fix(telemetry): do not display otel bug if the trace is sampled (#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. --------- Signed-off-by: Benjamin Coenen <[email protected]>
apollographql · Sep 15, 2023 · 641bfac · 641bfac
1 parent ba0f8f8
commit 641bfac
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 5 deletions.
diff --git a/.changesets/fix_bnjjj_fix_otel_data_from_log.md 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
diff --git 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
@@ -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::<SampledSpan>().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::<SampledSpan>().is_some()
+    }
+}
 /// prevents span fields from being formatted to a string when writing logs
 pub(crate) struct NullFieldFormatter;