Skip to content

Commit

Permalink
feat(api): per-endpoint metric tags
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 committed May 23, 2019
1 parent 5eb4bfd commit fc1f937
Show file tree
Hide file tree
Showing 8 changed files with 110 additions and 10 deletions.
10 changes: 10 additions & 0 deletions src/sentry/api/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,12 @@ def load_json_body(self, request):
except JSONDecodeError:
return

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

self.request._metric_tags.update(**kwargs)

def initialize_request(self, request, *args, **kwargs):
rv = super(Endpoint, self).initialize_request(request, *args, **kwargs)
# If our request is being made via our internal API client, we need to
Expand All @@ -151,6 +157,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
21 changes: 17 additions & 4 deletions src/sentry/api/bases/sentryapps.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,13 @@ 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):
def decorator(self, *args, **kwargs):
self.add_metric_tags(integration_platform=True)
return func(self, *args, **kwargs)
return decorator


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


class SentryAppsBaseEndpoint(Endpoint):
class IntegrationPlatformEndpoint(Endpoint):
def dispatch(self, request, *args, **kwargs):
self.add_metric_tags(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 +144,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 +188,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 +239,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
13 changes: 9 additions & 4 deletions src/sentry/middleware/stats.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ class RequestTimingMiddleware(object):
)

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

if request.method not in self.allowed_methods:
return

Expand Down Expand Up @@ -56,13 +58,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
9 changes: 9 additions & 0 deletions tests/sentry/api/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,15 @@ def test_basic_cors(self):

assert response['Access-Control-Allow-Origin'] == 'http://example.com'

def test_add_metric_tags(self):
request = HttpRequest()
endpoint = DummyEndpoint()
endpoint.request = request

endpoint.add_metric_tags(foo='bar')

assert endpoint.request._metric_tags['foo'] == 'bar'


class EndpointJSONBodyTest(APITestCase):
def setUp(self):
Expand Down
53 changes: 53 additions & 0 deletions tests/sentry/middleware/test_stats.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
from __future__ import absolute_import

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

from sentry.middleware.stats import RequestTimingMiddleware
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,
)

0 comments on commit fc1f937

Please sign in to comment.