From 14d41d1bacc1c7c2cc73c11837ef409c21dd86db Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Wed, 15 May 2024 14:56:10 +0200 Subject: [PATCH 1/6] feat(metrics): remove percentiles from meta endpoint in Metrics API --- src/sentry/snuba/metrics/datasource.py | 9 ++++++++- .../api/endpoints/test_organization_metrics_details.py | 10 ++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/sentry/snuba/metrics/datasource.py b/src/sentry/snuba/metrics/datasource.py index 7659fab695811b..96a3e89d541ee8 100644 --- a/src/sentry/snuba/metrics/datasource.py +++ b/src/sentry/snuba/metrics/datasource.py @@ -312,12 +312,19 @@ def get_metrics_blocking_state_of_projects( def _build_metric_meta( parsed_mri: ParsedMRI, project_ids: Sequence[int], blocking_status: Sequence[BlockedMetric] ) -> MetricMeta: + available_operations = get_available_operations(parsed_mri) + blocked_operations = ["p50", "p75", "p90", "p95", "p99"] + if parsed_mri.namespace == "custom": + available_operations = [ + operation for operation in available_operations if operation not in blocked_operations + ] + return MetricMeta( type=parsed_mri.entity, name=parsed_mri.name, unit=cast(MetricUnit, parsed_mri.unit), mri=parsed_mri.mri_string, - operations=cast(Sequence[MetricOperationType], get_available_operations(parsed_mri)), + operations=cast(Sequence[MetricOperationType], available_operations), projectIds=project_ids, blockingStatus=blocking_status, ) diff --git a/tests/sentry/api/endpoints/test_organization_metrics_details.py b/tests/sentry/api/endpoints/test_organization_metrics_details.py index ad19524fbcafc3..64682821e1e4fc 100644 --- a/tests/sentry/api/endpoints/test_organization_metrics_details.py +++ b/tests/sentry/api/endpoints/test_organization_metrics_details.py @@ -220,3 +220,13 @@ def test_metrics_details_for_custom_metrics(self): assert data[2]["blockingStatus"] == [ {"isBlocked": True, "blockedTags": [], "projectId": project_1.id} ] + assert sorted(data[1]["operations"]) == [ + "avg", + "count", + "histogram", + "max", + "max_timestamp", + "min", + "min_timestamp", + "sum", + ] From cb027206da12c5c6b93cc8316e2985d6234b0ae0 Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Thu, 16 May 2024 09:15:03 +0200 Subject: [PATCH 2/6] move build_metric_meta to metrics API and move hidden operations to constant --- .../querying/metadata/metrics.py | 33 ++++++- .../sentry_metrics/querying/metadata/utils.py | 2 + src/sentry/snuba/metrics/datasource.py | 93 +------------------ 3 files changed, 32 insertions(+), 96 deletions(-) diff --git a/src/sentry/sentry_metrics/querying/metadata/metrics.py b/src/sentry/sentry_metrics/querying/metadata/metrics.py index 21d4ec5f0fbb47..36a86fb841bcd0 100644 --- a/src/sentry/sentry_metrics/querying/metadata/metrics.py +++ b/src/sentry/sentry_metrics/querying/metadata/metrics.py @@ -1,15 +1,15 @@ from collections import defaultdict from collections.abc import Sequence +from typing import cast from sentry.models.organization import Organization from sentry.models.project import Project +from sentry.sentry_metrics.querying.metadata.utils import METRICS_API_HIDDEN_OPERATIONS from sentry.sentry_metrics.use_case_id_registry import UseCaseID from sentry.snuba.metrics import parse_mri -from sentry.snuba.metrics.datasource import ( - _build_metric_meta, - get_metrics_blocking_state_of_projects, -) -from sentry.snuba.metrics.utils import BlockedMetric, MetricMeta +from sentry.snuba.metrics.datasource import get_metrics_blocking_state_of_projects +from sentry.snuba.metrics.naming_layer.mri import ParsedMRI, get_available_operations +from sentry.snuba.metrics.utils import BlockedMetric, MetricMeta, MetricOperationType, MetricUnit from sentry.snuba.metrics_layer.query import fetch_metric_mris @@ -95,3 +95,26 @@ def _convert_to_mris_to_project_ids_mapping(project_id_to_mris: dict[int, list[s mris_to_project_ids.setdefault(mri, []).append(project_id) return mris_to_project_ids + + +def _build_metric_meta( + parsed_mri: ParsedMRI, project_ids: Sequence[int], blocking_status: Sequence[BlockedMetric] +) -> MetricMeta: + available_operations = get_available_operations(parsed_mri) + + if parsed_mri.namespace == "custom": + available_operations = [ + operation + for operation in available_operations + if operation not in METRICS_API_HIDDEN_OPERATIONS + ] + + return MetricMeta( + type=parsed_mri.entity, + name=parsed_mri.name, + unit=cast(MetricUnit, parsed_mri.unit), + mri=parsed_mri.mri_string, + operations=cast(Sequence[MetricOperationType], available_operations), + projectIds=project_ids, + blockingStatus=blocking_status, + ) diff --git a/src/sentry/sentry_metrics/querying/metadata/utils.py b/src/sentry/sentry_metrics/querying/metadata/utils.py index cc7df54f7e8b8e..71e259e5e47327 100644 --- a/src/sentry/sentry_metrics/querying/metadata/utils.py +++ b/src/sentry/sentry_metrics/querying/metadata/utils.py @@ -1,6 +1,8 @@ from sentry.snuba.metrics import get_mri from sentry.snuba.metrics.naming_layer.mri import is_mri +METRICS_API_HIDDEN_OPERATIONS = ["p50", "p75", "p90", "p95", "p99"] + def convert_metric_names_to_mris(metric_names: list[str]) -> list[str]: mris: list[str] = [] diff --git a/src/sentry/snuba/metrics/datasource.py b/src/sentry/snuba/metrics/datasource.py index 96a3e89d541ee8..c9492a5697160f 100644 --- a/src/sentry/snuba/metrics/datasource.py +++ b/src/sentry/snuba/metrics/datasource.py @@ -13,7 +13,6 @@ """ __all__ = ( - "get_metrics_meta", "get_all_tags", "get_tag_values", "get_series", @@ -27,7 +26,7 @@ from dataclasses import dataclass, replace from datetime import datetime from operator import itemgetter -from typing import Any, cast +from typing import Any from snuba_sdk import And, Column, Condition, Function, Op, Or, Query, Request from snuba_sdk.conditions import ConditionGroup @@ -54,13 +53,7 @@ org_id_from_projects, ) from sentry.snuba.metrics.naming_layer.mapping import get_mri -from sentry.snuba.metrics.naming_layer.mri import ( - ParsedMRI, - get_available_operations, - is_custom_measurement, - is_mri, - parse_mri, -) +from sentry.snuba.metrics.naming_layer.mri import is_custom_measurement, is_mri, parse_mri from sentry.snuba.metrics.query import Groupable, MetricField, MetricsQuery from sentry.snuba.metrics.query_builder import ( SnubaQueryBuilder, @@ -73,14 +66,11 @@ CUSTOM_MEASUREMENT_DATASETS, METRIC_TYPE_TO_ENTITY, UNALLOWED_TAGS, - BlockedMetric, DerivedMetricParseException, MetricDoesNotExistInIndexer, MetricMeta, MetricMetaWithTagKeys, - MetricOperationType, MetricType, - MetricUnit, NotSupportedOverCompositeEntityException, Tag, TagValue, @@ -309,85 +299,6 @@ def get_metrics_blocking_state_of_projects( return metrics_blocking_state_by_mri -def _build_metric_meta( - parsed_mri: ParsedMRI, project_ids: Sequence[int], blocking_status: Sequence[BlockedMetric] -) -> MetricMeta: - available_operations = get_available_operations(parsed_mri) - blocked_operations = ["p50", "p75", "p90", "p95", "p99"] - if parsed_mri.namespace == "custom": - available_operations = [ - operation for operation in available_operations if operation not in blocked_operations - ] - - return MetricMeta( - type=parsed_mri.entity, - name=parsed_mri.name, - unit=cast(MetricUnit, parsed_mri.unit), - mri=parsed_mri.mri_string, - operations=cast(Sequence[MetricOperationType], available_operations), - projectIds=project_ids, - blockingStatus=blocking_status, - ) - - -def get_metrics_meta( - projects: Sequence[Project], - use_case_ids: Sequence[UseCaseID], - start: datetime | None = None, - end: datetime | None = None, -) -> Sequence[MetricMeta]: - if not projects: - return [] - - stored_metrics = get_stored_metrics_of_projects(projects, use_case_ids, start, end) - metrics_blocking_state = ( - get_metrics_blocking_state_of_projects(projects) if UseCaseID.CUSTOM in use_case_ids else {} - ) - - metrics_metas = [] - for metric_mri, project_ids in stored_metrics.items(): - parsed_mri = parse_mri(metric_mri) - if parsed_mri is None: - with sentry_sdk.push_scope() as scope: - scope.set_extra("project_ids", project_ids) - scope.set_extra("metric_mri", metric_mri) - sentry_sdk.capture_message("Invalid metric MRI detected") - - continue - - blocking_status = [] - if (metric_blocking := metrics_blocking_state.get(metric_mri)) is not None: - blocking_status = [ - BlockedMetric(isBlocked=is_blocked, blockedTags=blocked_tags, projectId=project_id) - for is_blocked, blocked_tags, project_id in metric_blocking - ] - # We delete the metric so that in the next steps we can just merge the remaining blocked metrics that are - # not stored. - del metrics_blocking_state[metric_mri] - - metrics_metas.append(_build_metric_meta(parsed_mri, project_ids, blocking_status)) - - for metric_mri, metric_blocking in metrics_blocking_state.items(): - parsed_mri = parse_mri(metric_mri) - if parsed_mri is None: - continue - - metrics_metas.append( - _build_metric_meta( - parsed_mri, - [], - [ - BlockedMetric( - isBlocked=is_blocked, blockedTags=blocked_tags, projectId=project_id - ) - for is_blocked, blocked_tags, project_id in metric_blocking - ], - ) - ) - - return metrics_metas - - def get_stored_metrics_of_projects( projects: Sequence[Project], use_case_ids: Sequence[UseCaseID], From e80cdfed717dfa1a9fa9001f85e6cc689f454a67 Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Thu, 16 May 2024 09:29:45 +0200 Subject: [PATCH 3/6] add option for hiding of percentiles --- src/sentry/options/defaults.py | 7 +++++++ src/sentry/sentry_metrics/querying/metadata/metrics.py | 3 ++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/sentry/options/defaults.py b/src/sentry/options/defaults.py index ff85730ad8f193..bf0429b614a0f6 100644 --- a/src/sentry/options/defaults.py +++ b/src/sentry/options/defaults.py @@ -2506,3 +2506,10 @@ register( "api.organization-activity.brownout-duration", default="PT1M", flags=FLAG_AUTOMATOR_MODIFIABLE ) +# Disable +register( + "sentry-metrics.metrics-api.hide-percentile-operations", + type=Bool, + default=False, + flags=FLAG_AUTOMATOR_MODIFIABLE, +) diff --git a/src/sentry/sentry_metrics/querying/metadata/metrics.py b/src/sentry/sentry_metrics/querying/metadata/metrics.py index 36a86fb841bcd0..9f7826e1fc9ac8 100644 --- a/src/sentry/sentry_metrics/querying/metadata/metrics.py +++ b/src/sentry/sentry_metrics/querying/metadata/metrics.py @@ -2,6 +2,7 @@ from collections.abc import Sequence from typing import cast +from sentry import options from sentry.models.organization import Organization from sentry.models.project import Project from sentry.sentry_metrics.querying.metadata.utils import METRICS_API_HIDDEN_OPERATIONS @@ -102,7 +103,7 @@ def _build_metric_meta( ) -> MetricMeta: available_operations = get_available_operations(parsed_mri) - if parsed_mri.namespace == "custom": + if options.get("sentry-metrics.metrics-api.hide-percentile-operations"): available_operations = [ operation for operation in available_operations From d17d3e19e0086f8705d176e677f5d3fb30b288b8 Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Thu, 16 May 2024 09:36:26 +0200 Subject: [PATCH 4/6] do testing with hidden percentile operations --- .../test_organization_metrics_details.py | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/sentry/api/endpoints/test_organization_metrics_details.py b/tests/sentry/api/endpoints/test_organization_metrics_details.py index 64682821e1e4fc..f4fe99823d6d8b 100644 --- a/tests/sentry/api/endpoints/test_organization_metrics_details.py +++ b/tests/sentry/api/endpoints/test_organization_metrics_details.py @@ -9,6 +9,7 @@ ) from sentry.sentry_metrics.visibility import block_metric, block_tags_of_metric from sentry.testutils.cases import MetricsAPIBaseTestCase, OrganizationMetricsIntegrationTestCase +from sentry.testutils.helpers import override_options from sentry.testutils.skips import requires_snuba pytestmark = [pytest.mark.sentry_metrics, requires_snuba] @@ -228,5 +229,28 @@ def test_metrics_details_for_custom_metrics(self): "max_timestamp", "min", "min_timestamp", + "p50", + "p75", + "p90", + "p95", + "p99", "sum", ] + + with override_options( + {"sentry-metrics.metrics-api.hide-percentile-operations": True}, + ): + response = self.get_success_response( + self.organization.slug, project=[project_1.id, project_2.id], useCase="custom" + ) + data = sorted(response.data, key=lambda d: d["mri"]) + assert sorted(data[1]["operations"]) == [ + "avg", + "count", + "histogram", + "max", + "max_timestamp", + "min", + "min_timestamp", + "sum", + ] From ba75a0fb18977703808edca1469a04d6074062fa Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Thu, 16 May 2024 11:01:03 +0200 Subject: [PATCH 5/6] satisfy mypy types --- src/sentry/options/defaults.py | 3 ++- src/sentry/sentry_metrics/querying/metadata/metrics.py | 10 ++++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/sentry/options/defaults.py b/src/sentry/options/defaults.py index bf0429b614a0f6..e7e7ab7866209f 100644 --- a/src/sentry/options/defaults.py +++ b/src/sentry/options/defaults.py @@ -2506,7 +2506,8 @@ register( "api.organization-activity.brownout-duration", default="PT1M", flags=FLAG_AUTOMATOR_MODIFIABLE ) -# Disable +# Hide percentile operations from appearing in the metrics/meta endpoint in the Metrics API, thereby also hiding those +# operations from view in the Metrics UI. register( "sentry-metrics.metrics-api.hide-percentile-operations", type=Bool, diff --git a/src/sentry/sentry_metrics/querying/metadata/metrics.py b/src/sentry/sentry_metrics/querying/metadata/metrics.py index 9f7826e1fc9ac8..3309654f826eb8 100644 --- a/src/sentry/sentry_metrics/querying/metadata/metrics.py +++ b/src/sentry/sentry_metrics/querying/metadata/metrics.py @@ -10,7 +10,13 @@ from sentry.snuba.metrics import parse_mri from sentry.snuba.metrics.datasource import get_metrics_blocking_state_of_projects from sentry.snuba.metrics.naming_layer.mri import ParsedMRI, get_available_operations -from sentry.snuba.metrics.utils import BlockedMetric, MetricMeta, MetricOperationType, MetricUnit +from sentry.snuba.metrics.utils import ( + BlockedMetric, + MetricMeta, + MetricOperationType, + MetricType, + MetricUnit, +) from sentry.snuba.metrics_layer.query import fetch_metric_mris @@ -111,7 +117,7 @@ def _build_metric_meta( ] return MetricMeta( - type=parsed_mri.entity, + type=cast(MetricType, parsed_mri.entity), name=parsed_mri.name, unit=cast(MetricUnit, parsed_mri.unit), mri=parsed_mri.mri_string, From e0eb8ac6f7bdb2cb08f867a55bab1636f7a6bb96 Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Thu, 16 May 2024 13:12:07 +0200 Subject: [PATCH 6/6] use org-level option to hide percentiles from metrics API --- src/sentry/options/defaults.py | 13 +++++++------ .../sentry_metrics/querying/metadata/metrics.py | 14 +++++++++++--- .../test_organization_metrics_details.py | 16 ++++++++++------ 3 files changed, 28 insertions(+), 15 deletions(-) diff --git a/src/sentry/options/defaults.py b/src/sentry/options/defaults.py index e7e7ab7866209f..a662410935a09d 100644 --- a/src/sentry/options/defaults.py +++ b/src/sentry/options/defaults.py @@ -2506,11 +2506,12 @@ register( "api.organization-activity.brownout-duration", default="PT1M", flags=FLAG_AUTOMATOR_MODIFIABLE ) -# Hide percentile operations from appearing in the metrics/meta endpoint in the Metrics API, thereby also hiding those -# operations from view in the Metrics UI. + +# Enable percentile operations in the metrics/meta endpoint in the Metrics API for the orgs in the list. This is used to +# also hide those expensive operations from view in the Metrics UI for everyone except the whitelist. register( - "sentry-metrics.metrics-api.hide-percentile-operations", - type=Bool, - default=False, - flags=FLAG_AUTOMATOR_MODIFIABLE, + "sentry-metrics.metrics-api.enable-percentile-operations-for-orgs", + type=Sequence, + default=[], + flags=FLAG_ALLOW_EMPTY | FLAG_AUTOMATOR_MODIFIABLE, ) diff --git a/src/sentry/sentry_metrics/querying/metadata/metrics.py b/src/sentry/sentry_metrics/querying/metadata/metrics.py index 3309654f826eb8..7c3703def7c437 100644 --- a/src/sentry/sentry_metrics/querying/metadata/metrics.py +++ b/src/sentry/sentry_metrics/querying/metadata/metrics.py @@ -56,7 +56,9 @@ def get_metrics_meta( # not stored. del metrics_blocking_state[metric_mri] - metrics_metas.append(_build_metric_meta(parsed_mri, project_ids, blocking_status)) + metrics_metas.append( + _build_metric_meta(organization, parsed_mri, project_ids, blocking_status) + ) for metric_mri, metric_blocking in metrics_blocking_state.items(): parsed_mri = parse_mri(metric_mri) @@ -65,6 +67,7 @@ def get_metrics_meta( metrics_metas.append( _build_metric_meta( + organization, parsed_mri, [], [ @@ -105,11 +108,16 @@ def _convert_to_mris_to_project_ids_mapping(project_id_to_mris: dict[int, list[s def _build_metric_meta( - parsed_mri: ParsedMRI, project_ids: Sequence[int], blocking_status: Sequence[BlockedMetric] + organization: Organization, + parsed_mri: ParsedMRI, + project_ids: Sequence[int], + blocking_status: Sequence[BlockedMetric], ) -> MetricMeta: available_operations = get_available_operations(parsed_mri) - if options.get("sentry-metrics.metrics-api.hide-percentile-operations"): + if organization.id not in options.get( + "sentry-metrics.metrics-api.enable-percentile-operations-for-orgs" + ): available_operations = [ operation for operation in available_operations diff --git a/tests/sentry/api/endpoints/test_organization_metrics_details.py b/tests/sentry/api/endpoints/test_organization_metrics_details.py index f4fe99823d6d8b..3e6c7b8661d8cc 100644 --- a/tests/sentry/api/endpoints/test_organization_metrics_details.py +++ b/tests/sentry/api/endpoints/test_organization_metrics_details.py @@ -229,16 +229,15 @@ def test_metrics_details_for_custom_metrics(self): "max_timestamp", "min", "min_timestamp", - "p50", - "p75", - "p90", - "p95", - "p99", "sum", ] with override_options( - {"sentry-metrics.metrics-api.hide-percentile-operations": True}, + { + "sentry-metrics.metrics-api.enable-percentile-operations-for-orgs": [ + self.organization.id + ] + }, ): response = self.get_success_response( self.organization.slug, project=[project_1.id, project_2.id], useCase="custom" @@ -252,5 +251,10 @@ def test_metrics_details_for_custom_metrics(self): "max_timestamp", "min", "min_timestamp", + "p50", + "p75", + "p90", + "p95", + "p99", "sum", ]