From b8320bc9c2bb7056363c67fb4bb3cf53ae96fbee Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 9 Aug 2023 11:03:33 -0400 Subject: [PATCH 1/3] feat(spans): Improve span data http method extraction Check `http.request.method` and `method` fields when trying to extract `span.action` from an http span. --- relay-general/src/store/normalize/span/tag_extraction.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/relay-general/src/store/normalize/span/tag_extraction.rs b/relay-general/src/store/normalize/span/tag_extraction.rs index 107165daed..4469592795 100644 --- a/relay-general/src/store/normalize/span/tag_extraction.rs +++ b/relay-general/src/store/normalize/span/tag_extraction.rs @@ -200,15 +200,18 @@ pub(crate) fn extract_tags(span: &Span, config: &Config) -> BTreeMap span .data .value() - // TODO(iker): some SDKs extract this as method - .and_then(|v| v.get("http.method")) + .and_then(|v| { + v.get("http.request.method") + .or(v.get("http.method")) + .or(v.get("method")) + }) .and_then(|method| method.as_str()) .map(|s| s.to_uppercase()), (_, "db.redis", Some(desc)) => { From 2144d6a170ca1c3983afe00afa0e5684ebeb5779 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 9 Aug 2023 13:57:14 -0400 Subject: [PATCH 2/3] Add snapshot tests --- .../src/metrics_extraction/spans/mod.rs | 34 +++- ..._spans__tests__extract_span_metrics-2.snap | 102 +++++++++-- ...n__spans__tests__extract_span_metrics.snap | 168 ++++++++++++++++-- 3 files changed, 279 insertions(+), 25 deletions(-) diff --git a/relay-server/src/metrics_extraction/spans/mod.rs b/relay-server/src/metrics_extraction/spans/mod.rs index 081cce959f..b27295e6a1 100644 --- a/relay-server/src/metrics_extraction/spans/mod.rs +++ b/relay-server/src/metrics_extraction/spans/mod.rs @@ -168,6 +168,30 @@ mod tests { "http.method": "GET" } }, + { + "description": "POST http://domain.tld/hi", + "op": "http.client", + "parent_span_id": "8f5a2b8768cafb4e", + "span_id": "bd429c44b67a3eb4", + "start_timestamp": 1597976300.0000000, + "timestamp": 1597976302.0000000, + "trace_id": "ff62a8b040f340bda5d830223def1d81", + "data": { + "http.request.method": "POST" + } + }, + { + "description": "PUT http://domain.tld/hi", + "op": "http.client", + "parent_span_id": "8f5a2b8768cafb4e", + "span_id": "bd429c44b67a3eb4", + "start_timestamp": 1597976300.0000000, + "timestamp": 1597976302.0000000, + "trace_id": "ff62a8b040f340bda5d830223def1d81", + "data": { + "method": "PUT" + } + }, { "description": "GET /hi/this/is/just/the/path", "op": "http.client", @@ -279,7 +303,7 @@ mod tests { "trace_id": "ff62a8b040f340bda5d830223def1d81", "status": "ok", "data": { - "db.system": "MyDatabase", + "db.system": "postgresql", "db.operation": "SELECT" } }, @@ -303,7 +327,7 @@ mod tests { "trace_id": "ff62a8b040f340bda5d830223def1d81", "status": "ok", "data": { - "db.system": "MyDatabase", + "db.system": "postgresql", "db.operation": "INSERT" } }, @@ -317,7 +341,7 @@ mod tests { "trace_id": "ff62a8b040f340bda5d830223def1d81", "status": "ok", "data": { - "db.system": "MyDatabase", + "db.system": "postgresql", "db.operation": "INSERT" } }, @@ -341,7 +365,7 @@ mod tests { "trace_id": "ff62a8b040f340bda5d830223def1d81", "status": "ok", "data": { - "db.system": "MyDatabase", + "db.system": "postgresql", "db.operation": "SELECT" } }, @@ -355,7 +379,7 @@ mod tests { "trace_id": "ff62a8b040f340bda5d830223def1d81", "status": "ok", "data": { - "db.system": "MyDatabase", + "db.system": "postgresql", "db.operation": "SELECT" } }, diff --git a/relay-server/src/metrics_extraction/spans/snapshots/relay_server__metrics_extraction__spans__tests__extract_span_metrics-2.snap b/relay-server/src/metrics_extraction/spans/snapshots/relay_server__metrics_extraction__spans__tests__extract_span_metrics-2.snap index 7a77037371..8bf00a0a00 100644 --- a/relay-server/src/metrics_extraction/spans/snapshots/relay_server__metrics_extraction__spans__tests__extract_span_metrics-2.snap +++ b/relay-server/src/metrics_extraction/spans/snapshots/relay_server__metrics_extraction__spans__tests__extract_span_metrics-2.snap @@ -75,6 +75,88 @@ expression: metrics "transaction.op": "myop", }, }, + Metric { + name: "d:spans/exclusive_time@millisecond", + value: Distribution( + 2000.0, + ), + timestamp: UnixTimestamp(1597976302), + tags: { + "environment": "fake_environment", + "http.status_code": "500", + "span.action": "POST", + "span.category": "http", + "span.description": "POST http://domain.tld", + "span.domain": "domain.tld", + "span.group": "25faf23529f71d3e", + "span.module": "http", + "span.op": "http.client", + "transaction": "gEt /api/:version/users/", + "transaction.method": "POST", + "transaction.op": "myop", + }, + }, + Metric { + name: "d:spans/exclusive_time_light@millisecond", + value: Distribution( + 2000.0, + ), + timestamp: UnixTimestamp(1597976302), + tags: { + "environment": "fake_environment", + "http.status_code": "500", + "span.action": "POST", + "span.category": "http", + "span.description": "POST http://domain.tld", + "span.domain": "domain.tld", + "span.group": "25faf23529f71d3e", + "span.module": "http", + "span.op": "http.client", + "transaction.method": "POST", + "transaction.op": "myop", + }, + }, + Metric { + name: "d:spans/exclusive_time@millisecond", + value: Distribution( + 2000.0, + ), + timestamp: UnixTimestamp(1597976302), + tags: { + "environment": "fake_environment", + "http.status_code": "500", + "span.action": "PUT", + "span.category": "http", + "span.description": "PUT http://domain.tld", + "span.domain": "domain.tld", + "span.group": "488f09b46e5978be", + "span.module": "http", + "span.op": "http.client", + "transaction": "gEt /api/:version/users/", + "transaction.method": "POST", + "transaction.op": "myop", + }, + }, + Metric { + name: "d:spans/exclusive_time_light@millisecond", + value: Distribution( + 2000.0, + ), + timestamp: UnixTimestamp(1597976302), + tags: { + "environment": "fake_environment", + "http.status_code": "500", + "span.action": "PUT", + "span.category": "http", + "span.description": "PUT http://domain.tld", + "span.domain": "domain.tld", + "span.group": "488f09b46e5978be", + "span.module": "http", + "span.op": "http.client", + "transaction.method": "POST", + "transaction.op": "myop", + }, + }, Metric { name: "d:spans/exclusive_time@millisecond", value: Distribution( @@ -399,7 +481,7 @@ expression: metrics "span.module": "db", "span.op": "db.sql.query", "span.status": "ok", - "span.system": "mydatabase", + "span.system": "postgresql", "transaction": "gEt /api/:version/users/", "transaction.method": "POST", "transaction.op": "myop", @@ -422,7 +504,7 @@ expression: metrics "span.module": "db", "span.op": "db.sql.query", "span.status": "ok", - "span.system": "mydatabase", + "span.system": "postgresql", "transaction.method": "POST", "transaction.op": "myop", }, @@ -485,7 +567,7 @@ expression: metrics "span.module": "db", "span.op": "db.sql.query", "span.status": "ok", - "span.system": "mydatabase", + "span.system": "postgresql", "transaction": "gEt /api/:version/users/", "transaction.method": "POST", "transaction.op": "myop", @@ -506,7 +588,7 @@ expression: metrics "span.module": "db", "span.op": "db.sql.query", "span.status": "ok", - "span.system": "mydatabase", + "span.system": "postgresql", "transaction.method": "POST", "transaction.op": "myop", }, @@ -526,7 +608,7 @@ expression: metrics "span.module": "db", "span.op": "db.sql.query", "span.status": "ok", - "span.system": "mydatabase", + "span.system": "postgresql", "transaction": "gEt /api/:version/users/", "transaction.method": "POST", "transaction.op": "myop", @@ -547,7 +629,7 @@ expression: metrics "span.module": "db", "span.op": "db.sql.query", "span.status": "ok", - "span.system": "mydatabase", + "span.system": "postgresql", "transaction.method": "POST", "transaction.op": "myop", }, @@ -608,7 +690,7 @@ expression: metrics "span.module": "db", "span.op": "db.sql.query", "span.status": "ok", - "span.system": "mydatabase", + "span.system": "postgresql", "transaction": "gEt /api/:version/users/", "transaction.method": "POST", "transaction.op": "myop", @@ -631,7 +713,7 @@ expression: metrics "span.module": "db", "span.op": "db.sql.query", "span.status": "ok", - "span.system": "mydatabase", + "span.system": "postgresql", "transaction.method": "POST", "transaction.op": "myop", }, @@ -653,7 +735,7 @@ expression: metrics "span.module": "db", "span.op": "db", "span.status": "ok", - "span.system": "mydatabase", + "span.system": "postgresql", "transaction": "gEt /api/:version/users/", "transaction.method": "POST", "transaction.op": "myop", @@ -676,7 +758,7 @@ expression: metrics "span.module": "db", "span.op": "db", "span.status": "ok", - "span.system": "mydatabase", + "span.system": "postgresql", "transaction.method": "POST", "transaction.op": "myop", }, diff --git a/relay-server/src/metrics_extraction/spans/snapshots/relay_server__metrics_extraction__spans__tests__extract_span_metrics.snap b/relay-server/src/metrics_extraction/spans/snapshots/relay_server__metrics_extraction__spans__tests__extract_span_metrics.snap index 5f461c99ca..a2236d4e78 100644 --- a/relay-server/src/metrics_extraction/spans/snapshots/relay_server__metrics_extraction__spans__tests__extract_span_metrics.snap +++ b/relay-server/src/metrics_extraction/spans/snapshots/relay_server__metrics_extraction__spans__tests__extract_span_metrics.snap @@ -130,6 +130,154 @@ expression: event.value().unwrap().spans }, other: {}, }, + Span { + timestamp: Timestamp( + 2020-08-21T02:18:22Z, + ), + start_timestamp: Timestamp( + 2020-08-21T02:18:20Z, + ), + exclusive_time: 2000.0, + description: "POST http://domain.tld/hi", + op: "http.client", + span_id: SpanId( + "bd429c44b67a3eb4", + ), + parent_span_id: SpanId( + "8f5a2b8768cafb4e", + ), + trace_id: TraceId( + "ff62a8b040f340bda5d830223def1d81", + ), + status: ~, + tags: ~, + origin: ~, + data: { + "description.scrubbed": String( + "POST http://domain.tld", + ), + "environment": String( + "fake_environment", + ), + "http.request.method": String( + "POST", + ), + "http.status_code": String( + "500", + ), + "release": String( + "1.2.3", + ), + "span.action": String( + "POST", + ), + "span.category": String( + "http", + ), + "span.description": String( + "POST http://domain.tld", + ), + "span.domain": String( + "domain.tld", + ), + "span.group": String( + "25faf23529f71d3e", + ), + "span.module": String( + "http", + ), + "span.op": String( + "http.client", + ), + "transaction": String( + "gEt /api/:version/users/", + ), + "transaction.method": String( + "POST", + ), + "transaction.op": String( + "myop", + ), + "user": String( + "id:user123", + ), + }, + other: {}, + }, + Span { + timestamp: Timestamp( + 2020-08-21T02:18:22Z, + ), + start_timestamp: Timestamp( + 2020-08-21T02:18:20Z, + ), + exclusive_time: 2000.0, + description: "PUT http://domain.tld/hi", + op: "http.client", + span_id: SpanId( + "bd429c44b67a3eb4", + ), + parent_span_id: SpanId( + "8f5a2b8768cafb4e", + ), + trace_id: TraceId( + "ff62a8b040f340bda5d830223def1d81", + ), + status: ~, + tags: ~, + origin: ~, + data: { + "description.scrubbed": String( + "PUT http://domain.tld", + ), + "environment": String( + "fake_environment", + ), + "http.status_code": String( + "500", + ), + "method": String( + "PUT", + ), + "release": String( + "1.2.3", + ), + "span.action": String( + "PUT", + ), + "span.category": String( + "http", + ), + "span.description": String( + "PUT http://domain.tld", + ), + "span.domain": String( + "domain.tld", + ), + "span.group": String( + "488f09b46e5978be", + ), + "span.module": String( + "http", + ), + "span.op": String( + "http.client", + ), + "transaction": String( + "gEt /api/:version/users/", + ), + "transaction.method": String( + "POST", + ), + "transaction.op": String( + "myop", + ), + "user": String( + "id:user123", + ), + }, + other: {}, + }, Span { timestamp: Timestamp( 2020-08-21T02:18:22Z, @@ -728,7 +876,7 @@ expression: event.value().unwrap().spans "SELECT", ), "db.system": String( - "MyDatabase", + "postgresql", ), "description.scrubbed": String( "SeLeCt column FROM tAbLe WHERE id IN (%s)", @@ -767,7 +915,7 @@ expression: event.value().unwrap().spans "ok", ), "span.system": String( - "mydatabase", + "postgresql", ), "transaction": String( "gEt /api/:version/users/", @@ -885,7 +1033,7 @@ expression: event.value().unwrap().spans "INSERT", ), "db.system": String( - "MyDatabase", + "postgresql", ), "environment": String( "fake_environment", @@ -915,7 +1063,7 @@ expression: event.value().unwrap().spans "ok", ), "span.system": String( - "mydatabase", + "postgresql", ), "transaction": String( "gEt /api/:version/users/", @@ -959,7 +1107,7 @@ expression: event.value().unwrap().spans "INSERT", ), "db.system": String( - "MyDatabase", + "postgresql", ), "environment": String( "fake_environment", @@ -989,7 +1137,7 @@ expression: event.value().unwrap().spans "ok", ), "span.system": String( - "mydatabase", + "postgresql", ), "transaction": String( "gEt /api/:version/users/", @@ -1098,7 +1246,7 @@ expression: event.value().unwrap().spans "SELECT", ), "db.system": String( - "MyDatabase", + "postgresql", ), "description.scrubbed": String( "SELECT * FROM table WHERE id IN (val)", @@ -1137,7 +1285,7 @@ expression: event.value().unwrap().spans "ok", ), "span.system": String( - "mydatabase", + "postgresql", ), "transaction": String( "gEt /api/:version/users/", @@ -1181,7 +1329,7 @@ expression: event.value().unwrap().spans "SELECT", ), "db.system": String( - "MyDatabase", + "postgresql", ), "description.scrubbed": String( "SELECT col FROM table WHERE col = %s", @@ -1220,7 +1368,7 @@ expression: event.value().unwrap().spans "ok", ), "span.system": String( - "mydatabase", + "postgresql", ), "transaction": String( "gEt /api/:version/users/", From fa29e0d3252229293579d3a824ef30ba89ef2f59 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Thu, 10 Aug 2023 09:47:52 -0400 Subject: [PATCH 3/3] amend docstring to add details about spec used --- relay-general/src/store/normalize/span/tag_extraction.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/relay-general/src/store/normalize/span/tag_extraction.rs b/relay-general/src/store/normalize/span/tag_extraction.rs index 4469592795..5cd0b61ebc 100644 --- a/relay-general/src/store/normalize/span/tag_extraction.rs +++ b/relay-general/src/store/normalize/span/tag_extraction.rs @@ -169,6 +169,11 @@ fn extract_shared_tags(event: &Event) -> BTreeMap { } /// Writes fields into [`Span::data`]. +/// +/// Generating new span data fields is based on a combination of looking at +/// [span operations](https://develop.sentry.dev/sdk/performance/span-operations/) and +/// existing [span data](https://develop.sentry.dev/sdk/performance/span-data-conventions/) fields, +/// and rely on Sentry conventions and heuristics. pub(crate) fn extract_tags(span: &Span, config: &Config) -> BTreeMap { let mut span_tags: BTreeMap = BTreeMap::new(); if let Some(unsanitized_span_op) = span.op.value() { @@ -200,9 +205,6 @@ pub(crate) fn extract_tags(span: &Span, config: &Config) -> BTreeMap span .data