Skip exporting spans that are not part of / descendants of the allowed root spans list.

jimmygchen · jimmygchen · commit 7762c16bdcd3 · 2025-08-13T16:28:50.000+10:00
diff --git a/beacon_node/lighthouse_tracing/src/lib.rs b/beacon_node/lighthouse_tracing/src/lib.rs
@@ -1,5 +1,4 @@
-use tracing::{Metadata, Subscriber};
-use tracing_subscriber::layer::{Context, Filter};
+use tracing::Subscriber;
 
 /// DA checker spans
 pub const SPAN_PENDING_COMPONENTS: &str = "pending_components";
@@ -51,28 +50,112 @@ pub const LH_BN_ROOT_SPAN_NAMES: &[&str] = &[
     SPAN_HANDLE_LIGHT_CLIENT_FINALITY_UPDATE,
 ];
 
-/// Filter that only allows spans that either:
-/// - Have a parent span, or
-/// - Are root spans with names matching the predefined allowed list
-pub struct AllowedRootSpanFilter(&'static [&'static str]);
+use std::collections::HashSet;
+use tracing::span::Id;
+use tracing_subscriber::Layer;
 
-impl AllowedRootSpanFilter {
-    pub fn new(allowed_names: &'static [&'static str]) -> Self {
-        AllowedRootSpanFilter(allowed_names)
+/// A filtering layer that wraps another layer and only forwards spans from allowed root spans and their descendants
+pub struct AllowedRootSpanLayer<L> {
+    inner: L,
+    allowed_names: &'static [&'static str],
+    allowed_spans: std::sync::Mutex<HashSet<Id>>,
+}
+
+impl<L> AllowedRootSpanLayer<L> {
+    pub fn new(inner: L, allowed_names: &'static [&'static str]) -> Self {
+        AllowedRootSpanLayer {
+            inner,
+            allowed_names,
+            allowed_spans: std::sync::Mutex::new(HashSet::new()),
+        }
+    }
+
+    fn is_span_allowed(&self, span_id: &Id) -> bool {
+        self.allowed_spans.lock().unwrap().contains(span_id)
+    }
+
+    fn mark_span_allowed(&self, span_id: Id) {
+        self.allowed_spans.lock().unwrap().insert(span_id);
+    }
+
+    fn remove_span(&self, span_id: &Id) {
+        self.allowed_spans.lock().unwrap().remove(span_id);
     }
 }
 
-impl<S> Filter<S> for AllowedRootSpanFilter
+impl<S, L> Layer<S> for AllowedRootSpanLayer<L>
 where
-    S: Subscriber,
+    S: Subscriber + for<'lookup> tracing_subscriber::registry::LookupSpan<'lookup>,
+    L: Layer<S>,
 {
-    fn enabled(&self, metadata: &Metadata<'_>, ctx: &Context<'_, S>) -> bool {
-        // Has a parent span: allow
-        if ctx.current_span().id().is_some() {
-            return true;
+    fn on_new_span(
+        &self,
+        attrs: &tracing::span::Attributes<'_>,
+        id: &Id,
+        ctx: tracing_subscriber::layer::Context<'_, S>,
+    ) {
+        let should_allow = if let Some(parent_id) = ctx.current_span().id() {
+            // This span has a parent - allow it if the parent is allowed
+            self.is_span_allowed(parent_id)
+        } else {
+            // This is a root span - allow it if it's in the allowed list
+            self.allowed_names.contains(&attrs.metadata().name())
+        };
+
+        if should_allow {
+            self.mark_span_allowed(id.clone());
+            self.inner.on_new_span(attrs, id, ctx);
         }
+    }
+
+    fn on_record(
+        &self,
+        span: &Id,
+        values: &tracing::span::Record<'_>,
+        ctx: tracing_subscriber::layer::Context<'_, S>,
+    ) {
+        if self.is_span_allowed(span) {
+            self.inner.on_record(span, values, ctx);
+        }
+    }
 
-        // Root span: only allow if in allowed list
-        self.0.contains(&metadata.name())
+    fn on_follows_from(
+        &self,
+        span: &Id,
+        follows: &Id,
+        ctx: tracing_subscriber::layer::Context<'_, S>,
+    ) {
+        if self.is_span_allowed(span) {
+            self.inner.on_follows_from(span, follows, ctx);
+        }
+    }
+
+    fn on_event(&self, event: &tracing::Event<'_>, ctx: tracing_subscriber::layer::Context<'_, S>) {
+        // Check if we're in an allowed span context
+        if let Some(current_span_id) = ctx.current_span().id()
+            && self.is_span_allowed(current_span_id)
+        {
+            self.inner.on_event(event, ctx);
+        }
+        // If no current span, we could choose to allow or disallow events - for now, disallow
+    }
+
+    fn on_enter(&self, id: &Id, ctx: tracing_subscriber::layer::Context<'_, S>) {
+        if self.is_span_allowed(id) {
+            self.inner.on_enter(id, ctx);
+        }
+    }
+
+    fn on_exit(&self, id: &Id, ctx: tracing_subscriber::layer::Context<'_, S>) {
+        if self.is_span_allowed(id) {
+            self.inner.on_exit(id, ctx);
+        }
+    }
+
+    fn on_close(&self, id: Id, ctx: tracing_subscriber::layer::Context<'_, S>) {
+        if self.is_span_allowed(&id) {
+            self.inner.on_close(id.clone(), ctx);
+        }
+        self.remove_span(&id);
     }
 }
diff --git a/lighthouse/src/main.rs b/lighthouse/src/main.rs
@@ -16,7 +16,7 @@ use environment::{EnvironmentBuilder, LoggerConfig};
 use eth2_network_config::{DEFAULT_HARDCODED_NETWORK, Eth2NetworkConfig, HARDCODED_NET_NAMES};
 use ethereum_hashing::have_sha_extensions;
 use futures::TryFutureExt;
-use lighthouse_tracing::{AllowedRootSpanFilter, LH_BN_ROOT_SPAN_NAMES};
+use lighthouse_tracing::{AllowedRootSpanLayer, LH_BN_ROOT_SPAN_NAMES};
 use lighthouse_version::VERSION;
 use logging::{MetricsLayer, build_workspace_filter, crit};
 use malloc_utils::configure_memory_allocator;
@@ -721,12 +721,12 @@ fn run<E: EthSpec>(
                 .build();
 
             let tracer = provider.tracer("lighthouse");
-            Ok::<_, String>(
+            Ok::<_, String>(AllowedRootSpanLayer::new(
                 tracing_opentelemetry::layer()
                     .with_tracer(tracer)
-                    .with_filter(workspace_filter)
-                    .with_filter(AllowedRootSpanFilter::new(LH_BN_ROOT_SPAN_NAMES)),
-            )
+                    .with_filter(workspace_filter),
+                LH_BN_ROOT_SPAN_NAMES,
+            ))
         })?;
 
         logging_layers.push(telemetry_layer.boxed());
