From 9fbda5a7507d2c74676c145ccaa932cc4e7f312b Mon Sep 17 00:00:00 2001 From: anthony sottile Date: Wed, 29 Jan 2025 16:45:23 -0500 Subject: [PATCH] ref: ensure unit is MetricUnit --- .../querying/metadata/metrics.py | 4 +- src/sentry/snuba/metrics/naming_layer/mri.py | 48 +++++++++++-------- src/sentry/snuba/metrics/utils.py | 30 ++++++++++++ .../sentry/snuba/metrics/test_naming_layer.py | 16 +------ 4 files changed, 62 insertions(+), 36 deletions(-) diff --git a/src/sentry/sentry_metrics/querying/metadata/metrics.py b/src/sentry/sentry_metrics/querying/metadata/metrics.py index 09ba8d840968ca..a94cf6219e83ce 100644 --- a/src/sentry/sentry_metrics/querying/metadata/metrics.py +++ b/src/sentry/sentry_metrics/querying/metadata/metrics.py @@ -18,7 +18,7 @@ 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, MetricType, MetricUnit +from sentry.snuba.metrics.utils import BlockedMetric, MetricMeta, MetricType from sentry.snuba.metrics_layer.query import fetch_metric_mris @@ -144,7 +144,7 @@ def _build_metric_meta( return MetricMeta( type=cast(MetricType, parsed_mri.entity), name=parsed_mri.name, - unit=cast(MetricUnit, parsed_mri.unit), + unit=parsed_mri.unit, mri=parsed_mri.mri_string, operations=available_operations, projectIds=project_ids, diff --git a/src/sentry/snuba/metrics/naming_layer/mri.py b/src/sentry/snuba/metrics/naming_layer/mri.py index 284a1e34d3daa3..952d873677dc9b 100644 --- a/src/sentry/snuba/metrics/naming_layer/mri.py +++ b/src/sentry/snuba/metrics/naming_layer/mri.py @@ -17,21 +17,6 @@ metric that is queryable by the API. """ -__all__ = ( - "SessionMRI", - "TransactionMRI", - "SpanMRI", - "MRI_SCHEMA_REGEX", - "MRI_EXPRESSION_REGEX", - "ErrorsMRI", - "parse_mri", - "get_available_operations", - "is_mri_field", - "parse_mri_field", - "format_mri_field", - "format_mri_field_value", -) - import re from dataclasses import dataclass from enum import Enum @@ -49,6 +34,22 @@ MetricEntity, MetricOperationType, MetricUnit, + is_metric_unit, +) + +__all__ = ( + "SessionMRI", + "TransactionMRI", + "SpanMRI", + "MRI_SCHEMA_REGEX", + "MRI_EXPRESSION_REGEX", + "ErrorsMRI", + "parse_mri", + "get_available_operations", + "is_mri_field", + "parse_mri_field", + "format_mri_field", + "format_mri_field_value", ) MRI_SCHEMA_REGEX_STRING = r"(?P[^:]+):(?P[^/]+)/(?P[^@]+)@(?P.+)" @@ -201,7 +202,7 @@ class ParsedMRI: entity: str namespace: str name: str - unit: str + unit: MetricUnit @property def mri_string(self) -> str: @@ -228,7 +229,10 @@ def parse_mri_field(field: str | None) -> ParsedMRIField | None: try: op = cast(MetricOperationType, matches[1]) - mri = ParsedMRI(**matches.groupdict()) + params = matches.groupdict() + unit = params.pop("unit") + assert is_metric_unit(unit), unit + mri = ParsedMRI(unit=unit, **params) except (IndexError, TypeError): return None @@ -274,8 +278,9 @@ def format_mri_field_value(field: str, value: str) -> str: if parsed_mri_field is None: return value - unit = cast(MetricUnit, parsed_mri_field.mri.unit) - return format_value_using_unit_and_op(float(value), unit, parsed_mri_field.op) + return format_value_using_unit_and_op( + float(value), parsed_mri_field.mri.unit, parsed_mri_field.op + ) except InvalidParams: return value @@ -292,7 +297,10 @@ def parse_mri(mri_string: str | None) -> ParsedMRI | None: if match is None: return None - return ParsedMRI(**match.groupdict()) + params = match.groupdict() + unit = params.pop("unit") + assert is_metric_unit(unit), unit + return ParsedMRI(unit=unit, **params) def is_mri(mri_string: str | None) -> bool: diff --git a/src/sentry/snuba/metrics/utils.py b/src/sentry/snuba/metrics/utils.py index 4df8a57e40c4f0..c1825b1af2e8c9 100644 --- a/src/sentry/snuba/metrics/utils.py +++ b/src/sentry/snuba/metrics/utils.py @@ -119,6 +119,36 @@ "exabyte", "none", ] + + +def is_metric_unit(s: str) -> TypeIs[MetricUnit]: + return s in { + "nanosecond", + "microsecond", + "millisecond", + "second", + "minute", + "hour", + "day", + "week", + "bit", + "byte", + "kibibyte", + "mebibyte", + "gibibyte", + "tebibyte", + "pebibyte", + "exbibyte", + "kilobyte", + "megabyte", + "gigabyte", + "terabyte", + "petabyte", + "exabyte", + "none", + } + + #: The type of metric, which determines the snuba entity to query MetricType = Literal[ "counter", diff --git a/tests/sentry/snuba/metrics/test_naming_layer.py b/tests/sentry/snuba/metrics/test_naming_layer.py index 043b810b5e16c7..b59f7deee03dfa 100644 --- a/tests/sentry/snuba/metrics/test_naming_layer.py +++ b/tests/sentry/snuba/metrics/test_naming_layer.py @@ -60,20 +60,8 @@ def test_invalid_public_name_regex(name): ParsedMRI("s", "sessions", "error", "none"), ), ( - "dist:my_namespace/organizations/v1/my endpoint@{none}", - ParsedMRI("dist", "my_namespace", "organizations/v1/my endpoint", "{none}"), - ), - ( - "d:transactions/measurements.disk_io@byte/second", - ParsedMRI("d", "transactions", "measurements.disk_io", "byte/second"), - ), - ( - "c:custom/http.client.open_connections@{connection}", - ParsedMRI("c", "custom", "http.client.open_connections", "{connection}"), - ), - ( - "c:custom/http.client.active_requests@{request}", - ParsedMRI("c", "custom", "http.client.active_requests", "{request}"), + "dist:my_namespace/organizations/v1/my endpoint@none", + ParsedMRI("dist", "my_namespace", "organizations/v1/my endpoint", "none"), ), ], )