feat(mep): Expose new tagging rules interface for metrics extracted from transactions [INGEST-947] [INGEST-541]

[untitaker]

We're building this such that we can supersede certain kinds of duration-range based tagging we do in metrics extraction right now.

https://www.notion.so/sentry/Metrics-outlier-filtering-in-Relay-93c1a9a8c6c24277a2c9e8d61876f968?d=d1035fca98804ef697997439d14a06b2#3e08fc7f89a24de8a8c85b7d717d35f6=

[untitaker]

I think this should be fine but tbh i'm not super confident. I'm trying to think of cases where both values don't have decimal points, but can't be cast to the same int type:

e.g.: self.value is negative, event value is u64::MAX (self.value can't be cast to u64, and event value can't be cast to i64)

but i think in those cases we should be fine with casting those values to floats for comparison...

[jjbayer]

This is pretty awesome! I have some minor comments, but nothing that should keep you from merging.

[jjbayer]

Would add comment here to explain why we skip if the tag already exists.

[jjbayer]

nit

Suggested change

	x if x.starts_with("transaction.measurements.") => {
	field_name if field_name.starts_with("transaction.measurements.") => {

[jjbayer]

Out of curiosity, would there be a way to do this without a macro? Something like type GtCondition = CmpCondition<std::cmp::PartialOrd::gt>. I guess you would run into problems because you have three different operand types i64, u64, f64 inside?

[untitaker]

I didn't find a way, you'd have to be generic over a type, and functions are either:

• a singular type fn(a: i64, b: i64) -> bool (regular functions)
• an unnameable type (closures)

there's no const generics for function pointers yet

In order to make generics truly work, you'd have to define a trait Comparator, implement it for a few zero-sized types Lt, Gt, ...:

trait Comparator {
    fn matches(a: Number, b: Number) -> bool;
}

struct Lt;
impl Comparator for Lt { ... }

struct CmpCondition<A> { ... }
impl<A: Comparator> CmpCondition<A> {
    fn matches(self, event: ...) -> bool {
        A::matches(self.number, get_event_number(event))
    }
}

type LtCondition = CmpCondition<Lt>;

but this seemed like more effort

I think instead of being generic over a type what you could do is to write a single non-generic type:

struct CmpCondition {
    cmp_fn: fn(a: Number, b: Number) -> bool;
    ...
}

but then you have to write custom deserialization logic, and serialization just won't work unless you keep the enum variants:

enum RuleCondition {
    Gte(CmpCondition),
    Lt(CmpCondition)
}

[jjbayer]

Thanks for the rundown!

[jjbayer]

This is basically the else branch, right?

[untitaker]

yup. i felt that we wouldn't need an always-true condition since we can already write this today