-
Notifications
You must be signed in to change notification settings - Fork 104
feat(spans): Add more fields to span <-> transaction conversion #3456
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 14 commits
12c5a46
b089c25
0cfeb26
d429e7f
f9534d8
0a12ca6
0ad1b78
b409f25
14cfbdb
b1bd53a
d03e861
f9218a9
3b1f4b4
bec3ac2
1ca9d96
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -165,7 +165,7 @@ pub struct SpanData { | |||||||||
|
|
||||||||||
| /// The client's browser name. | ||||||||||
| #[metastructure(field = "browser.name")] | ||||||||||
| pub browser_name: Annotated<Value>, | ||||||||||
| pub browser_name: Annotated<String>, | ||||||||||
|
|
||||||||||
| /// The source code file name that identifies the code unit as uniquely as possible. | ||||||||||
| #[metastructure(field = "code.filepath", pii = "maybe")] | ||||||||||
|
|
@@ -274,7 +274,8 @@ pub struct SpanData { | |||||||||
| /// Origin Transaction name of the span. | ||||||||||
| /// | ||||||||||
| /// For INP spans, this is the route name where the interaction occurred. | ||||||||||
| pub transaction: Annotated<String>, | ||||||||||
| #[metastructure(field = "segment.name", legacy_alias = "transaction")] | ||||||||||
| pub segment_name: Annotated<String>, | ||||||||||
|
|
||||||||||
| /// Name of the UI component (e.g. React). | ||||||||||
| #[metastructure(field = "ui.component_name")] | ||||||||||
|
|
@@ -292,6 +293,10 @@ pub struct SpanData { | |||||||||
| #[metastructure(field = "replay_id")] | ||||||||||
| pub replay_id: Annotated<Value>, | ||||||||||
|
|
||||||||||
| #[metastructure(field = "sdk.name")] | ||||||||||
| /// The sentry SDK (see [`crate::protocol::ClientSdkInfo`]). | ||||||||||
|
||||||||||
| #[metastructure(field = "sdk.name")] | |
| /// The sentry SDK (see [`crate::protocol::ClientSdkInfo`]). | |
| /// The sentry SDK (see [`crate::protocol::ClientSdkInfo`]). | |
| #[metastructure(field = "sdk.name")] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| //! This module defines bidirectional field mappings between spans and transactions. | ||
| use crate::protocol::{Contexts, Event, ProfileContext, Span, TraceContext}; | ||
| use crate::protocol::{BrowserContext, Contexts, Event, ProfileContext, Span, TraceContext}; | ||
|
|
||
| use relay_base_schema::events::EventType; | ||
| use relay_protocol::Annotated; | ||
|
|
@@ -40,15 +40,18 @@ macro_rules! context_write_path ( | |
| ); | ||
|
|
||
| macro_rules! event_write_path( | ||
| ($event:expr, contexts browser $context_field:ident) => { | ||
| context_write_path!($event, BrowserContext, $context_field) | ||
| }; | ||
| ($event:expr, contexts trace $context_field:ident) => { | ||
| context_write_path!($event, TraceContext, $context_field) | ||
| }; | ||
| ($event:expr, contexts profile $context_field:ident) => { | ||
| context_write_path!($event, ProfileContext, $context_field) | ||
| }; | ||
| ($event:expr, $path_root:ident $(. $path_segment:ident)*) => { | ||
| ($event:expr, $path_root:ident $($path_segment:ident)*) => { | ||
| { | ||
| write_path!($event, $path_root $(. $path_segment:ident)*) | ||
| write_path!($event, $path_root $($path_segment)*) | ||
| } | ||
| }; | ||
| ); | ||
|
|
@@ -63,17 +66,20 @@ macro_rules! context_value ( | |
| ); | ||
|
|
||
| macro_rules! event_value( | ||
| ($event:expr, contexts browser $context_field:ident) => { | ||
| context_value!($event, BrowserContext, $context_field) | ||
| }; | ||
| ($event:expr, contexts trace $context_field:ident) => { | ||
| context_value!($event, TraceContext, $context_field) | ||
| }; | ||
| ($event:expr, contexts profile $context_field:ident) => { | ||
| context_value!($event, ProfileContext, $context_field) | ||
| }; | ||
| ($event:expr, $path_root:ident $(. $path_segment:ident)*) => { | ||
| ($event:expr, $path_root:ident $($path_segment:ident)*) => { | ||
| { | ||
| let value = ($event).$path_root.value(); | ||
| $( | ||
| let value = value.and_then(|value|&value.$path_segment.value()); | ||
| let value = value.and_then(|value|value.$path_segment.value()); | ||
|
Comment on lines
+69
to
+82
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Turned out I had some unused code branches here that needed to be fixed for deep access. |
||
| )* | ||
| value | ||
| } | ||
|
|
@@ -137,6 +143,7 @@ macro_rules! map_fields { | |
| map_fields!( | ||
| span._metrics_summary <=> event._metrics_summary, | ||
| span.description <=> event.transaction, | ||
| span.data.segment_name <=> event.transaction, | ||
| span.measurements <=> event.measurements, | ||
| span.platform <=> event.platform, | ||
| span.received <=> event.received, | ||
|
|
@@ -153,7 +160,9 @@ map_fields!( | |
| span.trace_id <=> event.contexts.trace.trace_id, | ||
| span.profile_id <=> event.contexts.profile.profile_id, | ||
| span.data.release <=> event.release, | ||
| span.data.environment <=> event.environment | ||
| span.data.environment <=> event.environment, | ||
| span.data.browser_name <=> event.contexts.browser.name, | ||
| span.data.sdk_name <=> event.client_sdk.name | ||
| ; | ||
| span.is_segment <= true, | ||
| span.was_transaction <= true | ||
|
|
@@ -165,16 +174,22 @@ map_fields!( | |
| mod tests { | ||
| use relay_protocol::Annotated; | ||
|
|
||
| use crate::protocol::SpanData; | ||
|
|
||
| use super::*; | ||
|
|
||
| #[test] | ||
| fn roundtrip() { | ||
| let event = Annotated::<Event>::from_json( | ||
| r#"{ | ||
| "type": "transaction", | ||
| "platform": "php", | ||
| "sdk": {"name": "sentry.php"}, | ||
| "release": "[email protected]", | ||
| "environment": "prod", | ||
| "transaction": "my 1st transaction", | ||
| "contexts": { | ||
| "browser": {"name": "Chrome"}, | ||
| "profile": {"profile_id": "a0aaaaaaaaaaaaaaaaaaaaaaaaaaaaab"}, | ||
| "trace": { | ||
| "trace_id": "4C79F60C11214EB38604F4AE0781BFB2", | ||
|
|
@@ -217,7 +232,7 @@ mod tests { | |
| timestamp: ~, | ||
| start_timestamp: ~, | ||
| exclusive_time: 123.4, | ||
| description: ~, | ||
| description: "my 1st transaction", | ||
| op: "myop", | ||
| span_id: SpanId( | ||
| "fa90fdead5f74052", | ||
|
|
@@ -240,7 +255,7 @@ mod tests { | |
| ), | ||
| data: SpanData { | ||
| app_start_type: ~, | ||
| browser_name: ~, | ||
| browser_name: "Chrome", | ||
| code_filepath: ~, | ||
| code_lineno: ~, | ||
| code_function: ~, | ||
|
|
@@ -266,11 +281,12 @@ mod tests { | |
| ai_total_tokens_used: ~, | ||
| ai_responses: ~, | ||
| thread_name: ~, | ||
| transaction: ~, | ||
| segment_name: "my 1st transaction", | ||
| ui_component_name: ~, | ||
| url_scheme: ~, | ||
| user: ~, | ||
| replay_id: ~, | ||
| sdk_name: "sentry.php", | ||
| other: {}, | ||
| }, | ||
| sentry_tags: ~, | ||
|
|
@@ -300,7 +316,7 @@ mod tests { | |
| ], | ||
| }, | ||
| ), | ||
| platform: ~, | ||
| platform: "php", | ||
| was_transaction: true, | ||
| other: {}, | ||
| } | ||
|
|
@@ -310,6 +326,23 @@ mod tests { | |
| assert_eq!(event, roundtripped); | ||
| } | ||
|
|
||
| #[test] | ||
| fn segment_name_takes_precedence() { | ||
|
||
| let span = Span { | ||
| is_segment: true.into(), | ||
| description: "This is the description".to_owned().into(), | ||
| data: SpanData { | ||
| segment_name: "This is the segment name".to_owned().into(), | ||
| ..Default::default() | ||
| } | ||
| .into(), | ||
| ..Default::default() | ||
| }; | ||
| let event = Event::try_from(&span).unwrap(); | ||
|
|
||
| assert_eq!(event.transaction.as_str(), Some("This is the segment name")); | ||
| } | ||
|
|
||
| #[test] | ||
| fn no_empty_profile_context() { | ||
| let span = Span { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed to future key.
transactionbecomes a legacy alias.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we update the docs too?