Skip to content

Commit

Permalink
feat(spans): Trace-based sampling for standalone spans (#3476)
Browse files Browse the repository at this point in the history
Apply trace-based dynamic sampling rules to spans. This either drops or
retains span envelopes based on the dynamic sampling context (DSC)
defined in the envelope header.

*Not* in this PR:

- Compute sampling biases from spans on sentry-side (all
transaction-based for now)
- Split envelopes by DSC (when spans in a single envelope have different
DSCs)
- Allow DSC per item, i.e. in the item header.
  • Loading branch information
jjbayer committed May 15, 2024
1 parent f8843d2 commit 64310da
Show file tree
Hide file tree
Showing 9 changed files with 321 additions and 118 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@
- Properly handle AI metrics from the Python SDK's `@ai_track` decorator. ([#3539](https://github.com/getsentry/relay/pull/3539))
- Mitigate occasional slowness and timeouts of the healthcheck endpoint. The endpoint will now respond promptly an unhealthy state. ([#3567](https://github.com/getsentry/relay/pull/3567))

**Features**:

- Apple trace-based sampling rules to standalone spans. ([#3476](https://github.com/getsentry/relay/pull/3476))

**Internal**:

- Add metrics extraction config to global config. ([#3490](https://github.com/getsentry/relay/pull/3490), [#3504](https://github.com/getsentry/relay/pull/3504))
Expand Down
8 changes: 4 additions & 4 deletions relay-sampling/src/dsc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,11 @@ pub struct DynamicSamplingContext {
/// The environment.
#[serde(default)]
pub environment: Option<String>,
/// The name of the transaction extracted from the `transaction` field in the starting
/// transaction.
/// In the transaction-based model, this is the name of the transaction extracted from the `transaction`
/// field in the starting transaction and set on transaction start, or via `scope.transaction`.
///
/// Set on transaction start, or via `scope.transaction`.
#[serde(default)]
/// In the spans-only model, this is the segment name for the segment that started the trace.
#[serde(default, alias = "segment_name")]
pub transaction: Option<String>,
/// The sample rate with which this trace was sampled in the client. This is a float between
/// `0.0` and `1.0`.
Expand Down
26 changes: 15 additions & 11 deletions relay-sampling/src/evaluation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,20 +154,24 @@ pub struct SamplingEvaluator<'a> {
}

impl<'a> SamplingEvaluator<'a> {
/// Constructor for [`SamplingEvaluator`].
pub fn new(now: DateTime<Utc>) -> Self {
/// Constructs an evaluator with reservoir sampling.
pub fn new_with_reservoir(now: DateTime<Utc>, reservoir: &'a ReservoirEvaluator<'a>) -> Self {
Self {
now,
rule_ids: vec![],
factor: 1.0,
reservoir: None,
reservoir: Some(reservoir),
}
}

/// Sets a [`ReservoirEvaluator`].
pub fn set_reservoir(mut self, reservoir: &'a ReservoirEvaluator) -> Self {
self.reservoir = Some(reservoir);
self
/// Constructs an evaluator without reservoir sampling.
pub fn new(now: DateTime<Utc>) -> Self {
Self {
now,
rule_ids: vec![],
factor: 1.0,
reservoir: None,
}
}

/// Attempts to find a match for sampling rules using `ControlFlow`.
Expand Down Expand Up @@ -533,19 +537,19 @@ mod tests {
// shares state among multiple evaluator instances.
let reservoir = mock_reservoir_evaluator(vec![]);

let evaluator = SamplingEvaluator::new(Utc::now()).set_reservoir(&reservoir);
let evaluator = SamplingEvaluator::new_with_reservoir(Utc::now(), &reservoir);
let matched_rules =
get_matched_rules(&evaluator.match_rules(Uuid::default(), &dsc, rules.iter()));
// Reservoir rule overrides 0 and 2.
assert_eq!(&matched_rules, &[1]);

let evaluator = SamplingEvaluator::new(Utc::now()).set_reservoir(&reservoir);
let evaluator = SamplingEvaluator::new_with_reservoir(Utc::now(), &reservoir);
let matched_rules =
get_matched_rules(&evaluator.match_rules(Uuid::default(), &dsc, rules.iter()));
// Reservoir rule overrides 0 and 2.
assert_eq!(&matched_rules, &[1]);

let evaluator = SamplingEvaluator::new(Utc::now()).set_reservoir(&reservoir);
let evaluator = SamplingEvaluator::new_with_reservoir(Utc::now(), &reservoir);
let matched_rules =
get_matched_rules(&evaluator.match_rules(Uuid::default(), &dsc, rules.iter()));
// Reservoir rule reached its limit, rule 0 and 2 are now matched instead.
Expand Down Expand Up @@ -783,7 +787,7 @@ mod tests {
let mut rule = mocked_sampling_rule();

let reservoir = ReservoirEvaluator::new(ReservoirCounters::default());
let mut eval = SamplingEvaluator::new(Utc::now()).set_reservoir(&reservoir);
let mut eval = SamplingEvaluator::new_with_reservoir(Utc::now(), &reservoir);

rule.sampling_value = SamplingValue::SampleRate { value: 1.0 };
assert_eq!(eval.try_compute_sample_rate(&rule), Some(1.0));
Expand Down
49 changes: 42 additions & 7 deletions relay-server/src/services/processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,9 +167,29 @@ macro_rules! processing_group {
/// Should be used only with groups which are responsible for processing envelopes with events.
pub trait EventProcessing {}

/// A trait for processing groups that can be dynamically sampled.
pub trait Sampling {
/// Whether dynamic sampling should run under the given project's conditions.
fn supports_sampling(project_state: &ProjectState) -> bool;

/// Whether reservoir sampling applies to this processing group (a.k.a. data type).
fn supports_reservoir_sampling() -> bool;
}

processing_group!(TransactionGroup, Transaction);
impl EventProcessing for TransactionGroup {}

impl Sampling for TransactionGroup {
fn supports_sampling(project_state: &ProjectState) -> bool {
// For transactions, we require transaction metrics to be enabled before sampling.
matches!(&project_state.config.transaction_metrics, Some(ErrorBoundary::Ok(c)) if c.is_enabled())
}

fn supports_reservoir_sampling() -> bool {
true
}
}

processing_group!(ErrorGroup, Error);
impl EventProcessing for ErrorGroup {}

Expand All @@ -179,6 +199,18 @@ processing_group!(ClientReportGroup, ClientReport);
processing_group!(ReplayGroup, Replay);
processing_group!(CheckInGroup, CheckIn);
processing_group!(SpanGroup, Span);

impl Sampling for SpanGroup {
fn supports_sampling(project_state: &ProjectState) -> bool {
// If no metrics could be extracted, do not sample anything.
matches!(&project_state.config().metric_extraction, ErrorBoundary::Ok(c) if c.is_supported())
}

fn supports_reservoir_sampling() -> bool {
false
}
}

processing_group!(ProfileChunkGroup, ProfileChunk);
processing_group!(MetricsGroup, Metrics);
processing_group!(ForwardUnknownGroup, ForwardUnknown);
Expand Down Expand Up @@ -1303,12 +1335,6 @@ impl EnvelopeProcessorService {
}
};

if let Some(sampling_state) = state.sampling_project_state.clone() {
state
.envelope_mut()
.parametrize_dsc_transaction(&sampling_state.config.tx_name_rules);
}

let request_meta = state.managed_envelope.envelope().meta();
let client_ipaddr = request_meta.client_addr().map(IpAddr::from);

Expand Down Expand Up @@ -1650,7 +1676,7 @@ impl EnvelopeProcessorService {

fn process_envelope(
&self,
managed_envelope: ManagedEnvelope,
mut managed_envelope: ManagedEnvelope,
project_id: ProjectId,
project_state: Arc<ProjectState>,
sampling_project_state: Option<Arc<ProjectState>>,
Expand All @@ -1660,6 +1686,15 @@ impl EnvelopeProcessorService {
// from the contents of the envelope.
let group = managed_envelope.group();

// Pre-process the envelope headers.
if let Some(sampling_state) = sampling_project_state.as_ref() {
// Both transactions and standalone span envelopes need a normalized DSC header
// to make sampling rules based on the segment/transaction name work correctly.
managed_envelope
.envelope_mut()
.parametrize_dsc_transaction(&sampling_state.config.tx_name_rules);
}

macro_rules! run {
($fn:ident) => {{
let managed_envelope = managed_envelope.try_into()?;
Expand Down
Loading

0 comments on commit 64310da

Please sign in to comment.