From f2504da8036d71fee3e0c8bad4f802b334e7ec4f Mon Sep 17 00:00:00 2001 From: trinity-1686a Date: Wed, 23 Oct 2024 12:20:00 +0200 Subject: [PATCH] fix unit conversion in jaeger http search endpoint --- .../quickwit-serve/src/jaeger_api/model.rs | 2 ++ .../src/jaeger_api/parse_duration.rs | 4 ++-- .../src/jaeger_api/rest_handler.rs | 20 +++++++++++++++++-- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/quickwit/quickwit-serve/src/jaeger_api/model.rs b/quickwit/quickwit-serve/src/jaeger_api/model.rs index 502e4d251a6..917d31fc68b 100644 --- a/quickwit/quickwit-serve/src/jaeger_api/model.rs +++ b/quickwit/quickwit-serve/src/jaeger_api/model.rs @@ -55,9 +55,11 @@ pub struct TracesSearchQueryParams { pub service: Option, #[serde(default)] pub operation: Option, + // these are microsecond precision pub start: Option, pub end: Option, pub tags: Option, + // these are unit-suffixed numbers. in practice we only support precision up to the ms pub min_duration: Option, pub max_duration: Option, pub lookback: Option, diff --git a/quickwit/quickwit-serve/src/jaeger_api/parse_duration.rs b/quickwit/quickwit-serve/src/jaeger_api/parse_duration.rs index c1d444d3d7a..65341eeb8e8 100644 --- a/quickwit/quickwit-serve/src/jaeger_api/parse_duration.rs +++ b/quickwit/quickwit-serve/src/jaeger_api/parse_duration.rs @@ -30,8 +30,8 @@ pub(crate) fn parse_duration_with_units(duration_string: String) -> anyhow::Resu } pub(crate) fn to_well_known_timestamp(timestamp_nanos: i64) -> ProstTimestamp { - let seconds = timestamp_nanos / 1_000_000; - let nanos = (timestamp_nanos % 1_000_000) as i32; + let seconds = timestamp_nanos / 1_000_000_000; + let nanos = (timestamp_nanos % 1_000_000_000) as i32; ProstTimestamp { seconds, nanos } } diff --git a/quickwit/quickwit-serve/src/jaeger_api/rest_handler.rs b/quickwit/quickwit-serve/src/jaeger_api/rest_handler.rs index 15d76413f47..544c30afda4 100644 --- a/quickwit/quickwit-serve/src/jaeger_api/rest_handler.rs +++ b/quickwit/quickwit-serve/src/jaeger_api/rest_handler.rs @@ -248,8 +248,12 @@ async fn jaeger_traces_search( service_name: search_params.service.unwrap_or_default(), operation_name: search_params.operation.unwrap_or_default(), tags, - start_time_min: search_params.start.map(to_well_known_timestamp), - start_time_max: search_params.end.map(to_well_known_timestamp), + start_time_min: search_params + .start + .map(|ts| to_well_known_timestamp(ts * 1000)), + start_time_max: search_params + .end + .map(|ts| to_well_known_timestamp(ts * 1000)), duration_min, duration_max, num_traces: search_params.limit.unwrap_or(DEFAULT_NUMBER_OF_TRACES), @@ -449,6 +453,18 @@ mod tests { "{\"type\":\"term\",\"field\":\"resource_attributes.tag.second\",\"value\":\"\ true\"}" )); + assert!(req.query_ast.contains( + "{\"type\":\"term\",\"field\":\"resource_attributes.tag.second\",\"value\":\"\ + true\"}" + )); + // no lowerbound because minDuration < 1ms, + assert!(req.query_ast.contains( + "{\"type\":\"range\",\"field\":\"span_duration_millis\",\"lower_bound\":\"\ + Unbounded\",\"upper_bound\":{\"Included\":1200}}" + )); + assert_eq!(req.start_timestamp, Some(1702352106)); + // TODO(trinity) i think we have an off by 1 here, imo this should be rounded up + assert_eq!(req.end_timestamp, Some(1702373706)); assert_eq!( req.index_id_patterns, vec![OTEL_TRACES_INDEX_ID.to_string()]