Skip to content

Commit

Permalink
feat(api): per-endpoint metric tags (#13374)
Browse files Browse the repository at this point in the history
This allows endpoints to add tags to the resulting `view.response`
metric that gets recorded in middleware.

The immediate use for this is to tag all Integration Platform endpoints
with a corresponding tag so that we can split them out downstream.
  • Loading branch information
mnoble authored Jun 5, 2019
1 parent 8ca17bd commit d550af8
Show file tree
Hide file tree
Showing 8 changed files with 149 additions and 10 deletions.
4 changes: 4 additions & 0 deletions src/sentry/api/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,10 @@ def dispatch(self, request, *args, **kwargs):
self.request = request
self.headers = self.default_response_headers # deprecate?

# Tags that will ultimately flow into the metrics backend at the end of
# the request (happens via middleware/stats.py).
request._metric_tags = {}

if settings.SENTRY_API_RESPONSE_DELAY:
time.sleep(settings.SENTRY_API_RESPONSE_DELAY / 1000.0)

Expand Down
24 changes: 20 additions & 4 deletions src/sentry/api/bases/sentryapps.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
from __future__ import absolute_import

from django.http import Http404
from functools import wraps

from sentry.utils.sdk import configure_scope
from sentry.api.authentication import ClientIdSecretAuthentication
from sentry.api.base import Endpoint
from sentry.api.permissions import SentryPermission
from sentry.auth.superuser import is_active_superuser
from sentry.middleware.stats import add_request_metric_tags
from sentry.models import SentryApp, SentryAppInstallation, Organization


Expand All @@ -33,6 +35,14 @@ def ensure_scoped_permission(request, allowed_scopes):
return any(request.access.has_scope(s) for s in set(allowed_scopes))


def add_integration_platform_metric_tag(func):
@wraps(func)
def wrapped(self, *args, **kwargs):
add_request_metric_tags(self.request, integration_platform=True)
return func(self, *args, **kwargs)
return wrapped


class SentryAppsPermission(SentryPermission):
scope_map = {
'GET': (), # Public endpoint.
Expand All @@ -58,7 +68,13 @@ def has_object_permission(self, request, view, organization):
)


class SentryAppsBaseEndpoint(Endpoint):
class IntegrationPlatformEndpoint(Endpoint):
def dispatch(self, request, *args, **kwargs):
add_request_metric_tags(request, integration_platform=True)
return super(IntegrationPlatformEndpoint, self).dispatch(request, *args, **kwargs)


class SentryAppsBaseEndpoint(IntegrationPlatformEndpoint):
permission_classes = (SentryAppsPermission, )

def convert_args(self, request, *args, **kwargs):
Expand Down Expand Up @@ -131,7 +147,7 @@ def _scopes_for_sentry_app(self, sentry_app):
return self.unpublished_scope_map


class SentryAppBaseEndpoint(Endpoint):
class SentryAppBaseEndpoint(IntegrationPlatformEndpoint):
permission_classes = (SentryAppPermission, )

def convert_args(self, request, sentry_app_slug, *args, **kwargs):
Expand Down Expand Up @@ -175,7 +191,7 @@ def has_object_permission(self, request, view, organization):
)


class SentryAppInstallationsBaseEndpoint(Endpoint):
class SentryAppInstallationsBaseEndpoint(IntegrationPlatformEndpoint):
permission_classes = (SentryAppInstallationsPermission, )

def convert_args(self, request, organization_slug, *args, **kwargs):
Expand Down Expand Up @@ -226,7 +242,7 @@ def has_object_permission(self, request, view, installation):
)


class SentryAppInstallationBaseEndpoint(Endpoint):
class SentryAppInstallationBaseEndpoint(IntegrationPlatformEndpoint):
permission_classes = (SentryAppInstallationPermission, )

def convert_args(self, request, uuid, *args, **kwargs):
Expand Down
3 changes: 2 additions & 1 deletion src/sentry/api/endpoints/organization_sentry_apps.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
from __future__ import absolute_import

from sentry.api.bases import OrganizationEndpoint
from sentry.api.bases import OrganizationEndpoint, add_integration_platform_metric_tag
from sentry.api.paginator import OffsetPaginator
from sentry.api.serializers import serialize
from sentry.models import SentryApp


