feat(metrics): Add bucket width to bucket protocol [INGEST-350] #1103
Conversation
There was a problem hiding this comment.
Metrics buckets are also parsed in store.rs, not sure if we have to do any error handling here:
https://github.com/getsentry/relay/blob/master/relay-server/src/actors/store.rs#L327
| pub timestamp: UnixTimestamp, | ||
| /// The length of the time window in seconds. | ||
| pub width: u64, |
There was a problem hiding this comment.
A u32 would give us 136 years, but I decided to stick with u64 to reduce the number of conversions, and because
- there's no difference in size for the serialized JSON,
Bucketobjects are not stored in the aggregator (it storesBucketValueobjects).
| @@ -54,8 +54,8 @@ def test_metrics(mini_sentry, relay): | |||
| received_metrics = json.loads(metrics_item.get_bytes().decode()) | |||
| received_metrics = sorted(received_metrics, key=lambda x: x["name"]) | |||
| assert received_metrics == [ | |||
| {"timestamp": timestamp, "name": "bar", "value": 17.0, "type": "c"}, | |||
| {"timestamp": timestamp, "name": "foo", "value": 42.0, "type": "c"}, | |||
| {"timestamp": timestamp, "width": 1, "name": "bar", "value": 17.0, "type": "c"}, | |||
There was a problem hiding this comment.
Is this a realistic test?
Don't we have min width 10s ( will this test need to change when we enforce/merge buckets)
There was a problem hiding this comment.
That's a good point. I think we used a one second bucket interval (and a delay of zero) in the integration test to make everything flush as quickly as possible. But we will have to change that once we enforce a min. bucket width.
Co-authored-by: Radu Woinaroski <5281987+RaduW@users.noreply.github.com>
Previously, incoming metrics buckets' timestamps were not checked at all, so it was possible to have overlapping buckets in the aggregator. This PR:
widthfield to the bucket protocol, so the upstream relay can decide what to do with bucket widths different than its own.See also https://www.notion.so/sentry/Extension-to-the-bucket-protocol-6e7eb432e73f44eba868b6007d711b6c.