diff --git a/src/sentry/api/base.py b/src/sentry/api/base.py index d1eaa77ea3c656..2d873523f48506 100644 --- a/src/sentry/api/base.py +++ b/src/sentry/api/base.py @@ -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) diff --git a/src/sentry/api/bases/sentryapps.py b/src/sentry/api/bases/sentryapps.py index b48ce4ee867045..85f395bfa547e3 100644 --- a/src/sentry/api/bases/sentryapps.py +++ b/src/sentry/api/bases/sentryapps.py @@ -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 @@ -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. @@ -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): @@ -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): @@ -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): @@ -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): diff --git a/src/sentry/api/endpoints/organization_sentry_apps.py b/src/sentry/api/endpoints/organization_sentry_apps.py index e64f70c20cd25c..f5945b71f45a2e 100644 --- a/src/sentry/api/endpoints/organization_sentry_apps.py +++ b/src/sentry/api/endpoints/organization_sentry_apps.py @@ -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, diff --git a/src/sentry/api/endpoints/sentry_app_components.py b/src/sentry/api/endpoints/sentry_app_components.py index 0ac9c960201644..97cc4e7fdb78bd 100644 --- a/src/sentry/api/endpoints/sentry_app_components.py +++ b/src/sentry/api/endpoints/sentry_app_components.py @@ -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 @@ -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( diff --git a/src/sentry/middleware/stats.py b/src/sentry/middleware/stats.py index cb65950c7dd124..1eccf79ee4d961 100644 --- a/src/sentry/middleware/stats.py +++ b/src/sentry/middleware/stats.py @@ -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) @@ -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 @@ -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, ) diff --git a/src/sentry/testutils/helpers/faux.py b/src/sentry/testutils/helpers/faux.py index a3632c7c3ff715..18e2d643954751 100644 --- a/src/sentry/testutils/helpers/faux.py +++ b/src/sentry/testutils/helpers/faux.py @@ -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]) diff --git a/tests/sentry/api/bases/test_sentryapps.py b/tests/sentry/api/bases/test_sentryapps.py index ccf18ff2b5e8a3..29466d5c43e14c 100644 --- a/tests/sentry/api/bases/test_sentryapps.py +++ b/tests/sentry/api/bases/test_sentryapps.py @@ -1,6 +1,7 @@ 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 ( @@ -8,7 +9,9 @@ SentryAppBaseEndpoint, SentryAppInstallationPermission, SentryAppInstallationBaseEndpoint, + add_integration_platform_metric_tag, ) +from sentry.testutils.helpers.faux import Mock class SentryAppPermissionTest(TestCase): @@ -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, + ) diff --git a/tests/sentry/middleware/test_stats.py b/tests/sentry/middleware/test_stats.py new file mode 100644 index 00000000000000..659f7fb7881446 --- /dev/null +++ b/tests/sentry/middleware/test_stats.py @@ -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, + )