class OrganizationSentryAppsEndpoint(OrganizationEndpoint):
@add_integration_platform_metric_tag
def get(self, request, organization):
queryset = SentryApp.objects.filter(
owner=organization,
Expand Down
5 changes: 4 additions & 1 deletion src/sentry/api/endpoints/sentry_app_components.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@

from rest_framework.response import Response

from sentry.api.bases import OrganizationEndpoint, SentryAppBaseEndpoint
from sentry.api.bases import (
OrganizationEndpoint, SentryAppBaseEndpoint, add_integration_platform_metric_tag,
)
from sentry.api.paginator import OffsetPaginator
from sentry.api.serializers import serialize
from sentry.coreapi import APIError
Expand All @@ -21,6 +23,7 @@ def get(self, request, sentry_app):


class OrganizationSentryAppComponentsEndpoint(OrganizationEndpoint):
@add_integration_platform_metric_tag
def get(self, request, organization):
try:
project = Project.objects.get(
Expand Down
21 changes: 17 additions & 4 deletions src/sentry/middleware/stats.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@
from sentry.utils import metrics


def add_request_metric_tags(request, **kwargs):
if not hasattr(request, '_metric_tags'):
request._metric_tags = {}

request._metric_tags.update(**kwargs)


class ResponseCodeMiddleware(object):
def process_response(self, request, response):
metrics.incr('response', instance=six.text_type(response.status_code), skip_internal=False)
Expand All @@ -27,6 +34,9 @@ class RequestTimingMiddleware(object):
)

def process_view(self, request, view_func, view_args, view_kwargs):
if not hasattr(request, '_metric_tags'):
request._metric_tags = {}

if request.method not in self.allowed_methods:
return

Expand Down Expand Up @@ -56,13 +66,16 @@ def _record_time(self, request, status_code):
if not hasattr(request, '_view_path'):
return

tags = request._metric_tags if hasattr(request, '_metric_tags') else {}
tags.update({
'method': request.method,
'status_code': status_code,
})

metrics.incr(
'view.response',
instance=request._view_path,
tags={
'method': request.method,
'status_code': status_code,
},
tags=tags,
skip_internal=False,
)

Expand Down
6 changes: 6 additions & 0 deletions src/sentry/testutils/helpers/faux.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,12 @@ def _kwargs_to_s(self, **kwargs):
return ', '.join(u'{}={!r}'.format(k, v) for k, v in six.iteritems(kwargs))


class Mock(object):
def __init__(self, *args, **kwargs):
for k, v in six.iteritems(kwargs):
setattr(self, k, v)


def faux(mock, call=None):
if call is not None:
return Faux(mock.mock_calls[call])
Expand Down
21 changes: 21 additions & 0 deletions tests/sentry/api/bases/test_sentryapps.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
from __future__ import absolute_import

from django.http import Http404
from mock import patch

from sentry.testutils import TestCase
from sentry.api.bases.sentryapps import (
SentryAppPermission,
SentryAppBaseEndpoint,
SentryAppInstallationPermission,
SentryAppInstallationBaseEndpoint,
add_integration_platform_metric_tag,
)
from sentry.testutils.helpers.faux import Mock


class SentryAppPermissionTest(TestCase):
Expand Down Expand Up @@ -132,3 +135,21 @@ def test_retrieves_installation(self):
def test_raises_when_sentry_app_not_found(self):
with self.assertRaises(Http404):
self.endpoint.convert_args(self.request, '1234')


class AddIntegrationPlatformMetricTagTest(TestCase):
@patch('sentry.api.bases.sentryapps.add_request_metric_tags')
def test_record_platform_integration_metric(self, add_request_metric_tags):
@add_integration_platform_metric_tag
def get(self, request, *args, **kwargs):
pass

request = Mock()
endpoint = Mock(request=request)

get(endpoint, request)

add_request_metric_tags.assert_called_with(
request,
integration_platform=True,
)
75 changes: 75 additions & 0 deletions tests/sentry/middleware/test_stats.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
from __future__ import absolute_import

from django.test import RequestFactory
from exam import fixture
from mock import patch

from sentry.middleware.stats import RequestTimingMiddleware, add_request_metric_tags
from sentry.testutils import TestCase
from sentry.testutils.helpers.faux import Mock


class RequestTimingMiddlewareTest(TestCase):
middleware = fixture(RequestTimingMiddleware)
factory = fixture(RequestFactory)

@patch('sentry.utils.metrics.incr')
def test_records_default_api_metrics(self, incr):
request = self.factory.get('/')
request._view_path = '/'
response = Mock(status_code=200)

self.middleware.process_response(request, response)

incr.assert_called_with(
'view.response',
instance=request._view_path,
tags={
'method': 'GET',
'status_code': 200,
},
skip_internal=False,
)

@patch('sentry.utils.metrics.incr')
def test_records_endpoint_specific_metrics(self, incr):
request = self.factory.get('/')
request._view_path = '/'
request._metric_tags = {'a': 'b'}

response = Mock(status_code=200)

self.middleware.process_response(request, response)

incr.assert_called_with(
'view.response',
instance=request._view_path,
tags={
'method': 'GET',
'status_code': 200,
'a': 'b',
},
skip_internal=False,
)

@patch('sentry.utils.metrics.incr')
def test_add_request_metric_tags(self, incr):
request = self.factory.get('/')
request._view_path = '/'

add_request_metric_tags(request, foo='bar')

response = Mock(status_code=200)

self.middleware.process_response(request, response)

incr.assert_called_with(
'view.response',
instance=request._view_path,
tags={
'method': 'GET',
'status_code': 200,
'foo': 'bar',
},
skip_internal=False,
)

0 comments on commit d550af8

Please sign in to comment.