diff --git a/.changesets/fix_bnjjj_fix_otel_data_from_log.md b/.changesets/fix_bnjjj_fix_otel_data_from_log.md new file mode 100644 index 0000000000..8d2fb003cc --- /dev/null +++ b/.changesets/fix_bnjjj_fix_otel_data_from_log.md @@ -0,0 +1,5 @@ +### fix(telemetry): do not display otel bug if the trace is sampled ([PR #3832](https://github.com/apollographql/router/pull/3832)) + +We changed the way we are sampling spans. If you had logs in an evicted span it displays that log `Unable to find OtelData in extensions; this is a bug` which is incorrect, it's not a bug, we just can't display the `trace_id` because there is no `trace_id` as the span has not been sampled. + +By [@bnjjj](https://github.com/bnjjj) in https://github.com/apollographql/router/pull/3832 \ No newline at end of file diff --git a/apollo-router/src/plugins/telemetry/formatters/text.rs b/apollo-router/src/plugins/telemetry/formatters/text.rs index 1ed1a79b2c..1ea09dcc31 100644 --- a/apollo-router/src/plugins/telemetry/formatters/text.rs +++ b/apollo-router/src/plugins/telemetry/formatters/text.rs @@ -17,6 +17,8 @@ use tracing_subscriber::fmt::time::SystemTime; use tracing_subscriber::fmt::FmtContext; use tracing_subscriber::registry::LookupSpan; +use crate::plugins::telemetry::reload::IsSampled; + #[derive(Debug, Clone)] pub(crate) struct TextFormatter { pub(crate) timer: SystemTime, @@ -185,7 +187,11 @@ impl TextFormatter { } writer.write_char(' ')?; } - None => eprintln!("Unable to find OtelData in extensions; this is a bug"), + None => { + if span.is_sampled() { + eprintln!("Unable to find OtelData in extensions; this is a bug"); + } + } } } diff --git a/apollo-router/src/plugins/telemetry/reload.rs b/apollo-router/src/plugins/telemetry/reload.rs index 96296341e5..2dcf270f41 100644 --- a/apollo-router/src/plugins/telemetry/reload.rs +++ b/apollo-router/src/plugins/telemetry/reload.rs @@ -21,6 +21,7 @@ use tracing_subscriber::layer::Layer; use tracing_subscriber::layer::Layered; use tracing_subscriber::layer::SubscriberExt; use tracing_subscriber::registry::LookupSpan; +use tracing_subscriber::registry::SpanRef; use tracing_subscriber::reload::Handle; use tracing_subscriber::util::SubscriberInitExt; use tracing_subscriber::EnvFilter; @@ -220,10 +221,7 @@ where .id() .and_then(|id| cx.span(id)) { - // if this extension is set, that means the parent span was accepted, and so the - // entire trace is accepted - let extensions = spanref.extensions(); - return extensions.get::().is_some(); + return spanref.is_sampled(); } // we only make the sampling decision on the root span. If we reach here for any other span, @@ -257,6 +255,22 @@ where } struct SampledSpan; + +pub(crate) trait IsSampled { + fn is_sampled(&self) -> bool; +} + +impl<'a, T> IsSampled for SpanRef<'a, T> +where + T: tracing_subscriber::registry::LookupSpan<'a>, +{ + fn is_sampled(&self) -> bool { + // if this extension is set, that means the parent span was accepted, and so the + // entire trace is accepted + let extensions = self.extensions(); + extensions.get::().is_some() + } +} /// prevents span fields from being formatted to a string when writing logs pub(crate) struct NullFieldFormatter;