feat(metrics): Tag the sample decision on count_per_root_project #1870
Conversation
| let decision = match sampling_result { | ||
| SamplingResult::Keep => "keep".to_owned(), | ||
| SamplingResult::Drop(_) => "drop".to_owned(), | ||
| }; | ||
| root_counter_tags.insert("decision".to_owned(), decision); |
There was a problem hiding this comment.
Does it make sense to implement ToString in SamplingResult, and directly sampling_result.to_string()?
There was a problem hiding this comment.
In this case I would likely not use it, because if we look at SamplingResult in isolation we would also render the rule IDs from Drop (see the Display implementation of MatchedRuleIds).
| let result = service.sample_envelope(&mut state); | ||
| assert_eq!(result.is_ok(), shouldkeep); | ||
|
|
||
| service.compute_sampling_decision(&mut state); |
There was a problem hiding this comment.
nit: compute_sampling_decision is now a very thin wrapper around should_keep_event(), so we might as well only test should_keep_event() here. Alternatively, if we want to test that sample_envelope() correctly converts a sampling result to a Result<...>, revert to calling sample_envelope() here.
There was a problem hiding this comment.
Good observation. Instead of testing should_keep_event here, we can even remove this test, since there are unit tests for this function already.
I'd probably test an entire process run then, that asserts we wire everything up correctly. Will look into this.
|
|
* master: feat(metrics): Tag the sample decision on count_per_root_project (#1870) feat(protocol): Add sourcemap debug image type to protocol (#1869) ref(statsd): Revert back the adition of metric names as tag on Sentry errors (#1873) feat(profiling): Add PHP support (#1871) fix(panic): revert sentry-types to 0.20.1 (#1872) ref(server): Use async/await in all endpoints (#1862) ref: Buffer envelopes for broken project states (#1856) meta: Remove accidentally added GeoIP file (#1866)
Adds a binary
decisiontag toc:transactions/count_per_root_project. This allows us to query the effective sample rate on projects.Background
Dynamic sampling is based on a target sample rate, called the organization's "fidelity". On top of that, Sentry computes a set of priorities (formerly called "biases") to increase visibility in under-represented areas. For example, Sentry boosts releases during adoption or small projects.
The priorities currently apply on top of the fidelity rate and increase the sample rate for a subset of transactions. Overall, this increases the effective sample rate applied for a project. The extent of this is not deterministic, as it depends on the match rate of sample rules.
The final goal is to adjust the biases and base sample rate in such a way that the effective sample rate matches the target fidelity.
Follow Up
In Sentry, compute the effective sample rate ($e$ ) and compare it with the target sample rate ($t$ ). Use this to apply an adjustment to the base sample rate as: $b = t * t/e$ .
Requires getsentry/sentry#45031
Fixes https://github.com/getsentry/team-ingest/issues/16