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

fix(spans): Handle strings with escaped characters #2926

Merged
merged 4 commits into from
Jan 9, 2024
Merged

fix(spans): Handle strings with escaped characters #2926

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d1c40e2
fix(spans): Handle strings with escaped characters
phacops Jan 9, 2024
395c88c
Add a CHANGELOG entry
phacops Jan 9, 2024
9eeb1b7
Set field as optional
phacops Jan 9, 2024
dae8132
Add a test for a span without description
phacops Jan 9, 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
3 changes: 1 addition & 2 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@
- Add `duration` metric for mobile app start spans. ([#2906](https://github.com/getsentry/relay/pull/2906))
- Introduce the configuration option `http.global_metrics`. When enabled, Relay submits metric buckets not through regular project-scoped Envelopes, but instead through the global endpoint. When this Relay serves a high number of projects, this can reduce the overall request volume. ([#2902](https://github.com/getsentry/relay/pull/2902))
- Record the size of global metrics requests in statsd as `upstream.metrics.body_size`. ([#2908](https://github.com/getsentry/relay/pull/2908))
- Make Kafka spans compatible with the Snuba span schema. ([#2917](https://github.com/getsentry/relay/pull/2917))
- Only extract span metrics / tags when they are needed. ([#2907](https://github.com/getsentry/relay/pull/2907))
- Make Kafka spans compatible with the Snuba span schema. ([#2917](https://github.com/getsentry/relay/pull/2917), [#2926](https://github.com/getsentry/relay/pull/2926))
- Only extract span metrics / tags when they are needed. ([#2907](https://github.com/getsentry/relay/pull/2907), [#2923](https://github.com/getsentry/relay/pull/2923))
- Normalize metric resource identifiers in `event._metrics_summary` and `span._metrics_summary`. ([#2914](https://github.com/getsentry/relay/pull/2914))

Expand Down
3 changes: 2 additions & 1 deletion relay-server/src/actors/store.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1138,7 +1138,8 @@ struct SpanKafkaMessage<'a> {
#[serde(rename(deserialize = "timestamp"), skip_serializing)]
end_timestamp: f64,

description: &'a str,
#[serde(default, skip_serializing_if = "Option::is_none")]
description: Option<&'a RawValue>,
#[serde(default)]
duration_ms: u32,
/// The ID of the transaction event associated to this span, if any.
Expand Down
100 changes: 85 additions & 15 deletions tests/integration/test_store.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1449,7 +1449,7 @@ def test_span_ingestion(
bytes=json.dumps(
{
"traceId": "89143b0763095bd9c9955e8175d1fb23",
"spanId": "e342abb1214ca181",
"spanId": "a342abb1214ca181",
"name": "my 1st OTel span",
"startTimeUnixNano": int(start.timestamp() * 1e9),
"endTimeUnixNano": int(end.timestamp() * 1e9),
Expand Down Expand Up @@ -1491,6 +1491,43 @@ def test_span_ingestion(
),
)
)
envelope.add_item(
Item(
type="span",
payload=PayloadRef(
bytes=json.dumps(
{
"description": r"test \" with \" escaped \" chars",
"op": "default",
"span_id": "cd429c44b67a3eb1",
"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",
},
).encode()
),
)
)
envelope.add_item(
Item(
type="span",
payload=PayloadRef(
bytes=json.dumps(
{
"op": "default",
"span_id": "ed429c44b67a3eb1",
"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",
},
).encode()
),
)
)
relay.send_envelope(project_id, envelope)

# 2 - Send OTel span via endpoint
Expand All @@ -1504,7 +1541,7 @@ def test_span_ingestion(
"spans": [
{
"traceId": "89143b0763095bd9c9955e8175d1fb24",
"spanId": "e342abb1214ca182",
"spanId": "d342abb1214ca182",
"name": "my 2nd OTel span",
"startTimeUnixNano": int(start.timestamp() * 1e9),
"endTimeUnixNano": int(end.timestamp() * 1e9),
Expand All @@ -1531,11 +1568,23 @@ def test_span_ingestion(
for span in spans:
span.pop("received", None)

spans.sort(
key=lambda msg: msg.get("description", "")
) # endpoint might overtake envelope
spans.sort(key=lambda msg: msg["span_id"]) # endpoint might overtake envelope

assert spans == [
{
"description": "my 1st OTel span",
"duration_ms": 500,
"exclusive_time_ms": 500.0,
"is_segment": True,
"parent_span_id": "",
"project_id": 42,
"retention_days": 90,
"segment_id": "a342abb1214ca181",
"sentry_tags": {"category": "db", "op": "db.query"},
"span_id": "a342abb1214ca181",
"start_timestamp_ms": int(start.timestamp() * 1e3),
"trace_id": "89143b0763095bd9c9955e8175d1fb23",
},
{
"description": "https://example.com/p/blah.js",
"duration_ms": 1500,
Expand All @@ -1557,18 +1606,17 @@ def test_span_ingestion(
"trace_id": "ff62a8b040f340bda5d830223def1d81",
},
{
"description": "my 1st OTel span",
"duration_ms": 500,
"exclusive_time_ms": 500.0,
"description": r"test \" with \" escaped \" chars",
"duration_ms": 1500,
"exclusive_time_ms": 345.0,
"is_segment": True,
"parent_span_id": "",
"project_id": 42,
"retention_days": 90,
"segment_id": "e342abb1214ca181",
"sentry_tags": {"category": "db", "op": "db.query"},
"span_id": "e342abb1214ca181",
"segment_id": "cd429c44b67a3eb1",
"sentry_tags": {"op": "default"},
"span_id": "cd429c44b67a3eb1",
"start_timestamp_ms": int(start.timestamp() * 1e3),
"trace_id": "89143b0763095bd9c9955e8175d1fb23",
"trace_id": "ff62a8b040f340bda5d830223def1d81",
},
{
"description": "my 2nd OTel span",
Expand All @@ -1578,12 +1626,24 @@ def test_span_ingestion(
"parent_span_id": "",
"project_id": 42,
"retention_days": 90,
"segment_id": "e342abb1214ca182",
"segment_id": "d342abb1214ca182",
"sentry_tags": {"op": "default"},
"span_id": "e342abb1214ca182",
"span_id": "d342abb1214ca182",
"start_timestamp_ms": int(start.timestamp() * 1e3),
"trace_id": "89143b0763095bd9c9955e8175d1fb24",
},
{
"duration_ms": 1500,
"exclusive_time_ms": 345.0,
"is_segment": True,
"project_id": 42,
"retention_days": 90,
"segment_id": "ed429c44b67a3eb1",
"sentry_tags": {"op": "default"},
"span_id": "ed429c44b67a3eb1",
"start_timestamp_ms": int(start.timestamp() * 1e3),
"trace_id": "ff62a8b040f340bda5d830223def1d81",
},
]

metrics = [metric for (metric, _headers) in metrics_consumer.get_metrics()]
Expand Down Expand Up @@ -1627,6 +1687,16 @@ def test_span_ingestion(
"tags": {"span.op": "default"},
"retention_days": 90,
},
{
"org_id": 1,
"project_id": 42,
"name": "c:spans/count_per_op@none",
"type": "c",
"value": 2.0,
"timestamp": expected_timestamp + 1,
"tags": {"span.op": "default"},
"retention_days": 90,
},
{
"org_id": 1,
"project_id": 42,
Expand Down