Add support for tracing::Span::recorded fields in metrics-tracing-context

metrics-tracing-context/src/lib.rs

@@ -55,19 +55,17 @@
 //!
 //! # Implementation
 //!
-//! The integration layer works by capturing all fields present when a span is created and storing
-//! them as an extension to the span.  If a metric is emitted while a span is entered, we check that
-//! span to see if it has any fields in the extension data, and if it does, we add those fields as
-//! labels to the metric key.
+//! The integration layer works by capturing all fields present when a span is created or then
+//! new fields are recorded afterward, and storing them as an extension to the span.  If a metric
+//! is emitted while a span is entered, we check that span to see if it has any fields in the
+//! extension data, and if it does, we add those fields as labels to the metric key.
 //!
-//! There are two important behaviors to be aware of:
-//! - we only capture the fields present when the span is created
-//! - we store all fields that a span has, including the fields of its parent span(s)
+//! Be aware that we store all fields that a span has, including the fields of its parent span(s).
 //!
-//! ## Lack of dynamism
+//! ## Support for dynamism
 //!
-//! This means that if you use [`Span::record`][tracing::Span::record] to add fields to a span after
-//! it has been created, those fields will not be captured and added to your metric key.
+//! If you use [`Span::record`][tracing::Span::record] to add fields to a span after it has been
+//! created, those fields will be captured and added to your metric key.
 //!
 //! ## Span fields and ancestry
 //!

metrics-tracing-context/src/tracing_integration.rs

