diff --git a/docs/examples/django/manage.py b/docs/examples/django/manage.py index 3a67dbf8296..bc2d44886b4 100755 --- a/docs/examples/django/manage.py +++ b/docs/examples/django/manage.py @@ -21,13 +21,13 @@ def main(): + os.environ.setdefault( + "DJANGO_SETTINGS_MODULE", "instrumentation_example.settings" + ) # This call is what makes the Django application be instrumented DjangoInstrumentor().instrument() - os.environ.setdefault( - "DJANGO_SETTINGS_MODULE", "instrumentation_example.settings" - ) try: from django.core.management import execute_from_command_line except ImportError as exc: diff --git a/instrumentation/opentelemetry-instrumentation-django/CHANGELOG.md b/instrumentation/opentelemetry-instrumentation-django/CHANGELOG.md index 439b65b17c9..36962fbdcef 100644 --- a/instrumentation/opentelemetry-instrumentation-django/CHANGELOG.md +++ b/instrumentation/opentelemetry-instrumentation-django/CHANGELOG.md @@ -11,6 +11,8 @@ Released 2020-10-13 - Changed span name extraction from request to comply semantic convention ([#992](https://github.com/open-telemetry/opentelemetry-python/pull/992)) - Added support for `OTEL_PYTHON_DJANGO_TRACED_REQUEST_ATTRS` ([#1154](https://github.com/open-telemetry/opentelemetry-python/pull/1154)) - Added capture of http.route ([#1226](https://github.com/open-telemetry/opentelemetry-python/issues/1226)) +- Add support for tracking http metrics + ([#1230](https://github.com/open-telemetry/opentelemetry-python/pull/1230)) ## Version 0.13b0 diff --git a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/__init__.py b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/__init__.py index 9ba3bbb9157..26e21a1f7fa 100644 --- a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/__init__.py @@ -18,12 +18,18 @@ from opentelemetry.configuration import Configuration from opentelemetry.instrumentation.django.middleware import _DjangoMiddleware +from opentelemetry.instrumentation.django.version import __version__ from opentelemetry.instrumentation.instrumentor import BaseInstrumentor +from opentelemetry.instrumentation.metric import ( + HTTPMetricRecorder, + HTTPMetricType, + MetricMixin, +) _logger = getLogger(__name__) -class DjangoInstrumentor(BaseInstrumentor): +class DjangoInstrumentor(BaseInstrumentor, MetricMixin): """An instrumentor for Django See `BaseInstrumentor` @@ -57,6 +63,11 @@ def _instrument(self, **kwargs): settings_middleware = list(settings_middleware) settings_middleware.insert(0, self._opentelemetry_middleware) + self.init_metrics( + __name__, __version__, + ) + metric_recorder = HTTPMetricRecorder(self.meter, HTTPMetricType.SERVER) + setattr(settings, "OTEL_METRIC_RECORDER", metric_recorder) setattr(settings, "MIDDLEWARE", settings_middleware) def _uninstrument(self, **kwargs): diff --git a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py index e3cb78dbd88..0503d7bbcb6 100644 --- a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py @@ -12,8 +12,11 @@ # See the License for the specific language governing permissions and # limitations under the License. +import time from logging import getLogger +from django.conf import settings + from opentelemetry.configuration import Configuration from opentelemetry.context import attach, detach from opentelemetry.instrumentation.django.version import __version__ @@ -41,11 +44,16 @@ MiddlewareMixin = object _logger = getLogger(__name__) +_attributes_by_preference = [ + ["http.scheme", "http.host", "http.target"], + ["http.scheme", "http.server_name", "net.host.port", "http.target"], + ["http.scheme", "net.host.name", "net.host.port", "http.target"], + ["http.url"], +] class _DjangoMiddleware(MiddlewareMixin): - """Django Middleware for OpenTelemetry - """ + """Django Middleware for OpenTelemetry""" _environ_activation_key = ( "opentelemetry-instrumentor-django.activation_key" @@ -88,6 +96,21 @@ def _get_span_name(request): except Resolver404: return "HTTP {}".format(request.method) + @staticmethod + def _get_metric_labels_from_attributes(attributes): + labels = {} + labels["http.method"] = attributes.get("http.method", "") + for attrs in _attributes_by_preference: + labels_from_attributes = { + attr: attributes.get(attr, None) for attr in attrs + } + if set(attrs).issubset(attributes.keys()): + labels.update(labels_from_attributes) + break + if attributes.get("http.flavor"): + labels["http.flavor"] = attributes.get("http.flavor") + return labels + def process_request(self, request): # request.META is a dictionary containing all available HTTP headers # Read more about request.META here: @@ -96,6 +119,9 @@ def process_request(self, request): if self._excluded_urls.url_disabled(request.build_absolute_uri("?")): return + # pylint:disable=W0212 + request._otel_start_time = time.time() + environ = request.META token = attach(extract(get_header_from_environ, environ)) @@ -110,8 +136,13 @@ def process_request(self, request): ), ) + attributes = collect_request_attributes(environ) + # pylint:disable=W0212 + request._otel_labels = self._get_metric_labels_from_attributes( + attributes + ) + if span.is_recording(): - attributes = collect_request_attributes(environ) attributes = extract_attributes_from_object( request, self._traced_request_attrs, attributes ) @@ -176,6 +207,10 @@ def process_response(self, request, response): "{} {}".format(response.status_code, response.reason_phrase), response, ) + # pylint:disable=W0212 + request._otel_labels["http.status_code"] = str( + response.status_code + ) request.META.pop(self._environ_span_key) request.META[self._environ_activation_key].__exit__( @@ -187,4 +222,14 @@ def process_response(self, request, response): detach(request.environ.get(self._environ_token)) request.META.pop(self._environ_token) + try: + metric_recorder = getattr(settings, "OTEL_METRIC_RECORDER", None) + if metric_recorder is not None: + # pylint:disable=W0212 + metric_recorder.record_server_duration_range( + request._otel_start_time, time.time(), request._otel_labels + ) + except Exception as ex: # pylint: disable=W0703 + _logger.warning("Error recording duration metrics: %s", ex) + return response diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py index 6e3196e3ef2..5c034a23af2 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py @@ -23,6 +23,8 @@ from opentelemetry.configuration import Configuration from opentelemetry.instrumentation.django import DjangoInstrumentor +from opentelemetry.sdk.util import get_dict_as_key +from opentelemetry.test.test_base import TestBase from opentelemetry.test.wsgitestutil import WsgiTestBase from opentelemetry.trace import SpanKind from opentelemetry.trace.status import StatusCanonicalCode @@ -53,7 +55,7 @@ _django_instrumentor = DjangoInstrumentor() -class TestMiddleware(WsgiTestBase): +class TestMiddleware(TestBase, WsgiTestBase): @classmethod def setUpClass(cls): super().setUpClass() @@ -121,6 +123,26 @@ def test_traced_get(self): self.assertEqual(span.attributes["http.status_code"], 200) self.assertEqual(span.attributes["http.status_text"], "OK") + self.assertIsNotNone(_django_instrumentor.meter) + self.assertEqual(len(_django_instrumentor.meter.metrics), 1) + recorder = _django_instrumentor.meter.metrics.pop() + match_key = get_dict_as_key( + { + "http.flavor": "1.1", + "http.method": "GET", + "http.status_code": "200", + "http.url": "http://testserver/traced/", + } + ) + for key in recorder.bound_instruments.keys(): + self.assertEqual(key, match_key) + # pylint: disable=protected-access + bound = recorder.bound_instruments.get(key) + for view_data in bound.view_datas: + self.assertEqual(view_data.labels, key) + self.assertEqual(view_data.aggregator.current.count, 1) + self.assertGreaterEqual(view_data.aggregator.current.sum, 0) + def test_not_recording(self): mock_tracer = Mock() mock_span = Mock() @@ -180,6 +202,23 @@ def test_error(self): ) self.assertEqual(span.attributes["http.route"], "^error/") self.assertEqual(span.attributes["http.scheme"], "http") + self.assertIsNotNone(_django_instrumentor.meter) + self.assertEqual(len(_django_instrumentor.meter.metrics), 1) + recorder = _django_instrumentor.meter.metrics.pop() + match_key = get_dict_as_key( + { + "http.flavor": "1.1", + "http.method": "GET", + "http.url": "http://testserver/error/", + } + ) + for key in recorder.bound_instruments.keys(): + self.assertEqual(key, match_key) + # pylint: disable=protected-access + bound = recorder.bound_instruments.get(key) + for view_data in bound.view_datas: + self.assertEqual(view_data.labels, key) + self.assertEqual(view_data.aggregator.current.count, 1) @patch( "opentelemetry.instrumentation.django.middleware._DjangoMiddleware._excluded_urls", diff --git a/instrumentation/opentelemetry-instrumentation-requests/CHANGELOG.md b/instrumentation/opentelemetry-instrumentation-requests/CHANGELOG.md index c2e4dcdbb92..96089a9da15 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/CHANGELOG.md +++ b/instrumentation/opentelemetry-instrumentation-requests/CHANGELOG.md @@ -10,7 +10,7 @@ Released 2020-09-17 ([#1040](https://github.com/open-telemetry/opentelemetry-python/pull/1040)) - Drop support for Python 3.4 ([#1099](https://github.com/open-telemetry/opentelemetry-python/pull/1099)) -- Add support for http metrics +- Add support for tracking http metrics ([#1116](https://github.com/open-telemetry/opentelemetry-python/pull/1116)) ## Version 0.12b0 diff --git a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py index 4b5f73de9e6..770eacf5e27 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py @@ -45,6 +45,7 @@ from opentelemetry.instrumentation.instrumentor import BaseInstrumentor from opentelemetry.instrumentation.metric import ( HTTPMetricRecorder, + HTTPMetricType, MetricMixin, ) from opentelemetry.instrumentation.requests.version import __version__ @@ -135,7 +136,7 @@ def _instrumented_requests_call( __name__, __version__, tracer_provider ).start_as_current_span(span_name, kind=SpanKind.CLIENT) as span: exception = None - with recorder.record_duration(labels): + with recorder.record_client_duration(labels): if span.is_recording(): span.set_attribute("component", "http") span.set_attribute("http.method", method) @@ -176,7 +177,6 @@ def _instrumented_requests_call( ) ) labels["http.status_code"] = str(result.status_code) - labels["http.status_text"] = result.reason if result.raw and result.raw.version: labels["http.flavor"] = ( str(result.raw.version)[:1] @@ -253,7 +253,9 @@ def _instrument(self, **kwargs): __name__, __version__, ) # pylint: disable=W0201 - self.metric_recorder = HTTPMetricRecorder(self.meter, SpanKind.CLIENT) + self.metric_recorder = HTTPMetricRecorder( + self.meter, HTTPMetricType.CLIENT + ) def _uninstrument(self, **kwargs): _uninstrument() diff --git a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py index 678499e8794..f41e597b23e 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py +++ b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py @@ -97,7 +97,6 @@ def test_basic(self): "http.flavor": "1.1", "http.method": "GET", "http.status_code": "200", - "http.status_text": "OK", "http.url": "http://httpbin.org/status/200", } ) @@ -108,7 +107,7 @@ def test_basic(self): for view_data in bound.view_datas: self.assertEqual(view_data.labels, key) self.assertEqual(view_data.aggregator.current.count, 1) - self.assertGreater(view_data.aggregator.current.sum, 0) + self.assertGreaterEqual(view_data.aggregator.current.sum, 0) def test_not_foundbasic(self): url_404 = "http://httpbin.org/status/404" @@ -318,7 +317,6 @@ def test_requests_exception_with_response(self, *_, **__): { "http.method": "GET", "http.status_code": "500", - "http.status_text": "Internal Server Error", "http.url": "http://httpbin.org/status/200", } ) diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/metric.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/metric.py index 74445cbff1b..6aaca0bcaa0 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/metric.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/metric.py @@ -28,7 +28,7 @@ class HTTPMetricType(enum.Enum): CLIENT = 0 SERVER = 1 - # TODO: Add both + BOTH = 2 class MetricMixin: @@ -57,29 +57,54 @@ def __init__( ): super().__init__(meter) self._http_type = http_type - if self._meter: - self._duration = self._meter.create_metric( - name="{}.{}.duration".format( - "http", self._http_type.name.lower() - ), - description="measures the duration of the {} HTTP request".format( - "inbound" - if self._http_type is HTTPMetricType.SERVER - else "outbound" - ), - unit="ms", - value_type=float, - metric_type=ValueRecorder, - ) + self._client_duration = None + self._server_duration = None + if self._meter is not None: + if http_type in (HTTPMetricType.CLIENT, HTTPMetricType.BOTH): + self._client_duration = self._meter.create_metric( + name="{}.{}.duration".format("http", "client"), + description="measures the duration of the outbound HTTP request", + unit="ms", + value_type=float, + metric_type=ValueRecorder, + ) + if http_type is not HTTPMetricType.CLIENT: + self._server_duration = self._meter.create_metric( + name="{}.{}.duration".format("http", "server"), + description="measures the duration of the inbound HTTP request", + unit="ms", + value_type=float, + metric_type=ValueRecorder, + ) # Conventions for recording duration can be found at: # https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/metrics/semantic_conventions/http-metrics.md @contextmanager - def record_duration(self, labels: Dict[str, str]): + def record_client_duration(self, labels: Dict[str, str]): start_time = time() try: yield start_time finally: - if self._meter: - elapsed_time = (time() - start_time) * 1000 - self._duration.record(elapsed_time, labels) + self.record_client_duration_range(start_time, time(), labels) + + def record_client_duration_range( + self, start_time, end_time, labels: Dict[str, str] + ): + if self._client_duration is not None: + elapsed_time = (end_time - start_time) * 1000 + self._client_duration.record(elapsed_time, labels) + + @contextmanager + def record_server_duration(self, labels: Dict[str, str]): + start_time = time() + try: + yield start_time + finally: + self.record_server_duration_range(start_time, time(), labels) + + def record_server_duration_range( + self, start_time, end_time, labels: Dict[str, str] + ): + if self._server_duration is not None: + elapsed_time = (end_time - start_time) * 1000 + self._server_duration.record(elapsed_time, labels) diff --git a/opentelemetry-instrumentation/tests/test_metric.py b/opentelemetry-instrumentation/tests/test_metric.py index ea8724fc889..14c39d85e09 100644 --- a/opentelemetry-instrumentation/tests/test_metric.py +++ b/opentelemetry-instrumentation/tests/test_metric.py @@ -61,26 +61,73 @@ def test_ctor(self): recorder = HTTPMetricRecorder(meter, HTTPMetricType.CLIENT) # pylint: disable=protected-access self.assertEqual(recorder._http_type, HTTPMetricType.CLIENT) - self.assertTrue(isinstance(recorder._duration, metrics.ValueRecorder)) - self.assertEqual(recorder._duration.name, "http.client.duration") + self.assertTrue( + isinstance(recorder._client_duration, metrics.ValueRecorder) + ) + self.assertEqual( + recorder._client_duration.name, "http.client.duration" + ) self.assertEqual( - recorder._duration.description, + recorder._client_duration.description, "measures the duration of the outbound HTTP request", ) - def test_record_duration(self): + def test_ctor_types(self): + meter = metrics_api.get_meter(__name__) + recorder = HTTPMetricRecorder(meter, HTTPMetricType.CLIENT) + self.assertEqual(recorder._http_type, HTTPMetricType.CLIENT) + self.assertTrue( + isinstance(recorder._client_duration, metrics.ValueRecorder) + ) + self.assertIsNone(recorder._server_duration) + + recorder = HTTPMetricRecorder(meter, HTTPMetricType.SERVER) + self.assertEqual(recorder._http_type, HTTPMetricType.SERVER) + self.assertTrue( + isinstance(recorder._server_duration, metrics.ValueRecorder) + ) + self.assertIsNone(recorder._client_duration) + + recorder = HTTPMetricRecorder(meter, HTTPMetricType.BOTH) + self.assertEqual(recorder._http_type, HTTPMetricType.BOTH) + self.assertTrue( + isinstance(recorder._client_duration, metrics.ValueRecorder) + ) + self.assertTrue( + isinstance(recorder._server_duration, metrics.ValueRecorder) + ) + + def test_record_client_duration(self): meter = metrics_api.get_meter(__name__) recorder = HTTPMetricRecorder(meter, HTTPMetricType.CLIENT) labels = {"test": "asd"} with mock.patch("time.time") as time_patch: time_patch.return_value = 5.0 - with recorder.record_duration(labels): + with recorder.record_client_duration(labels): + labels["test2"] = "asd2" + match_key = get_dict_as_key({"test": "asd", "test2": "asd2"}) + for key in recorder._client_duration.bound_instruments.keys(): + self.assertEqual(key, match_key) + # pylint: disable=protected-access + bound = recorder._client_duration.bound_instruments.get(key) + for view_data in bound.view_datas: + self.assertEqual(view_data.labels, key) + self.assertEqual(view_data.aggregator.current.count, 1) + self.assertGreaterEqual(view_data.aggregator.current.sum, 0) + + def test_record_server_duration(self): + meter = metrics_api.get_meter(__name__) + recorder = HTTPMetricRecorder(meter, HTTPMetricType.SERVER) + labels = {"test": "asd"} + with mock.patch("time.time") as time_patch: + time_patch.return_value = 5.0 + with recorder.record_server_duration(labels): labels["test2"] = "asd2" match_key = get_dict_as_key({"test": "asd", "test2": "asd2"}) - for key in recorder._duration.bound_instruments.keys(): + for key in recorder._server_duration.bound_instruments.keys(): self.assertEqual(key, match_key) # pylint: disable=protected-access - bound = recorder._duration.bound_instruments.get(key) + bound = recorder._server_duration.bound_instruments.get(key) for view_data in bound.view_datas: self.assertEqual(view_data.labels, key) self.assertEqual(view_data.aggregator.current.count, 1)