feat(protocol): Support comparisons in rule conditions on strings #2721
Conversation
|
For reviewers: A regression test for the legacy config is still missing. |
| @@ -106,12 +106,12 @@ macro_rules! impl_cmp_condition { | |||
| /// Path of the field that should match the value. | |||
| pub name: String, | |||
| /// The numeric value to check against. | |||
| pub value: Number, | |||
| pub value: Value, | |||
There was a problem hiding this comment.
Now since this is a value, should we also add array support like Eq has?
There was a problem hiding this comment.
I didn't wanna add this since the semantics of a comparison with an array are questionable. For eq, we defined it as "one of", but for comparisons this seemed weird to me. Thoughts?
There was a problem hiding this comment.
I personally found it already surprising for Eq, usually this would be a In operator. So for me then I assumed the value can always be an array and it turns into something like value <OPERATOR> ANY(value).
I agree though this is weird but we already have a precedent for the array. Though I think we just leave it as is and implement it when we need it (we can bump versions now, yay)
| #[serde(default, skip_serializing)] | ||
| pub rules_v2: Vec<SamplingRule>, |
There was a problem hiding this comment.
In some enums we've renamed the option to deprecated and set a serde alias on it. Could that be helpful here?
There was a problem hiding this comment.
It's a good idea, however it creates a difference between how Sentry serializes the schema and how it looks in Relay. I'm afraid that it causes more confusion when looking at the code.
* master: feat(protocol): Add access to `device.model` on contexts (#2728) Revert "feat(protocol): Support comparisons in rule conditions on strings" (#2727) ref(normalization): Remove TransactionsProcessor (#2714) feat(protocol): Support comparisons in rule conditions on strings (#2721) test(normalization): Reduce span default op test scope (#2726) release: 23.11.0 fix(normalization): Restore performance score normalization (#2725)
Adds the ability to run all comparison operators (
>,>=,<,<=)on strings. This applies to dynamic sampling and metric extraction.
Since this change breaks backwards compatibility, the versioning for
metrics extraction and dynamic sampling were bumped.
Dynamic sampling configs did not have a versioning schema before. The
last breaking change introduced a
rules_v2array. To fix this, thereis now versioning built into the new config. Additionally, an
ErrorBoundaryis added around sampling configuration to preventaccidental deserialization failures.
Backwards compatibility is as follows:
rules_v2.Relates to getsentry/sentry#59896
Resolves https://github.com/getsentry/team-ingest/issues/244