Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat(spans): Extract trace context status for indexed spans #3407

Merged
merged 6 commits into from
Apr 15, 2024
Merged

feat(spans): Extract trace context status for indexed spans #3407

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4e23ffc
Extract trace context status
edwardgou-sentry Apr 10, 2024
c36cee1
fix comma
edwardgou-sentry Apr 10, 2024
e7b1de3
fix test and tag key
edwardgou-sentry Apr 10, 2024
fd126a4
snapshots
edwardgou-sentry Apr 10, 2024
11fbb77
integration test
edwardgou-sentry Apr 10, 2024
7c43528
fix more integration test
edwardgou-sentry Apr 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions relay-event-normalization/src/normalize/span/tag_extraction.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ pub enum SpanTagKey {
AppStartType,
ReplayId,
CacheHit,
TraceStatus,
}

impl SpanTagKey {
Expand Down Expand Up @@ -109,6 +110,7 @@ impl SpanTagKey {
SpanTagKey::OsName => "os.name",
SpanTagKey::AppStartType => "app_start_type",
SpanTagKey::ReplayId => "replay_id",
SpanTagKey::TraceStatus => "trace.status",
}
}
}
Expand Down Expand Up @@ -228,6 +230,10 @@ fn extract_shared_tags(event: &Event) -> BTreeMap<SpanTagKey, String> {
if let Some(op) = extract_transaction_op(trace_context) {
tags.insert(SpanTagKey::TransactionOp, op.to_lowercase().to_owned());
}

if let Some(status) = trace_context.status.value() {
tags.insert(SpanTagKey::TraceStatus, status.to_string());
}
}

if MOBILE_SDKS.contains(&event.sdk_name()) {
Expand Down Expand Up @@ -1635,6 +1641,49 @@ LIMIT 1
);
}

#[test]
fn test_extract_trace_status() {
let json = r#"

{
"type": "transaction",
"platform": "python",
"start_timestamp": "2021-04-26T07:59:01+0100",
"timestamp": "2021-04-26T08:00:00+0100",
"transaction": "foo",
"contexts": {
"trace": {
"status": "ok"
}
},
"spans": [
{
"op": "resource.script",
"span_id": "bd429c44b67a3eb1",
"start_timestamp": 1597976300.0000000,
"timestamp": 1597976302.0000000,
"trace_id": "ff62a8b040f340bda5d830223def1d81"
}
]
}
"#;

let mut event = Annotated::<Event>::from_json(json)
.unwrap()
.into_value()
.unwrap();

extract_span_tags_from_event(&mut event, 200);

let span = &event.spans.value().unwrap()[0];
let tags = span.value().unwrap().sentry_tags.value().unwrap();

assert_eq!(
tags.get("trace.status"),
Some(&Annotated::new("ok".to_string()))
);
}

fn extract_tags_supabase(description: impl Into<String>) -> BTreeMap<SpanTagKey, String> {
let json = r#"{
"description": "from(my_table)",
Expand Down
8 changes: 8 additions & 0 deletions ...napshots/relay_server__metrics_extraction__event__tests__extract_span_metrics_mobile.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ expression: "(&event.value().unwrap().spans, metrics)"
"release": "1.2.3",
"sdk.name": "sentry.javascript.react-native",
"sdk.version": "unknown",
"trace.status": "unknown",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where does this unknown come from and is that something we want?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ^. The code LGTM, but according to dev docs, unknown means:

An unknown error raised by APIs that don't return enough error information

And trace.status:

Optional. Whether the trace failed or succeeded.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We coalesce this tag to unknown if theres no trace status supplied. I think we do want this since unknown is a valid value for trace.status and it matches the intended usage to me.

I'll use this for now and we can update this later if needed.

"transaction": "gEt /api/:version/users/",
"transaction.method": "GET",
"ttid": "ttid",
Expand Down Expand Up @@ -84,6 +85,7 @@ expression: "(&event.value().unwrap().spans, metrics)"
"release": "1.2.3",
"sdk.name": "sentry.javascript.react-native",
"sdk.version": "unknown",
"trace.status": "unknown",
"transaction": "gEt /api/:version/users/",
"transaction.method": "GET",
"ttid": "ttid",
Expand Down Expand Up @@ -132,6 +134,7 @@ expression: "(&event.value().unwrap().spans, metrics)"
"release": "1.2.3",
"sdk.name": "sentry.javascript.react-native",
"sdk.version": "unknown",
"trace.status": "unknown",
"transaction": "gEt /api/:version/users/",
"transaction.method": "GET",
"ttid": "ttid",
Expand Down Expand Up @@ -177,6 +180,7 @@ expression: "(&event.value().unwrap().spans, metrics)"
"release": "1.2.3",
"sdk.name": "sentry.javascript.react-native",
"sdk.version": "unknown",
"trace.status": "unknown",
"transaction": "gEt /api/:version/users/",
"transaction.method": "GET",
"ttid": "ttid",
Expand Down Expand Up @@ -224,6 +228,7 @@ expression: "(&event.value().unwrap().spans, metrics)"
"release": "1.2.3",
"sdk.name": "sentry.javascript.react-native",
"sdk.version": "unknown",
"trace.status": "unknown",
"transaction": "gEt /api/:version/users/",
"transaction.method": "GET",
"ttid": "ttid",
Expand Down Expand Up @@ -271,6 +276,7 @@ expression: "(&event.value().unwrap().spans, metrics)"
"release": "1.2.3",
"sdk.name": "sentry.javascript.react-native",
"sdk.version": "unknown",
"trace.status": "unknown",
"transaction": "gEt /api/:version/users/",
"transaction.method": "GET",
"ttid": "ttid",
Expand Down Expand Up @@ -352,6 +358,7 @@ expression: "(&event.value().unwrap().spans, metrics)"
"release": "1.2.3",
"sdk.name": "sentry.javascript.react-native",
"sdk.version": "unknown",
"trace.status": "unknown",
"transaction": "gEt /api/:version/users/",
"transaction.method": "GET",
"ttid": "ttid",
Expand Down Expand Up @@ -431,6 +438,7 @@ expression: "(&event.value().unwrap().spans, metrics)"
"release": "1.2.3",
"sdk.name": "sentry.javascript.react-native",
"sdk.version": "unknown",
"trace.status": "unknown",
"transaction": "gEt /api/:version/users/",
"transaction.method": "GET",
"ttid": "ttid",
Expand Down
4 changes: 4 additions & 0 deletions tests/integration/test_spans.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ def test_span_extraction(
"platform": "other",
"sdk.name": "raven-node",
"sdk.version": "2.6.3",
"trace.status": "unknown",
"transaction": "hi",
"transaction.op": "hi",
},
Expand Down Expand Up @@ -133,6 +134,7 @@ def test_span_extraction(
"platform": "other",
"sdk.name": "raven-node",
"sdk.version": "2.6.3",
"trace.status": "unknown",
"transaction": "hi",
"transaction.op": "hi",
},
Expand Down Expand Up @@ -777,6 +779,7 @@ def test_span_extraction_with_metrics_summary(
"platform": "other",
"sdk.name": "raven-node",
"sdk.version": "2.6.3",
"trace.status": "unknown",
"transaction": "hi",
"transaction.op": "hi",
},
Expand Down Expand Up @@ -896,6 +899,7 @@ def test_span_extraction_with_ddm_missing_values(
"platform": "other",
"sdk.name": "raven-node",
"sdk.version": "2.6.3",
"trace.status": "unknown",
"transaction": "hi",
"transaction.op": "hi",
},
Expand Down
Loading