From 0f7e3d4426d68828c8f4bb8c607d669ce503f195 Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Wed, 31 Jan 2024 18:51:52 -0500 Subject: [PATCH] feat(spans): Extract INP metrics from spans (#2969) --- CHANGELOG.md | 1 + relay-dynamic-config/src/defaults.rs | 97 ++++++++++++++ relay-event-normalization/src/event.rs | 8 +- relay-event-normalization/src/lib.rs | 4 +- .../src/normalize/span/tag_extraction.rs | 61 ++++++++- relay-event-schema/src/protocol/span.rs | 17 +++ relay-server/src/metrics_extraction/event.rs | 36 ++++++ .../src/services/processor/span/processing.rs | 44 ++++++- tests/integration/test_outcome.py | 4 +- tests/integration/test_store.py | 118 +++++++++++++++++- 10 files changed, 373 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 19dfd8397a..d334a13ccd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ - Emit a usage metric for total spans. ([#3007](https://github.com/getsentry/relay/pull/3007)) - Drop timestamp from metrics partition key. ([#3025](https://github.com/getsentry/relay/pull/3025)) - Drop spans ending outside the valid timestamp range. ([#3013](https://github.com/getsentry/relay/pull/3013)) +- Extract INP metrics from spans. ([#2969](https://github.com/getsentry/relay/pull/2969)) ## 24.1.1 diff --git a/relay-dynamic-config/src/defaults.rs b/relay-dynamic-config/src/defaults.rs index 781eeb0bf7..0562daf709 100644 --- a/relay-dynamic-config/src/defaults.rs +++ b/relay-dynamic-config/src/defaults.rs @@ -73,6 +73,11 @@ fn span_metrics() -> impl IntoIterator { let is_mobile_sdk = RuleCondition::eq("span.sentry_tags.mobile", "true"); + let is_allowed_browser = RuleCondition::eq( + "span.sentry_tags.browser.name", + vec!["Chrome", "Firefox", "Safari", "Edge", "Opera"], + ); + // This filter is based on // https://github.com/getsentry/sentry/blob/e01885215ff1a5b4e0da3046b4d929398a946360/static/app/views/starfish/views/screens/screenLoadSpans/spanOpSelector.tsx#L31-L34 let is_screen = RuleCondition::eq("span.sentry_tags.transaction.op", "ui.load") @@ -400,5 +405,97 @@ fn span_metrics() -> impl IntoIterator { .always(), // already guarded by condition on metric ], }, + MetricSpec { + category: DataCategory::Span, + mri: "d:spans/webvital.score.total@ratio".into(), + field: Some("span.measurements.score.total.value".into()), + condition: Some(is_allowed_browser.clone()), + tags: vec![ + Tag::with_key("transaction.op") + .from_field("span.sentry_tags.transaction.op") + .always(), + Tag::with_key("transaction") + .from_field("span.sentry_tags.transaction") + .always(), + Tag::with_key("environment") + .from_field("span.sentry_tags.environment") + .always(), + Tag::with_key("release") + .from_field("span.sentry_tags.release") + .always(), + Tag::with_key("browser.name") + .from_field("span.browser.name") + .always(), // already guarded by condition on metric + ], + }, + MetricSpec { + category: DataCategory::Span, + mri: "d:spans/webvital.score.inp@ratio".into(), + field: Some("span.measurements.score.inp.value".into()), + condition: Some(is_allowed_browser.clone()), + tags: vec![ + Tag::with_key("span.op") + .from_field("span.sentry_tags.op") + .always(), + Tag::with_key("transaction") + .from_field("span.sentry_tags.transaction") + .always(), + Tag::with_key("environment") + .from_field("span.sentry_tags.environment") + .always(), + Tag::with_key("release") + .from_field("span.sentry_tags.release") + .always(), + Tag::with_key("browser.name") + .from_field("span.sentry_tags.browser.name") + .always(), // already guarded by condition on metric + ], + }, + MetricSpec { + category: DataCategory::Span, + mri: "d:spans/webvital.score.weight.inp@ratio".into(), + field: Some("span.measurements.score.weight.inp.value".into()), + condition: Some(is_allowed_browser.clone()), + tags: vec![ + Tag::with_key("span.op") + .from_field("span.sentry_tags.op") + .always(), + Tag::with_key("transaction") + .from_field("span.sentry_tags.transaction") + .always(), + Tag::with_key("environment") + .from_field("span.sentry_tags.environment") + .always(), + Tag::with_key("release") + .from_field("span.sentry_tags.release") + .always(), + Tag::with_key("browser.name") + .from_field("span.sentry_tags.browser.name") + .always(), // already guarded by condition on metric + ], + }, + MetricSpec { + category: DataCategory::Span, + mri: "d:spans/webvital.inp@millisecond".into(), + field: Some("span.measurements.inp.value".into()), + condition: Some(is_allowed_browser), + tags: vec![ + Tag::with_key("span.op") + .from_field("span.sentry_tags.op") + .always(), + Tag::with_key("transaction") + .from_field("span.sentry_tags.transaction") + .always(), + Tag::with_key("environment") + .from_field("span.sentry_tags.environment") + .always(), + Tag::with_key("release") + .from_field("span.sentry_tags.release") + .always(), + Tag::with_key("browser.name") + .from_field("span.sentry_tags.browser.name") + .always(), // already guarded by condition on metric + ], + }, ] } diff --git a/relay-event-normalization/src/event.rs b/relay-event-normalization/src/event.rs index 66d1498327..189098e73f 100644 --- a/relay-event-normalization/src/event.rs +++ b/relay-event-normalization/src/event.rs @@ -627,11 +627,11 @@ pub fn normalize_measurements( } } -/// Computes performance score measurements. +/// Computes performance score measurements for an event. /// /// This computes score from vital measurements, using config options to define how it is /// calculated. -fn normalize_performance_score( +pub fn normalize_performance_score( event: &mut Event, performance_score: Option<&PerformanceScoreConfig>, ) { @@ -655,8 +655,8 @@ fn normalize_performance_score( // a measurement with weight is missing. break; } - let mut score_total = 0.0; - let mut weight_total = 0.0; + let mut score_total = 0.0f64; + let mut weight_total = 0.0f64; for component in &profile.score_components { // Skip optional components if they are not present on the event. if component.optional diff --git a/relay-event-normalization/src/lib.rs b/relay-event-normalization/src/lib.rs index cd30f72bf6..d0320340ec 100644 --- a/relay-event-normalization/src/lib.rs +++ b/relay-event-normalization/src/lib.rs @@ -40,7 +40,9 @@ pub use validation::{ TransactionValidationConfig, }; pub mod replay; -pub use event::{normalize_event, normalize_measurements, NormalizationConfig}; +pub use event::{ + normalize_event, normalize_measurements, normalize_performance_score, NormalizationConfig, +}; pub use normalize::breakdowns::*; pub use normalize::*; pub use remove_other::RemoveOtherProcessor; diff --git a/relay-event-normalization/src/normalize/span/tag_extraction.rs b/relay-event-normalization/src/normalize/span/tag_extraction.rs index 82664dd689..5d09158159 100644 --- a/relay-event-normalization/src/normalize/span/tag_extraction.rs +++ b/relay-event-normalization/src/normalize/span/tag_extraction.rs @@ -8,7 +8,7 @@ use once_cell::sync::Lazy; use regex::Regex; use relay_base_schema::metrics::{InformationUnit, MetricUnit}; use relay_event_schema::protocol::{ - AppContext, Event, Measurement, OsContext, Span, Timestamp, TraceContext, + AppContext, BrowserContext, Event, Measurement, OsContext, Span, Timestamp, TraceContext, }; use relay_protocol::{Annotated, Value}; use sqlparser::ast::Visit; @@ -32,6 +32,7 @@ pub enum SpanTagKey { Transaction, TransactionMethod, TransactionOp, + BrowserName, // `"true"` if the transaction was sent by a mobile SDK. Mobile, DeviceClass, @@ -76,6 +77,7 @@ impl SpanTagKey { SpanTagKey::TransactionOp => "transaction.op", SpanTagKey::Mobile => "mobile", SpanTagKey::DeviceClass => "device.class", + SpanTagKey::BrowserName => "browser.name", SpanTagKey::Action => "action", SpanTagKey::Category => "category", @@ -228,6 +230,13 @@ pub fn extract_shared_tags(event: &Event) -> BTreeMap { tags.insert(SpanTagKey::DeviceClass, device_class.into()); } + if let Some(browser_name) = event + .context::() + .and_then(|v| v.name.value()) + { + tags.insert(SpanTagKey::BrowserName, browser_name.into()); + } + tags } @@ -1274,4 +1283,54 @@ LIMIT 1 let tags = span.value().unwrap().sentry_tags.value().unwrap(); assert_eq!(tags.get("main_thread"), None); } + + #[test] + fn test_browser_name_and_geo_country_code() { + let json = r#" + { + "type": "transaction", + "platform": "javascript", + "start_timestamp": "2021-04-26T07:59:01+0100", + "timestamp": "2021-04-26T08:00:00+0100", + "transaction": "foo", + "contexts": { + "trace": { + "trace_id": "ff62a8b040f340bda5d830223def1d81", + "span_id": "bd429c44b67a3eb4" + }, + "browser": { + "name": "Chrome" + } + }, + "spans": [ + { + "op": "resource.script", + "span_id": "bd429c44b67a3eb1", + "start_timestamp": 1597976300.0000000, + "timestamp": 1597976302.0000000, + "trace_id": "ff62a8b040f340bda5d830223def1d81" + } + ] + } + "#; + + let mut event = Annotated::::from_json(json) + .unwrap() + .into_value() + .unwrap(); + + extract_span_tags( + &mut event, + &Config { + max_tag_value_size: 200, + }, + ); + + let span = &event.spans.value().unwrap()[0]; + let tags = span.value().unwrap().sentry_tags.value().unwrap(); + assert_eq!( + tags.get("browser.name"), + Some(&Annotated::new("Chrome".to_string())) + ); + } } diff --git a/relay-event-schema/src/protocol/span.rs b/relay-event-schema/src/protocol/span.rs index 7c04fb7a3d..08d15edc54 100644 --- a/relay-event-schema/src/protocol/span.rs +++ b/relay-event-schema/src/protocol/span.rs @@ -160,6 +160,15 @@ impl Getter for Span { val.into() } else if let Some(key) = path.strip_prefix("sentry_tags.") { self.sentry_tags.value()?.get(key)?.as_str()?.into() + } else if let Some(rest) = path.strip_prefix("measurements.") { + let name = rest.strip_suffix(".value")?; + self.measurements + .value()? + .get(name)? + .value()? + .value + .value()? + .into() } else { return None; } @@ -233,6 +242,9 @@ mod tests { "data": { "foo": {"bar": 1}, "foo.bar": 2 + }, + "measurements": { + "some": {"value": 100.0} } }"#, ) @@ -246,6 +258,11 @@ mod tests { assert_eq!(span.get_value("span.data"), None); assert_eq!(span.get_value("span.data."), None); assert_eq!(span.get_value("span.data.x"), None); + + assert_eq!( + span.get_value("span.measurements.some.value"), + Some(Val::F64(100.0)) + ); } #[test] diff --git a/relay-server/src/metrics_extraction/event.rs b/relay-server/src/metrics_extraction/event.rs index b71dd9e50d..5b07aab273 100644 --- a/relay-server/src/metrics_extraction/event.rs +++ b/relay-server/src/metrics_extraction/event.rs @@ -1426,4 +1426,40 @@ mod tests { assert_eq!(metric.tag("ttfd"), Some("ttfd")); } } + + #[test] + fn test_extract_span_metrics_performance_score() { + let json = r#" + { + "op": "ui.interaction.click", + "parent_span_id": "8f5a2b8768cafb4e", + "span_id": "bd429c44b67a3eb4", + "start_timestamp": 1597976300.0000000, + "timestamp": 1597976302.0000000, + "exclusive_time": 2000.0, + "trace_id": "ff62a8b040f340bda5d830223def1d81", + "sentry_tags": { + "browser.name": "Chrome", + "op": "ui.interaction.click" + }, + "measurements": { + "score.total": {"value": 1.0}, + "score.inp": {"value": 1.0}, + "score.weight.inp": {"value": 1.0}, + "inp": {"value": 1.0} + } + } + "#; + let span = Annotated::from_json(json).unwrap(); + let metrics = extract_span_metrics(span.value().unwrap()); + + for mri in [ + "d:spans/webvital.inp@millisecond", + "d:spans/webvital.score.inp@ratio", + "d:spans/webvital.score.total@ratio", + "d:spans/webvital.score.weight.inp@ratio", + ] { + assert!(metrics.iter().any(|b| b.name == mri)); + } + } } diff --git a/relay-server/src/services/processor/span/processing.rs b/relay-server/src/services/processor/span/processing.rs index bfd41a0680..6227710968 100644 --- a/relay-server/src/services/processor/span/processing.rs +++ b/relay-server/src/services/processor/span/processing.rs @@ -9,13 +9,13 @@ use relay_config::Config; use relay_dynamic_config::{ErrorBoundary, Feature, GlobalConfig, ProjectConfig}; use relay_event_normalization::span::tag_extraction; use relay_event_normalization::{ - normalize_measurements, validate_span, DynamicMeasurementsConfig, MeasurementsConfig, - TransactionsProcessor, + normalize_measurements, normalize_performance_score, normalize_user_agent_info_generic, + validate_span, DynamicMeasurementsConfig, MeasurementsConfig, PerformanceScoreConfig, + RawUserAgentInfo, TransactionsProcessor, }; use relay_event_schema::processor::{process_value, ProcessingState}; -use relay_event_schema::protocol::Span; -use relay_metrics::UnixTimestamp; -use relay_metrics::{aggregator::AggregatorConfig, MetricNamespace}; +use relay_event_schema::protocol::{Contexts, Event, Span}; +use relay_metrics::{aggregator::AggregatorConfig, MetricNamespace, UnixTimestamp}; use relay_pii::PiiProcessor; use relay_protocol::{Annotated, Empty}; @@ -41,6 +41,19 @@ pub fn process( state.managed_envelope.received_at(), global_config.measurements.as_ref(), state.project_state.config().measurements.as_ref(), + state.project_state.config().performance_score.as_ref(), + ); + + let meta = state.managed_envelope.envelope().meta(); + let mut contexts = Contexts::new(); + let user_agent_info = RawUserAgentInfo { + user_agent: meta.user_agent(), + client_hints: meta.client_hints().as_deref(), + }; + normalize_user_agent_info_generic( + &mut contexts, + &Annotated::new("".to_string()), + &user_agent_info, ); state.managed_envelope.retain_items(|item| { @@ -65,7 +78,11 @@ pub fn process( _ => return ItemAction::Keep, }; - if let Err(e) = normalize(&mut annotated_span, normalize_span_config.clone()) { + if let Err(e) = normalize( + &mut annotated_span, + normalize_span_config.clone(), + Annotated::new(contexts.clone()), + ) { relay_log::debug!("failed to normalize span: {}", e); return ItemAction::Drop(Outcome::Invalid(DiscardReason::Internal)); }; @@ -222,6 +239,8 @@ struct NormalizeSpanConfig<'a> { pub timestamp_range: std::ops::Range, /// The maximum allowed size of tag values in bytes. Longer values will be cropped. pub max_tag_value_size: usize, + /// Configuration for generating performance score measurements for web vitals + pub performance_score: Option<&'a PerformanceScoreConfig>, /// Configuration for measurement normalization in transaction events. /// /// Has an optional [`relay_event_normalization::MeasurementsConfig`] from both the project and the global level. @@ -240,6 +259,7 @@ fn get_normalize_span_config<'a>( received_at: DateTime, global_measurements_config: Option<&'a MeasurementsConfig>, project_measurements_config: Option<&'a MeasurementsConfig>, + performance_score: Option<&'a PerformanceScoreConfig>, ) -> NormalizeSpanConfig<'a> { let aggregator_config = AggregatorConfig::from(config.aggregator_config_for(MetricNamespace::Spans)); @@ -259,6 +279,7 @@ fn get_normalize_span_config<'a>( .max_name_length .saturating_sub(MeasurementsConfig::MEASUREMENT_MRI_OVERHEAD), ), + performance_score, } } @@ -266,6 +287,7 @@ fn get_normalize_span_config<'a>( fn normalize( annotated_span: &mut Annotated, config: NormalizeSpanConfig, + contexts: Annotated, ) -> Result<(), ProcessingError> { use relay_event_normalization::{SchemaProcessor, TimestampProcessor, TrimmingProcessor}; @@ -273,6 +295,7 @@ fn normalize( received_at, timestamp_range: timestmap_range, max_tag_value_size, + performance_score, measurements, max_name_and_unit_len, } = config; @@ -335,6 +358,15 @@ fn normalize( .collect(), ); + let mut event = Event { + contexts, + measurements: span.measurements.clone(), + spans: Annotated::from(vec![Annotated::new(span.clone())]), + ..Default::default() + }; + normalize_performance_score(&mut event, performance_score); + span.measurements = event.measurements; + tag_extraction::extract_measurements(span); process_value( diff --git a/tests/integration/test_outcome.py b/tests/integration/test_outcome.py index 2a3c5d85e5..d424325ab7 100644 --- a/tests/integration/test_outcome.py +++ b/tests/integration/test_outcome.py @@ -1792,7 +1792,7 @@ def make_envelope(transaction_name): project_id, make_envelope("ho") ) # should be kept by dynamic sampling - outcomes = outcomes_consumer.get_outcomes() + outcomes = outcomes_consumer.get_outcomes(timeout=10.0) outcomes.sort(key=lambda o: sorted(o.items())) expected_source = { @@ -1895,7 +1895,7 @@ def make_envelope(): envelope = make_envelope() upstream.send_envelope(project_id, envelope) - outcomes = outcomes_consumer.get_outcomes() + outcomes = outcomes_consumer.get_outcomes(timeout=10.0) outcomes.sort(key=lambda o: sorted(o.items())) expected_outcomes = [ diff --git a/tests/integration/test_store.py b/tests/integration/test_store.py index ad22b16319..98fd3a693a 100644 --- a/tests/integration/test_store.py +++ b/tests/integration/test_store.py @@ -1564,7 +1564,8 @@ def test_span_ingestion( }, ) - spans = list(spans_consumer.get_spans()) + spans = list(spans_consumer.get_spans(timeout=10.0)) + for span in spans: span.pop("received", None) @@ -1946,9 +1947,120 @@ def test_span_reject_invalid_timestamps( ), ) ) - relay.send_envelope(project_id, envelope) - spans = list(spans_consumer.get_spans()) + spans = list(spans_consumer.get_spans(timeout=10.0)) + assert len(spans) == 1 assert spans[0]["description"] == "span with valid timestamps" + + +def test_span_ingestion_with_performance_scores( + mini_sentry, + relay_with_processing, + spans_consumer, +): + spans_consumer = spans_consumer() + relay = relay_with_processing() + + project_id = 42 + project_config = mini_sentry.add_full_project_config(project_id) + project_config["config"]["performanceScore"] = { + "profiles": [ + { + "name": "Desktop", + "scoreComponents": [ + {"measurement": "fcp", "weight": 0.15, "p10": 900, "p50": 1600}, + {"measurement": "lcp", "weight": 0.30, "p10": 1200, "p50": 2400}, + {"measurement": "fid", "weight": 0.30, "p10": 100, "p50": 300}, + {"measurement": "cls", "weight": 0.25, "p10": 0.1, "p50": 0.25}, + {"measurement": "ttfb", "weight": 0.0, "p10": 0.2, "p50": 0.4}, + ], + "condition": { + "op": "eq", + "name": "event.contexts.browser.name", + "value": "Python Requests", + }, + }, + ], + } + project_config["config"]["features"] = [ + "organizations:performance-calculate-score-relay", + "organizations:standalone-span-ingestion", + "projects:span-metrics-extraction", + "projects:span-metrics-extraction-all-modules", + ] + + duration = timedelta(milliseconds=500) + end = datetime.utcnow().replace(tzinfo=timezone.utc) - timedelta(seconds=1) + start = end - duration + + envelope = Envelope() + envelope.add_item( + Item( + type="span", + payload=PayloadRef( + bytes=json.dumps( + { + "op": "ui.interaction.click", + "span_id": "bd429c44b67a3eb1", + "segment_id": "968cff94913ebb07", + "start_timestamp": start.timestamp(), + "timestamp": end.timestamp() + 1, + "exclusive_time": 345.0, # The SDK knows that this span has a lower exclusive time + "trace_id": "ff62a8b040f340bda5d830223def1d81", + "measurements": { + "cls": {"value": 100}, + "fcp": {"value": 200}, + "fid": {"value": 300}, + "lcp": {"value": 400}, + "ttfb": {"value": 500}, + }, + }, + ).encode() + ), + ) + ) + relay.send_envelope(project_id, envelope) + + spans = list(spans_consumer.get_spans(timeout=10.0)) + + for span in spans: + span.pop("received", None) + + spans.sort(key=lambda msg: msg["span_id"]) # endpoint might overtake envelope + + assert spans == [ + { + "duration_ms": 1500, + "exclusive_time_ms": 345.0, + "is_segment": True, + "project_id": 42, + "retention_days": 90, + "segment_id": "bd429c44b67a3eb1", + "sentry_tags": { + "op": "ui.interaction.click", + }, + "span_id": "bd429c44b67a3eb1", + "start_timestamp_ms": int(start.timestamp() * 1e3), + "trace_id": "ff62a8b040f340bda5d830223def1d81", + "measurements": { + "score.fcp": {"value": 0.14999972769539766}, + "score.fid": {"value": 0.14999999985}, + "score.lcp": {"value": 0.29986141375718806}, + "score.total": {"value": 0.5998611413025857}, + "score.ttfb": {"value": 0.0}, + "score.weight.cls": {"value": 0.25}, + "score.weight.fcp": {"value": 0.15}, + "score.weight.fid": {"value": 0.3}, + "score.weight.lcp": {"value": 0.3}, + "score.weight.ttfb": {"value": 0.0}, + "cls": {"value": 100.0}, + "fcp": {"value": 200.0}, + "fid": {"value": 300.0}, + "lcp": {"value": 400.0}, + "ttfb": {"value": 500.0}, + "score.cls": {"value": 0.0}, + }, + }, + ]