@@ -65,9 +65,8 @@ impl Visit for Labels {
 }
 
 impl Labels {
-    fn from_attributes(attrs: &Attributes<'_>) -> Labels {
+    fn from_record(record: &Record) -> Labels {
         let mut labels = Labels::default();
-        let record = Record::new(attrs.values());
         record.record(&mut labels);
         labels
     }
@@ -135,7 +134,7 @@ where
 {
     fn on_new_span(&self, attrs: &Attributes<'_>, id: &Id, cx: Context<'_, S>) {
         let span = cx.span(id).expect("span must already exist!");
-        let mut labels = Labels::from_attributes(attrs);
+        let mut labels = Labels::from_record(&Record::new(attrs.values()));
 
         if let Some(parent) = span.parent() {
             if let Some(parent_labels) = parent.extensions().get::<Labels>() {
@@ -146,6 +145,18 @@ where
         span.extensions_mut().insert(labels);
     }
 
+    fn on_record(&self, id: &Id, values: &Record<'_>, cx: Context<'_, S>) {
+        let span = cx.span(id).expect("span must already exist!");
+        let labels = Labels::from_record(values);
+
+        let ext = &mut span.extensions_mut();
+        if let Some(existing) = ext.get_mut::<Labels>() {
+            existing.extend_from_labels(&labels);
+        } else {
+            ext.insert(labels);
+        }
+    }
+
     unsafe fn downcast_raw(&self, id: TypeId) -> Option<*const ()> {
         match id {
             id if id == TypeId::of::<Self>() => Some(self as *const _ as *const ()),

metrics-tracing-context/tests/integration.rs

@@ -8,54 +8,61 @@ use tracing::{span, Level};
 use tracing_subscriber::{layer::SubscriberExt, Registry};
 
 static TEST_MUTEX: Mutex<()> = const_mutex(());
-static LOGIN_ATTEMPTS: &'static str = "login_attempts";
-static LOGIN_ATTEMPTS_NONE: &'static str = "login_attempts_no_labels";
-static LOGIN_ATTEMPTS_STATIC: &'static str = "login_attempts_static_labels";
-static LOGIN_ATTEMPTS_DYNAMIC: &'static str = "login_attempts_dynamic_labels";
-static LOGIN_ATTEMPTS_BOTH: &'static str = "login_attempts_static_and_dynamic_labels";
-static MY_COUNTER: &'static str = "my_counter";
-static USER_EMAIL: &'static [Label] = &[
+static LOGIN_ATTEMPTS: &str = "login_attempts";
+static LOGIN_ATTEMPTS_NONE: &str = "login_attempts_no_labels";
+static LOGIN_ATTEMPTS_STATIC: &str = "login_attempts_static_labels";
+static LOGIN_ATTEMPTS_DYNAMIC: &str = "login_attempts_dynamic_labels";
+static LOGIN_ATTEMPTS_BOTH: &str = "login_attempts_static_and_dynamic_labels";
+static MY_COUNTER: &str = "my_counter";
+static USER_EMAIL: &[Label] = &[
     Label::from_static_parts("user", "ferris"),
     Label::from_static_parts("user.email", "[email protected]"),
 ];
-static EMAIL_USER: &'static [Label] = &[
+static USER_ID: &[Label] = &[Label::from_static_parts("user.id", "42")];
+static EMAIL_USER: &[Label] = &[
     Label::from_static_parts("user.email", "[email protected]"),
     Label::from_static_parts("user", "ferris"),
 ];
-static SVC_ENV: &'static [Label] = &[
+static SVC_ENV: &[Label] = &[
     Label::from_static_parts("service", "login_service"),
     Label::from_static_parts("env", "test"),
 ];
-static SVC_USER_EMAIL: &'static [Label] = &[
+static SVC_USER_EMAIL: &[Label] = &[
     Label::from_static_parts("service", "login_service"),
     Label::from_static_parts("user", "ferris"),
     Label::from_static_parts("user.email", "[email protected]"),
 ];
-static NODE_USER_EMAIL: &'static [Label] = &[
+static SVC_USER_EMAIL_ID: &[Label] = &[
+    Label::from_static_parts("service", "login_service"),
+    Label::from_static_parts("user", "ferris"),
+    Label::from_static_parts("user.email", "[email protected]"),
+    Label::from_static_parts("user.id", "42"),
+];
+static NODE_USER_EMAIL: &[Label] = &[
     Label::from_static_parts("node_name", "localhost"),
     Label::from_static_parts("user", "ferris"),
     Label::from_static_parts("user.email", "[email protected]"),
 ];
-static SVC_NODE_USER_EMAIL: &'static [Label] = &[
+static SVC_NODE_USER_EMAIL: &[Label] = &[
     Label::from_static_parts("service", "login_service"),
     Label::from_static_parts("node_name", "localhost"),
     Label::from_static_parts("user", "ferris"),
     Label::from_static_parts("user.email", "[email protected]"),
 ];
-static COMBINED_LABELS: &'static [Label] = &[
+static COMBINED_LABELS: &[Label] = &[
     Label::from_static_parts("shared_field", "inner"),
     Label::from_static_parts("inner_specific", "foo"),
     Label::from_static_parts("inner_specific_dynamic", "foo_dynamic"),
     Label::from_static_parts("shared_field", "outer"),
     Label::from_static_parts("outer_specific", "bar"),
     Label::from_static_parts("outer_specific_dynamic", "bar_dynamic"),
 ];
-static SAME_CALLSITE_PATH_1: &'static [Label] = &[
+static SAME_CALLSITE_PATH_1: &[Label] = &[
     Label::from_static_parts("shared_field", "path1"),
     Label::from_static_parts("path1_specific", "foo"),
     Label::from_static_parts("path1_specific_dynamic", "foo_dynamic"),
 ];
-static SAME_CALLSITE_PATH_2: &'static [Label] = &[
+static SAME_CALLSITE_PATH_2: &[Label] = &[
     Label::from_static_parts("shared_field", "path2"),
     Label::from_static_parts("path2_specific", "bar"),
     Label::from_static_parts("path2_specific_dynamic", "bar_dynamic"),
@@ -110,7 +117,137 @@ fn test_basic_functionality() {
             None,
             DebugValue::Counter(1),
         )]
-    )
+    );
+}
+
+#[test]
+fn test_basic_functionality_record() {
+    let (_guard, snapshotter) = setup(TracingContextLayer::all());
+
+    let user = "ferris";
+    let email = "[email protected]";
+    let span = span!(
+        Level::TRACE,
+        "login",
+        user,
+        user.email = email,
+        user.id = tracing_core::field::Empty,
+    );
+    let _guard = span.enter();
+
+    span.record("user.id", 42);
+    counter!("login_attempts", "service" => "login_service").increment(1);
+
+    let snapshot = snapshotter.snapshot().into_vec();
+
+    assert_eq!(
+        snapshot,
+        vec![(
+            CompositeKey::new(
+                MetricKind::Counter,
+                Key::from_static_parts(LOGIN_ATTEMPTS, SVC_USER_EMAIL_ID)
+            ),
+            None,
+            None,
+            DebugValue::Counter(1),
+        )]
+    );
+}
+
+#[test]
+fn test_basic_functionality_then_record() {
+    let (_guard, snapshotter) = setup(TracingContextLayer::all());
+
+    let user = "ferris";
+    let email = "[email protected]";
+    let span = span!(
+        Level::TRACE,
+        "login",
+        user,
+        user.email = email,
+        user.id = tracing_core::field::Empty,
+    );
+    let _guard = span.enter();
+    let mut snapshots = vec![];
+    {
+        counter!("login_attempts", "service" => "login_service").increment(1);
+
+        let snapshot = snapshotter.snapshot().into_vec();
+
+        snapshots.push((
+            CompositeKey::new(
+                MetricKind::Counter,
+                Key::from_static_parts(LOGIN_ATTEMPTS, SVC_USER_EMAIL),
+            ),
+            None,
+            None,
+            DebugValue::Counter(1),
+        ));
+
+        assert_eq!(snapshot, snapshots);
+    }
+    span.record("user.id", 42);
+    {
+        counter!("login_attempts", "service" => "login_service").increment(1);
+
+        let snapshot = snapshotter.snapshot().into_vec();
+
+        snapshots.push((
+            CompositeKey::new(
+                MetricKind::Counter,
+                Key::from_static_parts(LOGIN_ATTEMPTS, SVC_USER_EMAIL_ID),
+            ),
+            None,
+            None,
+            DebugValue::Counter(1),
+        ));
+
+        assert_eq!(snapshot, snapshots);
+    }
+}
+
+#[test]
+fn test_rerecord() {
+    static USER_ID_42: &[Label] = &[Label::from_static_parts("user.id", "42")];
+    static USER_ID_123: &[Label] =
+        &[Label::from_static_parts("user.id", "42"), Label::from_static_parts("user.id", "123")];
+
+    let (_guard, snapshotter) = setup(TracingContextLayer::all());
+
+    let span = span!(Level::TRACE, "login", user.id = tracing_core::field::Empty);
+    let _guard = span.enter();
+
+    span.record("user.id", 42);
+    counter!("login_attempts").increment(1);
+
+    span.record("user.id", 123);
+    counter!("login_attempts").increment(1);
+
+    let snapshot = snapshotter.snapshot().into_vec();
+
+    assert_eq!(
+        snapshot,
+        vec![
+            (
+                CompositeKey::new(
+                    MetricKind::Counter,
+                    Key::from_static_parts(LOGIN_ATTEMPTS, USER_ID_42)
+                ),
+                None,
+                None,
+                DebugValue::Counter(1),
+            ),
+            (
+                CompositeKey::new(
+                    MetricKind::Counter,
+                    Key::from_static_parts(LOGIN_ATTEMPTS, USER_ID_123)
+                ),
+                None,
+                None,
+                DebugValue::Counter(1),
+            )
+        ]
+    );
 }
 
 #[test]
@@ -130,8 +267,11 @@ fn test_macro_forms() {
     let node_name = "localhost".to_string();
     counter!("login_attempts_dynamic_labels", "node_name" => node_name.clone()).increment(1);
     // Static and dynamic.
-    counter!("login_attempts_static_and_dynamic_labels",
-        "service" => "login_service", "node_name" => node_name.clone())
+    counter!(
+        "login_attempts_static_and_dynamic_labels",
+        "service" => "login_service",
+        "node_name" => node_name,
+    )
     .increment(1);
 
     let snapshot = snapshotter.snapshot().into_vec();
@@ -176,7 +316,7 @@ fn test_macro_forms() {
                 DebugValue::Counter(1),
             ),
         ]
-    )
+    );
 }
 
 #[test]
@@ -198,7 +338,30 @@ fn test_no_labels() {
             None,
             DebugValue::Counter(1),
         )]
