From ece785ff926533c467440238772a9168d786f08b Mon Sep 17 00:00:00 2001 From: Troy Sankey Date: Thu, 3 Nov 2016 10:18:17 -0400 Subject: [PATCH] Implement the newrelic_custom_metrics middleware Use this middleware and its helpers to accumulate custom New Relic metrics on a per-request basis. If, while handling a request, the app does not accumulate any metrics, this middleware is a very lightweight no-op. This commit also enables the middleware! PERF-354 --- .../newrelic_custom_metrics/__init__.py | 43 +++++++++++++ .../newrelic_custom_metrics/middleware.py | 62 +++++++++++++++++++ .../newrelic_custom_metrics/tests.py | 46 ++++++++++++++ lms/envs/common.py | 1 + 4 files changed, 152 insertions(+) create mode 100644 common/djangoapps/newrelic_custom_metrics/__init__.py create mode 100644 common/djangoapps/newrelic_custom_metrics/middleware.py create mode 100644 common/djangoapps/newrelic_custom_metrics/tests.py diff --git a/common/djangoapps/newrelic_custom_metrics/__init__.py b/common/djangoapps/newrelic_custom_metrics/__init__.py new file mode 100644 index 0000000000..cd920c807b --- /dev/null +++ b/common/djangoapps/newrelic_custom_metrics/__init__.py @@ -0,0 +1,43 @@ +""" +This is an interface to the newrelic_custom_metrics middleware. Functions +defined in this module can be used to report custom metrics to New Relic. For +example: + + import newrelic_custom_metrics + ... + newrelic_custom_metrics.accumulate('xb_user_state.get_many.num_items', 4) + +There is no need to do anything else. The metrics are automatically cleared +before the next request. +""" + +from newrelic_custom_metrics import middleware + + +def accumulate(name, value): + """ + Queue up a custom New Relic metric for the current request. At the end of + the request, the newrelic_custom_metrics middleware will batch report all + queued metrics to NR. + + Q: What style of names should I use? + A: Metric names should be comma delimited, becoming more specific from left + to right. + + Q: What type can values be? + A: numbers only. + + Q: What happens when I call this multiple times with the same name? + A: Like-named metrics will be accumulated using the sum. + """ + middleware.NewRelicCustomMetrics.accumulate_metric(name, value) + + +def increment(name): + """ + Increment a custom New Relic metric representing a counter. + + Here we simply accumulate a new custom metric with a value of 1, and the + middleware should automatically aggregate this metric. + """ + accumulate(name, 1) diff --git a/common/djangoapps/newrelic_custom_metrics/middleware.py b/common/djangoapps/newrelic_custom_metrics/middleware.py new file mode 100644 index 0000000000..3af8cb5ef1 --- /dev/null +++ b/common/djangoapps/newrelic_custom_metrics/middleware.py @@ -0,0 +1,62 @@ +""" +Middleware for handling the storage, aggregation, and reporing of custom New +Relic metrics. + +This middleware will only call on the newrelic agent if there are any metrics +to report for this request, so it will not incur any processing overhead for +request handlers which do not record custom metrics. +""" +import newrelic.agent +import request_cache + +REQUEST_CACHE_KEY = 'newrelic_custom_metrics' + + +class NewRelicCustomMetrics(object): + """ + The middleware class. Make sure to add below the request cache in + MIDDLEWARE_CLASSES. + """ + + @classmethod + def _get_metrics_cache(cls): + """ + Get a reference to the part of the request cache wherein we store New + Relic custom metrics related to the current request. + """ + return request_cache.get_cache(name=REQUEST_CACHE_KEY) + + @classmethod + def accumulate_metric(cls, name, value): + """ + Accumulate a custom metric (name and value) in the metrics cache. + """ + metrics_cache = cls._get_metrics_cache() + metrics_cache.setdefault(name, 0) + metrics_cache[name] += value + + @classmethod + def _batch_report(cls): + """ + Report the collected custom metrics to New Relic. + """ + metrics_cache = cls._get_metrics_cache() + for metric_name, metric_value in metrics_cache.iteritems(): + newrelic.agent.add_custom_parameter(metric_name, metric_value) + + # Whether or not there was an exception, report any custom NR metrics that + # may have been collected. + + def process_response(self, request, response): # pylint: disable=unused-argument + """ + Django middleware handler to process a response + """ + self._batch_report() + return response + + def process_exception(self, request, exception): # pylint: disable=unused-argument + """ + Django middleware handler to process an exception + """ + self._batch_report() + return None diff --git a/common/djangoapps/newrelic_custom_metrics/tests.py b/common/djangoapps/newrelic_custom_metrics/tests.py new file mode 100644 index 0000000000..b1fc931b1a --- /dev/null +++ b/common/djangoapps/newrelic_custom_metrics/tests.py @@ -0,0 +1,46 @@ +""" +Tests for newrelic custom metrics. +""" +from django.test import TestCase +from mock import patch, call + +import newrelic_custom_metrics + + +class TestNewRelicCustomMetrics(TestCase): + """ + Test the newrelic_custom_metrics middleware and helpers + """ + + @patch('newrelic.agent') + def test_cache_normal_contents(self, mock_newrelic_agent): + """ + Test normal usage of collecting and reporting custom New Relic metrics + """ + newrelic_custom_metrics.accumulate('hello', 10) + newrelic_custom_metrics.accumulate('world', 10) + newrelic_custom_metrics.accumulate('world', 10) + newrelic_custom_metrics.increment('foo') + newrelic_custom_metrics.increment('foo') + + # based on the metric data above, we expect the following calls to newrelic: + nr_agent_calls_expected = [ + call('hello', 10), + call('world', 20), + call('foo', 2), + ] + + # fake a response to trigger metrics reporting + newrelic_custom_metrics.middleware.NewRelicCustomMetrics().process_response( + 'fake request', + 'fake response', + ) + + # Assert call counts to newrelic.agent.add_custom_parameter() + expected_call_count = len(nr_agent_calls_expected) + measured_call_count = mock_newrelic_agent.add_custom_parameter.call_count + self.assertEqual(expected_call_count, measured_call_count) + + # Assert call args to newrelic.agent.add_custom_parameter(). Due to + # the nature of python dicts, call order is undefined. + mock_newrelic_agent.add_custom_parameter.has_calls(nr_agent_calls_expected, any_order=True) diff --git a/lms/envs/common.py b/lms/envs/common.py index d4623ac53c..52a9349f7c 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -1096,6 +1096,7 @@ MIDDLEWARE_CLASSES = ( 'crum.CurrentRequestUserMiddleware', 'request_cache.middleware.RequestCache', + 'newrelic_custom_metrics.middleware.NewRelicCustomMetrics', 'mobile_api.middleware.AppVersionUpgrade', 'openedx.core.djangoapps.header_control.middleware.HeaderControlMiddleware',