Skip to content

Commit

Permalink
feat(metrics): MRI support for metrics API (#50404)
Browse files Browse the repository at this point in the history
  • Loading branch information
obostjancic authored Jun 12, 2023
1 parent 268804e commit 7528f45
Show file tree
Hide file tree
Showing 8 changed files with 196 additions and 332 deletions.
40 changes: 18 additions & 22 deletions src/sentry/api/endpoints/organization_metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,17 @@
from sentry.utils.cursors import Cursor, CursorResult


def get_use_case_id(use_case: str) -> UseCaseKey:
def get_use_case_id(request: Request) -> UseCaseKey:
"""
Get use_case from str and validate it against UseCaseKey enum type
if use_case parameter has wrong value just raise an ParseError.
Get useCase from query params and validate it against UseCaseKey enum type
Raise a ParseError if the use_case parameter is invalid.
"""
try:
if use_case == "releaseHealth":
use_case = "release-health"

return UseCaseKey(use_case)
try:
return UseCaseKey(request.GET.get("useCase", "release-health"))
except ValueError:
raise ParseError(
detail=f"Invalid useCase parameter. Please use one of: {', '.join(use_case.value for use_case in UseCaseKey)}"
detail=f"Invalid useCase parameter. Please use one of: {[uc.value for uc in UseCaseKey]}"
)


Expand All @@ -47,14 +45,9 @@ def get(self, request: Request, organization) -> Response:
return Response(status=404)

projects = self.get_projects(request, organization)
metrics = get_metrics(
projects, use_case_id=get_use_case_id(request.GET.get("useCase", "release-health"))
)
# TODO: replace this with a serializer so that if the structure of MetricMeta changes the response of this
# endpoint does not
for metric in metrics:
del metric["metric_id"]
del metric["mri_string"]

metrics = get_metrics(projects, use_case_id=get_use_case_id(request))

return Response(metrics, status=200)


Expand All @@ -71,7 +64,7 @@ def get(self, request: Request, organization, metric_name) -> Response:
metric = get_single_metric_info(
projects,
metric_name,
use_case_id=get_use_case_id(request.GET.get("useCase", "release-health")),
use_case_id=get_use_case_id(request),
)
except InvalidParams as e:
raise ResourceDoesNotExist(e)
Expand Down Expand Up @@ -104,7 +97,7 @@ def get(self, request: Request, organization) -> Response:
tags = get_tags(
projects,
metric_names,
use_case_id=get_use_case_id(request.GET.get("useCase", "release-health")),
use_case_id=get_use_case_id(request),
)
except (InvalidParams, DerivedMetricParseException) as exc:
raise (ParseError(detail=str(exc)))
Expand All @@ -129,7 +122,7 @@ def get(self, request: Request, organization, tag_name) -> Response:
projects,
tag_name,
metric_names,
use_case_id=get_use_case_id(request.GET.get("useCase", "release-health")),
use_case_id=get_use_case_id(request),
)
except (InvalidParams, DerivedMetricParseException) as exc:
msg = str(exc)
Expand Down Expand Up @@ -158,12 +151,15 @@ def get(self, request: Request, organization) -> Response:
def data_fn(offset: int, limit: int):
try:
query = QueryDefinition(
projects, request.GET, paginator_kwargs={"limit": limit, "offset": offset}
projects,
request.GET,
allow_mri=True,
paginator_kwargs={"limit": limit, "offset": offset},
)
data = get_series(
projects,
query.to_metrics_query(),
use_case_id=get_use_case_id(request.GET.get("useCase", "release-health")),
metrics_query=query.to_metrics_query(),
use_case_id=get_use_case_id(request),
tenant_ids={"organization_id": organization.id},
)
data["query"] = query.query
Expand Down
84 changes: 32 additions & 52 deletions src/sentry/snuba/metrics/datasource.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
from sentry.snuba.dataset import Dataset, EntityKey
from sentry.snuba.metrics.fields import run_metrics_query
from sentry.snuba.metrics.fields.base import get_derived_metrics, org_id_from_projects
from sentry.snuba.metrics.naming_layer.mapping import get_mri, get_public_name_from_mri
from sentry.snuba.metrics.naming_layer.mapping import get_all_mris, get_mri
from sentry.snuba.metrics.naming_layer.mri import is_custom_measurement, parse_mri
from sentry.snuba.metrics.query import Groupable, MetricField, MetricsQuery
from sentry.snuba.metrics.query_builder import (
Expand Down Expand Up @@ -136,63 +136,43 @@ def get_available_derived_metrics(


def get_metrics(projects: Sequence[Project], use_case_id: UseCaseKey) -> Sequence[MetricMeta]:
assert projects

metrics_meta = []
metric_ids_in_entities = {}
ENTITY_TO_DATASET = {
"sessions": {
"c": "metrics_counters",
"s": "metrics_sets",
"d": "metrics_distributions",
"g": "metrics_gauges",
},
"transactions": {
"c": "generic_metrics_counters",
"s": "generic_metrics_sets",
"d": "generic_metrics_distributions",
"g": "generic_metrics_gauges",
},
}

for metric_type in ("counter", "set", "distribution"):
metric_ids_in_entities.setdefault(metric_type, set())
org_id = projects[0].organization_id
for row in _get_metrics_for_entity(
entity_key=METRIC_TYPE_TO_ENTITY[metric_type],
project_ids=[project.id for project in projects],
org_id=org_id,
):
try:
mri_string = reverse_resolve(use_case_id, org_id, row["metric_id"])
metrics_meta.append(
MetricMeta(
name=get_public_name_from_mri(mri_string),
type=metric_type,
operations=AVAILABLE_OPERATIONS[METRIC_TYPE_TO_ENTITY[metric_type].value],
unit=None, # snuba does not know the unit
metric_id=row["metric_id"],
mri_string=mri_string,
)
)
except InvalidParams:
# An instance of `InvalidParams` exception is raised here when there is no reverse
# mapping from MRI to public name because of the naming change
logger.error("datasource.get_metrics.get_public_name_from_mri.error", exc_info=True)
continue
metric_ids_in_entities[metric_type].add(row["metric_id"])

# In the previous loop, we find all available metric ids per entity with respect to the
# projects filter, and so to figure out which derived metrics are supported for these
# projects, we need to iterate over the list of derived metrics and generate the ids of
# their constituent metrics. A derived metric should be added to the response list if its
# metric ids are a subset of the metric ids in one of the entities i.e. Its an instance of
# SingularEntityDerivedMetric.
found_derived_metrics = get_available_derived_metrics(
projects, metric_ids_in_entities, use_case_id
)
public_derived_metrics = get_derived_metrics(exclude_private=True)
result = []
for mri in get_all_mris():
parsed = parse_mri(mri)

for derived_metric_mri in found_derived_metrics:
derived_metric_obj = public_derived_metrics[derived_metric_mri]
metrics_meta.append(
if parsed.entity not in ENTITY_TO_DATASET["sessions"].keys():
ops = []
elif parsed.namespace == "sessions":
ops = AVAILABLE_OPERATIONS[ENTITY_TO_DATASET[parsed.namespace][parsed.entity]]
else:
ops = AVAILABLE_GENERIC_OPERATIONS[ENTITY_TO_DATASET[parsed.namespace][parsed.entity]]

result.append(
MetricMeta(
name=get_public_name_from_mri(derived_metric_obj.metric_mri),
type=derived_metric_obj.result_type,
operations=derived_metric_obj.generate_available_operations(),
unit=derived_metric_obj.unit,
# Derived metrics won't have an id
metric_id=None,
mri_string=derived_metric_mri,
mri=mri,
unit=parsed.unit,
name=parsed.name,
operations=ops,
)
)
return sorted(metrics_meta, key=itemgetter("name"))

return result


def get_custom_measurements(
Expand Down
9 changes: 8 additions & 1 deletion src/sentry/snuba/metrics/naming_layer/mapping.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
__all__ = (
"create_name_mapping_layers",
"get_mri",
"get_all_mris",
"get_public_name_from_mri",
"parse_expression",
"get_operation_with_public_name",
)


from enum import Enum
from typing import Dict, Optional, Tuple, Union, cast
from typing import Dict, List, Optional, Tuple, Union, cast

from sentry.api.utils import InvalidParams
from sentry.snuba.metrics.naming_layer.mri import (
Expand Down Expand Up @@ -65,6 +66,12 @@ def get_mri(external_name: Union[Enum, str]) -> str:
)


def get_all_mris() -> List[str]:
if not len(NAME_TO_MRI):
create_name_mapping_layers()
return [entry.value for entry in NAME_TO_MRI.values()]


def get_public_name_from_mri(internal_name: Union[TransactionMRI, SessionMRI, str]) -> str:
"""Returns the public name from a MRI if it has a mapping to a public metric name, otherwise raise an exception"""
if not len(MRI_TO_NAME):
Expand Down
49 changes: 40 additions & 9 deletions src/sentry/snuba/metrics/query_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
get_operation_with_public_name,
parse_expression,
)
from sentry.snuba.metrics.naming_layer.mri import MRI_EXPRESSION_REGEX, MRI_SCHEMA_REGEX
from sentry.snuba.metrics.naming_layer.public import PUBLIC_EXPRESSION_REGEX
from sentry.snuba.metrics.query import (
MetricActionByField,
Expand Down Expand Up @@ -84,14 +85,36 @@
from sentry.utils.snuba import parse_snuba_datetime


def parse_field(field: str) -> MetricField:
def parse_field(field: str, allow_mri: bool = False) -> MetricField:

if allow_mri:
mri_matches = MRI_SCHEMA_REGEX.match(field) or MRI_EXPRESSION_REGEX.match(field)
if mri_matches:
return parse_mri_field(field)

return parse_public_field(field)


def parse_mri_field(field: str) -> MetricField:
matches = MRI_EXPRESSION_REGEX.match(field)

try:
operation = matches[1]
mri = matches[2]
except (IndexError, TypeError):
operation = None
mri = field
return MetricField(operation, mri, alias=mri)


def parse_public_field(field: str) -> MetricField:

matches = PUBLIC_EXPRESSION_REGEX.match(field)

try:
if matches is None:
raise TypeError
operation = matches[1]
metric_name = matches[2]
except (IndexError, TypeError):
except (TypeError, IndexError):
operation = None
metric_name = field
return MetricField(operation, get_mri(metric_name))
Expand Down Expand Up @@ -391,7 +414,13 @@ class QueryDefinition:
"""

def __init__(self, projects, query_params, paginator_kwargs: Optional[Dict] = None):
def __init__(
self,
projects,
query_params,
allow_mri: bool = False,
paginator_kwargs: Optional[Dict] = None,
):
self._projects = projects
paginator_kwargs = paginator_kwargs or {}

Expand All @@ -400,8 +429,10 @@ def __init__(self, projects, query_params, paginator_kwargs: Optional[Dict] = No
self.groupby = [
MetricGroupByField(groupby_col) for groupby_col in query_params.getlist("groupBy", [])
]
self.fields = [parse_field(key) for key in query_params.getlist("field", [])]
self.orderby = self._parse_orderby(query_params)
self.fields = [
parse_field(key, allow_mri=allow_mri) for key in query_params.getlist("field", [])
]
self.orderby = self._parse_orderby(query_params, allow_mri)
self.limit: Optional[Limit] = self._parse_limit(paginator_kwargs)
self.offset: Optional[Offset] = self._parse_offset(paginator_kwargs)
self.having: Optional[ConditionGroup] = query_params.getlist("having")
Expand Down Expand Up @@ -432,7 +463,7 @@ def to_metrics_query(self) -> MetricsQuery:
)

@staticmethod
def _parse_orderby(query_params):
def _parse_orderby(query_params, allow_mri: bool = False):
orderbys = query_params.getlist("orderBy", [])
if not orderbys:
return None
Expand All @@ -444,7 +475,7 @@ def _parse_orderby(query_params):
orderby = orderby[1:]
direction = Direction.DESC

field = parse_field(orderby)
field = parse_field(orderby, allow_mri=allow_mri)
orderby_list.append(MetricsOrderBy(field=field, direction=direction))

return orderby_list
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ def test_incorrect_use_case_id_value(self):
assert response.status_code == 400
assert (
response.json()["detail"]
== "Invalid useCase parameter. Please use one of: release-health, performance"
== "Invalid useCase parameter. Please use one of: ['release-health', 'performance']"
)

def test_invalid_field(self):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class OrganizationMetricDetailsIntegrationTest(OrganizationMetricMetaIntegration
mocked_mri_resolver(["metric1", "metric2", "metric3"], get_mri),
)
@patch(
"sentry.snuba.metrics.datasource.get_public_name_from_mri",
"sentry.snuba.metrics.get_public_name_from_mri",
mocked_mri_resolver(["metric1", "metric2", "metric3"], get_public_name_from_mri),
)
def test_metric_details(self):
Expand Down Expand Up @@ -114,7 +114,7 @@ def test_metric_details_metric_does_not_exist_in_indexer(self):

@patch("sentry.snuba.metrics.datasource.get_mri", mocked_mri_resolver(["foo.bar"], get_mri))
@patch(
"sentry.snuba.metrics.datasource.get_public_name_from_mri",
"sentry.snuba.metrics.get_public_name_from_mri",
mocked_mri_resolver(["foo.bar"], get_public_name_from_mri),
)
def test_metric_details_metric_does_not_have_data(self):
Expand Down Expand Up @@ -190,7 +190,7 @@ def test_incorrectly_setup_derived_metric(self, mocked_derived_metrics, mocked_g
mocked_mri_resolver(["metric_foo_doe", "derived_metric.multiple_metrics"], get_mri),
)
@patch(
"sentry.snuba.metrics.datasource.get_public_name_from_mri",
"sentry.snuba.metrics.get_public_name_from_mri",
mocked_mri_resolver(
["metric_foo_doe", "derived_metric.multiple_metrics"], get_public_name_from_mri
),
Expand Down Expand Up @@ -227,7 +227,7 @@ def test_same_entity_multiple_metric_ids_missing_data(self, mocked_derived_metri
mocked_mri_resolver(["metric_foo_doe", "derived_metric.multiple_metrics"], get_mri),
)
@patch(
"sentry.snuba.metrics.datasource.get_public_name_from_mri",
"sentry.snuba.metrics.get_public_name_from_mri",
mocked_mri_resolver(
["metric_foo_doe", "derived_metric.multiple_metrics"], get_public_name_from_mri
),
Expand Down
Loading

0 comments on commit 7528f45

Please sign in to comment.