From 6e415767507d1cbc2b1fc5f6d0ae3c344a092388 Mon Sep 17 00:00:00 2001 From: zohnannor Date: Mon, 6 Nov 2023 16:22:15 +0400 Subject: [PATCH 01/15] Add support for recorded fields --- .../src/tracing_integration.rs | 23 +- metrics-tracing-context/tests/integration.rs | 259 ++++++++++++++++-- 2 files changed, 256 insertions(+), 26 deletions(-) diff --git a/metrics-tracing-context/src/tracing_integration.rs b/metrics-tracing-context/src/tracing_integration.rs index 7507b34e..d8ec53f7 100644 --- a/metrics-tracing-context/src/tracing_integration.rs +++ b/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::() { @@ -146,6 +145,24 @@ 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 mut labels = Labels::from_record(values); + + if let Some(parent) = span.parent() { + if let Some(parent_labels) = parent.extensions().get::() { + labels.extend_from_labels(parent_labels); + } + } + + let ext = &mut span.extensions_mut(); + if let Some(existing) = ext.get_mut::() { + 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::() => Some(self as *const _ as *const ()), diff --git a/metrics-tracing-context/tests/integration.rs b/metrics-tracing-context/tests/integration.rs index 0229393e..44d8f5de 100644 --- a/metrics-tracing-context/tests/integration.rs +++ b/metrics-tracing-context/tests/integration.rs @@ -8,41 +8,66 @@ 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", "ferris@rust-lang.org"), ]; -static EMAIL_USER: &'static [Label] = &[ +static USER_ID: &[Label] = &[Label::from_static_parts("user.id", "42")]; +static USER_EMAIL_ID: &[Label] = &[ + Label::from_static_parts("user", "ferris"), + Label::from_static_parts("user.email", "ferris@rust-lang.org"), + Label::from_static_parts("user.id", "42"), +]; +static EMAIL_USER: &[Label] = &[ Label::from_static_parts("user.email", "ferris@rust-lang.org"), 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", "ferris@rust-lang.org"), ]; -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", "ferris@rust-lang.org"), + 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", "ferris@rust-lang.org"), +]; +static NODE_USER_EMAIL_ID: &[Label] = &[ Label::from_static_parts("node_name", "localhost"), Label::from_static_parts("user", "ferris"), Label::from_static_parts("user.email", "ferris@rust-lang.org"), + Label::from_static_parts("user.id", "42"), ]; -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", "ferris@rust-lang.org"), ]; -static COMBINED_LABELS: &'static [Label] = &[ +static SVC_NODE_USER_EMAIL_ID: &[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", "ferris@rust-lang.org"), + Label::from_static_parts("user.id", "42"), +]; +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"), @@ -50,12 +75,12 @@ static COMBINED_LABELS: &'static [Label] = &[ 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 +135,93 @@ 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 = "ferris@rust-lang.org"; + 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 = "ferris@rust-lang.org"; + 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] @@ -130,8 +241,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 +290,83 @@ fn test_macro_forms() { DebugValue::Counter(1), ), ] + ); +} + +#[test] +fn test_macro_forms_record() { + let (_guard, snapshotter) = setup(TracingContextLayer::all()); + + let user = "ferris"; + let email = "ferris@rust-lang.org"; + 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); + // No labels. + counter!("login_attempts_no_labels").increment(1); + // Static labels only. + counter!("login_attempts_static_labels", "service" => "login_service").increment(1); + // Dynamic labels only. + 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, ) + .increment(1); + + let snapshot = snapshotter.snapshot().into_vec(); + + assert_eq!( + snapshot, + vec![ + ( + CompositeKey::new( + MetricKind::Counter, + Key::from_static_parts(LOGIN_ATTEMPTS_NONE, USER_EMAIL_ID) + ), + None, + None, + DebugValue::Counter(1), + ), + ( + CompositeKey::new( + MetricKind::Counter, + Key::from_static_parts(LOGIN_ATTEMPTS_STATIC, SVC_USER_EMAIL_ID), + ), + None, + None, + DebugValue::Counter(1), + ), + ( + CompositeKey::new( + MetricKind::Counter, + Key::from_static_parts(LOGIN_ATTEMPTS_DYNAMIC, NODE_USER_EMAIL_ID), + ), + None, + None, + DebugValue::Counter(1), + ), + ( + CompositeKey::new( + MetricKind::Counter, + Key::from_static_parts(LOGIN_ATTEMPTS_BOTH, SVC_NODE_USER_EMAIL_ID), + ), + None, + None, + DebugValue::Counter(1), + ), + ] + ); } #[test] @@ -198,7 +388,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 +475,7 @@ fn test_multiple_paths_to_the_same_callsite() { DebugValue::Counter(1), ) ] - ) + ); } #[test] @@ -347,12 +560,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 = "ferris@rust-lang.org"; @@ -378,5 +591,5 @@ fn test_label_allowlist() { None, DebugValue::Counter(1), )] - ) + ); } From 825bd40d8c1cacbc1ccff81dae89d2248e873040 Mon Sep 17 00:00:00 2001 From: zohnannor Date: Mon, 6 Nov 2023 16:40:23 +0400 Subject: [PATCH 02/15] Update docs --- metrics-tracing-context/src/lib.rs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/metrics-tracing-context/src/lib.rs b/metrics-tracing-context/src/lib.rs index 71d219d4..fc780cbf 100644 --- a/metrics-tracing-context/src/lib.rs +++ b/metrics-tracing-context/src/lib.rs @@ -55,19 +55,18 @@ //! //! # 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. +//! it has been created, those fields will be captured and added to your metric key. //! //! ## Span fields and ancestry //! From e129f632795f5c1fb6717351b211a9acff2a4307 Mon Sep 17 00:00:00 2001 From: zohnannor Date: Tue, 7 Nov 2023 13:18:15 +0400 Subject: [PATCH 03/15] Remove parent's fields inheritance in `on_record` --- metrics-tracing-context/src/tracing_integration.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/metrics-tracing-context/src/tracing_integration.rs b/metrics-tracing-context/src/tracing_integration.rs index d8ec53f7..98d32321 100644 --- a/metrics-tracing-context/src/tracing_integration.rs +++ b/metrics-tracing-context/src/tracing_integration.rs @@ -147,13 +147,7 @@ where fn on_record(&self, id: &Id, values: &Record<'_>, cx: Context<'_, S>) { let span = cx.span(id).expect("span must already exist!"); - let mut labels = Labels::from_record(values); - - if let Some(parent) = span.parent() { - if let Some(parent_labels) = parent.extensions().get::() { - labels.extend_from_labels(parent_labels); - } - } + let labels = Labels::from_record(values); let ext = &mut span.extensions_mut(); if let Some(existing) = ext.get_mut::() { From 449b21e675c2102495ae05aa2538e10b9b7090b5 Mon Sep 17 00:00:00 2001 From: zohnannor Date: Tue, 7 Nov 2023 13:18:30 +0400 Subject: [PATCH 04/15] Reformat docs --- metrics-tracing-context/src/lib.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/metrics-tracing-context/src/lib.rs b/metrics-tracing-context/src/lib.rs index fc780cbf..a5237acf 100644 --- a/metrics-tracing-context/src/lib.rs +++ b/metrics-tracing-context/src/lib.rs @@ -55,18 +55,17 @@ //! //! # Implementation //! -//! 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. +//! 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. //! //! Be aware that we store all fields that a span has, including the fields of its parent span(s). //! //! ## 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 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 //! From ba53763ffcc5b6d087979730df824d822c16a0ed Mon Sep 17 00:00:00 2001 From: zohnannor Date: Tue, 7 Nov 2023 14:08:33 +0400 Subject: [PATCH 05/15] Add test --- metrics-tracing-context/tests/integration.rs | 90 +++++--------------- 1 file changed, 20 insertions(+), 70 deletions(-) diff --git a/metrics-tracing-context/tests/integration.rs b/metrics-tracing-context/tests/integration.rs index 44d8f5de..a89e2171 100644 --- a/metrics-tracing-context/tests/integration.rs +++ b/metrics-tracing-context/tests/integration.rs @@ -19,11 +19,6 @@ static USER_EMAIL: &[Label] = &[ Label::from_static_parts("user.email", "ferris@rust-lang.org"), ]; static USER_ID: &[Label] = &[Label::from_static_parts("user.id", "42")]; -static USER_EMAIL_ID: &[Label] = &[ - Label::from_static_parts("user", "ferris"), - Label::from_static_parts("user.email", "ferris@rust-lang.org"), - Label::from_static_parts("user.id", "42"), -]; static EMAIL_USER: &[Label] = &[ Label::from_static_parts("user.email", "ferris@rust-lang.org"), Label::from_static_parts("user", "ferris"), @@ -48,25 +43,12 @@ static NODE_USER_EMAIL: &[Label] = &[ Label::from_static_parts("user", "ferris"), Label::from_static_parts("user.email", "ferris@rust-lang.org"), ]; -static NODE_USER_EMAIL_ID: &[Label] = &[ - Label::from_static_parts("node_name", "localhost"), - Label::from_static_parts("user", "ferris"), - Label::from_static_parts("user.email", "ferris@rust-lang.org"), - Label::from_static_parts("user.id", "42"), -]; 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", "ferris@rust-lang.org"), ]; -static SVC_NODE_USER_EMAIL_ID: &[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", "ferris@rust-lang.org"), - Label::from_static_parts("user.id", "42"), -]; static COMBINED_LABELS: &[Label] = &[ Label::from_static_parts("shared_field", "inner"), Label::from_static_parts("inner_specific", "foo"), @@ -225,28 +207,21 @@ fn test_basic_functionality_then_record() { } #[test] -fn test_macro_forms() { +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 user = "ferris"; - let email = "ferris@rust-lang.org"; - let span = span!(Level::TRACE, "login", user, user.email = email); + let span = span!(Level::TRACE, "login", user.id = tracing_core::field::Empty); let _guard = span.enter(); - // No labels. - counter!("login_attempts_no_labels").increment(1); - // Static labels only. - counter!("login_attempts_static_labels", "service" => "login_service").increment(1); - // Dynamic labels only. - 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, - ) - .increment(1); + 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(); @@ -256,7 +231,7 @@ fn test_macro_forms() { ( CompositeKey::new( MetricKind::Counter, - Key::from_static_parts(LOGIN_ATTEMPTS_NONE, USER_EMAIL) + Key::from_static_parts(LOGIN_ATTEMPTS, USER_ID_42) ), None, None, @@ -265,50 +240,25 @@ fn test_macro_forms() { ( CompositeKey::new( MetricKind::Counter, - Key::from_static_parts(LOGIN_ATTEMPTS_STATIC, SVC_USER_EMAIL), + Key::from_static_parts(LOGIN_ATTEMPTS, USER_ID_123) ), None, None, DebugValue::Counter(1), - ), - ( - CompositeKey::new( - MetricKind::Counter, - Key::from_static_parts(LOGIN_ATTEMPTS_DYNAMIC, NODE_USER_EMAIL), - ), - None, - None, - DebugValue::Counter(1), - ), - ( - CompositeKey::new( - MetricKind::Counter, - Key::from_static_parts(LOGIN_ATTEMPTS_BOTH, SVC_NODE_USER_EMAIL), - ), - None, - None, - DebugValue::Counter(1), - ), + ) ] ); } #[test] -fn test_macro_forms_record() { +fn test_macro_forms() { let (_guard, snapshotter) = setup(TracingContextLayer::all()); let user = "ferris"; let email = "ferris@rust-lang.org"; - let span = span!( - Level::TRACE, - "login", - user, - user.email = email, - user.id = tracing_core::field::Empty, - ); + let span = span!(Level::TRACE, "login", user, user.email = email); let _guard = span.enter(); - span.record("user.id", 42); // No labels. counter!("login_attempts_no_labels").increment(1); // Static labels only. @@ -332,7 +282,7 @@ fn test_macro_forms_record() { ( CompositeKey::new( MetricKind::Counter, - Key::from_static_parts(LOGIN_ATTEMPTS_NONE, USER_EMAIL_ID) + Key::from_static_parts(LOGIN_ATTEMPTS_NONE, USER_EMAIL) ), None, None, @@ -341,7 +291,7 @@ fn test_macro_forms_record() { ( CompositeKey::new( MetricKind::Counter, - Key::from_static_parts(LOGIN_ATTEMPTS_STATIC, SVC_USER_EMAIL_ID), + Key::from_static_parts(LOGIN_ATTEMPTS_STATIC, SVC_USER_EMAIL), ), None, None, @@ -350,7 +300,7 @@ fn test_macro_forms_record() { ( CompositeKey::new( MetricKind::Counter, - Key::from_static_parts(LOGIN_ATTEMPTS_DYNAMIC, NODE_USER_EMAIL_ID), + Key::from_static_parts(LOGIN_ATTEMPTS_DYNAMIC, NODE_USER_EMAIL), ), None, None, @@ -359,7 +309,7 @@ fn test_macro_forms_record() { ( CompositeKey::new( MetricKind::Counter, - Key::from_static_parts(LOGIN_ATTEMPTS_BOTH, SVC_NODE_USER_EMAIL_ID), + Key::from_static_parts(LOGIN_ATTEMPTS_BOTH, SVC_NODE_USER_EMAIL), ), None, None, From 22688afe3d321fb1e30da0c25afced6213b64ec6 Mon Sep 17 00:00:00 2001 From: zohnannor Date: Fri, 10 Nov 2023 13:23:23 +0400 Subject: [PATCH 06/15] Change Labels storage to be an IndexMap --- metrics-tracing-context/Cargo.toml | 1 + metrics-tracing-context/benches/visit.rs | 11 +++-- metrics-tracing-context/src/lib.rs | 25 +++++----- .../src/tracing_integration.rs | 41 ++++++++++------ metrics-tracing-context/tests/integration.rs | 49 +++++++++++++++++-- 5 files changed, 88 insertions(+), 39 deletions(-) diff --git a/metrics-tracing-context/Cargo.toml b/metrics-tracing-context/Cargo.toml index 0a692f7f..8160af84 100644 --- a/metrics-tracing-context/Cargo.toml +++ b/metrics-tracing-context/Cargo.toml @@ -32,6 +32,7 @@ itoa = { version = "1", default-features = false } metrics = { version = "^0.21", path = "../metrics" } metrics-util = { version = "^0.15", path = "../metrics-util" } lockfree-object-pool = { version = "0.1.3", default-features = false } +indexmap = "2.1" once_cell = { version = "1", default-features = false, features = ["std"] } tracing = { version = "0.1.29", default-features = false } tracing-core = { version = "0.1.21", default-features = false } diff --git a/metrics-tracing-context/benches/visit.rs b/metrics-tracing-context/benches/visit.rs index a3128af9..5f806700 100644 --- a/metrics-tracing-context/benches/visit.rs +++ b/metrics-tracing-context/benches/visit.rs @@ -1,8 +1,9 @@ use std::sync::Arc; use criterion::{criterion_group, criterion_main, BatchSize, Criterion}; +use indexmap::IndexMap; use lockfree_object_pool::LinearObjectPool; -use metrics::Label; +use metrics::{Label, SharedString}; use metrics_tracing_context::Labels; use once_cell::sync::OnceCell; use tracing::Metadata; @@ -13,9 +14,11 @@ use tracing_core::{ Callsite, Interest, }; -fn get_pool() -> &'static Arc>> { - static POOL: OnceCell>>> = OnceCell::new(); - POOL.get_or_init(|| Arc::new(LinearObjectPool::new(|| Vec::new(), |vec| vec.clear()))) +pub(crate) type Map = IndexMap; + +fn get_pool() -> &'static Arc> { + static POOL: OnceCell>> = OnceCell::new(); + POOL.get_or_init(|| Arc::new(LinearObjectPool::new(Map::new, Map::clear))) } const BATCH_SIZE: usize = 1000; diff --git a/metrics-tracing-context/src/lib.rs b/metrics-tracing-context/src/lib.rs index a5237acf..7fb9c362 100644 --- a/metrics-tracing-context/src/lib.rs +++ b/metrics-tracing-context/src/lib.rs @@ -65,7 +65,9 @@ //! ## Support for dynamism //! //! 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. +//! created, those fields will be captured and added to your metric key. Multiple records of the +//! same field would overwrite it, leaving the most recent value. This way you can change any of +//! the metrics keys coming from span fields. //! //! ## Span fields and ancestry //! @@ -93,20 +95,17 @@ #![deny(missing_docs)] #![cfg_attr(docsrs, feature(doc_cfg), deny(rustdoc::broken_intra_doc_links))] -use metrics::{ - Counter, Gauge, Histogram, Key, KeyName, Label, Metadata, Recorder, SharedString, Unit, -}; +use metrics::{Counter, Gauge, Histogram, Key, KeyName, Metadata, Recorder, SharedString, Unit}; use metrics_util::layers::Layer; pub mod label_filter; mod tracing_integration; pub use label_filter::LabelFilter; -use tracing_integration::WithContext; pub use tracing_integration::{Labels, MetricsLayer}; +use tracing_integration::{Map, WithContext}; -/// [`TracingContextLayer`] provides an implementation of a [`Layer`][metrics_util::layers::Layer] -/// for [`TracingContext`]. +/// [`TracingContextLayer`] provides an implementation of a [`Layer`] for [`TracingContext`]. pub struct TracingContextLayer { label_filter: F, } @@ -172,22 +171,20 @@ where // We're currently within a live tracing span, so see if we have an available // metrics context to grab any fields/labels out of. if let Some(ctx) = dispatch.downcast_ref::() { - let mut f = |new_labels: &[Label]| { - if !new_labels.is_empty() { + let mut f = |new_labels: &Map| { + (!new_labels.is_empty()).then(|| { let (name, mut labels) = key.clone().into_parts(); let filtered_labels = new_labels - .iter() + .values() .filter(|label| { self.label_filter.should_include_label(&name, label) }) .cloned(); labels.extend(filtered_labels); - Some(Key::from_parts(name, labels)) - } else { - None - } + Key::from_parts(name, labels) + }) }; // Pull in the span's fields/labels if they exist. diff --git a/metrics-tracing-context/src/tracing_integration.rs b/metrics-tracing-context/src/tracing_integration.rs index 98d32321..ca1a58b2 100644 --- a/metrics-tracing-context/src/tracing_integration.rs +++ b/metrics-tracing-context/src/tracing_integration.rs @@ -1,28 +1,37 @@ //! The code that integrates with the `tracing` crate. +use indexmap::IndexMap; use lockfree_object_pool::{LinearObjectPool, LinearOwnedReusable}; -use metrics::{Key, Label}; +use metrics::{Key, Label, SharedString}; use once_cell::sync::OnceCell; use std::sync::Arc; -use std::{any::TypeId, marker::PhantomData}; +use std::{any::TypeId, cmp, marker::PhantomData}; use tracing_core::span::{Attributes, Id, Record}; use tracing_core::{field::Visit, Dispatch, Field, Subscriber}; use tracing_subscriber::{layer::Context, registry::LookupSpan, Layer}; -fn get_pool() -> &'static Arc>> { - static POOL: OnceCell>>> = OnceCell::new(); - POOL.get_or_init(|| Arc::new(LinearObjectPool::new(Vec::new, Vec::clear))) +pub(crate) type Map = IndexMap; + +fn get_pool() -> &'static Arc> { + static POOL: OnceCell>> = OnceCell::new(); + POOL.get_or_init(|| Arc::new(LinearObjectPool::new(Map::new, Map::clear))) } + /// Span fields mapped as metrics labels. /// /// Hidden from documentation as there is no need for end users to ever touch this type, but it must /// be public in order to be pulled in by external benchmark code. #[doc(hidden)] -pub struct Labels(pub LinearOwnedReusable>); +pub struct Labels(pub LinearOwnedReusable); impl Labels { pub(crate) fn extend_from_labels(&mut self, other: &Labels) { - self.0.extend_from_slice(other.as_ref()); + let new_len = cmp::max(self.as_ref().len(), other.as_ref().len()); + let additional = new_len - self.as_ref().len(); + self.0.reserve(additional); + for (k, v) in other.as_ref() { + self.0.insert(k.clone(), v.clone()); + } } } @@ -35,32 +44,32 @@ impl Default for Labels { impl Visit for Labels { fn record_str(&mut self, field: &Field, value: &str) { let label = Label::new(field.name(), value.to_string()); - self.0.push(label); + self.0.insert(field.name().into(), label); } fn record_bool(&mut self, field: &Field, value: bool) { let label = Label::from_static_parts(field.name(), if value { "true" } else { "false" }); - self.0.push(label); + self.0.insert(field.name().into(), label); } fn record_i64(&mut self, field: &Field, value: i64) { let mut buf = itoa::Buffer::new(); let s = buf.format(value); let label = Label::new(field.name(), s.to_string()); - self.0.push(label); + self.0.insert(field.name().into(), label); } fn record_u64(&mut self, field: &Field, value: u64) { let mut buf = itoa::Buffer::new(); let s = buf.format(value); let label = Label::new(field.name(), s.to_string()); - self.0.push(label); + self.0.insert(field.name().into(), label); } fn record_debug(&mut self, field: &Field, value: &dyn std::fmt::Debug) { - let value_string = format!("{:?}", value); + let value_string = format!("{value:?}"); let label = Label::new(field.name(), value_string); - self.0.push(label); + self.0.insert(field.name().into(), label); } } @@ -72,8 +81,8 @@ impl Labels { } } -impl AsRef<[Label]> for Labels { - fn as_ref(&self) -> &[Label] { +impl AsRef for Labels { + fn as_ref(&self) -> &Map { &self.0 } } @@ -87,7 +96,7 @@ impl WithContext { &self, dispatch: &Dispatch, id: &Id, - f: &mut dyn FnMut(&[Label]) -> Option, + f: &mut dyn FnMut(&Map) -> Option, ) -> Option { let mut ff = |labels: &Labels| f(labels.as_ref()); (self.with_labels)(dispatch, id, &mut ff) diff --git a/metrics-tracing-context/tests/integration.rs b/metrics-tracing-context/tests/integration.rs index a89e2171..3cae15be 100644 --- a/metrics-tracing-context/tests/integration.rs +++ b/metrics-tracing-context/tests/integration.rs @@ -18,6 +18,11 @@ static USER_EMAIL: &[Label] = &[ Label::from_static_parts("user", "ferris"), Label::from_static_parts("user.email", "ferris@rust-lang.org"), ]; +static USER_EMAIL_ATTEMPT: &[Label] = &[ + Label::from_static_parts("user", "ferris"), + Label::from_static_parts("user.email", "ferris@rust-lang.org"), + Label::from_static_parts("attempt", "42"), +]; static USER_ID: &[Label] = &[Label::from_static_parts("user.id", "42")]; static EMAIL_USER: &[Label] = &[ Label::from_static_parts("user.email", "ferris@rust-lang.org"), @@ -50,10 +55,9 @@ static SVC_NODE_USER_EMAIL: &[Label] = &[ Label::from_static_parts("user.email", "ferris@rust-lang.org"), ]; static COMBINED_LABELS: &[Label] = &[ - Label::from_static_parts("shared_field", "inner"), + Label::from_static_parts("shared_field", "outer"), 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"), ]; @@ -209,8 +213,7 @@ fn test_basic_functionality_then_record() { #[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")]; + static USER_ID_123: &[Label] = &[Label::from_static_parts("user.id", "123")]; let (_guard, snapshotter) = setup(TracingContextLayer::all()); @@ -250,6 +253,42 @@ fn test_rerecord() { ); } +#[test] +fn test_loop() { + let (_guard, snapshotter) = setup(TracingContextLayer::all()); + + let user = "ferris"; + let email = "ferris@rust-lang.org"; + let span = span!( + Level::TRACE, + "login", + user, + user.email = email, + attempt = tracing_core::field::Empty, + ); + let _guard = span.enter(); + + for attempt in 1..=42 { + span.record("attempt", attempt); + } + 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_EMAIL_ATTEMPT) + ), + None, + None, + DebugValue::Counter(1), + )] + ); +} + #[test] fn test_macro_forms() { let (_guard, snapshotter) = setup(TracingContextLayer::all()); @@ -345,7 +384,7 @@ fn test_no_labels() { fn test_no_labels_record() { let (_guard, snapshotter) = setup(TracingContextLayer::all()); - let span = span!(Level::TRACE, "login", user.id = tracing_core::field::Empty,); + let span = span!(Level::TRACE, "login", user.id = tracing_core::field::Empty); let _guard = span.enter(); span.record("user.id", 42); From cbdc89d4b1e2ba0b5d9c5aa288309c95670e7ee6 Mon Sep 17 00:00:00 2001 From: zohnannor Date: Mon, 13 Nov 2023 14:46:38 +0400 Subject: [PATCH 07/15] Small correction --- metrics-tracing-context/src/tracing_integration.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/metrics-tracing-context/src/tracing_integration.rs b/metrics-tracing-context/src/tracing_integration.rs index ca1a58b2..edcb8fbd 100644 --- a/metrics-tracing-context/src/tracing_integration.rs +++ b/metrics-tracing-context/src/tracing_integration.rs @@ -131,8 +131,7 @@ where .expect("subscriber should downcast to expected type; this is a bug!"); let span = subscriber.span(id).expect("registry should have a span for the current ID"); - let result = - if let Some(labels) = span.extensions().get::() { f(labels) } else { None }; + let result = span.extensions().get::().and_then(f); result } } From e470aa738bdca108bc3f56cc54fa890b1b38f15b Mon Sep 17 00:00:00 2001 From: zohnannor Date: Mon, 13 Nov 2023 14:46:46 +0400 Subject: [PATCH 08/15] Mention in CHANGELOG.md --- metrics-tracing-context/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/metrics-tracing-context/CHANGELOG.md b/metrics-tracing-context/CHANGELOG.md index 389f8da2..63ed2d4d 100644 --- a/metrics-tracing-context/CHANGELOG.md +++ b/metrics-tracing-context/CHANGELOG.md @@ -8,6 +8,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] - ReleaseDate +### Added + +- Support for dynamism using `tracing::Span::record` to add label values. ([#408](https://github.com/metrics-rs/metrics/pull/408)) + ## [0.14.0] - 2023-04-16 ### Changed From 1439f89db01e297c45d2d9540ff264e64f259ad0 Mon Sep 17 00:00:00 2001 From: zohnannor Date: Mon, 13 Nov 2023 20:16:27 +0400 Subject: [PATCH 09/15] Post-review corrections --- metrics-tracing-context/Cargo.toml | 2 +- metrics-tracing-context/benches/visit.rs | 4 +-- metrics-tracing-context/src/lib.rs | 32 +++++++++++-------- .../src/tracing_integration.rs | 20 ++++++------ 4 files changed, 32 insertions(+), 26 deletions(-) diff --git a/metrics-tracing-context/Cargo.toml b/metrics-tracing-context/Cargo.toml index 8160af84..915a60d3 100644 --- a/metrics-tracing-context/Cargo.toml +++ b/metrics-tracing-context/Cargo.toml @@ -32,7 +32,7 @@ itoa = { version = "1", default-features = false } metrics = { version = "^0.21", path = "../metrics" } metrics-util = { version = "^0.15", path = "../metrics-util" } lockfree-object-pool = { version = "0.1.3", default-features = false } -indexmap = "2.1" +indexmap = { version = "2.1", default-features = false, features = ["std"] } once_cell = { version = "1", default-features = false, features = ["std"] } tracing = { version = "0.1.29", default-features = false } tracing-core = { version = "0.1.21", default-features = false } diff --git a/metrics-tracing-context/benches/visit.rs b/metrics-tracing-context/benches/visit.rs index 5f806700..87d2e75d 100644 --- a/metrics-tracing-context/benches/visit.rs +++ b/metrics-tracing-context/benches/visit.rs @@ -3,7 +3,7 @@ use std::sync::Arc; use criterion::{criterion_group, criterion_main, BatchSize, Criterion}; use indexmap::IndexMap; use lockfree_object_pool::LinearObjectPool; -use metrics::{Label, SharedString}; +use metrics::Label; use metrics_tracing_context::Labels; use once_cell::sync::OnceCell; use tracing::Metadata; @@ -14,7 +14,7 @@ use tracing_core::{ Callsite, Interest, }; -pub(crate) type Map = IndexMap; +type Map = IndexMap<&'static str, Label>; fn get_pool() -> &'static Arc> { static POOL: OnceCell>> = OnceCell::new(); diff --git a/metrics-tracing-context/src/lib.rs b/metrics-tracing-context/src/lib.rs index 7fb9c362..f2e4d9b1 100644 --- a/metrics-tracing-context/src/lib.rs +++ b/metrics-tracing-context/src/lib.rs @@ -55,19 +55,25 @@ //! //! # Implementation //! -//! 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. -//! -//! Be aware that we store all fields that a span has, including the fields of its parent span(s). -//! -//! ## Support for dynamism -//! -//! 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. Multiple records of the -//! same field would overwrite it, leaving the most recent value. This way you can change any of -//! the metrics keys coming from span fields. +//! The integration layer works by capturing all fields that are present when a span is created, +//! as well as fields recorded after the fact, and storing them as an extension to the span. If +//! a metric is emitted while a span is entered, any fields captured for that span will be added +//! to the metric as additional labels. +//! +//! Be aware that we recursively capture the fields of a span, including fields from +//! parent spans, and use them when generating metric labels. This means that if a metric is being +//! emitted in span B, which is a child of span A, and span A has field X, and span B has field Y, +//! then the metric labels will include both field X and Y. This applies regardless of how many +//! nested spans are currently entered. +//! +//! ## Duplicate span fields +//! +//! When span fields are captured, they are deduplicated such that only the most recent value is kept. +//! For merging parent span fields into the current span fields, the fields from the current span have +//! the highest priority. Additionally, when using [`Span::record`][tracing::Span::record] to add fields +//! to a span after it has been created, the same behavior applies. This means that recording a field +//! multiple times only keeps the most recently recorded value, including if a field was already present +//! from a parent span and is then recorded dynamically in the current span. //! //! ## Span fields and ancestry //! diff --git a/metrics-tracing-context/src/tracing_integration.rs b/metrics-tracing-context/src/tracing_integration.rs index edcb8fbd..4db442b8 100644 --- a/metrics-tracing-context/src/tracing_integration.rs +++ b/metrics-tracing-context/src/tracing_integration.rs @@ -2,7 +2,7 @@ use indexmap::IndexMap; use lockfree_object_pool::{LinearObjectPool, LinearOwnedReusable}; -use metrics::{Key, Label, SharedString}; +use metrics::{Key, Label}; use once_cell::sync::OnceCell; use std::sync::Arc; use std::{any::TypeId, cmp, marker::PhantomData}; @@ -10,7 +10,7 @@ use tracing_core::span::{Attributes, Id, Record}; use tracing_core::{field::Visit, Dispatch, Field, Subscriber}; use tracing_subscriber::{layer::Context, registry::LookupSpan, Layer}; -pub(crate) type Map = IndexMap; +pub(crate) type Map = IndexMap<&'static str, Label>; fn get_pool() -> &'static Arc> { static POOL: OnceCell>> = OnceCell::new(); @@ -30,7 +30,7 @@ impl Labels { let additional = new_len - self.as_ref().len(); self.0.reserve(additional); for (k, v) in other.as_ref() { - self.0.insert(k.clone(), v.clone()); + self.0.insert(k, v.clone()); } } } @@ -44,32 +44,32 @@ impl Default for Labels { impl Visit for Labels { fn record_str(&mut self, field: &Field, value: &str) { let label = Label::new(field.name(), value.to_string()); - self.0.insert(field.name().into(), label); + self.0.insert(field.name(), label); } fn record_bool(&mut self, field: &Field, value: bool) { let label = Label::from_static_parts(field.name(), if value { "true" } else { "false" }); - self.0.insert(field.name().into(), label); + self.0.insert(field.name(), label); } fn record_i64(&mut self, field: &Field, value: i64) { let mut buf = itoa::Buffer::new(); let s = buf.format(value); let label = Label::new(field.name(), s.to_string()); - self.0.insert(field.name().into(), label); + self.0.insert(field.name(), label); } fn record_u64(&mut self, field: &Field, value: u64) { let mut buf = itoa::Buffer::new(); let s = buf.format(value); let label = Label::new(field.name(), s.to_string()); - self.0.insert(field.name().into(), label); + self.0.insert(field.name(), label); } fn record_debug(&mut self, field: &Field, value: &dyn std::fmt::Debug) { let value_string = format!("{value:?}"); let label = Label::new(field.name(), value_string); - self.0.insert(field.name().into(), label); + self.0.insert(field.name(), label); } } @@ -131,8 +131,8 @@ where .expect("subscriber should downcast to expected type; this is a bug!"); let span = subscriber.span(id).expect("registry should have a span for the current ID"); - let result = span.extensions().get::().and_then(f); - result + let ext = span.extensions(); + ext.get::().and_then(f) } } From 77f02bfd405077dcc1cc468dc77a8c0d1f16d68a Mon Sep 17 00:00:00 2001 From: zohnannor Date: Thu, 16 Nov 2023 19:58:42 +0400 Subject: [PATCH 10/15] Fix label overwrites --- metrics-tracing-context/Cargo.toml | 1 + metrics-tracing-context/benches/visit.rs | 4 +- metrics-tracing-context/src/lib.rs | 51 +++++++++---------- .../src/tracing_integration.rs | 28 ++++------ metrics-tracing-context/tests/integration.rs | 45 ++++++++++++---- 5 files changed, 74 insertions(+), 55 deletions(-) diff --git a/metrics-tracing-context/Cargo.toml b/metrics-tracing-context/Cargo.toml index 915a60d3..1b6cdc68 100644 --- a/metrics-tracing-context/Cargo.toml +++ b/metrics-tracing-context/Cargo.toml @@ -43,3 +43,4 @@ criterion = { version = "=0.3.3", default-features = false } parking_lot = { version = "0.12.1", default-features = false } tracing = { version = "0.1.29", default-features = false, features = ["std"] } tracing-subscriber = { version = "0.3.1", default-features = false, features = ["registry"] } +pretty_assertions = "*" \ No newline at end of file diff --git a/metrics-tracing-context/benches/visit.rs b/metrics-tracing-context/benches/visit.rs index 87d2e75d..9b6db089 100644 --- a/metrics-tracing-context/benches/visit.rs +++ b/metrics-tracing-context/benches/visit.rs @@ -3,7 +3,7 @@ use std::sync::Arc; use criterion::{criterion_group, criterion_main, BatchSize, Criterion}; use indexmap::IndexMap; use lockfree_object_pool::LinearObjectPool; -use metrics::Label; +use metrics::SharedString; use metrics_tracing_context::Labels; use once_cell::sync::OnceCell; use tracing::Metadata; @@ -14,7 +14,7 @@ use tracing_core::{ Callsite, Interest, }; -type Map = IndexMap<&'static str, Label>; +type Map = IndexMap; fn get_pool() -> &'static Arc> { static POOL: OnceCell>> = OnceCell::new(); diff --git a/metrics-tracing-context/src/lib.rs b/metrics-tracing-context/src/lib.rs index f2e4d9b1..bc7078e7 100644 --- a/metrics-tracing-context/src/lib.rs +++ b/metrics-tracing-context/src/lib.rs @@ -101,7 +101,9 @@ #![deny(missing_docs)] #![cfg_attr(docsrs, feature(doc_cfg), deny(rustdoc::broken_intra_doc_links))] -use metrics::{Counter, Gauge, Histogram, Key, KeyName, Metadata, Recorder, SharedString, Unit}; +use metrics::{ + Counter, Gauge, Histogram, Key, KeyName, Label, Metadata, Recorder, SharedString, Unit, +}; use metrics_util::layers::Layer; pub mod label_filter; @@ -173,32 +175,27 @@ where // doing the iteration would likely exceed the cost of simply constructing the new key. tracing::dispatcher::get_default(|dispatch| { let current = dispatch.current_span(); - if let Some(id) = current.id() { - // We're currently within a live tracing span, so see if we have an available - // metrics context to grab any fields/labels out of. - if let Some(ctx) = dispatch.downcast_ref::() { - let mut f = |new_labels: &Map| { - (!new_labels.is_empty()).then(|| { - let (name, mut labels) = key.clone().into_parts(); - - let filtered_labels = new_labels - .values() - .filter(|label| { - self.label_filter.should_include_label(&name, label) - }) - .cloned(); - labels.extend(filtered_labels); - - Key::from_parts(name, labels) - }) - }; - - // Pull in the span's fields/labels if they exist. - return ctx.with_labels(dispatch, id, &mut f); - } - } - - None + let id = current.id()?; + let ctx = dispatch.downcast_ref::()?; + + let mut f = |mut span_labels: Map| { + (!span_labels.is_empty()).then(|| { + let (name, labels) = key.clone().into_parts(); + + span_labels.extend(labels.into_iter().map(Label::into_parts)); + + let labels = span_labels + .into_iter() + .map(|(key, value)| Label::new(key.clone(), value.clone())) + .filter(|label| self.label_filter.should_include_label(&name, label)) + .collect::>(); + + Key::from_parts(name, labels) + }) + }; + + // Pull in the span's fields/labels if they exist. + ctx.with_labels(dispatch, id, &mut f) }) } } diff --git a/metrics-tracing-context/src/tracing_integration.rs b/metrics-tracing-context/src/tracing_integration.rs index 4db442b8..9f846de4 100644 --- a/metrics-tracing-context/src/tracing_integration.rs +++ b/metrics-tracing-context/src/tracing_integration.rs @@ -2,7 +2,7 @@ use indexmap::IndexMap; use lockfree_object_pool::{LinearObjectPool, LinearOwnedReusable}; -use metrics::{Key, Label}; +use metrics::{Key, SharedString}; use once_cell::sync::OnceCell; use std::sync::Arc; use std::{any::TypeId, cmp, marker::PhantomData}; @@ -10,7 +10,7 @@ use tracing_core::span::{Attributes, Id, Record}; use tracing_core::{field::Visit, Dispatch, Field, Subscriber}; use tracing_subscriber::{layer::Context, registry::LookupSpan, Layer}; -pub(crate) type Map = IndexMap<&'static str, Label>; +pub(crate) type Map = IndexMap; fn get_pool() -> &'static Arc> { static POOL: OnceCell>> = OnceCell::new(); @@ -30,7 +30,7 @@ impl Labels { let additional = new_len - self.as_ref().len(); self.0.reserve(additional); for (k, v) in other.as_ref() { - self.0.insert(k, v.clone()); + self.0.insert(k.clone(), v.clone()); } } } @@ -43,33 +43,27 @@ impl Default for Labels { impl Visit for Labels { fn record_str(&mut self, field: &Field, value: &str) { - let label = Label::new(field.name(), value.to_string()); - self.0.insert(field.name(), label); + self.0.insert(field.name().into(), value.to_owned().into()); } fn record_bool(&mut self, field: &Field, value: bool) { - let label = Label::from_static_parts(field.name(), if value { "true" } else { "false" }); - self.0.insert(field.name(), label); + self.0.insert(field.name().into(), if value { "true" } else { "false" }.into()); } fn record_i64(&mut self, field: &Field, value: i64) { let mut buf = itoa::Buffer::new(); let s = buf.format(value); - let label = Label::new(field.name(), s.to_string()); - self.0.insert(field.name(), label); + self.0.insert(field.name().into(), s.to_owned().into()); } fn record_u64(&mut self, field: &Field, value: u64) { let mut buf = itoa::Buffer::new(); let s = buf.format(value); - let label = Label::new(field.name(), s.to_string()); - self.0.insert(field.name(), label); + self.0.insert(field.name().into(), s.to_owned().into()); } fn record_debug(&mut self, field: &Field, value: &dyn std::fmt::Debug) { - let value_string = format!("{value:?}"); - let label = Label::new(field.name(), value_string); - self.0.insert(field.name(), label); + self.0.insert(field.name().into(), format!("{value:?}").into()); } } @@ -96,9 +90,9 @@ impl WithContext { &self, dispatch: &Dispatch, id: &Id, - f: &mut dyn FnMut(&Map) -> Option, + f: &mut dyn FnMut(Map) -> Option, ) -> Option { - let mut ff = |labels: &Labels| f(labels.as_ref()); + let mut ff = |labels: &Labels| f(labels.0.clone()); (self.with_labels)(dispatch, id, &mut ff) } } @@ -132,7 +126,7 @@ where let span = subscriber.span(id).expect("registry should have a span for the current ID"); let ext = span.extensions(); - ext.get::().and_then(f) + f(ext.get::()?) } } diff --git a/metrics-tracing-context/tests/integration.rs b/metrics-tracing-context/tests/integration.rs index 3cae15be..d5d0792e 100644 --- a/metrics-tracing-context/tests/integration.rs +++ b/metrics-tracing-context/tests/integration.rs @@ -7,6 +7,8 @@ use tracing::dispatcher::{set_default, DefaultGuard, Dispatch}; use tracing::{span, Level}; use tracing_subscriber::{layer::SubscriberExt, Registry}; +use pretty_assertions::assert_eq; + static TEST_MUTEX: Mutex<()> = const_mutex(()); static LOGIN_ATTEMPTS: &str = "login_attempts"; static LOGIN_ATTEMPTS_NONE: &str = "login_attempts_no_labels"; @@ -24,35 +26,32 @@ static USER_EMAIL_ATTEMPT: &[Label] = &[ Label::from_static_parts("attempt", "42"), ]; static USER_ID: &[Label] = &[Label::from_static_parts("user.id", "42")]; -static EMAIL_USER: &[Label] = &[ - Label::from_static_parts("user.email", "ferris@rust-lang.org"), - Label::from_static_parts("user", "ferris"), -]; +static EMAIL_USER: &[Label] = &[Label::from_static_parts("user", "ferris")]; static SVC_ENV: &[Label] = &[ Label::from_static_parts("service", "login_service"), Label::from_static_parts("env", "test"), ]; static SVC_USER_EMAIL: &[Label] = &[ - Label::from_static_parts("service", "login_service"), Label::from_static_parts("user", "ferris"), Label::from_static_parts("user.email", "ferris@rust-lang.org"), + Label::from_static_parts("service", "login_service"), ]; 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", "ferris@rust-lang.org"), Label::from_static_parts("user.id", "42"), + Label::from_static_parts("service", "login_service"), ]; static NODE_USER_EMAIL: &[Label] = &[ - Label::from_static_parts("node_name", "localhost"), Label::from_static_parts("user", "ferris"), Label::from_static_parts("user.email", "ferris@rust-lang.org"), + Label::from_static_parts("node_name", "localhost"), ]; 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", "ferris@rust-lang.org"), + Label::from_static_parts("service", "login_service"), + Label::from_static_parts("node_name", "localhost"), ]; static COMBINED_LABELS: &[Label] = &[ Label::from_static_parts("shared_field", "outer"), @@ -289,6 +288,34 @@ fn test_loop() { ); } +#[test] +fn test_record_does_not_overwrite() { + static USER_ID_42: &[Label] = &[Label::from_static_parts("user.id", "42")]; + + 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", 666); + counter!("login_attempts", "user.id" => "42").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), + )] + ); +} + #[test] fn test_macro_forms() { let (_guard, snapshotter) = setup(TracingContextLayer::all()); From 4cb3de9fdcab68a42eb1d07cdb31d1dec966f930 Mon Sep 17 00:00:00 2001 From: zohnannor Date: Thu, 16 Nov 2023 23:31:35 +0400 Subject: [PATCH 11/15] Correction --- metrics-tracing-context/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/metrics-tracing-context/src/lib.rs b/metrics-tracing-context/src/lib.rs index bc7078e7..f13a4a95 100644 --- a/metrics-tracing-context/src/lib.rs +++ b/metrics-tracing-context/src/lib.rs @@ -186,7 +186,7 @@ where let labels = span_labels .into_iter() - .map(|(key, value)| Label::new(key.clone(), value.clone())) + .map(|(key, value)| Label::new(key, value)) .filter(|label| self.label_filter.should_include_label(&name, label)) .collect::>(); From 5ca2f14a503af845a601cb50ae3d0e55969da499 Mon Sep 17 00:00:00 2001 From: zohnannor Date: Wed, 22 Nov 2023 20:37:15 +0400 Subject: [PATCH 12/15] Tests --- metrics-tracing-context/Cargo.toml | 6 +- metrics-tracing-context/src/lib.rs | 4 +- .../src/tracing_integration.rs | 103 ++++----- metrics-tracing-context/tests/integration.rs | 215 +++++++++++++++++- 4 files changed, 263 insertions(+), 65 deletions(-) diff --git a/metrics-tracing-context/Cargo.toml b/metrics-tracing-context/Cargo.toml index 1b6cdc68..bf66df50 100644 --- a/metrics-tracing-context/Cargo.toml +++ b/metrics-tracing-context/Cargo.toml @@ -37,10 +37,12 @@ once_cell = { version = "1", default-features = false, features = ["std"] } tracing = { version = "0.1.29", default-features = false } tracing-core = { version = "0.1.21", default-features = false } tracing-subscriber = { version = "0.3.1", default-features = false, features = ["std"] } +tracing-test = "0.2.4" [dev-dependencies] criterion = { version = "=0.3.3", default-features = false } parking_lot = { version = "0.12.1", default-features = false } tracing = { version = "0.1.29", default-features = false, features = ["std"] } -tracing-subscriber = { version = "0.3.1", default-features = false, features = ["registry"] } -pretty_assertions = "*" \ No newline at end of file +tracing-subscriber = { version = "0.3.1", default-features = false, features = ["registry", "fmt"] } +pretty_assertions = "*" +itertools = "*" diff --git a/metrics-tracing-context/src/lib.rs b/metrics-tracing-context/src/lib.rs index f13a4a95..49c70bc0 100644 --- a/metrics-tracing-context/src/lib.rs +++ b/metrics-tracing-context/src/lib.rs @@ -110,8 +110,8 @@ pub mod label_filter; mod tracing_integration; pub use label_filter::LabelFilter; +use tracing_integration::Map; pub use tracing_integration::{Labels, MetricsLayer}; -use tracing_integration::{Map, WithContext}; /// [`TracingContextLayer`] provides an implementation of a [`Layer`] for [`TracingContext`]. pub struct TracingContextLayer { @@ -176,7 +176,7 @@ where tracing::dispatcher::get_default(|dispatch| { let current = dispatch.current_span(); let id = current.id()?; - let ctx = dispatch.downcast_ref::()?; + let ctx = dispatch.downcast_ref::()?; let mut f = |mut span_labels: Map| { (!span_labels.is_empty()).then(|| { diff --git a/metrics-tracing-context/src/tracing_integration.rs b/metrics-tracing-context/src/tracing_integration.rs index 9f846de4..1cf748ab 100644 --- a/metrics-tracing-context/src/tracing_integration.rs +++ b/metrics-tracing-context/src/tracing_integration.rs @@ -4,8 +4,9 @@ use indexmap::IndexMap; use lockfree_object_pool::{LinearObjectPool, LinearOwnedReusable}; use metrics::{Key, SharedString}; use once_cell::sync::OnceCell; +use std::cmp; use std::sync::Arc; -use std::{any::TypeId, cmp, marker::PhantomData}; +use tracing::span; use tracing_core::span::{Attributes, Id, Record}; use tracing_core::{field::Visit, Dispatch, Field, Subscriber}; use tracing_subscriber::{layer::Context, registry::LookupSpan, Layer}; @@ -24,13 +25,23 @@ fn get_pool() -> &'static Arc> { #[doc(hidden)] pub struct Labels(pub LinearOwnedReusable); +impl std::fmt::Debug for Labels { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_tuple("Labels").field(self.as_ref()).finish() + } +} + impl Labels { - pub(crate) fn extend_from_labels(&mut self, other: &Labels) { + pub(crate) fn extend_from_labels(&mut self, other: &Labels, overwrite: bool) { let new_len = cmp::max(self.as_ref().len(), other.as_ref().len()); let additional = new_len - self.as_ref().len(); self.0.reserve(additional); for (k, v) in other.as_ref() { - self.0.insert(k.clone(), v.clone()); + if overwrite { + self.0.insert(k.clone(), v.clone()); + } else { + self.0.entry(k.clone()).or_insert_with(|| v.clone()); + } } } } @@ -81,66 +92,57 @@ impl AsRef for Labels { } } -pub struct WithContext { - with_labels: fn(&Dispatch, &Id, f: &mut dyn FnMut(&Labels) -> Option) -> Option, -} - -impl WithContext { - pub fn with_labels( - &self, - dispatch: &Dispatch, - id: &Id, - f: &mut dyn FnMut(Map) -> Option, - ) -> Option { - let mut ff = |labels: &Labels| f(labels.0.clone()); - (self.with_labels)(dispatch, id, &mut ff) - } -} - /// [`MetricsLayer`] is a [`tracing_subscriber::Layer`] that captures the span /// fields and allows them to be later on used as metrics labels. -pub struct MetricsLayer { - ctx: WithContext, - _subscriber: PhantomData, +#[derive(Default)] +pub struct MetricsLayer { + with_labels: + Option Option) -> Option>, } -impl MetricsLayer -where - S: Subscriber + for<'span> LookupSpan<'span>, -{ - /// Create a new `MetricsLayer`. +impl MetricsLayer { + /// Create a new [`MetricsLayer`]. pub fn new() -> Self { - let ctx = WithContext { with_labels: Self::with_labels }; - - Self { ctx, _subscriber: PhantomData } + Self::default() } - fn with_labels( + pub(crate) fn with_labels( + &self, dispatch: &Dispatch, id: &Id, - f: &mut dyn FnMut(&Labels) -> Option, + f: &mut dyn FnMut(Map) -> Option, ) -> Option { - let subscriber = dispatch - .downcast_ref::() - .expect("subscriber should downcast to expected type; this is a bug!"); - let span = subscriber.span(id).expect("registry should have a span for the current ID"); - - let ext = span.extensions(); - f(ext.get::()?) + let mut ff = |labels: &Labels| f(labels.0.clone()); + (self.with_labels?)(dispatch, id, &mut ff) } } -impl Layer for MetricsLayer +impl Layer for MetricsLayer where S: Subscriber + for<'a> LookupSpan<'a>, { + fn on_layer(&mut self, _: &mut S) { + self.with_labels = Some( + |dispatch: &Dispatch, id: &span::Id, f: &mut dyn FnMut(&Labels) -> Option| { + let subscriber = dispatch + .downcast_ref::() + .expect("subscriber should downcast to expected type; this is a bug!"); + let span = + subscriber.span(id).expect("registry should have a span for the current ID"); + + let ext = span.extensions(); + f(ext.get::()?) + }, + ); + } + 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_record(&Record::new(attrs.values())); if let Some(parent) = span.parent() { if let Some(parent_labels) = parent.extensions().get::() { - labels.extend_from_labels(parent_labels); + labels.extend_from_labels(parent_labels, false); } } @@ -153,26 +155,9 @@ where let ext = &mut span.extensions_mut(); if let Some(existing) = ext.get_mut::() { - existing.extend_from_labels(&labels); + existing.extend_from_labels(&labels, true); } else { ext.insert(labels); } } - - unsafe fn downcast_raw(&self, id: TypeId) -> Option<*const ()> { - match id { - id if id == TypeId::of::() => Some(self as *const _ as *const ()), - id if id == TypeId::of::() => Some(&self.ctx as *const _ as *const ()), - _ => None, - } - } -} - -impl Default for MetricsLayer -where - S: Subscriber + for<'span> LookupSpan<'span>, -{ - fn default() -> Self { - MetricsLayer::new() - } } diff --git a/metrics-tracing-context/tests/integration.rs b/metrics-tracing-context/tests/integration.rs index d5d0792e..3871873a 100644 --- a/metrics-tracing-context/tests/integration.rs +++ b/metrics-tracing-context/tests/integration.rs @@ -1,4 +1,9 @@ -use metrics::{counter, Key, KeyName, Label}; +#![deny(unreachable_patterns)] + +use std::panic; + +use itertools::Itertools as _; +use metrics::{counter, Key, KeyName, Label, SharedString}; use metrics_tracing_context::{LabelFilter, MetricsLayer, TracingContextLayer}; use metrics_util::debugging::{DebugValue, DebuggingRecorder, Snapshotter}; use metrics_util::{layers::Layer, CompositeKey, MetricKind}; @@ -54,7 +59,7 @@ static SVC_NODE_USER_EMAIL: &[Label] = &[ Label::from_static_parts("node_name", "localhost"), ]; static COMBINED_LABELS: &[Label] = &[ - Label::from_static_parts("shared_field", "outer"), + 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("outer_specific", "bar"), @@ -609,3 +614,209 @@ fn test_label_allowlist() { )] ); } + +#[tracing_test::traced_test] +#[test] +fn test_all_permutations() { + let perms = (0..9).map(|_| [false, true]).multi_cartesian_product(); + + for v in perms { + let &[metric_has_labels, in_span, span_has_fields, span_field_same_as_metric, span_has_parent, parent_field_same_as_span, span_field_is_empty, record_field, emit_before_recording] = + &*v + else { + unreachable!("{:?}, {}", v, v.len()); + }; + + test( + metric_has_labels, + in_span, + span_has_fields, + span_field_same_as_metric, + span_has_parent, + parent_field_same_as_span, + span_field_is_empty, + record_field, + emit_before_recording, + ); + } +} + +#[allow(clippy::fn_params_excessive_bools, clippy::too_many_arguments, clippy::too_many_lines)] +fn test( + metric_has_labels: bool, + in_span: bool, + span_has_fields: bool, + span_field_same_as_metric: bool, + span_has_parent: bool, + parent_field_same_as_span: bool, + span_field_is_empty: bool, + record_field: bool, + emit_before_recording: bool, +) { + let (_guard, snapshotter) = setup(TracingContextLayer::all()); + + let parent = if span_field_same_as_metric && parent_field_same_as_span { + tracing::trace_span!("outer", user.email = "changed@domain.com") + } else { + tracing::trace_span!("outer", user.id = 999) + }; + + let _parent_guard = span_has_parent.then(|| parent.enter()); + + let span = if span_has_fields { + match (span_field_same_as_metric, span_field_is_empty) { + (false, false) => tracing::trace_span!("login", user.id = 666), + (false, true) => tracing::trace_span!("login", user.id = tracing_core::field::Empty), + (true, false) => tracing::trace_span!("login", user.email = "user@domain.com"), + (true, true) => tracing::trace_span!("login", user.email = tracing_core::field::Empty), + } + } else { + tracing::trace_span!("login") + }; + + let _guard = in_span.then(|| span.enter()); + + let inc = || { + if metric_has_labels { + counter!("login_attempts", "user.email" => "ferris@rust-lang.org").increment(1); + } else { + counter!("login_attempts").increment(1); + } + }; + + if emit_before_recording { + inc(); + } + + if record_field { + span.record("user.id", 42); + } + + inc(); + + let snapshot = snapshotter.snapshot().into_vec(); + + let expected1: (CompositeKey, Option, Option, DebugValue) = ( + CompositeKey::new( + MetricKind::Counter, + Key::from_parts( + LOGIN_ATTEMPTS, + IntoIterator::into_iter([ + (metric_has_labels + && in_span + && span_has_fields + && span_field_same_as_metric + && span_has_parent + && !span_field_is_empty) + .then(|| Label::new("user.email", "ferris@rust-lang.org")), + (in_span + && span_has_fields + && !span_field_same_as_metric + && !span_field_is_empty + && record_field + && emit_before_recording) + .then(|| Label::new("user.id", "666")), + (in_span + && span_has_fields + && !span_field_same_as_metric + && span_has_parent + && span_field_is_empty + && record_field + && emit_before_recording) + .then(|| Label::new("user.id", "999")), + (metric_has_labels + && !(in_span + && span_has_fields + && span_field_same_as_metric + && span_has_parent + && !span_field_is_empty)) + .then(|| Label::new("user.email", "ferris@rust-lang.org")), + ]) + .flatten() + .collect::>(), + ), + ), + None, + None, + DebugValue::Counter(1), + ); + + let labels2 = IntoIterator::into_iter([ + (metric_has_labels + && in_span + && span_has_fields + && span_field_same_as_metric + && span_has_parent + && !span_field_is_empty) + .then(|| Label::new("user.email", "ferris@rust-lang.org")), + (in_span && span_has_fields) + .then(|| { + match ( + metric_has_labels, + span_field_same_as_metric, + span_field_is_empty, + record_field, + ) { + (_, false, _, true) => Some(Label::new("user.id", "42")), + (_, false, false, false) => Some(Label::new("user.id", "666")), + (false, true, false, _) => Some(Label::new("user.email", "user@domain.com")), + _ => None, + } + }) + .flatten(), + if span_has_parent + && (!in_span || !span_has_fields || span_field_is_empty) + && parent_field_same_as_span + && span_field_same_as_metric + && !metric_has_labels + { + Some(Label::new("user.email", "changed@domain.com")) + } else if !span_has_parent + || span_field_same_as_metric && metric_has_labels && parent_field_same_as_span + || in_span + && span_has_fields + && ((parent_field_same_as_span || !span_field_same_as_metric) + && !span_field_is_empty + || !span_field_same_as_metric && record_field) + { + None + } else { + Some(Label::new("user.id", "999")) + }, + (metric_has_labels + && !(in_span + && span_has_fields + && span_field_same_as_metric + && span_has_parent + && !span_field_is_empty)) + .then(|| Label::new("user.email", "ferris@rust-lang.org")), + ]) + .flatten() + .collect::>(); + + let expected2 = ( + CompositeKey::new(MetricKind::Counter, Key::from_parts(LOGIN_ATTEMPTS, labels2)), + None, + None, + DebugValue::Counter( + if !emit_before_recording + || in_span && span_has_fields && !span_field_same_as_metric && record_field + { + 1 + } else { + 2 + }, + ), + ); + let expected: Vec<_> = (in_span + && span_has_fields + && !span_field_same_as_metric + && record_field + && emit_before_recording) + .then(|| expected1) + .into_iter() + .chain([expected2]) + .collect(); + // let expected = vec![expected2]; + assert_eq!(snapshot, expected); +} From d28e1692354d705d93a8fa93b4f1959c143ffc75 Mon Sep 17 00:00:00 2001 From: zohnannor Date: Thu, 23 Nov 2023 14:01:53 +0400 Subject: [PATCH 13/15] WIP --- .../src/tracing_integration.rs | 30 +-- metrics-tracing-context/tests/integration.rs | 190 ++++++++---------- 2 files changed, 101 insertions(+), 119 deletions(-) diff --git a/metrics-tracing-context/src/tracing_integration.rs b/metrics-tracing-context/src/tracing_integration.rs index 1cf748ab..2720f4e1 100644 --- a/metrics-tracing-context/src/tracing_integration.rs +++ b/metrics-tracing-context/src/tracing_integration.rs @@ -25,25 +25,27 @@ fn get_pool() -> &'static Arc> { #[doc(hidden)] pub struct Labels(pub LinearOwnedReusable); -impl std::fmt::Debug for Labels { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_tuple("Labels").field(self.as_ref()).finish() - } -} - impl Labels { - pub(crate) fn extend_from_labels(&mut self, other: &Labels, overwrite: bool) { + fn extend(&mut self, other: &Labels, f: impl Fn(&mut Map, &SharedString, &SharedString)) { let new_len = cmp::max(self.as_ref().len(), other.as_ref().len()); let additional = new_len - self.as_ref().len(); self.0.reserve(additional); for (k, v) in other.as_ref() { - if overwrite { - self.0.insert(k.clone(), v.clone()); - } else { - self.0.entry(k.clone()).or_insert_with(|| v.clone()); - } + f(&mut self.0, k, v); } } + + fn extend_from_labels(&mut self, other: &Labels) { + self.extend(other, |map, k, v| { + map.entry(k.clone()).or_insert_with(|| v.clone()); + }); + } + + fn extend_from_labels_overwrite(&mut self, other: &Labels) { + self.extend(other, |map, k, v| { + map.insert(k.clone(), v.clone()); + }); + } } impl Default for Labels { @@ -142,7 +144,7 @@ where if let Some(parent) = span.parent() { if let Some(parent_labels) = parent.extensions().get::() { - labels.extend_from_labels(parent_labels, false); + labels.extend_from_labels(parent_labels); } } @@ -155,7 +157,7 @@ where let ext = &mut span.extensions_mut(); if let Some(existing) = ext.get_mut::() { - existing.extend_from_labels(&labels, true); + existing.extend_from_labels_overwrite(&labels); } else { ext.insert(labels); } diff --git a/metrics-tracing-context/tests/integration.rs b/metrics-tracing-context/tests/integration.rs index 3871873a..f7293ea7 100644 --- a/metrics-tracing-context/tests/integration.rs +++ b/metrics-tracing-context/tests/integration.rs @@ -1,9 +1,5 @@ -#![deny(unreachable_patterns)] - -use std::panic; - use itertools::Itertools as _; -use metrics::{counter, Key, KeyName, Label, SharedString}; +use metrics::{counter, Key, KeyName, Label}; use metrics_tracing_context::{LabelFilter, MetricsLayer, TracingContextLayer}; use metrics_util::debugging::{DebugValue, DebuggingRecorder, Snapshotter}; use metrics_util::{layers::Layer, CompositeKey, MetricKind}; @@ -621,8 +617,8 @@ fn test_all_permutations() { let perms = (0..9).map(|_| [false, true]).multi_cartesian_product(); for v in perms { - let &[metric_has_labels, in_span, span_has_fields, span_field_same_as_metric, span_has_parent, parent_field_same_as_span, span_field_is_empty, record_field, emit_before_recording] = - &*v + let [metric_has_labels, in_span, span_has_fields, span_field_same_as_metric, span_has_parent, parent_field_same_as_span, span_field_is_empty, record_field, emit_before_recording] = + v[..] else { unreachable!("{:?}, {}", v, v.len()); }; @@ -696,40 +692,93 @@ fn test( let snapshot = snapshotter.snapshot().into_vec(); - let expected1: (CompositeKey, Option, Option, DebugValue) = ( + let mut expected = vec![]; + + let in_both_spans_with_metric_label = in_span + && span_has_fields + && !span_field_is_empty + && span_field_same_as_metric + && span_has_parent; + + if in_span + && span_has_fields + && !span_field_same_as_metric + && record_field + && emit_before_recording + { + expected.push(( + CompositeKey::new( + MetricKind::Counter, + Key::from_parts( + LOGIN_ATTEMPTS, + IntoIterator::into_iter([ + (metric_has_labels && in_both_spans_with_metric_label) + .then(|| Label::new("user.email", "ferris@rust-lang.org")), + (!span_field_is_empty).then(|| Label::new("user.id", "666")), + (span_field_is_empty && span_has_parent) + .then(|| Label::new("user.id", "999")), + (metric_has_labels && !in_both_spans_with_metric_label) + .then(|| Label::new("user.email", "ferris@rust-lang.org")), + ]) + .flatten() + .collect::>(), + ), + ), + None, + None, + DebugValue::Counter(1), + )); + } + + expected.push(( CompositeKey::new( MetricKind::Counter, Key::from_parts( LOGIN_ATTEMPTS, IntoIterator::into_iter([ - (metric_has_labels - && in_span - && span_has_fields - && span_field_same_as_metric - && span_has_parent - && !span_field_is_empty) + (metric_has_labels && in_both_spans_with_metric_label) .then(|| Label::new("user.email", "ferris@rust-lang.org")), - (in_span - && span_has_fields - && !span_field_same_as_metric - && !span_field_is_empty - && record_field - && emit_before_recording) - .then(|| Label::new("user.id", "666")), - (in_span - && span_has_fields - && !span_field_same_as_metric - && span_has_parent - && span_field_is_empty - && record_field - && emit_before_recording) - .then(|| Label::new("user.id", "999")), - (metric_has_labels - && !(in_span - && span_has_fields + if in_span && span_has_fields { + if span_field_same_as_metric { + if !metric_has_labels && !span_field_is_empty { + Some(Label::new("user.email", "user@domain.com")) + } else { + None + } + } else if record_field { + Some(Label::new("user.id", "42")) + } else if !span_field_is_empty { + Some(Label::new("user.id", "666")) + } else { + None + } + } else { + None + }, + if span_has_parent { + if !(in_span && span_has_fields && !span_field_is_empty) + && parent_field_same_as_span && span_field_same_as_metric - && span_has_parent - && !span_field_is_empty)) + && !metric_has_labels + { + Some(Label::new("user.email", "changed@domain.com")) + } else if !metric_has_labels && (!in_span || !span_has_fields) + || span_field_same_as_metric && !parent_field_same_as_span + || !in_span && !span_field_same_as_metric + || !span_has_fields && !span_field_same_as_metric + || !metric_has_labels + && span_field_is_empty + && span_field_same_as_metric + || span_field_is_empty && !span_field_same_as_metric && !record_field + { + Some(Label::new("user.id", "999")) + } else { + None + } + } else { + None + }, + (metric_has_labels && !in_both_spans_with_metric_label) .then(|| Label::new("user.email", "ferris@rust-lang.org")), ]) .flatten() @@ -738,66 +787,6 @@ fn test( ), None, None, - DebugValue::Counter(1), - ); - - let labels2 = IntoIterator::into_iter([ - (metric_has_labels - && in_span - && span_has_fields - && span_field_same_as_metric - && span_has_parent - && !span_field_is_empty) - .then(|| Label::new("user.email", "ferris@rust-lang.org")), - (in_span && span_has_fields) - .then(|| { - match ( - metric_has_labels, - span_field_same_as_metric, - span_field_is_empty, - record_field, - ) { - (_, false, _, true) => Some(Label::new("user.id", "42")), - (_, false, false, false) => Some(Label::new("user.id", "666")), - (false, true, false, _) => Some(Label::new("user.email", "user@domain.com")), - _ => None, - } - }) - .flatten(), - if span_has_parent - && (!in_span || !span_has_fields || span_field_is_empty) - && parent_field_same_as_span - && span_field_same_as_metric - && !metric_has_labels - { - Some(Label::new("user.email", "changed@domain.com")) - } else if !span_has_parent - || span_field_same_as_metric && metric_has_labels && parent_field_same_as_span - || in_span - && span_has_fields - && ((parent_field_same_as_span || !span_field_same_as_metric) - && !span_field_is_empty - || !span_field_same_as_metric && record_field) - { - None - } else { - Some(Label::new("user.id", "999")) - }, - (metric_has_labels - && !(in_span - && span_has_fields - && span_field_same_as_metric - && span_has_parent - && !span_field_is_empty)) - .then(|| Label::new("user.email", "ferris@rust-lang.org")), - ]) - .flatten() - .collect::>(); - - let expected2 = ( - CompositeKey::new(MetricKind::Counter, Key::from_parts(LOGIN_ATTEMPTS, labels2)), - None, - None, DebugValue::Counter( if !emit_before_recording || in_span && span_has_fields && !span_field_same_as_metric && record_field @@ -807,16 +796,7 @@ fn test( 2 }, ), - ); - let expected: Vec<_> = (in_span - && span_has_fields - && !span_field_same_as_metric - && record_field - && emit_before_recording) - .then(|| expected1) - .into_iter() - .chain([expected2]) - .collect(); - // let expected = vec![expected2]; + )); + assert_eq!(snapshot, expected); } From d8eb1f742bbcf474685031de248aec6b00d0ad79 Mon Sep 17 00:00:00 2001 From: zohnannor Date: Mon, 27 Nov 2023 18:31:52 +0400 Subject: [PATCH 14/15] WIP --- metrics-tracing-context/Cargo.toml | 3 +- metrics-tracing-context/src/lib.rs | 8 +- .../src/tracing_integration.rs | 20 +-- metrics-tracing-context/tests/integration.rs | 167 +++++++++--------- 4 files changed, 99 insertions(+), 99 deletions(-) diff --git a/metrics-tracing-context/Cargo.toml b/metrics-tracing-context/Cargo.toml index bf66df50..5be3283f 100644 --- a/metrics-tracing-context/Cargo.toml +++ b/metrics-tracing-context/Cargo.toml @@ -37,7 +37,6 @@ once_cell = { version = "1", default-features = false, features = ["std"] } tracing = { version = "0.1.29", default-features = false } tracing-core = { version = "0.1.21", default-features = false } tracing-subscriber = { version = "0.3.1", default-features = false, features = ["std"] } -tracing-test = "0.2.4" [dev-dependencies] criterion = { version = "=0.3.3", default-features = false } @@ -45,4 +44,4 @@ parking_lot = { version = "0.12.1", default-features = false } tracing = { version = "0.1.29", default-features = false, features = ["std"] } tracing-subscriber = { version = "0.3.1", default-features = false, features = ["registry", "fmt"] } pretty_assertions = "*" -itertools = "*" +itertools = { version = "0.12.0", default-features = false, features = ["use_std"] } diff --git a/metrics-tracing-context/src/lib.rs b/metrics-tracing-context/src/lib.rs index 49c70bc0..656e6449 100644 --- a/metrics-tracing-context/src/lib.rs +++ b/metrics-tracing-context/src/lib.rs @@ -182,12 +182,18 @@ where (!span_labels.is_empty()).then(|| { let (name, labels) = key.clone().into_parts(); + // Filter only span labels + span_labels.retain(|key: &SharedString, value: &mut SharedString| { + let label = Label::new(key.clone(), value.clone()); + self.label_filter.should_include_label(&name, &label) + }); + + // Overwrites labels from spans span_labels.extend(labels.into_iter().map(Label::into_parts)); let labels = span_labels .into_iter() .map(|(key, value)| Label::new(key, value)) - .filter(|label| self.label_filter.should_include_label(&name, label)) .collect::>(); Key::from_parts(name, labels) diff --git a/metrics-tracing-context/src/tracing_integration.rs b/metrics-tracing-context/src/tracing_integration.rs index 2720f4e1..d6e235f4 100644 --- a/metrics-tracing-context/src/tracing_integration.rs +++ b/metrics-tracing-context/src/tracing_integration.rs @@ -6,7 +6,6 @@ use metrics::{Key, SharedString}; use once_cell::sync::OnceCell; use std::cmp; use std::sync::Arc; -use tracing::span; use tracing_core::span::{Attributes, Id, Record}; use tracing_core::{field::Visit, Dispatch, Field, Subscriber}; use tracing_subscriber::{layer::Context, registry::LookupSpan, Layer}; @@ -124,18 +123,13 @@ where S: Subscriber + for<'a> LookupSpan<'a>, { fn on_layer(&mut self, _: &mut S) { - self.with_labels = Some( - |dispatch: &Dispatch, id: &span::Id, f: &mut dyn FnMut(&Labels) -> Option| { - let subscriber = dispatch - .downcast_ref::() - .expect("subscriber should downcast to expected type; this is a bug!"); - let span = - subscriber.span(id).expect("registry should have a span for the current ID"); - - let ext = span.extensions(); - f(ext.get::()?) - }, - ); + self.with_labels = Some(|dispatch, id, f| { + let subscriber = dispatch.downcast_ref::()?; + let span = subscriber.span(id)?; + + let ext = span.extensions(); + f(ext.get::()?) + }); } fn on_new_span(&self, attrs: &Attributes<'_>, id: &Id, cx: Context<'_, S>) { diff --git a/metrics-tracing-context/tests/integration.rs b/metrics-tracing-context/tests/integration.rs index f7293ea7..fa07130f 100644 --- a/metrics-tracing-context/tests/integration.rs +++ b/metrics-tracing-context/tests/integration.rs @@ -1,4 +1,4 @@ -use itertools::Itertools as _; +use itertools::Itertools; use metrics::{counter, Key, KeyName, Label}; use metrics_tracing_context::{LabelFilter, MetricsLayer, TracingContextLayer}; use metrics_util::debugging::{DebugValue, DebuggingRecorder, Snapshotter}; @@ -27,7 +27,10 @@ static USER_EMAIL_ATTEMPT: &[Label] = &[ Label::from_static_parts("attempt", "42"), ]; static USER_ID: &[Label] = &[Label::from_static_parts("user.id", "42")]; -static EMAIL_USER: &[Label] = &[Label::from_static_parts("user", "ferris")]; +static EMAIL_USER: &[Label] = &[ + Label::from_static_parts("user", "ferris"), + Label::from_static_parts("user.email", "ferris@rust-lang.org"), +]; static SVC_ENV: &[Label] = &[ Label::from_static_parts("service", "login_service"), Label::from_static_parts("env", "test"), @@ -611,7 +614,6 @@ fn test_label_allowlist() { ); } -#[tracing_test::traced_test] #[test] fn test_all_permutations() { let perms = (0..9).map(|_| [false, true]).multi_cartesian_product(); @@ -651,55 +653,55 @@ fn test( ) { let (_guard, snapshotter) = setup(TracingContextLayer::all()); - let parent = if span_field_same_as_metric && parent_field_same_as_span { - tracing::trace_span!("outer", user.email = "changed@domain.com") - } else { - tracing::trace_span!("outer", user.id = 999) - }; + { + let parent = if span_field_same_as_metric && parent_field_same_as_span { + tracing::trace_span!("outer", user.email = "changed@domain.com") + } else { + tracing::trace_span!("outer", user.id = 999) + }; - let _parent_guard = span_has_parent.then(|| parent.enter()); + let _parent_guard = span_has_parent.then(|| parent.enter()); + + let span = if span_has_fields { + match (span_field_same_as_metric, span_field_is_empty) { + (false, false) => tracing::trace_span!("login", user.id = 666), + (false, true) => { + tracing::trace_span!("login", user.id = tracing_core::field::Empty) + } + (true, false) => tracing::trace_span!("login", user.email = "user@domain.com"), + (true, true) => { + tracing::trace_span!("login", user.email = tracing_core::field::Empty) + } + } + } else { + tracing::trace_span!("login") + }; - let span = if span_has_fields { - match (span_field_same_as_metric, span_field_is_empty) { - (false, false) => tracing::trace_span!("login", user.id = 666), - (false, true) => tracing::trace_span!("login", user.id = tracing_core::field::Empty), - (true, false) => tracing::trace_span!("login", user.email = "user@domain.com"), - (true, true) => tracing::trace_span!("login", user.email = tracing_core::field::Empty), - } - } else { - tracing::trace_span!("login") - }; + let _guard = in_span.then(|| span.enter()); + + let inc = || { + if metric_has_labels { + counter!("login_attempts", "user.email" => "ferris@rust-lang.org").increment(1); + } else { + counter!("login_attempts").increment(1); + } + }; - let _guard = in_span.then(|| span.enter()); + if emit_before_recording { + inc(); + } - let inc = || { - if metric_has_labels { - counter!("login_attempts", "user.email" => "ferris@rust-lang.org").increment(1); - } else { - counter!("login_attempts").increment(1); + if record_field { + span.record("user.id", 42); } - }; - if emit_before_recording { inc(); } - if record_field { - span.record("user.id", 42); - } - - inc(); - let snapshot = snapshotter.snapshot().into_vec(); let mut expected = vec![]; - let in_both_spans_with_metric_label = in_span - && span_has_fields - && !span_field_is_empty - && span_field_same_as_metric - && span_has_parent; - if in_span && span_has_fields && !span_field_same_as_metric @@ -712,13 +714,10 @@ fn test( Key::from_parts( LOGIN_ATTEMPTS, IntoIterator::into_iter([ - (metric_has_labels && in_both_spans_with_metric_label) - .then(|| Label::new("user.email", "ferris@rust-lang.org")), - (!span_field_is_empty).then(|| Label::new("user.id", "666")), - (span_field_is_empty && span_has_parent) - .then(|| Label::new("user.id", "999")), - (metric_has_labels && !in_both_spans_with_metric_label) - .then(|| Label::new("user.email", "ferris@rust-lang.org")), + (span_has_parent || !span_field_is_empty).then(|| { + Label::new("user.id", if span_field_is_empty { "999" } else { "666" }) + }), + metric_has_labels.then(|| Label::new("user.email", "ferris@rust-lang.org")), ]) .flatten() .collect::>(), @@ -730,55 +729,57 @@ fn test( )); } + let in_span_with_metric_field = + in_span && span_has_fields && span_field_same_as_metric && !span_field_is_empty; + let has_other_labels = !(!span_has_parent + && (!in_span + || (span_field_same_as_metric || !span_has_fields) + || (!record_field && span_field_is_empty))) + && !(span_field_same_as_metric && parent_field_same_as_span) + && !in_span_with_metric_field; + expected.push(( CompositeKey::new( MetricKind::Counter, Key::from_parts( LOGIN_ATTEMPTS, IntoIterator::into_iter([ - (metric_has_labels && in_both_spans_with_metric_label) + (metric_has_labels && !has_other_labels) .then(|| Label::new("user.email", "ferris@rust-lang.org")), - if in_span && span_has_fields { - if span_field_same_as_metric { - if !metric_has_labels && !span_field_is_empty { - Some(Label::new("user.email", "user@domain.com")) + (!metric_has_labels + && (in_span_with_metric_field + || span_field_same_as_metric + && span_has_parent + && parent_field_same_as_span)) + .then(|| { + if in_span_with_metric_field { + Label::new("user.email", "user@domain.com") } else { - None + Label::new("user.email", "changed@domain.com") } - } else if record_field { - Some(Label::new("user.id", "42")) - } else if !span_field_is_empty { - Some(Label::new("user.id", "666")) - } else { - None - } - } else { - None - }, - if span_has_parent { - if !(in_span && span_has_fields && !span_field_is_empty) - && parent_field_same_as_span - && span_field_same_as_metric - && !metric_has_labels - { - Some(Label::new("user.email", "changed@domain.com")) - } else if !metric_has_labels && (!in_span || !span_has_fields) - || span_field_same_as_metric && !parent_field_same_as_span - || !in_span && !span_field_same_as_metric - || !span_has_fields && !span_field_same_as_metric - || !metric_has_labels - && span_field_is_empty - && span_field_same_as_metric - || span_field_is_empty && !span_field_same_as_metric && !record_field - { - Some(Label::new("user.id", "999")) - } else { - None - } + }), + if in_span && span_has_fields && !span_field_same_as_metric && record_field { + Some(Label::new("user.id", "42")) + } else if in_span + && span_has_fields + && !span_field_same_as_metric + && !span_field_is_empty + && !record_field + { + Some(Label::new("user.id", "666")) + } else if (!in_span || !span_has_fields || span_field_same_as_metric) + && (!span_field_same_as_metric || !parent_field_same_as_span) + && span_has_parent + || span_has_parent + && span_field_is_empty + && !record_field + && !span_field_same_as_metric + { + Some(Label::new("user.id", "999")) } else { None }, - (metric_has_labels && !in_both_spans_with_metric_label) + (metric_has_labels && has_other_labels) .then(|| Label::new("user.email", "ferris@rust-lang.org")), ]) .flatten() From 1f623f649bd7dc8db89968b7cbe300e03923fbf2 Mon Sep 17 00:00:00 2001 From: zohnannor Date: Tue, 28 Nov 2023 17:30:15 +0400 Subject: [PATCH 15/15] Corrections --- metrics-tracing-context/Cargo.toml | 3 +- metrics-tracing-context/tests/integration.rs | 78 +++++++++----------- 2 files changed, 36 insertions(+), 45 deletions(-) diff --git a/metrics-tracing-context/Cargo.toml b/metrics-tracing-context/Cargo.toml index 5be3283f..96287598 100644 --- a/metrics-tracing-context/Cargo.toml +++ b/metrics-tracing-context/Cargo.toml @@ -42,6 +42,5 @@ tracing-subscriber = { version = "0.3.1", default-features = false, features = [ criterion = { version = "=0.3.3", default-features = false } parking_lot = { version = "0.12.1", default-features = false } tracing = { version = "0.1.29", default-features = false, features = ["std"] } -tracing-subscriber = { version = "0.3.1", default-features = false, features = ["registry", "fmt"] } -pretty_assertions = "*" +tracing-subscriber = { version = "0.3.1", default-features = false, features = ["registry"] } itertools = { version = "0.12.0", default-features = false, features = ["use_std"] } diff --git a/metrics-tracing-context/tests/integration.rs b/metrics-tracing-context/tests/integration.rs index fa07130f..526b0c63 100644 --- a/metrics-tracing-context/tests/integration.rs +++ b/metrics-tracing-context/tests/integration.rs @@ -8,8 +8,6 @@ use tracing::dispatcher::{set_default, DefaultGuard, Dispatch}; use tracing::{span, Level}; use tracing_subscriber::{layer::SubscriberExt, Registry}; -use pretty_assertions::assert_eq; - static TEST_MUTEX: Mutex<()> = const_mutex(()); static LOGIN_ATTEMPTS: &str = "login_attempts"; static LOGIN_ATTEMPTS_NONE: &str = "login_attempts_no_labels"; @@ -619,7 +617,7 @@ fn test_all_permutations() { let perms = (0..9).map(|_| [false, true]).multi_cartesian_product(); for v in perms { - let [metric_has_labels, in_span, span_has_fields, span_field_same_as_metric, span_has_parent, parent_field_same_as_span, span_field_is_empty, record_field, emit_before_recording] = + let [metric_has_labels, in_span, span_has_fields, span_field_same_as_metric, span_has_parent, parent_field_same_as_span, span_field_is_empty, record_field, increment_before_recording] = v[..] else { unreachable!("{:?}, {}", v, v.len()); @@ -634,7 +632,7 @@ fn test_all_permutations() { parent_field_same_as_span, span_field_is_empty, record_field, - emit_before_recording, + increment_before_recording, ); } } @@ -649,55 +647,49 @@ fn test( parent_field_same_as_span: bool, span_field_is_empty: bool, record_field: bool, - emit_before_recording: bool, + increment_before_recording: bool, ) { let (_guard, snapshotter) = setup(TracingContextLayer::all()); - { - let parent = if span_field_same_as_metric && parent_field_same_as_span { - tracing::trace_span!("outer", user.email = "changed@domain.com") - } else { - tracing::trace_span!("outer", user.id = 999) - }; + let parent = if span_field_same_as_metric && parent_field_same_as_span { + tracing::trace_span!("outer", user.email = "changed@domain.com") + } else { + tracing::trace_span!("outer", user.id = 999) + }; - let _parent_guard = span_has_parent.then(|| parent.enter()); - - let span = if span_has_fields { - match (span_field_same_as_metric, span_field_is_empty) { - (false, false) => tracing::trace_span!("login", user.id = 666), - (false, true) => { - tracing::trace_span!("login", user.id = tracing_core::field::Empty) - } - (true, false) => tracing::trace_span!("login", user.email = "user@domain.com"), - (true, true) => { - tracing::trace_span!("login", user.email = tracing_core::field::Empty) - } - } - } else { - tracing::trace_span!("login") - }; + let _guard = span_has_parent.then(|| parent.enter()); - let _guard = in_span.then(|| span.enter()); + let span = if span_has_fields { + match (span_field_same_as_metric, span_field_is_empty) { + (false, false) => tracing::trace_span!("login", user.id = 666), + (false, true) => tracing::trace_span!("login", user.id = tracing_core::field::Empty), + (true, false) => tracing::trace_span!("login", user.email = "user@domain.com"), + (true, true) => tracing::trace_span!("login", user.email = tracing_core::field::Empty), + } + } else { + tracing::trace_span!("login") + }; - let inc = || { - if metric_has_labels { - counter!("login_attempts", "user.email" => "ferris@rust-lang.org").increment(1); - } else { - counter!("login_attempts").increment(1); - } - }; + let _guard = in_span.then(|| span.enter()); - if emit_before_recording { - inc(); + let increment = || { + if metric_has_labels { + counter!("login_attempts", "user.email" => "ferris@rust-lang.org").increment(1); + } else { + counter!("login_attempts").increment(1); } + }; - if record_field { - span.record("user.id", 42); - } + if increment_before_recording { + increment(); + } - inc(); + if record_field { + span.record("user.id", 42); } + increment(); + let snapshot = snapshotter.snapshot().into_vec(); let mut expected = vec![]; @@ -706,7 +698,7 @@ fn test( && span_has_fields && !span_field_same_as_metric && record_field - && emit_before_recording + && increment_before_recording { expected.push(( CompositeKey::new( @@ -789,7 +781,7 @@ fn test( None, None, DebugValue::Counter( - if !emit_before_recording + if !increment_before_recording || in_span && span_has_fields && !span_field_same_as_metric && record_field { 1