Skip to content

Commit

Permalink
fix unit conversion in jaeger http search endpoint (#5519)
Browse files Browse the repository at this point in the history
  • Loading branch information
trinity-1686a authored Oct 23, 2024
1 parent f0ff912 commit 3a1532a
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 2 deletions.
2 changes: 2 additions & 0 deletions quickwit/quickwit-serve/src/jaeger_api/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,11 @@ pub struct TracesSearchQueryParams {
pub service: Option<String>,
#[serde(default)]
pub operation: Option<String>,
// these are microsecond precision
pub start: Option<i64>,
pub end: Option<i64>,
pub tags: Option<String>,
// these are unit-suffixed numbers. in practice we only support precision up to the ms
pub min_duration: Option<String>,
pub max_duration: Option<String>,
pub lookback: Option<String>,
Expand Down
20 changes: 18 additions & 2 deletions quickwit/quickwit-serve/src/jaeger_api/rest_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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()]
Expand Down

0 comments on commit 3a1532a

Please sign in to comment.