From 460874b1c91c404c347558b1ed0e076575cd5887 Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Fri, 30 Aug 2024 11:24:47 +0200 Subject: [PATCH 1/3] chore(metrics): remove usages of span metric models --- .../querying/visitors/query_expression.py | 7 - src/sentry/snuba/metrics/naming_layer/mri.py | 42 ------ src/sentry/testutils/factories.py | 8 -- src/sentry/testutils/fixtures.py | 3 - .../sentry_metrics/querying/data/test_api.py | 135 ------------------ tests/snuba/metrics/naming_layer/test_mri.py | 86 ----------- 6 files changed, 281 deletions(-) diff --git a/src/sentry/sentry_metrics/querying/visitors/query_expression.py b/src/sentry/sentry_metrics/querying/visitors/query_expression.py index 61bda4b7b1b3e..6d23694850cff 100644 --- a/src/sentry/sentry_metrics/querying/visitors/query_expression.py +++ b/src/sentry/sentry_metrics/querying/visitors/query_expression.py @@ -7,7 +7,6 @@ from sentry.models.environment import Environment from sentry.models.project import Project -from sentry.sentry_metrics.models import SpanAttributeExtractionRuleCondition from sentry.sentry_metrics.querying.constants import COEFFICIENT_OPERATORS from sentry.sentry_metrics.querying.data.mapping.base import ( Mapper, @@ -426,12 +425,6 @@ def _extract_unit(self, timeseries: Timeseries) -> str | 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 - except SpanAttributeExtractionRuleCondition.DoesNotExist: - return None - return parsed_mri.unit return None diff --git a/src/sentry/snuba/metrics/naming_layer/mri.py b/src/sentry/snuba/metrics/naming_layer/mri.py index 5cd9cbe064743..9ad5e2f3b774a 100644 --- a/src/sentry/snuba/metrics/naming_layer/mri.py +++ b/src/sentry/snuba/metrics/naming_layer/mri.py @@ -38,12 +38,9 @@ from enum import Enum from typing import cast -import sentry_sdk from sentry_kafka_schemas.codecs import ValidationError from sentry.exceptions import InvalidParams -from sentry.sentry_metrics.extraction_rules import SPAN_ATTRIBUTE_PREFIX -from sentry.sentry_metrics.models import SpanAttributeExtractionRuleCondition from sentry.sentry_metrics.use_case_id_registry import UseCaseID from sentry.snuba.dataset import EntityKey from sentry.snuba.metrics.units import format_value_using_unit_and_op @@ -210,15 +207,6 @@ class ParsedMRI: def mri_string(self) -> str: return f"{self.entity}:{self.namespace}/{self.name}@{self.unit}" - @property - def span_attribute_rule_id(self): - if self.name.startswith(SPAN_ATTRIBUTE_PREFIX): - try: - return int(self.name[len(SPAN_ATTRIBUTE_PREFIX) :]) - except ValueError: - return None - return None - @dataclass class ParsedMRIField: @@ -264,24 +252,6 @@ def format_mri_field(field: str) -> str: parsed = parse_mri_field(field) if parsed: - if condition_id := parsed.mri.span_attribute_rule_id: - try: - condition = SpanAttributeExtractionRuleCondition.objects.get(id=condition_id) - config = condition.config - if condition.value: - filter_str = f' filtered by "{condition.value}"' - else: - filter_str = "" - - return f"{parsed.op}({config.span_attribute}){filter_str}" - except SpanAttributeExtractionRuleCondition.DoesNotExist: - with sentry_sdk.new_scope() as scope: - scope.set_tag("field", field) - sentry_sdk.capture_message( - "Trying to format MRI field for non-existent span metric." - ) - return field - return str(parsed) else: @@ -304,18 +274,6 @@ def format_mri_field_value(field: str, value: str) -> str: if parsed_mri_field is None: return value - if condition_id := parsed_mri_field.mri.span_attribute_rule_id: - try: - condition = SpanAttributeExtractionRuleCondition.objects.get(id=condition_id) - unit = cast(MetricUnit, condition.config.unit) - except SpanAttributeExtractionRuleCondition.DoesNotExist: - with sentry_sdk.new_scope() as scope: - scope.set_tag("field", field) - sentry_sdk.capture_message( - "Trying to format MRI field for non-existent span metric." - ) - return value - else: unit = cast(MetricUnit, parsed_mri_field.mri.unit) diff --git a/src/sentry/testutils/factories.py b/src/sentry/testutils/factories.py index 1d6339661d4a4..798d7eb46bd2d 100644 --- a/src/sentry/testutils/factories.py +++ b/src/sentry/testutils/factories.py @@ -144,7 +144,6 @@ ) from sentry.sentry_apps.services.app.serial import serialize_sentry_app_installation from sentry.sentry_apps.services.hook import hook_service -from sentry.sentry_metrics.models import SpanAttributeExtractionRuleConfig from sentry.signals import project_created from sentry.silo.base import SiloMode from sentry.snuba.dataset import Dataset @@ -1219,13 +1218,6 @@ def create_sentry_app_installation_for_provider( sentry_app_installation=installation, ) - @staticmethod - @assume_test_silo_mode(SiloMode.REGION) - def create_span_attribute_extraction_config( - dictionary: dict[str, Any], user_id: int, project: Project - ) -> SpanAttributeExtractionRuleConfig: - return SpanAttributeExtractionRuleConfig.from_dict(dictionary, user_id, project) - @staticmethod @assume_test_silo_mode(SiloMode.CONTROL) def create_stacktrace_link_schema(): diff --git a/src/sentry/testutils/fixtures.py b/src/sentry/testutils/fixtures.py index cf7665c16e73c..858c4e159c510 100644 --- a/src/sentry/testutils/fixtures.py +++ b/src/sentry/testutils/fixtures.py @@ -346,9 +346,6 @@ def create_sentry_app_installation(self, *args, **kwargs): def create_sentry_app_installation_for_provider(self, *args, **kwargs): return Factories.create_sentry_app_installation_for_provider(*args, **kwargs) - def create_span_attribute_extraction_config(self, *args, **kwargs): - return Factories.create_span_attribute_extraction_config(*args, **kwargs) - def create_stacktrace_link_schema(self, *args, **kwargs): return Factories.create_stacktrace_link_schema(*args, **kwargs) diff --git a/tests/sentry/sentry_metrics/querying/data/test_api.py b/tests/sentry/sentry_metrics/querying/data/test_api.py index 08ef5884dab53..f09e1365db9ab 100644 --- a/tests/sentry/sentry_metrics/querying/data/test_api.py +++ b/tests/sentry/sentry_metrics/querying/data/test_api.py @@ -29,7 +29,6 @@ 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 @@ -1411,140 +1410,6 @@ def test_query_with_basic_formula_and_coercible_units(self): 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 = [ - { - "spanAttribute": "my_duration_nano", - "aggregates": ["count", "p50", "p75", "p95", "p99"], - "unit": "nanosecond", - "tags": [], - "conditions": [ - {"value": ""}, - ], - }, - { - "spanAttribute": "my_duration_micro", - "aggregates": ["count", "p50", "p75", "p95", "p99"], - "unit": "microsecond", - "tags": [], - "conditions": [ - {"value": ""}, - ], - }, - ] - objects = [] - for config in configs: - o = self.create_span_attribute_extraction_config( - dictionary=config, user_id=self.user.id, project=self.project - ) - objects.append(o) - - 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, - self.project.id, - mri, - {}, - self.ts(self.now()), - value, - ) - - for formula, expected_result, expected_unit_family in ( - # (($query_2 * 1000) + 10000.0) - ("($query_2 + 10)", 30000.0, UnitFamily.DURATION.value), - # (($query_2 * 1000) + (10 * 2) * 1000) - ("($query_2 + (10 * 2))", 40000.0, UnitFamily.DURATION.value), - # (10000.0 + ($query_2 * 1000)) - ("(10 + $query_2)", 30000.0, UnitFamily.DURATION.value), - # (($query_2 + 1000) + (10000.0 + 20000.0)) - ("($query_2 + (10 + 20))", 50000.0, UnitFamily.DURATION.value), - # ((10000.0 + 20000.0) + ($query_2 + 1000)) - ("((10 + 20) + $query_2)", 50000.0, UnitFamily.DURATION.value), - # ($query_2 * 1000 + 10000.0) + ($query_2 * 1000) - ("($query_2 + 10) + $query_2", 50000.0, UnitFamily.DURATION.value), - # ($query_2 * 1000 + 10000.0) + $query_1 - ("($query_2 + 10) + $query_1", 30015.0, UnitFamily.DURATION.value), - # ($query_1 + 10) + ($query_2 * 1000) - ("($query_1 + 10) + $query_2", 20025.0, UnitFamily.DURATION.value), - ): - query_1 = self.mql("avg", mri_1) - query_2 = self.mql("sum", mri_2) - - results = self.run_query( - mql_queries=[ - MQLQuery(formula, query_1=MQLQuery(query_1), query_2=MQLQuery(query_2)) - ], - 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, expected_result, None] - assert data[0][0]["totals"] == expected_result - meta = results["meta"] - assert len(meta) == 1 - assert meta[0][1]["unit_family"] == expected_unit_family - 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" diff --git a/tests/snuba/metrics/naming_layer/test_mri.py b/tests/snuba/metrics/naming_layer/test_mri.py index 5158bf39ad47d..7777d97fe1227 100644 --- a/tests/snuba/metrics/naming_layer/test_mri.py +++ b/tests/snuba/metrics/naming_layer/test_mri.py @@ -27,89 +27,3 @@ def test_format_mri_field_value(self): assert format_mri_field_value("sum(d:spans/exclusive_time@millisecond)", "1000") == "1 s" assert format_mri_field_value("invalid_mri_field", "100") == "100" assert format_mri_field_value(cast(str, None), "100") == "100" - - def test_span_metric_mri_field(self): - config = { - "spanAttribute": "browser.name", - "aggregates": ["count_unique"], - "unit": "none", - "tags": [], - "conditions": [ - {"value": "browser.name:Chrome or browser.name:Firefox"}, - ], - } - project = self.create_project(organization=self.organization, name="my new project") - config_object = self.create_span_attribute_extraction_config( - dictionary=config, user_id=self.user.id, project=project - ) - condition_id = config_object.conditions.get().id - assert ( - format_mri_field(f"count_unique(c:custom/span_attribute_{condition_id}@none)") - == 'count_unique(browser.name) filtered by "browser.name:Chrome or browser.name:Firefox"' - ) - - def test_span_metric_mri_field_empty_condition(self): - config = { - "spanAttribute": "browser.name", - "aggregates": ["count_unique"], - "unit": "none", - "tags": [], - "conditions": [ - {"value": ""}, - ], - } - project = self.create_project(organization=self.organization, name="my new project") - config_object = self.create_span_attribute_extraction_config( - dictionary=config, user_id=self.user.id, project=project - ) - condition_id = config_object.conditions.get().id - assert ( - format_mri_field(f"count_unique(c:custom/span_attribute_{condition_id}@none)") - == "count_unique(browser.name)" - ) - - def test_span_metric_mri_field_value(self): - config = { - "spanAttribute": "my_duration", - "aggregates": ["avg", "min", "max", "sum"], - "unit": "millisecond", - "tags": [], - "conditions": [ - {"value": "browser.name:Chrome or browser.name:Firefox"}, - ], - } - project = self.create_project(organization=self.organization, name="my new project") - config_object = self.create_span_attribute_extraction_config( - dictionary=config, user_id=self.user.id, project=project - ) - condition_id = config_object.conditions.get().id - assert ( - format_mri_field_value(f"avg(c:custom/span_attribute_{condition_id}@none)", "1000") - == "1 s" - ) - - def test_span_metric_does_not_exist(self): - config = { - "spanAttribute": "my_duration", - "aggregates": ["avg", "min", "max", "sum"], - "unit": "millisecond", - "tags": [], - "conditions": [ - {"value": "browser.name:Chrome or browser.name:Firefox"}, - ], - } - project = self.create_project(organization=self.organization, name="my new project") - self.create_span_attribute_extraction_config( - dictionary=config, user_id=self.user.id, project=project - ) - non_existent_id = 65537 - - assert ( - format_mri_field_value(f"avg(c:custom/span_attribute_{non_existent_id}@none)", "1000") - == "1000" - ) - - assert ( - format_mri_field(f"avg(c:custom/span_attribute_{non_existent_id}@none)") - == f"avg(c:custom/span_attribute_{non_existent_id}@none)" - ) From 2c81c7c31ae7ff7d1b2ad95562b1f4f2ea8cc811 Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Mon, 2 Sep 2024 12:34:43 +0200 Subject: [PATCH 2/3] remove superfluous else --- src/sentry/snuba/metrics/naming_layer/mri.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/sentry/snuba/metrics/naming_layer/mri.py b/src/sentry/snuba/metrics/naming_layer/mri.py index 9ad5e2f3b774a..9d5d82c37be88 100644 --- a/src/sentry/snuba/metrics/naming_layer/mri.py +++ b/src/sentry/snuba/metrics/naming_layer/mri.py @@ -274,10 +274,9 @@ def format_mri_field_value(field: str, value: str) -> str: if parsed_mri_field is None: return value - else: - unit = cast(MetricUnit, parsed_mri_field.mri.unit) - + unit = cast(MetricUnit, parsed_mri_field.mri.unit) return format_value_using_unit_and_op(float(value), unit, parsed_mri_field.op) + except InvalidParams: return value From 412e73740d64fe250549900144f0abefad0c7652 Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Mon, 2 Sep 2024 14:12:23 +0200 Subject: [PATCH 3/3] remove span metric CRUD functions --- .../span_attribute_extraction_rules.py | 68 ------------------- 1 file changed, 68 deletions(-) delete mode 100644 src/sentry/sentry_metrics/span_attribute_extraction_rules.py diff --git a/src/sentry/sentry_metrics/span_attribute_extraction_rules.py b/src/sentry/sentry_metrics/span_attribute_extraction_rules.py deleted file mode 100644 index 26659d100181a..0000000000000 --- a/src/sentry/sentry_metrics/span_attribute_extraction_rules.py +++ /dev/null @@ -1,68 +0,0 @@ -from typing import Any - -import sentry_sdk -from rest_framework.request import Request - -from sentry.models.project import Project -from sentry.sentry_metrics.configuration import HARD_CODED_UNITS -from sentry.sentry_metrics.models import ( - SpanAttributeExtractionRuleCondition, - SpanAttributeExtractionRuleConfig, -) - - -def create_extraction_rule_config(request: Request, project: Project, config_update: Any): - configs = [] - for obj in config_update: - configs.append(SpanAttributeExtractionRuleConfig.from_dict(obj, request.user.id, project)) - if ( - "conditions" not in obj - or len(obj["conditions"]) == 0 - or (len(obj["conditions"]) == 1 and obj["conditions"][0] == "") - ): - with sentry_sdk.new_scope() as scope: - scope.set_tag("project", project.slug) - sentry_sdk.capture_message( - "A MetricExtractionRuleConfig without conditions was created.", - ) - - return configs - - -def delete_extraction_rule_config(project: Project, config_update: Any): - for obj in config_update: - SpanAttributeExtractionRuleConfig.objects.filter( - project=project, span_attribute=obj["spanAttribute"] - ).delete() - - -def update_extraction_rule_config(request: Request, project: Project, config_update: Any): - configs = [] - for obj in config_update: - config = SpanAttributeExtractionRuleConfig.objects.get( - project=project, span_attribute=obj["spanAttribute"] - ) - config.aggregates = obj["aggregates"] - config.unit = HARD_CODED_UNITS.get(obj["spanAttribute"], obj["unit"]) - config.tags = obj["tags"] - config.save() - config.refresh_from_db() - configs.append(config) - - # delete conditions not present in update - included_conditions = [x["id"] for x in obj["conditions"] if "id" in x] - SpanAttributeExtractionRuleCondition.objects.filter(config=config).exclude( - id__in=included_conditions - ).delete() - - for condition in obj["conditions"]: - condition_id = condition["id"] if "id" in condition else None - SpanAttributeExtractionRuleCondition.objects.update_or_create( - id=condition_id, - config=config, - defaults={ - "value": condition["value"], - "created_by_id": request.user.id, - }, - ) - return configs