From 7c02318ee0b47bb0cc9434cefdc088cfc870d108 Mon Sep 17 00:00:00 2001 From: Nar Saynorath Date: Mon, 22 Apr 2024 22:48:38 -0400 Subject: [PATCH 1/8] feat(mobile-ui): Collect frame delay metric --- relay-dynamic-config/src/defaults.rs | 32 ++++++++++ relay-event-schema/src/protocol/span.rs | 6 +- relay-server/src/metrics_extraction/event.rs | 5 +- ...t__tests__extract_span_metrics_mobile.snap | 61 ++++++++++++++++++- 4 files changed, 101 insertions(+), 3 deletions(-) diff --git a/relay-dynamic-config/src/defaults.rs b/relay-dynamic-config/src/defaults.rs index 179259398b5..43a43e9fb96 100644 --- a/relay-dynamic-config/src/defaults.rs +++ b/relay-dynamic-config/src/defaults.rs @@ -622,6 +622,38 @@ fn span_metrics( .always(), // already guarded by condition on metric ], }, + MetricSpec { + category: DataCategory::Span, + mri: "d:spans/mobile.frames_delay@millisecond".into(), + field: Some("span.data.frames\\.delay".into()), + condition: Some(is_mobile.clone() & duration_condition.clone()), + tags: vec![ + 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("span.description") + .from_field("span.sentry_tags.description") + .always(), + Tag::with_key("span.op") + .from_field("span.sentry_tags.op") + .always(), + Tag::with_key("span.group") + .from_field("span.sentry_tags.group") + .always(), + Tag::with_key("device.class") + .from_field("span.sentry_tags.device.class") + .always(), + Tag::with_key("os.name") + .from_field("span.sentry_tags.os.name") + .always(), + ], + }, ]; if double_write_distributions_as_gauges { diff --git a/relay-event-schema/src/protocol/span.rs b/relay-event-schema/src/protocol/span.rs index 1e1932f8fd1..de1ca6a9bf9 100644 --- a/relay-event-schema/src/protocol/span.rs +++ b/relay-event-schema/src/protocol/span.rs @@ -493,7 +493,8 @@ mod tests { "code.filepath": "task.py", "code.lineno": 123, "code.function": "fn()", - "code.namespace": "ns" + "code.namespace": "ns", + "frames.delay": 100 }"#; let data = Annotated::::from_json(data) .unwrap() @@ -549,6 +550,9 @@ mod tests { "foo": I64( 2, ), + "frames.delay": I64( + 100, + ), }, } "###); diff --git a/relay-server/src/metrics_extraction/event.rs b/relay-server/src/metrics_extraction/event.rs index 0f08e829cf8..8157802ac12 100644 --- a/relay-server/src/metrics_extraction/event.rs +++ b/relay-server/src/metrics_extraction/event.rs @@ -1133,7 +1133,10 @@ mod tests { "span_id": "bd429c44b67a3eb2", "start_timestamp": 1597976300.0000000, "timestamp": 1597976303.0000000, - "trace_id": "ff62a8b040f340bda5d830223def1d81" + "trace_id": "ff62a8b040f340bda5d830223def1d81", + "data": { + "frames.delay": 123 + } }, { "op": "app.start.cold", diff --git a/relay-server/src/metrics_extraction/snapshots/relay_server__metrics_extraction__event__tests__extract_span_metrics_mobile.snap b/relay-server/src/metrics_extraction/snapshots/relay_server__metrics_extraction__event__tests__extract_span_metrics_mobile.snap index 9de0b77f1cb..18057b45a84 100644 --- a/relay-server/src/metrics_extraction/snapshots/relay_server__metrics_extraction__event__tests__extract_span_metrics_mobile.snap +++ b/relay-server/src/metrics_extraction/snapshots/relay_server__metrics_extraction__event__tests__extract_span_metrics_mobile.snap @@ -74,7 +74,44 @@ expression: "(&event.value().unwrap().spans, metrics)" tags: ~, origin: ~, profile_id: ~, - data: ~, + data: SpanData { + app_start_type: ~, + browser_name: ~, + code_filepath: ~, + code_lineno: ~, + code_function: ~, + code_namespace: ~, + db_operation: ~, + db_system: ~, + environment: ~, + release: ~, + http_decoded_response_content_length: ~, + http_request_method: ~, + http_response_content_length: ~, + http_response_transfer_size: ~, + resource_render_blocking_status: ~, + server_address: ~, + cache_hit: ~, + cache_item_size: ~, + http_response_status_code: ~, + ai_input_messages: ~, + ai_completion_tokens_used: ~, + ai_prompt_tokens_used: ~, + ai_total_tokens_used: ~, + ai_responses: ~, + thread_name: ~, + segment_name: ~, + ui_component_name: ~, + url_scheme: ~, + user: ~, + replay_id: ~, + sdk_name: ~, + other: { + "frames.delay": I64( + 123, + ), + }, + }, sentry_tags: { "app_start_type": "warm", "device.class": "1", @@ -715,6 +752,28 @@ expression: "(&event.value().unwrap().spans, metrics)" merges: 1, }, }, + Bucket { + timestamp: UnixTimestamp(1597976303), + width: 0, + name: MetricName( + "d:spans/mobile.frames_delay@millisecond", + ), + value: Distribution( + [ + 123.0, + ], + ), + tags: { + "device.class": "1", + "os.name": "iOS", + "release": "1.2.3", + "span.op": "ui.load.initial_display", + "transaction": "gEt /api/:version/users/", + }, + metadata: BucketMetadata { + merges: 1, + }, + }, Bucket { timestamp: UnixTimestamp(1597976303), width: 0, From 16416d8f69fd34223c63fec7385bf19238cbb241 Mon Sep 17 00:00:00 2001 From: Nar Saynorath Date: Mon, 22 Apr 2024 22:53:21 -0400 Subject: [PATCH 2/8] Changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 446a7775354..c4a87a5c0ed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,6 @@ ## Unreleased - **Bug fixes:** - Respect country code TLDs when scrubbing span tags. ([#3458](https://github.com/getsentry/relay/pull/3458)) @@ -11,6 +10,7 @@ - Use same keys for OTel span attributes and Sentry span data. ([#3457](https://github.com/getsentry/relay/pull/3457)) - Support passing owner when upserting Monitors. ([#3468](https://github.com/getsentry/relay/pull/3468)) +- Extract `frames.delay` metric from mobile spans. ([#3472](https://github.com/getsentry/relay/pull/3472)) **Internal**: From eefb202c9ff62c4732a9061b6b2fac646f128435 Mon Sep 17 00:00:00 2001 From: Nar Saynorath Date: Mon, 22 Apr 2024 23:36:05 -0400 Subject: [PATCH 3/8] Add frame delay to span data definitions --- relay-event-schema/src/protocol/span.rs | 5 +++++ relay-event-schema/src/protocol/span/convert.rs | 1 + 2 files changed, 6 insertions(+) diff --git a/relay-event-schema/src/protocol/span.rs b/relay-event-schema/src/protocol/span.rs index de1ca6a9bf9..d96ee9765ce 100644 --- a/relay-event-schema/src/protocol/span.rs +++ b/relay-event-schema/src/protocol/span.rs @@ -299,6 +299,10 @@ pub struct SpanData { #[metastructure(field = "sentry.sdk.name")] pub sdk_name: Annotated, + // Frames Delay (in ms) + #[metastructure(field = "sentry.frames.delay")] + pub frames_delay: Annotated, + /// Other fields in `span.data`. #[metastructure(additional_properties, pii = "true", retain = "true")] other: Object, @@ -543,6 +547,7 @@ mod tests { user: ~, replay_id: ~, sdk_name: ~, + frames_delay: ~, other: { "bar": String( "3", diff --git a/relay-event-schema/src/protocol/span/convert.rs b/relay-event-schema/src/protocol/span/convert.rs index ecf449d2f71..0578ecd8440 100644 --- a/relay-event-schema/src/protocol/span/convert.rs +++ b/relay-event-schema/src/protocol/span/convert.rs @@ -287,6 +287,7 @@ mod tests { user: ~, replay_id: ~, sdk_name: "sentry.php", + frames_delay: ~, other: {}, }, sentry_tags: ~, From 554670de5958b94c693d5e4333e9d836fa1001fd Mon Sep 17 00:00:00 2001 From: Nar Saynorath Date: Mon, 22 Apr 2024 23:44:48 -0400 Subject: [PATCH 4/8] Update snapshots --- relay-event-schema/src/protocol/span.rs | 9 +++-- ...t__tests__extract_span_metrics_mobile.snap | 33 ++++--------------- relay-spans/src/span.rs | 1 + 3 files changed, 11 insertions(+), 32 deletions(-) diff --git a/relay-event-schema/src/protocol/span.rs b/relay-event-schema/src/protocol/span.rs index d96ee9765ce..b8f430a9c2a 100644 --- a/relay-event-schema/src/protocol/span.rs +++ b/relay-event-schema/src/protocol/span.rs @@ -300,7 +300,7 @@ pub struct SpanData { pub sdk_name: Annotated, // Frames Delay (in ms) - #[metastructure(field = "sentry.frames.delay")] + #[metastructure(field = "frames.delay")] pub frames_delay: Annotated, /// Other fields in `span.data`. @@ -547,7 +547,9 @@ mod tests { user: ~, replay_id: ~, sdk_name: ~, - frames_delay: ~, + frames_delay: I64( + 100, + ), other: { "bar": String( "3", @@ -555,9 +557,6 @@ mod tests { "foo": I64( 2, ), - "frames.delay": I64( - 100, - ), }, } "###); diff --git a/relay-server/src/metrics_extraction/snapshots/relay_server__metrics_extraction__event__tests__extract_span_metrics_mobile.snap b/relay-server/src/metrics_extraction/snapshots/relay_server__metrics_extraction__event__tests__extract_span_metrics_mobile.snap index 18057b45a84..257c79b7da0 100644 --- a/relay-server/src/metrics_extraction/snapshots/relay_server__metrics_extraction__event__tests__extract_span_metrics_mobile.snap +++ b/relay-server/src/metrics_extraction/snapshots/relay_server__metrics_extraction__event__tests__extract_span_metrics_mobile.snap @@ -106,11 +106,10 @@ expression: "(&event.value().unwrap().spans, metrics)" user: ~, replay_id: ~, sdk_name: ~, - other: { - "frames.delay": I64( - 123, - ), - }, + frames_delay: I64( + 123, + ), + other: {}, }, sentry_tags: { "app_start_type": "warm", @@ -382,6 +381,7 @@ expression: "(&event.value().unwrap().spans, metrics)" user: ~, replay_id: ~, sdk_name: ~, + frames_delay: ~, other: {}, }, sentry_tags: { @@ -465,6 +465,7 @@ expression: "(&event.value().unwrap().spans, metrics)" user: ~, replay_id: ~, sdk_name: ~, + frames_delay: ~, other: {}, }, sentry_tags: { @@ -752,28 +753,6 @@ expression: "(&event.value().unwrap().spans, metrics)" merges: 1, }, }, - Bucket { - timestamp: UnixTimestamp(1597976303), - width: 0, - name: MetricName( - "d:spans/mobile.frames_delay@millisecond", - ), - value: Distribution( - [ - 123.0, - ], - ), - tags: { - "device.class": "1", - "os.name": "iOS", - "release": "1.2.3", - "span.op": "ui.load.initial_display", - "transaction": "gEt /api/:version/users/", - }, - metadata: BucketMetadata { - merges: 1, - }, - }, Bucket { timestamp: UnixTimestamp(1597976303), width: 0, diff --git a/relay-spans/src/span.rs b/relay-spans/src/span.rs index 723f85cd795..7698411d300 100644 --- a/relay-spans/src/span.rs +++ b/relay-spans/src/span.rs @@ -545,6 +545,7 @@ mod tests { user: ~, replay_id: ~, sdk_name: "sentry.php", + frames_delay: ~, other: {}, }, sentry_tags: ~, From 004bad61133258126565dfe47db0e5c96511768a Mon Sep 17 00:00:00 2001 From: Nar Saynorath Date: Tue, 23 Apr 2024 00:19:53 -0400 Subject: [PATCH 5/8] Fix span getter --- relay-event-schema/src/protocol/span.rs | 1 + ...t__tests__extract_span_metrics_mobile.snap | 22 +++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/relay-event-schema/src/protocol/span.rs b/relay-event-schema/src/protocol/span.rs index b8f430a9c2a..593dc8322b5 100644 --- a/relay-event-schema/src/protocol/span.rs +++ b/relay-event-schema/src/protocol/span.rs @@ -320,6 +320,7 @@ impl Getter for SpanData { "db.operation" => self.db_operation.value()?.into(), "db\\.system" => self.db_system.value()?.into(), "environment" => self.environment.as_str()?.into(), + "frames\\.delay" => self.frames_delay.value()?.into(), "http\\.decoded_response_content_length" => { self.http_decoded_response_content_length.value()?.into() } diff --git a/relay-server/src/metrics_extraction/snapshots/relay_server__metrics_extraction__event__tests__extract_span_metrics_mobile.snap b/relay-server/src/metrics_extraction/snapshots/relay_server__metrics_extraction__event__tests__extract_span_metrics_mobile.snap index 257c79b7da0..cc417b12e6c 100644 --- a/relay-server/src/metrics_extraction/snapshots/relay_server__metrics_extraction__event__tests__extract_span_metrics_mobile.snap +++ b/relay-server/src/metrics_extraction/snapshots/relay_server__metrics_extraction__event__tests__extract_span_metrics_mobile.snap @@ -753,6 +753,28 @@ expression: "(&event.value().unwrap().spans, metrics)" merges: 1, }, }, + Bucket { + timestamp: UnixTimestamp(1597976303), + width: 0, + name: MetricName( + "d:spans/mobile.frames_delay@millisecond", + ), + value: Distribution( + [ + 123.0, + ], + ), + tags: { + "device.class": "1", + "os.name": "iOS", + "release": "1.2.3", + "span.op": "ui.load.initial_display", + "transaction": "gEt /api/:version/users/", + }, + metadata: BucketMetadata { + merges: 1, + }, + }, Bucket { timestamp: UnixTimestamp(1597976303), width: 0, From 9f85a7245fb510b389b6d92c591a001931e84b06 Mon Sep 17 00:00:00 2001 From: Nar Saynorath Date: Wed, 24 Apr 2024 15:41:59 -0400 Subject: [PATCH 6/8] Fix metric unit type --- relay-dynamic-config/src/defaults.rs | 2 +- .../src/normalize/span/tag_extraction.rs | 17 +++++++++++------ relay-event-schema/src/protocol/span.rs | 1 - 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/relay-dynamic-config/src/defaults.rs b/relay-dynamic-config/src/defaults.rs index c9fe1fdcddc..fc675ac25bb 100644 --- a/relay-dynamic-config/src/defaults.rs +++ b/relay-dynamic-config/src/defaults.rs @@ -730,7 +730,7 @@ fn span_metrics( MetricSpec { category: DataCategory::Span, mri: "d:spans/mobile.frames_delay@second".into(), - field: Some("span.data.frames\\.delay".into()), + field: Some("span.measurements.frames.delay.value".into()), condition: Some(is_mobile.clone() & duration_condition.clone()), tags: vec![ Tag::with_key("transaction") diff --git a/relay-event-normalization/src/normalize/span/tag_extraction.rs b/relay-event-normalization/src/normalize/span/tag_extraction.rs index 13f49fe9f7c..ced57429313 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 std::ops::ControlFlow; use once_cell::sync::Lazy; use regex::Regex; -use relay_base_schema::metrics::{InformationUnit, MetricUnit}; +use relay_base_schema::metrics::{DurationUnit, InformationUnit, MetricUnit}; use relay_event_schema::protocol::{ AppContext, BrowserContext, Event, Measurement, OsContext, Span, Timestamp, TraceContext, }; @@ -662,10 +662,15 @@ pub fn extract_measurements(span: &mut Span, is_mobile: bool) { if is_mobile { if let Some(data) = span.data.value() { - for (field, key) in [ - (&data.frames_frozen, "frames.frozen"), - (&data.frames_slow, "frames.slow"), - (&data.frames_total, "frames.total"), + for (field, key, unit) in [ + (&data.frames_frozen, "frames.frozen", MetricUnit::None), + (&data.frames_slow, "frames.slow", MetricUnit::None), + (&data.frames_total, "frames.total", MetricUnit::None), + ( + &data.frames_delay, + "frames.delay", + MetricUnit::Duration(DurationUnit::Second), + ), ] { if let Some(value) = match field.value() { Some(Value::F64(f)) => Some(*f), @@ -678,7 +683,7 @@ pub fn extract_measurements(span: &mut Span, is_mobile: bool) { key.into(), Measurement { value: value.into(), - unit: MetricUnit::None.into(), + unit: unit.into(), } .into(), ); diff --git a/relay-event-schema/src/protocol/span.rs b/relay-event-schema/src/protocol/span.rs index 24ee9626703..5fcc9e5e5a3 100644 --- a/relay-event-schema/src/protocol/span.rs +++ b/relay-event-schema/src/protocol/span.rs @@ -336,7 +336,6 @@ impl Getter for SpanData { "db.operation" => self.db_operation.value()?.into(), "db\\.system" => self.db_system.value()?.into(), "environment" => self.environment.as_str()?.into(), - "frames\\.delay" => self.frames_delay.value()?.into(), "http\\.decoded_response_content_length" => { self.http_decoded_response_content_length.value()?.into() } From 85e51e02853ed181e6d3149a0f8971548c60917d Mon Sep 17 00:00:00 2001 From: Nar Saynorath Date: Wed, 24 Apr 2024 15:44:34 -0400 Subject: [PATCH 7/8] Add frames.delay to mobile event test case --- relay-server/src/metrics_extraction/event.rs | 3 +- ...t__tests__extract_span_metrics_mobile.snap | 32 ++++++++++++++++++- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/relay-server/src/metrics_extraction/event.rs b/relay-server/src/metrics_extraction/event.rs index f1f10a4a987..8b2b739c13d 100644 --- a/relay-server/src/metrics_extraction/event.rs +++ b/relay-server/src/metrics_extraction/event.rs @@ -1155,7 +1155,8 @@ mod tests { "data": { "frames.slow": 1, "frames.frozen": 2, - "frames.total": 9 + "frames.total": 9, + "frames.delay": 0.1 } }, { diff --git a/relay-server/src/metrics_extraction/snapshots/relay_server__metrics_extraction__event__tests__extract_span_metrics_mobile.snap b/relay-server/src/metrics_extraction/snapshots/relay_server__metrics_extraction__event__tests__extract_span_metrics_mobile.snap index 77de7ca41e0..e0294ea6be9 100644 --- a/relay-server/src/metrics_extraction/snapshots/relay_server__metrics_extraction__event__tests__extract_span_metrics_mobile.snap +++ b/relay-server/src/metrics_extraction/snapshots/relay_server__metrics_extraction__event__tests__extract_span_metrics_mobile.snap @@ -116,7 +116,9 @@ expression: "(&event.value().unwrap().spans, metrics)" frames_total: I64( 9, ), - frames_delay: ~, + frames_delay: F64( + 0.1, + ), other: {}, }, sentry_tags: { @@ -137,6 +139,12 @@ expression: "(&event.value().unwrap().spans, metrics)" received: ~, measurements: Measurements( { + "frames.delay": Measurement { + value: 0.1, + unit: Duration( + Second, + ), + }, "frames.frozen": Measurement { value: 2.0, unit: None, @@ -862,6 +870,28 @@ expression: "(&event.value().unwrap().spans, metrics)" merges: 1, }, }, + Bucket { + timestamp: UnixTimestamp(1597976303), + width: 0, + name: MetricName( + "d:spans/mobile.frames_delay@second", + ), + value: Distribution( + [ + 0.1, + ], + ), + tags: { + "device.class": "1", + "os.name": "iOS", + "release": "1.2.3", + "span.op": "ui.load.initial_display", + "transaction": "gEt /api/:version/users/", + }, + metadata: BucketMetadata { + merges: 1, + }, + }, Bucket { timestamp: UnixTimestamp(1597976303), width: 0, From c2c95ad1100153402ce67ce286ad408b7e32cc48 Mon Sep 17 00:00:00 2001 From: Nar Saynorath Date: Thu, 25 Apr 2024 08:39:51 -0400 Subject: [PATCH 8/8] Use gauge --- relay-dynamic-config/src/defaults.rs | 2 +- ..._event__tests__extract_span_metrics_mobile.snap | 14 +++++++++----- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/relay-dynamic-config/src/defaults.rs b/relay-dynamic-config/src/defaults.rs index fc675ac25bb..9b4bea4c016 100644 --- a/relay-dynamic-config/src/defaults.rs +++ b/relay-dynamic-config/src/defaults.rs @@ -729,7 +729,7 @@ fn span_metrics( }, MetricSpec { category: DataCategory::Span, - mri: "d:spans/mobile.frames_delay@second".into(), + mri: "g:spans/mobile.frames_delay@second".into(), field: Some("span.measurements.frames.delay.value".into()), condition: Some(is_mobile.clone() & duration_condition.clone()), tags: vec![ diff --git a/relay-server/src/metrics_extraction/snapshots/relay_server__metrics_extraction__event__tests__extract_span_metrics_mobile.snap b/relay-server/src/metrics_extraction/snapshots/relay_server__metrics_extraction__event__tests__extract_span_metrics_mobile.snap index e0294ea6be9..f49fc457a8e 100644 --- a/relay-server/src/metrics_extraction/snapshots/relay_server__metrics_extraction__event__tests__extract_span_metrics_mobile.snap +++ b/relay-server/src/metrics_extraction/snapshots/relay_server__metrics_extraction__event__tests__extract_span_metrics_mobile.snap @@ -874,12 +874,16 @@ expression: "(&event.value().unwrap().spans, metrics)" timestamp: UnixTimestamp(1597976303), width: 0, name: MetricName( - "d:spans/mobile.frames_delay@second", + "g:spans/mobile.frames_delay@second", ), - value: Distribution( - [ - 0.1, - ], + value: Gauge( + GaugeValue { + last: 0.1, + min: 0.1, + max: 0.1, + sum: 0.1, + count: 1, + }, ), tags: { "device.class": "1",