Browse Source

cleanup(metrics): remove snuba tag value meta table references (#85193)

These tag values tables aren't being accessed and we want to stop
writing to this table to reduce load on the
generic-metrics-distributions cluster in SaaS (see INC-1035)

(not making this change broke integration tests when I tried to merge
https://github.com/getsentry/snuba/pull/6878 and that had to be
reverted)
Oliver Newland 3 weeks ago
parent
commit
a4a7ddb283

+ 0 - 6
src/sentry/api/urls.py

@@ -514,7 +514,6 @@ from .endpoints.organization_metrics_meta import (
     OrganizationMetricsCompatibilitySums,
 )
 from .endpoints.organization_metrics_query import OrganizationMetricsQueryEndpoint
-from .endpoints.organization_metrics_tag_details import OrganizationMetricsTagDetailsEndpoint
 from .endpoints.organization_metrics_tags import OrganizationMetricsTagsEndpoint
 from .endpoints.organization_on_demand_metrics_estimation_stats import (
     OrganizationOnDemandMetricsEstimationStatsEndpoint,
@@ -2093,11 +2092,6 @@ ORGANIZATION_URLS: list[URLPattern | URLResolver] = [
         OrganizationMetricsTagsEndpoint.as_view(),
         name="sentry-api-0-organization-metrics-tags",
     ),
-    re_path(
-        r"^(?P<organization_id_or_slug>[^/]+)/metrics/tags/(?P<tag_name>[^/]+)/$",
-        OrganizationMetricsTagDetailsEndpoint.as_view(),
-        name="sentry-api-0-organization-metrics-tag-details",
-    ),
     re_path(
         r"^(?P<organization_id_or_slug>[^/]+)/profiling/",
         include(

+ 4 - 10
src/sentry/sentry_metrics/querying/metadata/tags.py

@@ -4,7 +4,7 @@ from dataclasses import dataclass
 from sentry.models.organization import Organization
 from sentry.models.project import Project
 from sentry.sentry_metrics.use_case_id_registry import UseCaseID
-from sentry.snuba.metrics_layer.query import fetch_metric_tag_keys, fetch_metric_tag_values
+from sentry.snuba.metrics_layer.query import fetch_metric_tag_keys
 
 
 @dataclass
@@ -45,14 +45,8 @@ def get_tag_values(
     tag_value_prefix: str = "",
 ) -> list[str]:
     """
+    DEPRECATED: This query path is not in use in production
+
     Get all available tag values for an MRI and tag key from metrics.
     """
-    project_ids = [project.id for project in projects]
-    tag_values: set[str] = set()
-    for use_case_id in use_case_ids:
-        use_case_tag_values = fetch_metric_tag_values(
-            organization.id, project_ids, use_case_id, mri, tag_key, tag_value_prefix
-        )
-        tag_values = tag_values.union(use_case_tag_values)
-
-    return list(tag_values)
+    return []

+ 0 - 68
src/sentry/snuba/metrics_layer/query.py

@@ -31,7 +31,6 @@ from sentry.exceptions import InvalidParams
 from sentry.sentry_metrics.use_case_id_registry import UseCaseID
 from sentry.sentry_metrics.utils import (
     bulk_reverse_resolve,
-    resolve_many_weak,
     resolve_weak,
     reverse_resolve_weak,
     string_to_use_case_id,
@@ -639,70 +638,3 @@ def _query_meta_table(
             grouped_results.setdefault(row["project_id"], list()).append(val)
 
     return grouped_results
-
-
-def fetch_metric_tag_values(
-    org_id: int,
-    project_ids: list[int],
-    use_case_id: UseCaseID,
-    mri: str,
-    tag_key: str,
-    tag_value_prefix: str = "",
-    app_id: str = "",
-) -> list[str]:
-    """
-    Find all the unique tag values for a given MRI and tag key. This will reverse resolve
-    all the values.
-    """
-    parsed_mri = parse_mri(mri)
-    if parsed_mri is None:
-        raise InvalidParams(f"'{mri}' is not a valid MRI")
-
-    metric_type = {
-        "c": "counters",
-        "d": "distributions",
-        "g": "gauges",
-        "s": "sets",
-    }[parsed_mri.entity]
-
-    resolved = resolve_many_weak(use_case_id, org_id, [mri, tag_key])
-    if len(resolved) != 2:
-        raise InvalidParams("Unknown metric or tag key")
-    metric_id, tag_key_id = resolved
-
-    conditions = [
-        Condition(Column("project_id"), Op.IN, project_ids),
-        Condition(Column("metric_id"), Op.EQ, metric_id),
-        Condition(Column("tag_key"), Op.EQ, tag_key_id),
-        Condition(Column("timestamp"), Op.GTE, datetime.now(UTC) - timedelta(days=90)),
-        Condition(Column("timestamp"), Op.LT, datetime.now(UTC) + timedelta(days=1)),
-    ]
-
-    if tag_value_prefix:
-        conditions.append(Condition(Column("tag_value"), Op.LIKE, f"{tag_value_prefix}%"))
-
-    tag_values_query = (
-        Query(Storage(f"generic_metrics_{metric_type}_meta_tag_values"))
-        .set_select([Column("tag_value")])
-        .set_groupby([Column("tag_value")])
-        .set_where(conditions)
-        .set_orderby([OrderBy(Column("tag_value"), Direction.ASC)])
-        .set_limit(1000)
-    )
-
-    request = Request(
-        dataset="generic_metrics",
-        app_id=use_case_id.value if app_id == "" else app_id,
-        query=tag_values_query,
-        tenant_ids={
-            "organization_id": org_id,
-            "referrer": "generic_metrics_meta_tag_values",
-        },
-    )
-
-    results = bulk_snuba_queries([request], "generic_metrics_meta_tag_values")
-    values = []
-    for result in results:
-        values.extend([row["tag_value"] for row in result["data"]])
-
-    return values

+ 9 - 13
tests/sentry/api/endpoints/test_organization_metrics.py

@@ -38,25 +38,21 @@ MOCKED_DERIVED_METRICS.update(
 
 
 class OrganizationMetricsPermissionTest(APITestCase):
-    endpoints = (
-        (
-            "get",
-            "sentry-api-0-organization-metrics-details",
-        ),
-        (
+    endpoints = [
+        ["get", "sentry-api-0-organization-metrics-details"],
+        [
             "get",
             "sentry-api-0-organization-metrics-tags",
-        ),
-        ("get", "sentry-api-0-organization-metrics-tag-details", "foo"),
-        (
+        ],
+        [
             "get",
             "sentry-api-0-organization-metrics-data",
-        ),
-        (
+        ],
+        [
             "post",
             "sentry-api-0-organization-metrics-query",
-        ),
-    )
+        ],
+    ]
 
     def setUp(self):
         self.create_project(name="Bar", slug="bar", teams=[self.team], fire_project_created=True)

+ 0 - 123
tests/sentry/api/endpoints/test_organization_metrics_metadata.py

@@ -1,123 +0,0 @@
-from datetime import timedelta
-
-import pytest
-
-from sentry.snuba.metrics.naming_layer import TransactionMRI
-from sentry.testutils.cases import MetricsAPIBaseTestCase
-from sentry.testutils.helpers.datetime import freeze_time
-
-pytestmark = pytest.mark.sentry_metrics
-
-
-@freeze_time(MetricsAPIBaseTestCase.MOCK_DATETIME)
-class OrganizationMetricsTagValues(MetricsAPIBaseTestCase):
-    method = "get"
-    endpoint = "sentry-api-0-organization-metrics-tag-details"
-
-    def setUp(self):
-        super().setUp()
-        self.login_as(self.user)
-
-        release_1 = self.create_release(
-            project=self.project, version="1.0", date_added=MetricsAPIBaseTestCase.MOCK_DATETIME
-        )
-        release_2 = self.create_release(
-            project=self.project,
-            version="2.0",
-            date_added=MetricsAPIBaseTestCase.MOCK_DATETIME + timedelta(minutes=5),
-        )
-
-        # Use Case: TRANSACTIONS
-        for value, transaction, platform, env, release, time in (
-            (1, "/hello", "android", "prod", release_1.version, self.now()),
-            (6, "/hello", "ios", "dev", release_2.version, self.now()),
-            (5, "/world", "windows", "prod", release_1.version, self.now() + timedelta(minutes=30)),
-            (3, "/hello", "ios", "dev", release_2.version, self.now() + timedelta(hours=1)),
-            (2, "/hello", "android", "dev", release_1.version, self.now() + timedelta(hours=1)),
-            (
-                4,
-                "/world",
-                "windows",
-                "prod",
-                release_2.version,
-                self.now() + timedelta(hours=1, minutes=30),
-            ),
-        ):
-            self.store_metric(
-                self.project.organization.id,
-                self.project.id,
-                TransactionMRI.DURATION.value,
-                {
-                    "transaction": transaction,
-                    "platform": platform,
-                    "environment": env,
-                    "release": release,
-                },
-                self.now().timestamp(),
-                value,
-            )
-
-        self.prod_env = self.create_environment(name="prod", project=self.project)
-        self.dev_env = self.create_environment(name="dev", project=self.project)
-
-    def now(self):
-        return MetricsAPIBaseTestCase.MOCK_DATETIME
-
-    def test_tag_details_for_transactions_use_case(self):
-        response = self.get_success_response(
-            self.project.organization.slug,
-            "transaction",
-            metric=["d:transactions/duration@millisecond"],
-            project=[self.project.id],
-            useCase="transactions",
-        )
-        assert sorted(response.data, key=lambda x: x["value"]) == [
-            {"key": "transaction", "value": "/hello"},
-            {"key": "transaction", "value": "/world"},
-        ]
-
-    def test_non_existing_tag_for_transactions_use_case(self):
-        response = self.get_error_response(
-            self.project.organization.slug,
-            "my_non_existent_tag",
-            metric=["d:transactions/duration@millisecond"],
-            project=[self.project.id],
-            useCase="transactions",
-        )
-        assert response.status_code == 404
-        assert (
-            response.json()["detail"]
-            == "No data found for metric: d:transactions/duration@millisecond and tag: my_non_existent_tag"
-        )
-
-    def test_tag_details_for_non_existent_metric(self):
-        response = self.get_error_response(
-            self.project.organization.slug,
-            "my_non_existent_tag",
-            metric=["d:transactions/my_non_existent_test_metric@percent"],
-            project=[self.project.id],
-            useCase="transactions",
-        )
-        assert response.status_code == 404
-        assert (
-            response.json()["detail"]
-            == "No data found for metric: d:transactions/my_non_existent_test_metric@percent and tag: my_non_existent_tag"
-        )
-
-    # fix this
-    def test_tag_details_for_multiple_supplied_metrics(self):
-        response = self.get_error_response(
-            self.project.organization.slug,
-            "my_non_existent_tag",
-            metric=[
-                "d:transactions/my_test_metric@percent",
-                "d:transactions/duration@millisecond",
-            ],
-            project=[self.project.id],
-            useCase="transactions",
-        )
-
-        assert (
-            response.json()["detail"]
-            == "Please supply only a single metric name. Specifying multiple metric names is not supported for this endpoint."
-        )

+ 0 - 57
tests/sentry/api/endpoints/test_organization_metrics_tag_details.py

@@ -1,57 +0,0 @@
-import time
-
-import pytest
-
-from sentry.sentry_metrics import indexer
-from sentry.sentry_metrics.use_case_id_registry import UseCaseID
-from sentry.testutils.cases import OrganizationMetricsIntegrationTestCase
-
-pytestmark = pytest.mark.sentry_metrics
-
-
-def _indexer_record(org_id: int, string: str) -> None:
-    indexer.record(use_case_id=UseCaseID.SESSIONS, org_id=org_id, string=string)
-
-
-class OrganizationMetricsTagDetailsTest(OrganizationMetricsIntegrationTestCase):
-
-    endpoint = "sentry-api-0-organization-metrics-tag-details"
-
-    def test_unknown_tag(self):
-        _indexer_record(self.organization.id, "bar")
-        response = self.get_response(self.project.organization.slug, "bar")
-        assert response.status_code == 404
-        assert response.json()["detail"] == "No data found for tag: bar"
-
-    def test_non_existing_tag(self):
-        response = self.get_response(self.project.organization.slug, "bar")
-        assert response.status_code == 404
-        assert response.json()["detail"] == "No data found for tag: bar"
-
-    def test_non_existing_metric_name(self):
-        _indexer_record(self.organization.id, "bar")
-        response = self.get_response(self.project.organization.slug, "bar", metric="bad")
-        assert response.status_code == 404
-        assert response.json()["detail"] == "No data found for metric: bad and tag: bar"
-
-    def test_metric_not_in_naming_layer(self):
-        self.store_session(
-            self.build_session(
-                project_id=self.project.id,
-                started=(time.time() // 60) * 60,
-                status="ok",
-                release="foobar@2.0",
-                errors=2,
-            )
-        )
-
-        response = self.get_response(
-            self.organization.slug,
-            "release",
-            metric=["session.abnormal_and_crashed"],
-        )
-        assert response.status_code == 404
-        assert (
-            response.json()["detail"]
-            == "No data found for metric: session.abnormal_and_crashed and tag: release"
-        )

+ 0 - 240
tests/sentry/api/endpoints/test_organization_metrics_tag_details_v2.py

@@ -1,240 +0,0 @@
-from datetime import timedelta
-
-import pytest
-
-from sentry.snuba.metrics.naming_layer import TransactionMRI
-from sentry.testutils.cases import MetricsAPIBaseTestCase
-from sentry.testutils.helpers.datetime import freeze_time
-
-pytestmark = pytest.mark.sentry_metrics
-
-
-@freeze_time(MetricsAPIBaseTestCase.MOCK_DATETIME)
-class OrganizationMetricsTagValues(MetricsAPIBaseTestCase):
-    method = "get"
-    endpoint = "sentry-api-0-organization-metrics-tag-details"
-
-    def setUp(self):
-        super().setUp()
-        self.login_as(self.user)
-
-        release_1 = self.create_release(
-            project=self.project, version="1.0", date_added=MetricsAPIBaseTestCase.MOCK_DATETIME
-        )
-        release_2 = self.create_release(
-            project=self.project,
-            version="2.0",
-            date_added=MetricsAPIBaseTestCase.MOCK_DATETIME + timedelta(minutes=5),
-        )
-
-        # Use Case: TRANSACTIONS
-        for value, transaction, platform, env, release, time in (
-            (1, "/hello", "android", "prod", release_1.version, self.now()),
-            (6, "/hello", "ios", "dev", release_2.version, self.now()),
-            (5, "/world", "windows", "prod", release_1.version, self.now() + timedelta(minutes=30)),
-            (3, "/hello", "ios", "dev", release_2.version, self.now() + timedelta(hours=1)),
-            (2, "/hello", "android", "dev", release_1.version, self.now() + timedelta(hours=1)),
-            (
-                4,
-                "/world",
-                "windows",
-                "prod",
-                release_2.version,
-                self.now() + timedelta(hours=1, minutes=30),
-            ),
-        ):
-            self.store_metric(
-                self.project.organization.id,
-                self.project.id,
-                TransactionMRI.DURATION.value,
-                {
-                    "transaction": transaction,
-                    "platform": platform,
-                    "environment": env,
-                    "release": release,
-                },
-                self.now().timestamp(),
-                value,
-            )
-        # Use Case: CUSTOM
-        for value, release, tag_value, time in (
-            (1, release_1.version, "tag_value_1", self.now()),
-            (1, release_1.version, "tag_value_1", self.now()),
-            (1, release_1.version, "tag_value_2", self.now() - timedelta(days=40)),
-            (1, release_2.version, "tag_value_3", self.now() - timedelta(days=50)),
-            (1, release_2.version, "tag_value_4", self.now() - timedelta(days=60)),
-            (1, release_2.version, "my_tag_value_5", self.now() - timedelta(days=60)),
-        ):
-            self.store_metric(
-                self.project.organization.id,
-                self.project.id,
-                "d:transactions/my_test_metric@percent",
-                {
-                    "transaction": "/hello",
-                    "platform": "platform",
-                    "environment": "prod",
-                    "release": release,
-                    "mytag": tag_value,
-                },
-                self.now().timestamp(),
-                value,
-            )
-
-        self.prod_env = self.create_environment(name="prod", project=self.project)
-        self.dev_env = self.create_environment(name="dev", project=self.project)
-
-    def now(self):
-        return MetricsAPIBaseTestCase.MOCK_DATETIME
-
-    def test_tag_details_for_transactions_use_case(self):
-        response = self.get_success_response(
-            self.project.organization.slug,
-            "transaction",
-            metric=["d:transactions/duration@millisecond"],
-            project=[self.project.id],
-            useCase="transactions",
-        )
-        assert sorted(response.data, key=lambda x: x["value"]) == [
-            {"key": "transaction", "value": "/hello"},
-            {"key": "transaction", "value": "/world"},
-        ]
-
-    def test_tag_details_prefix(self):
-        response = self.get_success_response(
-            self.project.organization.slug,
-            "mytag",
-            metric=["d:transactions/my_test_metric@percent"],
-            project=[self.project.id],
-            useCase="transactions",
-            prefix="tag_val",
-        )
-        assert sorted(response.data, key=lambda x: x["value"]) == [
-            {"key": "mytag", "value": "tag_value_1"},
-            {"key": "mytag", "value": "tag_value_2"},
-            {"key": "mytag", "value": "tag_value_3"},
-            {"key": "mytag", "value": "tag_value_4"},
-        ]
-
-    def test_tag_details_prefix_empty_result(self):
-        response = self.get_success_response(
-            self.project.organization.slug,
-            "mytag",
-            metric=["d:transactions/my_test_metric@percent"],
-            project=[self.project.id],
-            useCase="transactions",
-            prefix="this_does_not_exist",
-        )
-        assert len(response.data) == 0
-
-    def test_tag_details_prefix_non_existent_metric(self):
-        response = self.get_response(
-            self.project.organization.slug,
-            "mytag",
-            metric=["d:transactions/my_non_existent_metric@percent"],
-            project=[self.project.id],
-            useCase="transactions",
-            prefix="this_does_not_exist",
-        )
-        assert response.status_code == 404
-        assert (
-            response.json()["detail"]
-            == "No data found for metric: d:transactions/my_non_existent_metric@percent and tag: mytag"
-        )
-
-    def test_tag_details_prefix_non_existent_tag_key(self):
-        response = self.get_response(
-            self.project.organization.slug,
-            "mytagkeydoesnotexist",
-            metric=["d:transactions/my_non_existent_metric@percent"],
-            project=[self.project.id],
-            useCase="transactions",
-            prefix="this_does_not_exist",
-        )
-        assert response.status_code == 404
-        assert (
-            response.json()["detail"]
-            == "No data found for metric: d:transactions/my_non_existent_metric@percent and tag: mytagkeydoesnotexist"
-        )
-
-    def test_tag_details_empty_prefix(self):
-        response = self.get_success_response(
-            self.project.organization.slug,
-            "mytag",
-            metric=["d:transactions/my_test_metric@percent"],
-            project=[self.project.id],
-            useCase="transactions",
-            prefix="",
-        )
-        assert sorted(response.data, key=lambda x: x["value"]) == [
-            {"key": "mytag", "value": "my_tag_value_5"},
-            {"key": "mytag", "value": "tag_value_1"},
-            {"key": "mytag", "value": "tag_value_2"},
-            {"key": "mytag", "value": "tag_value_3"},
-            {"key": "mytag", "value": "tag_value_4"},
-        ]
-
-    def test_non_existing_tag_for_transactions_use_case(self):
-        response = self.get_error_response(
-            self.project.organization.slug,
-            "my_non_existent_tag",
-            metric=["d:transactions/duration@millisecond"],
-            project=[self.project.id],
-            useCase="transactions",
-        )
-        assert response.status_code == 404
-        assert (
-            response.json()["detail"]
-            == "No data found for metric: d:transactions/duration@millisecond and tag: my_non_existent_tag"
-        )
-
-    def test_non_existing_tag_for_custom_use_case(self):
-        response = self.get_error_response(
-            self.project.organization.slug,
-            "my_non_existent_tag",
-            metric=["d:transactions/my_test_metric@percent"],
-            project=[self.project.id],
-            useCase="transactions",
-        )
-        assert response.status_code == 404
-        assert (
-            response.json()["detail"]
-            == "No data found for metric: d:transactions/my_test_metric@percent and tag: my_non_existent_tag"
-        )
-
-    def test_tag_details_for_non_existent_metric(self):
-        response = self.get_error_response(
-            self.project.organization.slug,
-            "my_non_existent_tag",
-            metric=["d:transactions/my_non_existent_test_metric@percent"],
-            project=[self.project.id],
-            useCase="transactions",
-        )
-        assert response.status_code == 404
-        assert (
-            response.json()["detail"]
-            == "No data found for metric: d:transactions/my_non_existent_test_metric@percent and tag: my_non_existent_tag"
-        )
-
-    def test_tag_details_for_multiple_supplied_metrics(self):
-        response = self.get_error_response(
-            self.project.organization.slug,
-            "my_non_existent_tag",
-            metric=[
-                "d:transactions/my_test_metric@percent",
-                "d:transactions/duration@millisecond",
-            ],
-            project=[self.project.id],
-            useCase="transactions",
-        )
-        assert response.status_code == 400
-        assert (
-            response.json()["detail"]
-            == "Please supply only a single metric name. Specifying multiple metric names is not supported for this endpoint."
-        )
-
-    def test_metrics_tags_when_organization_has_no_projects(self):
-        organization_without_projects = self.create_organization()
-        self.create_member(user=self.user, organization=organization_without_projects)
-        response = self.get_error_response(organization_without_projects.slug, "mytag")
-        assert response.status_code == 404
-        assert response.data["detail"] == "You must supply at least one project to see its metrics"

+ 0 - 46
tests/snuba/test_metrics_layer.py

@@ -29,7 +29,6 @@ from sentry.snuba.metrics_layer.query import (
     bulk_run_query,
     fetch_metric_mris,
     fetch_metric_tag_keys,
-    fetch_metric_tag_values,
     run_query,
 )
 from sentry.testutils.cases import BaseMetricsTestCase, TestCase
@@ -989,48 +988,3 @@ class MQLMetaTest(TestCase, BaseMetricsTestCase):
         assert len(tag_keys) == 1
         assert len(tag_keys[self.project.id]) == 3
         assert tag_keys[self.project.id] == ["status_code", "device", "transaction"]
-
-    def test_fetch_metric_tag_values(self) -> None:
-        tag_values = fetch_metric_tag_values(
-            self.org_id,
-            [self.project.id],
-            UseCaseID.TRANSACTIONS,
-            "g:transactions/test_gauge@none",
-            "transaction",
-        )
-        assert len(tag_values) == 2
-        assert tag_values == ["transaction_0", "transaction_1"]
-
-    def test_fetch_metric_tag_values_with_prefix(self) -> None:
-        tag_values = fetch_metric_tag_values(
-            self.org_id,
-            [self.project.id],
-            UseCaseID.TRANSACTIONS,
-            "g:transactions/test_gauge@none",
-            "status_code",
-            "5",
-        )
-        assert len(tag_values) == 1
-        assert tag_values == ["500"]
-
-    def test_fetch_metric_tag_values_for_multiple_projects(self) -> None:
-        new_project = self.create_project(name="New Project")
-        self.store_metric(
-            self.org_id,
-            new_project.id,
-            "g:transactions/test_gauge@none",
-            {"status_code": "524"},
-            self.ts(self.hour_ago + timedelta(minutes=10)),
-            10,
-        )
-
-        tag_values = fetch_metric_tag_values(
-            self.org_id,
-            [self.project.id, new_project.id],
-            UseCaseID.TRANSACTIONS,
-            "g:transactions/test_gauge@none",
-            "status_code",
-            "5",
-        )
-        assert len(tag_values) == 2
-        assert tag_values == ["500", "524"]