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); }