-    )
+    );
+}
+
+#[test]
+fn test_no_labels_record() {
+    let (_guard, snapshotter) = setup(TracingContextLayer::all());
+
+    let span = span!(Level::TRACE, "login", user.id = tracing_core::field::Empty,);
+    let _guard = span.enter();
+
+    span.record("user.id", 42);
+    counter!("login_attempts").increment(1);
+
+    let snapshot = snapshotter.snapshot().into_vec();
+
+    assert_eq!(
+        snapshot,
+        vec![(
+            CompositeKey::new(MetricKind::Counter, Key::from_static_parts(LOGIN_ATTEMPTS, USER_ID)),
+            None,
+            None,
+            DebugValue::Counter(1),
+        )]
+    );
 }
 
 #[test]
@@ -262,7 +425,7 @@ fn test_multiple_paths_to_the_same_callsite() {
                 DebugValue::Counter(1),
             )
         ]
-    )
+    );
 }
 
 #[test]
@@ -347,12 +510,12 @@ fn test_label_filtering() {
             None,
             DebugValue::Counter(1),
         )]
-    )
+    );
 }
 
 #[test]
 fn test_label_allowlist() {
-    let (_guard, snapshotter) = setup(TracingContextLayer::only_allow(&["env", "service"]));
+    let (_guard, snapshotter) = setup(TracingContextLayer::only_allow(["env", "service"]));
 
     let user = "ferris";
     let email = "[email protected]";
@@ -378,5 +541,5 @@ fn test_label_allowlist() {
             None,
             DebugValue::Counter(1),
         )]
-    )
+    );
 }