Skip to content

Commit

Permalink
only return root spans for jaeger http api (#5358)
Browse files Browse the repository at this point in the history
* only return root spans for jaeger http api

* fix rest api tests
  • Loading branch information
trinity-1686a authored Aug 29, 2024
1 parent 7e6d4fc commit 67091c5
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 8 deletions.
17 changes: 17 additions & 0 deletions quickwit/quickwit-jaeger/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ impl JaegerService {
operation_name: &'static str,
request_start: Instant,
index_id_patterns: Vec<String>,
root_only: bool,
) -> JaegerResult<SpanStream> {
debug!(request=?request, "`find_traces` request");

Expand All @@ -210,6 +211,7 @@ impl JaegerService {
operation_name,
request_start,
index_id_patterns,
root_only,
)
.await?;
Ok(response)
Expand Down Expand Up @@ -237,6 +239,7 @@ impl JaegerService {
operation_name,
request_start,
index_id_patterns,
false,
)
.await?;
Ok(response)
Expand Down Expand Up @@ -300,6 +303,7 @@ impl JaegerService {
operation_name: &'static str,
request_start: Instant,
index_id_patterns: Vec<String>,
root_only: bool,
) -> Result<SpanStream, Status> {
if trace_ids.is_empty() {
let (_tx, rx) = mpsc::channel(1);
Expand All @@ -316,6 +320,15 @@ impl JaegerService {
};
query.should.push(term_query.into());
}
if root_only {
// TODO this isn't backward compatible. We could do NOT is_root:false with a lenient
// UserInputQuery once we support being lenient on missing fields
let term_query = TermQuery {
field: "is_root".to_string(),
value: "true".to_string(),
};
query.must.push(term_query.into());
}

let query_ast: QueryAst = query.into();
let query_ast =
Expand Down Expand Up @@ -523,6 +536,9 @@ impl SpanReaderPlugin for JaegerService {
"find_traces",
Instant::now(),
index_id_patterns,
false, /* if we use true, Jaeger will display "1 Span", and display an empty trace
* when clicking on the ui (but display the full trace after reloading the
* page) */
)
.await
.map(Response::new)
Expand Down Expand Up @@ -2160,6 +2176,7 @@ mod tests {
message: Some("An error occurred.".to_string()),
},
parent_span_id: Some(SpanId::new([3; 8])),
is_root: Some(false),
events: vec![QwEvent {
event_timestamp_nanos: 1000500003,
event_name: "event_name".to_string(),
Expand Down
12 changes: 11 additions & 1 deletion quickwit/quickwit-opentelemetry/src/otlp/traces.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ use super::{
use crate::otlp::metrics::OTLP_SERVICE_METRICS;
use crate::otlp::{extract_attributes, SpanId, TraceId};

pub const OTEL_TRACES_INDEX_ID: &str = "otel-traces-v0_7";
pub const OTEL_TRACES_INDEX_ID: &str = "otel-traces-v0_9";
pub const OTEL_TRACES_INDEX_ID_PATTERN: &str = "otel-traces-v0_*";

const OTEL_TRACES_INDEX_CONFIG: &str = r#"
Expand Down Expand Up @@ -144,6 +144,10 @@ doc_mapping:
input_format: hex
output_format: hex
indexed: false
- name: is_root
type: bool
indexed: true
stored: false
- name: events
type: array<json>
tokenizer: raw
Expand Down Expand Up @@ -231,6 +235,8 @@ pub struct Span {
pub span_status: SpanStatus,
#[serde(skip_serializing_if = "Option::is_none")]
pub parent_span_id: Option<SpanId>,
#[serde(skip_serializing_if = "Option::is_none")]
pub is_root: Option<bool>,
#[serde(default)]
#[serde(skip_serializing_if = "Vec::is_empty")]
pub events: Vec<Event>,
Expand Down Expand Up @@ -312,6 +318,7 @@ impl Span {
span_dropped_events_count: span.dropped_events_count,
span_dropped_links_count: span.dropped_links_count,
span_status: span.status.map(SpanStatus::from_otlp).unwrap_or_default(),
is_root: Some(parent_span_id.is_none()),
parent_span_id,
events,
event_names,
Expand Down Expand Up @@ -1278,6 +1285,7 @@ mod tests {
span_dropped_links_count: 0,
span_status: SpanStatus::default(),
parent_span_id: None,
is_root: Some(true),
events: Vec::new(),
event_names: Vec::new(),
links: Vec::new(),
Expand Down Expand Up @@ -1320,6 +1328,7 @@ mod tests {
message: None,
},
parent_span_id: Some(SpanId::new([3; 8])),
is_root: Some(false),
events: vec![Event {
event_timestamp_nanos: 1,
event_name: "event_name".to_string(),
Expand Down Expand Up @@ -1373,6 +1382,7 @@ mod tests {
span_dropped_links_count: 0,
span_status: SpanStatus::default(),
parent_span_id: None,
is_root: Some(true),
events: Vec::new(),
event_names: Vec::new(),
links: Vec::new(),
Expand Down
13 changes: 7 additions & 6 deletions quickwit/quickwit-serve/src/jaeger_api/rest_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ pub(crate) struct JaegerApi;
///
/// This is where all Jaeger handlers
/// should be registered.
/// Request are executed on the `otel traces v0_7` index.
/// Request are executed on the `otel-traces-v0_*` indexes.
pub(crate) fn jaeger_api_handlers(
jaeger_service_opt: Option<JaegerService>,
) -> impl Filter<Extract = (impl warp::Reply,), Error = Rejection> + Clone {
Expand Down Expand Up @@ -261,6 +261,7 @@ async fn jaeger_traces_search(
"find_traces",
Instant::now(),
index_id_patterns,
true,
)
.await
.map_err(|error| {
Expand Down Expand Up @@ -353,7 +354,7 @@ mod tests {
async fn test_when_jaeger_not_found() {
let jaeger_api_handler = jaeger_api_handlers(None).recover(crate::rest::recover_fn_final);
let resp = warp::test::request()
.path("/otel-traces-v0_7/jaeger/api/services")
.path("/otel-traces-v0_9/jaeger/api/services")
.reply(&jaeger_api_handler)
.await;
assert_eq!(resp.status(), 404);
Expand Down Expand Up @@ -385,7 +386,7 @@ mod tests {

let jaeger_api_handler = jaeger_api_handlers(Some(jaeger)).recover(recover_fn);
let resp = warp::test::request()
.path("/otel-traces-v0_7/jaeger/api/services")
.path("/otel-traces-v0_9/jaeger/api/services")
.reply(&jaeger_api_handler)
.await;
assert_eq!(resp.status(), 200);
Expand Down Expand Up @@ -421,7 +422,7 @@ mod tests {
let jaeger = JaegerService::new(JaegerConfig::default(), mock_search_service);
let jaeger_api_handler = jaeger_api_handlers(Some(jaeger)).recover(recover_fn);
let resp = warp::test::request()
.path("/otel-traces-v0_7/jaeger/api/services/service1/operations")
.path("/otel-traces-v0_9/jaeger/api/services/service1/operations")
.reply(&jaeger_api_handler)
.await;
assert_eq!(resp.status(), 200);
Expand Down Expand Up @@ -469,7 +470,7 @@ mod tests {
let jaeger_api_handler = jaeger_api_handlers(Some(jaeger)).recover(recover_fn);
let resp = warp::test::request()
.path(
"/otel-traces-v0_7/jaeger/api/traces?service=quickwit&\
"/otel-traces-v0_9/jaeger/api/traces?service=quickwit&\
operation=delete_splits_marked_for_deletion&minDuration=500us&maxDuration=1.2s&\
tags=%7B%22tag.first%22%3A%22common%22%2C%22tag.second%22%3A%22true%22%7D&\
limit=1&start=1702352106016000&end=1702373706016000&lookback=custom",
Expand Down Expand Up @@ -500,7 +501,7 @@ mod tests {

let jaeger_api_handler = jaeger_api_handlers(Some(jaeger)).recover(recover_fn);
let resp = warp::test::request()
.path("/otel-traces-v0_7/jaeger/api/traces/1506026ddd216249555653218dc88a6c")
.path("/otel-traces-v0_9/jaeger/api/traces/1506026ddd216249555653218dc88a6c")
.reply(&jaeger_api_handler)
.await;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ expected:
#uuid: gharchive:01HN2SDANHDN6WFAFNH7BBMQ8C
- index: otel-logs-v0_7
docs.count: '0'
- index: otel-traces-v0_7
- index: otel-traces-v0_9
docs.count: '0'
---
method: [GET]
Expand Down

0 comments on commit 67091c5

Please sign in to comment.