Skip to content

Commit 3699983

Browse files
authored
fix(ds): Emit dynamic sampling outcomes for spans contained in transaction (#4569)
Dynamic sampling currently only emits outcomes for the transaction but not the contained spans. With this PR sampled/filtered outcomes are also emitted for contained spans.
1 parent c1d9f09 commit 3699983

File tree

7 files changed

+174
-77
lines changed

7 files changed

+174
-77
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
**Bug Fixes**:
1717

1818
- Prevent partial trims in indexed and queryable span data. ([#4557](https://github.com/getsentry/relay/pull/4557))
19+
- Emit filtered/sampled outcomes for spans attached to a dropped transaction. ([#4569](https://github.com/getsentry/relay/pull/4569))
1920

2021
**Internal**:
2122

relay-server/src/services/processor.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1784,7 +1784,12 @@ impl EnvelopeProcessorService {
17841784
spans_extracted,
17851785
)?;
17861786

1787-
dynamic_sampling::drop_unsampled_items(managed_envelope, event, outcome);
1787+
dynamic_sampling::drop_unsampled_items(
1788+
managed_envelope,
1789+
event,
1790+
outcome,
1791+
spans_extracted,
1792+
);
17881793

17891794
// At this point we have:
17901795
// - An empty envelope.

relay-server/src/services/processor/dynamic_sampling.rs

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,16 @@ use relay_config::Config;
77
use relay_dynamic_config::ErrorBoundary;
88
use relay_event_schema::protocol::{Contexts, Event, TraceContext};
99
use relay_protocol::{Annotated, Empty};
10+
use relay_quotas::DataCategory;
1011
use relay_sampling::config::RuleType;
1112
use relay_sampling::evaluation::{ReservoirEvaluator, SamplingEvaluator};
1213
use relay_sampling::{DynamicSamplingContext, SamplingConfig};
1314

1415
use crate::envelope::{CountFor, ItemType};
1516
use crate::services::outcome::Outcome;
16-
use crate::services::processor::{event_category, EventProcessing, Sampling, TransactionGroup};
17+
use crate::services::processor::{
18+
event_category, EventProcessing, Sampling, SpansExtracted, TransactionGroup,
19+
};
1720
use crate::services::projects::project::ProjectInfo;
1821
use crate::utils::{self, SamplingResult, TypedEnvelope};
1922

@@ -112,6 +115,7 @@ pub fn drop_unsampled_items(
112115
managed_envelope: &mut TypedEnvelope<TransactionGroup>,
113116
event: Annotated<Event>,
114117
outcome: Outcome,
118+
spans_extracted: SpansExtracted,
115119
) {
116120
// Remove all items from the envelope which need to be dropped due to dynamic sampling.
117121
let dropped_items = managed_envelope
@@ -136,6 +140,23 @@ pub fn drop_unsampled_items(
136140
item.set_sampled(false);
137141
}
138142

143+
// Another 'hack' to emit outcomes from the container item for the contained items (spans).
144+
//
145+
// The entire tracking outcomes for contained elements is not handled in a systematic way
146+
// and whenever an event/transaction is discarded, contained elements are tracked in a 'best
147+
// effort' basis (basically in all the cases where someone figured out this is a problem).
148+
//
149+
// This is yet another case, when the spans have not yet been separated from the transaction
150+
// also emit dynamic sampling outcomes for the contained spans.
151+
if !spans_extracted.0 {
152+
let spans = event.value().and_then(|e| e.spans.value());
153+
let span_count = spans.map_or(0, |s| s.len());
154+
155+
// Track the amount of contained spans + 1 segment span (the transaction itself which would
156+
// be converted to a span).
157+
managed_envelope.track_outcome(outcome.clone(), DataCategory::SpanIndexed, span_count + 1);
158+
}
159+
139160
// All items have been dropped, now make sure the event is also handled and dropped.
140161
if let Some(category) = event_category(&event) {
141162
let category = category.index_category().unwrap_or(category);

tests/integration/test_dynamic_sampling.py

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -492,7 +492,9 @@ def test_uses_trace_public_key(mini_sentry, relay):
492492
mini_sentry.captured_events.get(timeout=1)
493493

494494
# and it should create an outcome
495-
outcomes = mini_sentry.captured_outcomes.get(timeout=2)
495+
outcomes = mini_sentry.captured_outcomes.get(timeout=2) # Spans
496+
assert outcomes is not None
497+
outcomes = mini_sentry.captured_outcomes.get(timeout=2) # Transactions
496498
assert outcomes is not None
497499
with pytest.raises(queue.Empty):
498500
mini_sentry.captured_outcomes.get(timeout=1)
@@ -586,6 +588,14 @@ def test_client_sample_rate_adjusted(mini_sentry, relay, rule_type, event_factor
586588
itself to 1.0. The chances of this test passing without the adjustment in
587589
place are very low (but not 0).
588590
"""
591+
592+
def assert_client_reports():
593+
# Relay is sending a client report, skip over it (1 for transactions, 1 for spans)
594+
for _ in range(2):
595+
received_envelope = mini_sentry.captured_events.get(timeout=1)
596+
assert received_envelope.get_transaction_event() is None
597+
assert received_envelope.get_event() is None
598+
589599
project_id = 42
590600
relay = relay(mini_sentry)
591601
config = mini_sentry.add_basic_project_config(project_id)
@@ -604,18 +614,12 @@ def test_client_sample_rate_adjusted(mini_sentry, relay, rule_type, event_factor
604614
)
605615

606616
relay.send_envelope(project_id, envelope)
607-
608-
received_envelope = mini_sentry.captured_events.get(timeout=1)
609-
received_envelope.get_transaction_event()
617+
assert_client_reports()
610618

611619
envelope, trace_id, event_id = event_factory(public_key, client_sample_rate=1.0)
612620

613621
relay.send_envelope(project_id, envelope)
614-
615-
# Relay is sending a client report, skip over it
616-
received_envelope = mini_sentry.captured_events.get(timeout=1)
617-
assert received_envelope.get_transaction_event() is None
618-
assert received_envelope.get_event() is None
622+
assert_client_reports()
619623

620624
with pytest.raises(queue.Empty):
621625
mini_sentry.captured_events.get(timeout=1)

tests/integration/test_metrics.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1019,6 +1019,8 @@ def test_transaction_metrics_extraction_external_relays(
10191019
if expect_metrics_extraction:
10201020
metrics_envelope = mini_sentry.captured_events.get(timeout=3)
10211021
assert len(metrics_envelope.items) == 1
1022+
metrics_envelope = mini_sentry.captured_events.get(timeout=3)
1023+
assert len(metrics_envelope.items) == 1
10221024

10231025
payload = json.loads(metrics_envelope.items[0].get_bytes().decode())
10241026
assert len(payload) == 4
@@ -1576,9 +1578,13 @@ def test_generic_metric_extraction(mini_sentry, relay):
15761578
relay = relay(relay(mini_sentry, options=TEST_CONFIG), options=TEST_CONFIG)
15771579
relay.send_transaction(PROJECT_ID, transaction)
15781580

1581+
# Skip client reports
15791582
envelope = mini_sentry.captured_events.get(timeout=3)
1583+
assert envelope.get_event() is None
15801584
envelope = mini_sentry.captured_events.get(timeout=3)
1585+
assert envelope.get_event() is None
15811586

1587+
envelope = mini_sentry.captured_events.get(timeout=3)
15821588
for item in envelope.items:
15831589
# Transaction items should be sampled and not among the envelope items.
15841590
assert item.headers.get("type") != "transaction"
@@ -2008,7 +2014,7 @@ def test_histogram_outliers(mini_sentry, relay):
20082014
relay.send_event(42, event)
20092015

20102016
tags = {}
2011-
for _ in range(2):
2017+
for _ in range(3):
20122018
envelope = mini_sentry.captured_events.get()
20132019
for item in envelope:
20142020
if item.type == "metric_buckets":

0 commit comments

Comments
 (0)