Browse Source

fix(metrics): dont normalize units when calling sum on counter metric in span metrics (#74491)

Simon Hellmayr 7 months ago
parent
commit
96bd59f43d

+ 3 - 0
src/sentry/sentry_metrics/querying/visitors/query_expression.py

@@ -423,6 +423,9 @@ class UnitsNormalizationVisitor(QueryExpressionVisitor[tuple[UnitMetadata, Query
 
         parsed_mri = parse_mri(timeseries.metric.mri)
         if parsed_mri is not None:
+            if parsed_mri.entity == "c":
+                return None
+
             if rule_id := parsed_mri.span_attribute_rule_id:
                 try:
                     return SpanAttributeExtractionRuleCondition.objects.get(id=rule_id).config.unit

+ 55 - 3
tests/sentry/sentry_metrics/querying/data/test_api.py

@@ -29,6 +29,7 @@ from sentry.sentry_metrics.visibility import block_metric, block_tags_of_metric
 from sentry.snuba.metrics.naming_layer import TransactionMRI
 from sentry.testutils.cases import BaseMetricsTestCase, TestCase
 from sentry.testutils.helpers.datetime import freeze_time
+from sentry.testutils.pytest.fixtures import django_db_all
 
 pytestmark = pytest.mark.sentry_metrics
 
@@ -1410,6 +1411,7 @@ class MetricsAPITestCase(TestCase, BaseMetricsTestCase):
             assert meta[0][1]["unit"] == "nanosecond"
             assert meta[0][1]["scaling_factor"] is None
 
+    @django_db_all(reset_sequences=True)
     def test_span_metrics_with_coercible_units(self):
 
         configs = [
@@ -1432,13 +1434,15 @@ class MetricsAPITestCase(TestCase, BaseMetricsTestCase):
                 ],
             },
         ]
+        objects = []
         for config in configs:
-            self.create_span_attribute_extraction_config(
+            o = self.create_span_attribute_extraction_config(
                 dictionary=config, user_id=self.user.id, project=self.project
             )
+            objects.append(o)
 
-        mri_1 = "d:custom/span_attribute_1@nanosecond"
-        mri_2 = "d:custom/span_attribute_2@microsecond"
+        mri_1 = f"d:custom/span_attribute_{objects[0].conditions.all().get().id}@none"
+        mri_2 = f"d:custom/span_attribute_{objects[1].conditions.all().get().id}@none"
         for mri, value in ((mri_1, 20), (mri_1, 10), (mri_2, 15), (mri_2, 5)):
             self.store_metric(
                 self.project.organization.id,
@@ -1493,6 +1497,54 @@ class MetricsAPITestCase(TestCase, BaseMetricsTestCase):
             assert meta[0][1]["unit"] == "nanosecond"
             assert meta[0][1]["scaling_factor"] is None
 
+    @django_db_all(reset_sequences=True)
+    def test_span_metrics_count_span_duration(self):
+        config = {
+            "spanAttribute": "span.duration",
+            "aggregates": ["count"],
+            "unit": "millisecond",
+            "tags": [],
+            "conditions": [
+                {"value": ""},
+            ],
+        }
+
+        o = self.create_span_attribute_extraction_config(
+            dictionary=config, user_id=self.user.id, project=self.project
+        )
+
+        mri_1 = f"c:custom/span_attribute_{o.conditions.all().get().id}@none"
+        for mri, value in ((mri_1, 1), (mri_1, 1)):
+            self.store_metric(
+                self.project.organization.id,
+                self.project.id,
+                mri,
+                {},
+                self.ts(self.now()),
+                value,
+            )
+
+        query_1 = self.mql("sum", mri_1)
+
+        results = self.run_query(
+            mql_queries=[MQLQuery(query_1)],
+            start=self.now() - timedelta(minutes=30),
+            end=self.now() + timedelta(hours=1, minutes=30),
+            interval=3600,
+            organization=self.project.organization,
+            projects=[self.project],
+            environments=[],
+            referrer="metrics.data.api",
+        )
+        data = results["data"]
+        assert len(data) == 1
+        assert data[0][0]["by"] == {}
+        assert data[0][0]["series"] == [None, 2.0, None]
+        meta = results["meta"]
+        assert len(meta) == 1
+        assert meta[0][1]["unit"] is None
+        assert meta[0][1]["scaling_factor"] is None
+
     def test_query_with_basic_formula_and_non_coercible_units(self):
         mri_1 = "d:custom/page_load@nanosecond"
         mri_2 = "d:custom/page_size@byte"