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

feat(spans): Forward span.data on the Kafka message #3523

Merged
merged 5 commits into from
May 6, 2024
Merged

feat(spans): Forward span.data on the Kafka message #3523

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1139b3c
feat(spans): Forward span.data on the Kafka message
phacops May 2, 2024
3ead108
Add a CHANGELOG entry
phacops May 2, 2024
276746c
Fix standalone span ingestion tests
phacops May 3, 2024
c656b2c
Merge branch 'master' into pierre/spans-send-data-in-the-kafka-message
phacops May 6, 2024
c6efdbd
Fix integration tests
phacops May 6, 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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
- Add metrics extraction config to global config. ([#3490](https://github.com/getsentry/relay/pull/3490), [#3504](https://github.com/getsentry/relay/pull/3504))
- Extract `cache.item_size` from measurements instead of data. ([#3510](https://github.com/getsentry/relay/pull/3510))
- Collect `enviornment` tag as part of exclusive_time_light for cache spans. ([#3510](https://github.com/getsentry/relay/pull/3510))
- Forward `span.data` on the Kafka message. ([#3523](https://github.com/getsentry/relay/pull/3523))

## 24.4.2

Expand Down
2 changes: 2 additions & 0 deletions relay-server/src/services/store.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1390,6 +1390,8 @@ struct SpanKafkaMessage<'a> {
#[serde(default)]
is_segment: bool,

#[serde(default, skip_serializing_if = "none_or_empty_object")]
data: Option<&'a RawValue>,
Copy link
Member

Choose a reason for hiding this comment

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

I was confused where this field is set, then saw that it's because of the useless serialization roundtrip 🤣

Copy link
Member

Choose a reason for hiding this comment

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

Yeah we're basically trying to make the roundtrip as cheap as possible.

#[serde(borrow, default, skip_serializing_if = "Option::is_none")]
measurements: Option<BTreeMap<Cow<'a, str>, Option<SpanMeasurement>>>,
#[serde(default)]
Expand Down
25 changes: 25 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 @@ -120,6 +120,10 @@ def test_span_extraction(
transaction_span = spans_consumer.get_span()
del transaction_span["received"]
assert transaction_span == {
"data": {
"sentry.sdk.name": "raven-node",
"sentry.segment.name": "hi",
},
"description": "hi",
"duration_ms": duration_ms,
"event_id": "cbf6960622e14a45abc1f03b2055b186",
Expand Down Expand Up @@ -492,6 +496,7 @@ def test_span_ingestion(

assert spans == [
{
"data": {"browser.name": "Chrome"},
"description": "my 1st OTel span",
"duration_ms": 500,
"exclusive_time_ms": 500.0,
Expand All @@ -510,6 +515,7 @@ def test_span_ingestion(
"trace_id": "89143b0763095bd9c9955e8175d1fb23",
},
{
"data": {"browser.name": "Chrome"},
"description": "https://example.com/p/blah.js",
"duration_ms": 1500,
"exclusive_time_ms": 345.0,
Expand All @@ -533,6 +539,7 @@ def test_span_ingestion(
"trace_id": "ff62a8b040f340bda5d830223def1d81",
},
{
"data": {"browser.name": "Chrome"},
"description": r"test \" with \" escaped \" chars",
"duration_ms": 1500,
"exclusive_time_ms": 345.0,
Expand All @@ -547,6 +554,7 @@ def test_span_ingestion(
"trace_id": "ff62a8b040f340bda5d830223def1d81",
},
{
"data": {"browser.name": "Python Requests"},
"description": "my 2nd OTel span",
"duration_ms": 500,
"exclusive_time_ms": 500.0,
Expand All @@ -564,6 +572,7 @@ def test_span_ingestion(
"trace_id": "89143b0763095bd9c9955e8175d1fb24",
},
{
"data": {"browser.name": "Chrome"},
"duration_ms": 1500,
"exclusive_time_ms": 345.0,
"is_segment": False,
Expand All @@ -580,6 +589,7 @@ def test_span_ingestion(
"trace_id": "ff62a8b040f340bda5d830223def1d81",
},
{
"data": {"browser.name": "Python Requests"},
"description": "my 3rd protobuf OTel span",
"duration_ms": 500,
"exclusive_time_ms": 500.0,
Expand Down Expand Up @@ -853,6 +863,10 @@ def test_span_extraction_with_metrics_summary(
transaction_span = spans_consumer.get_span()
del transaction_span["received"]
assert transaction_span == {
"data": {
"sentry.sdk.name": "raven-node",
"sentry.segment.name": "hi",
},
"description": "hi",
"duration_ms": duration_ms,
"event_id": "cbf6960622e14a45abc1f03b2055b186",
Expand Down Expand Up @@ -1026,6 +1040,10 @@ def test_span_extraction_with_ddm_missing_values(
transaction_span = spans_consumer.get_span()
del transaction_span["received"]
assert transaction_span == {
"data": {
"sentry.sdk.name": "raven-node",
"sentry.segment.name": "hi",
},
"description": "hi",
"duration_ms": duration_ms,
"event_id": "cbf6960622e14a45abc1f03b2055b186",
Expand Down Expand Up @@ -1256,6 +1274,7 @@ def test_span_ingestion_with_performance_scores(

assert spans == [
{
"data": {"browser.name": "Python Requests"},
"duration_ms": 1500,
"exclusive_time_ms": 345.0,
"is_segment": True,
Expand Down Expand Up @@ -1290,6 +1309,12 @@ def test_span_ingestion_with_performance_scores(
},
},
{
"data": {
"browser.name": "Python Requests",
"sentry.replay.id": "8477286c8e5148b386b71ade38374d58",
"sentry.segment.name": "/page/with/click/interaction/*/*",
"user": "admin@sentry.io",
},
"duration_ms": 1500,
"exclusive_time_ms": 345.0,
"is_segment": True,
Expand Down
Loading