feat(server): Add rule id to outcomes coming from transaction sampling #953
Conversation
| @@ -211,6 +215,46 @@ mod tests { | |||
| } | |||
| } | |||
|
|
|||
| /// ugly hack to build an envelope with an optional trace context | |||
| fn new_envelope(with_trace_context: bool) -> Envelope { | |||
There was a problem hiding this comment.
For a follow-up: Constructing a raw envelope like this for tests is not very ergonomic. I think we should invest more in the testing facilities.
We have a few envelope tests that already create envelopes like this one.
The same goes for project configuration above.
There was a problem hiding this comment.
Agreed, I toyed with the idea making some members public in order to construct what I needed easier, but I decided that it would need a bit more careful consideration and went for a brute force approach in tests.
There was a problem hiding this comment.
Yeah, rather than making members public we should create a good interface. We'll need similar facilities for testing metrics, so I can look into that.
| if !should_sample { | ||
| // finally we decided that we should sample the transaction | ||
| if let SamplingResult::Drop(rule_id) = trace_context.should_keep(client_ip, sampling_config) { | ||
| // TODO: Remove event-dependent items similar to `EnvelopeLimiter`. |
There was a problem hiding this comment.
I noticed that we don't drop dependents when we drop the transaction. It is unrelated to this change, however, so let's fix this in a follow-up.
* master: fix(server): Remove dependent items from envelope when dropping transaction item (#960) fix(clippy): Fix clippy 1.51.0 warnings (#965) feat(server): Add support for breakdowns ingestion (#934) build: Update schemars and remove workarounds (#961) feat(server): Add rule id to outcomes coming from transaction sampling (#953)
This PR adds rule ids to outcomes for transactions that were dropped due to transaction sampling.
The outcomes from these events has also changed from 3 (Invalid) to 1 (Filtered).