Skip to content

Commit 3aa4991

Browse files
jjbayerjjbayer
authored
ref(ds): Low cardinality outcome reason (#3623)
Reduce noise in outcome reasons by mapping ranges of dynamic sampling rule IDs together, and ordering them. That is ```python "123,1004,1500,1403,1404,1000" # becomes "1000,1004,1400,1500,?" ```
1 parent 692583e commit 3aa4991

File tree

8 files changed

+164
-29
lines changed

8 files changed

+164
-29
lines changed

‎CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
# Changelog
22

3+
## Unreleased
4+
5+
**Internal**:
6+
7+
- Map outcome reasons for dynamic sampling to reduced set of values. ([#3623](https://github.com/getsentry/relay/pull/3623))
8+
39
## 24.5.0
410

511
**Breaking Changes**:

‎relay-server/src/services/outcome.rs

Lines changed: 96 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
//! pipeline, outcomes may not be emitted if the item is accepted.
66
77
use std::borrow::Cow;
8-
use std::collections::BTreeMap;
8+
use std::collections::{BTreeMap, BTreeSet};
99
use std::error::Error;
1010
use std::net::IpAddr;
1111
use std::sync::Arc;
@@ -24,6 +24,7 @@ use relay_filter::FilterStatKey;
2424
#[cfg(feature = "processing")]
2525
use relay_kafka::{ClientError, KafkaClient, KafkaTopic};
2626
use relay_quotas::{DataCategory, ReasonCode, Scoping};
27+
use relay_sampling::config::RuleId;
2728
use relay_sampling::evaluation::MatchedRuleIds;
2829
use relay_statsd::metric;
2930
use relay_system::{Addr, FromMessage, Interface, NoResponse, Service};
@@ -164,7 +165,7 @@ pub enum Outcome {
164165
Filtered(FilterStatKey),
165166

166167
/// The event has been filtered by a Sampling Rule
167-
FilteredSampling(MatchedRuleIds),
168+
FilteredSampling(RuleCategories),
168169

169170
/// The event has been rate limited.
170171
RateLimited(Option<ReasonCode>),
@@ -251,6 +252,79 @@ impl fmt::Display for Outcome {
251252
}
252253
}
253254

255+
/// A lower-cardinality version of [`RuleId`].
256+
#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
257+
pub enum RuleCategory {
258+
BoostLowVolumeProjects,
259+
BoostEnvironments,
260+
IgnoreHealthChecks,
261+
BoostKeyTransactions,
262+
Recalibration,
263+
BoostReplayId,
264+
BoostLowVolumeTransactions,
265+
BoostLatestReleases,
266+
Custom,
267+
Other,
268+
}
269+
270+
impl RuleCategory {
271+
fn as_str(&self) -> &'static str {
272+
match self {
273+
Self::BoostLowVolumeProjects => "1000",
274+
Self::BoostEnvironments => "1001",
275+
Self::IgnoreHealthChecks => "1002",
276+
Self::BoostKeyTransactions => "1003",
277+
Self::Recalibration => "1004",
278+
Self::BoostReplayId => "1005",
279+
Self::BoostLowVolumeTransactions => "1400",
280+
Self::BoostLatestReleases => "1500",
281+
Self::Custom => "3000",
282+
Self::Other => "0",
283+
}
284+
}
285+
}
286+
287+
impl From<RuleId> for RuleCategory {
288+
fn from(value: RuleId) -> Self {
289+
match value.0 {
290+
1000 => Self::BoostLowVolumeProjects,
291+
1001 => Self::BoostEnvironments,
292+
1002 => Self::IgnoreHealthChecks,
293+
1003 => Self::BoostKeyTransactions,
294+
1004 => Self::Recalibration,
295+
1005 => Self::BoostReplayId,
296+
1400..=1499 => Self::BoostLowVolumeTransactions,
297+
1500..=1599 => Self::BoostLatestReleases,
298+
3000..=4999 => Self::Custom,
299+
_ => Self::Other,
300+
}
301+
}
302+
}
303+
304+
/// An ordered set of categories that can be used as outcome reason.
305+
#[derive(Clone, Debug, PartialEq, Eq, Hash)]
306+
pub struct RuleCategories(pub BTreeSet<RuleCategory>);
307+
308+
impl fmt::Display for RuleCategories {
309+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
310+
for (i, c) in self.0.iter().enumerate() {
311+
if i > 0 {
312+
write!(f, ",")?;
313+
}
314+
write!(f, "{}", c.as_str())?;
315+
}
316+
Ok(())
317+
}
318+
}
319+
320+
impl From<MatchedRuleIds> for RuleCategories {
321+
fn from(value: MatchedRuleIds) -> Self {
322+
RuleCategories(BTreeSet::from_iter(
323+
value.0.into_iter().map(RuleCategory::from),
324+
))
325+
}
326+
}
327+
254328
/// Reason for a discarded invalid event.
255329
///
256330
/// Used in `Outcome::Invalid`. Synchronize overlap with Sentry.
@@ -971,3 +1045,23 @@ impl Service for OutcomeProducerService {
9711045
});
9721046
}
9731047
}
1048+
1049+
#[cfg(test)]
1050+
mod tests {
1051+
use super::*;
1052+
1053+
#[test]
1054+
fn rule_category_roundtrip() {
1055+
let input = "123,1004,1500,1403,1403,1404,1000";
1056+
let rule_ids = MatchedRuleIds::parse(input).unwrap();
1057+
let rule_categories = RuleCategories::from(rule_ids);
1058+
1059+
let serialized = rule_categories.to_string();
1060+
assert_eq!(&serialized, "1000,1004,1400,1500,0");
1061+
1062+
assert_eq!(
1063+
MatchedRuleIds::parse(&serialized).unwrap(),
1064+
MatchedRuleIds([1000, 1004, 1400, 1500, 0].map(RuleId).into())
1065+
);
1066+
}
1067+
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ pub fn sample_envelope_items(
115115
let unsampled_profiles_enabled = forward_unsampled_profiles(state, global_config);
116116

117117
let matched_rules = sampling_match.into_matched_rules();
118-
let outcome = Outcome::FilteredSampling(matched_rules.clone());
118+
let outcome = Outcome::FilteredSampling(matched_rules.into());
119119
state.managed_envelope.retain_items(|item| {
120120
if unsampled_profiles_enabled && item.ty() == &ItemType::Profile {
121121
item.set_sampled(false);

‎relay-server/src/services/processor/report.rs

Lines changed: 39 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ use relay_sampling::evaluation::MatchedRuleIds;
1414
use relay_system::Addr;
1515

1616
use crate::envelope::{ContentType, ItemType};
17-
use crate::services::outcome::{Outcome, TrackOutcome};
17+
use crate::services::outcome::{Outcome, RuleCategories, TrackOutcome};
1818
use crate::services::processor::{ClientReportGroup, ProcessEnvelopeState, MINIMUM_CLOCK_DRIFT};
1919
use crate::utils::ItemAction;
2020

@@ -240,6 +240,7 @@ fn outcome_from_parts(field: ClientReportField, reason: &str) -> Result<Outcome,
240240
match field {
241241
ClientReportField::FilteredSampling => match reason.strip_prefix("Sampled:") {
242242
Some(rule_ids) => MatchedRuleIds::parse(rule_ids)
243+
.map(RuleCategories::from)
243244
.map(Outcome::FilteredSampling)
244245
.map_err(|_| ()),
245246
None => Err(()),
@@ -261,11 +262,11 @@ mod tests {
261262
use std::sync::Arc;
262263

263264
use relay_event_schema::protocol::EventId;
264-
use relay_sampling::config::RuleId;
265265
use relay_sampling::evaluation::ReservoirCounters;
266266

267267
use crate::envelope::{Envelope, Item};
268268
use crate::extractors::RequestMeta;
269+
use crate::services::outcome::RuleCategory;
269270
use crate::services::processor::{ProcessEnvelope, ProcessingGroup};
270271
use crate::services::project::ProjectState;
271272
use crate::testutils::{self, create_test_processor};
@@ -559,15 +560,46 @@ mod tests {
559560

560561
assert_eq!(
561562
outcome_from_parts(ClientReportField::FilteredSampling, "Sampled:123,456"),
562-
Ok(Outcome::FilteredSampling(MatchedRuleIds(vec![
563-
RuleId(123),
564-
RuleId(456),
565-
])))
563+
Ok(Outcome::FilteredSampling(RuleCategories(
564+
[RuleCategory::Other].into()
565+
)))
566566
);
567567

568568
assert_eq!(
569569
outcome_from_parts(ClientReportField::FilteredSampling, "Sampled:123"),
570-
Ok(Outcome::FilteredSampling(MatchedRuleIds(vec![RuleId(123)])))
570+
Ok(Outcome::FilteredSampling(RuleCategories(
571+
[RuleCategory::Other].into()
572+
)))
573+
);
574+
575+
assert_eq!(
576+
outcome_from_parts(ClientReportField::FilteredSampling, "Sampled:123"),
577+
Ok(Outcome::FilteredSampling(RuleCategories(
578+
[RuleCategory::Other].into()
579+
)))
580+
);
581+
582+
assert_eq!(
583+
outcome_from_parts(ClientReportField::FilteredSampling, "Sampled:1001"),
584+
Ok(Outcome::FilteredSampling(RuleCategories(
585+
[RuleCategory::BoostEnvironments].into()
586+
)))
587+
);
588+
589+
assert_eq!(
590+
outcome_from_parts(
591+
ClientReportField::FilteredSampling,
592+
"Sampled:1001,1456,1567,3333,4444"
593+
),
594+
Ok(Outcome::FilteredSampling(RuleCategories(
595+
[
596+
RuleCategory::BoostEnvironments,
597+
RuleCategory::BoostLowVolumeTransactions,
598+
RuleCategory::BoostLatestReleases,
599+
RuleCategory::Custom
600+
]
601+
.into()
602+
)))
571603
);
572604
}
573605

‎relay-server/src/services/processor/span/processing.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ use relay_spans::{otel_to_sentry_span, otel_trace::Span as OtelSpan};
2525

2626
use crate::envelope::{ContentType, Envelope, Item, ItemType};
2727
use crate::metrics_extraction::generic::extract_metrics;
28-
use crate::services::outcome::{DiscardReason, Outcome};
28+
use crate::services::outcome::{DiscardReason, Outcome, RuleCategories};
2929
use crate::services::processor::span::extract_transaction_span;
3030
use crate::services::processor::{
3131
dynamic_sampling, Addrs, ProcessEnvelope, ProcessEnvelopeState, ProcessingError,
@@ -53,7 +53,7 @@ pub fn process(
5353
// once for all spans in the envelope.
5454
let sampling_outcome = match dynamic_sampling::run(state, &config) {
5555
SamplingResult::Match(sampling_match) if sampling_match.should_drop() => Some(
56-
Outcome::FilteredSampling(sampling_match.into_matched_rules()),
56+
Outcome::FilteredSampling(RuleCategories::from(sampling_match.into_matched_rules())),
5757
),
5858
_ => None,
5959
};

‎tests/integration/test_dynamic_sampling.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,7 @@ def test_it_removes_events(mini_sentry, relay):
238238
public_key = config["publicKeys"][0]["publicKey"]
239239

240240
# add a sampling rule to project config that removes all transactions (sample_rate=0)
241-
rules = _add_sampling_config(config, sample_rate=0, rule_type="transaction")
241+
_add_sampling_config(config, sample_rate=0, rule_type="transaction")
242242

243243
# create an envelope with a trace context that is initiated by this project (for simplicity)
244244
envelope, trace_id, event_id = _create_transaction_envelope(public_key)
@@ -253,7 +253,7 @@ def test_it_removes_events(mini_sentry, relay):
253253
assert outcomes is not None
254254
outcome = outcomes["outcomes"][0]
255255
assert outcome.get("outcome") == 1
256-
assert outcome.get("reason") == f"Sampled:{rules[0]['id']}"
256+
assert outcome.get("reason") == "Sampled:0"
257257

258258

259259
def test_it_does_not_sample_error(mini_sentry, relay):
@@ -392,7 +392,7 @@ def test_sample_on_parametrized_root_transaction(mini_sentry, relay):
392392
relay.send_envelope(project_id, envelope)
393393

394394
outcome = mini_sentry.captured_outcomes.get(timeout=2)
395-
assert outcome["outcomes"][0]["reason"] == "Sampled:1"
395+
assert outcome["outcomes"][0]["reason"] == "Sampled:0"
396396

397397

398398
def test_it_keeps_events(mini_sentry, relay):

‎tests/integration/test_outcome.py

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -798,7 +798,7 @@ def test_outcome_to_client_report(relay, mini_sentry):
798798
"version": 2,
799799
"rules": [
800800
{
801-
"id": 1,
801+
"id": 3001,
802802
"samplingValue": {"type": "sampleRate", "value": 0.0},
803803
"type": "transaction",
804804
"condition": {
@@ -853,7 +853,7 @@ def test_outcome_to_client_report(relay, mini_sentry):
853853
"project_id": 42,
854854
"key_id": 123,
855855
"outcome": 1,
856-
"reason": "Sampled:1",
856+
"reason": "Sampled:3000",
857857
"category": 9,
858858
"quantity": 1,
859859
}
@@ -960,7 +960,7 @@ def test_outcomes_aggregate_dynamic_sampling(relay, mini_sentry):
960960
"version": 2,
961961
"rules": [
962962
{
963-
"id": 1,
963+
"id": 3001,
964964
"samplingValue": {"type": "sampleRate", "value": 0.0},
965965
"type": "transaction",
966966
"condition": {
@@ -1006,7 +1006,7 @@ def test_outcomes_aggregate_dynamic_sampling(relay, mini_sentry):
10061006
"project_id": 42,
10071007
"key_id": 123,
10081008
"outcome": 1,
1009-
"reason": "Sampled:1",
1009+
"reason": "Sampled:3000",
10101010
"category": 9,
10111011
"quantity": 2,
10121012
}
@@ -1069,7 +1069,7 @@ def test_graceful_shutdown(relay, mini_sentry):
10691069
"version": 2,
10701070
"rules": [
10711071
{
1072-
"id": 1,
1072+
"id": 3001,
10731073
"samplingValue": {"type": "sampleRate", "value": 0.0},
10741074
"type": "transaction",
10751075
"condition": {
@@ -1120,7 +1120,7 @@ def test_graceful_shutdown(relay, mini_sentry):
11201120
"project_id": 42,
11211121
"key_id": 123,
11221122
"outcome": 1,
1123-
"reason": "Sampled:1",
1123+
"reason": "Sampled:3000",
11241124
"category": 9,
11251125
"quantity": 1,
11261126
}
@@ -1157,7 +1157,7 @@ def test_profile_outcomes(
11571157
"version": 2,
11581158
"rules": [
11591159
{
1160-
"id": 1,
1160+
"id": 3001,
11611161
"samplingValue": {"type": "sampleRate", "value": 0.0},
11621162
"type": "transaction",
11631163
"condition": {
@@ -1242,7 +1242,7 @@ def make_envelope(transaction_name):
12421242
"outcome": 1,
12431243
"project_id": 42,
12441244
"quantity": 6, # len(b"foobar")
1245-
"reason": "Sampled:1",
1245+
"reason": "Sampled:3000",
12461246
"source": expected_source,
12471247
},
12481248
{
@@ -1252,7 +1252,7 @@ def make_envelope(transaction_name):
12521252
"outcome": 1, # Filtered
12531253
"project_id": 42,
12541254
"quantity": 1,
1255-
"reason": "Sampled:1",
1255+
"reason": "Sampled:3000",
12561256
"source": expected_source,
12571257
},
12581258
{
@@ -1262,7 +1262,7 @@ def make_envelope(transaction_name):
12621262
"outcome": 1, # Filtered
12631263
"project_id": 42,
12641264
"quantity": 1,
1265-
"reason": "Sampled:1",
1265+
"reason": "Sampled:3000",
12661266
"source": expected_source,
12671267
},
12681268
]
@@ -1751,7 +1751,7 @@ def test_span_outcomes(
17511751
"version": 2,
17521752
"rules": [
17531753
{
1754-
"id": 1,
1754+
"id": 3001,
17551755
"samplingValue": {"type": "sampleRate", "value": 0.0},
17561756
"type": "transaction",
17571757
"condition": {
@@ -1827,7 +1827,7 @@ def make_envelope(transaction_name):
18271827
"outcome": 1, # Filtered
18281828
"project_id": 42,
18291829
"quantity": 1,
1830-
"reason": "Sampled:1",
1830+
"reason": "Sampled:3000",
18311831
"source": expected_source,
18321832
},
18331833
{

0 commit comments

Comments
 (0)