From 67c963d9c8d5e7e9de6347aee0edcf0c58d9fb24 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Mon, 4 Dec 2023 22:56:08 +0100 Subject: [PATCH] feat(summary): Fixed the incorrect emission of span metric summaries (#2566) --- sentry_sdk/metrics.py | 18 +++++---- tests/test_metrics.py | 86 ++++++++++++++++++++++++++----------------- 2 files changed, 63 insertions(+), 41 deletions(-) diff --git a/sentry_sdk/metrics.py b/sentry_sdk/metrics.py index 0ffdcf6de5..69902ca1a7 100644 --- a/sentry_sdk/metrics.py +++ b/sentry_sdk/metrics.py @@ -375,20 +375,22 @@ def add( def to_json(self): # type: (...) -> Dict[str, Any] - rv = {} + rv = {} # type: Any for (export_key, tags), ( v_min, v_max, v_count, v_sum, ) in self._measurements.items(): - rv[export_key] = { - "tags": _tags_to_dict(tags), - "min": v_min, - "max": v_max, - "count": v_count, - "sum": v_sum, - } + rv.setdefault(export_key, []).append( + { + "tags": _tags_to_dict(tags), + "min": v_min, + "max": v_max, + "count": v_count, + "sum": v_sum, + } + ) return rv diff --git a/tests/test_metrics.py b/tests/test_metrics.py index 3decca31c2..3f8b6049d8 100644 --- a/tests/test_metrics.py +++ b/tests/test_metrics.py @@ -597,33 +597,37 @@ def test_metric_summaries(sentry_init, capture_envelopes): t = transaction.items[0].get_transaction_event() assert t["_metrics_summary"] == { - "c:root-counter@none": { - "count": 1, - "min": 1.0, - "max": 1.0, - "sum": 1.0, + "c:root-counter@none": [ + { + "count": 1, + "min": 1.0, + "max": 1.0, + "sum": 1.0, + "tags": { + "transaction": "/foo", + "release": "fun-release@1.0.0", + "environment": "not-fun-env", + }, + } + ] + } + + assert t["spans"][0]["_metrics_summary"]["d:my-dist@none"] == [ + { + "count": 10, + "min": 0.0, + "max": 9.0, + "sum": 45.0, "tags": { - "transaction": "/foo", - "release": "fun-release@1.0.0", "environment": "not-fun-env", + "release": "fun-release@1.0.0", + "transaction": "/foo", }, } - } - - assert t["spans"][0]["_metrics_summary"]["d:my-dist@none"] == { - "count": 10, - "min": 0.0, - "max": 9.0, - "sum": 45.0, - "tags": { - "environment": "not-fun-env", - "release": "fun-release@1.0.0", - "transaction": "/foo", - }, - } + ] assert t["spans"][0]["tags"] == {"a": "b"} - timer = t["spans"][0]["_metrics_summary"]["d:my-timer-metric@second"] + (timer,) = t["spans"][0]["_metrics_summary"]["d:my-timer-metric@second"] assert timer["count"] == 1 assert timer["max"] == timer["min"] == timer["sum"] assert timer["sum"] > 0 @@ -697,6 +701,7 @@ def should_summarize_metric(key, tags): op="stuff", name="/foo", source=TRANSACTION_SOURCE_ROUTE ) as transaction: metrics.timing("foo", value=1.0, tags={"a": "b"}, timestamp=ts) + metrics.timing("foo", value=1.0, tags={"b": "c"}, timestamp=ts) metrics.timing("bar", value=1.0, tags={"a": "b"}, timestamp=ts) Hub.current.flush() @@ -707,25 +712,40 @@ def should_summarize_metric(key, tags): assert envelope.items[0].headers["type"] == "statsd" m = parse_metrics(envelope.items[0].payload.get_bytes()) - assert len(m) == 2 + assert len(m) == 3 assert m[0][1] == "bar@second" assert m[1][1] == "foo@second" + assert m[2][1] == "foo@second" # Measurement Attachment t = transaction.items[0].get_transaction_event()["_metrics_summary"] assert t == { - "d:foo@second": { - "tags": { - "a": "b", - "environment": "not-fun-env", - "release": "fun-release@1.0.0", - "transaction": "/foo", + "d:foo@second": [ + { + "tags": { + "a": "b", + "environment": "not-fun-env", + "release": "fun-release@1.0.0", + "transaction": "/foo", + }, + "min": 1.0, + "max": 1.0, + "count": 1, + "sum": 1.0, }, - "min": 1.0, - "max": 1.0, - "count": 1, - "sum": 1.0, - } + { + "tags": { + "b": "c", + "environment": "not-fun-env", + "release": "fun-release@1.0.0", + "transaction": "/foo", + }, + "min": 1.0, + "max": 1.0, + "count": 1, + "sum": 1.0, + }, + ] }