From 89b4e6be483a57c72865e52676c0e2ee6c3dcd13 Mon Sep 17 00:00:00 2001 From: Leighton Date: Mon, 12 Oct 2020 14:45:10 -0400 Subject: [PATCH 01/12] django --- docs/examples/django/manage.py | 6 +- .../instrumentation/django/__init__.py | 21 +++--- .../instrumentation/django/middleware.py | 41 +++++++++++- .../instrumentation/requests/__init__.py | 6 +- .../opentelemetry/instrumentation/metric.py | 66 ++++++++++++++----- 5 files changed, 104 insertions(+), 36 deletions(-) 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/src/opentelemetry/instrumentation/django/__init__.py b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/__init__.py index a9bd620e77c..5b2544512e0 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` @@ -34,16 +40,6 @@ class DjangoInstrumentor(BaseInstrumentor): ) def _instrument(self, **kwargs): - - # FIXME this is probably a pattern that will show up in the rest of the - # ext. Find a better way of implementing this. - # FIXME Probably the evaluation of strings into boolean values can be - # built inside the Configuration class itself with the magic method - # __bool__ - - if not Configuration().DJANGO_INSTRUMENT: - return - # This can not be solved, but is an inherent problem of this approach: # the order of middleware entries matters, and here you have no control # on that: @@ -57,6 +53,9 @@ 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.BOTH) + 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 11991413eb8..6201b506e81 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,12 @@ # 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__ @@ -88,6 +92,28 @@ 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", "") + if attributes.get("http.url"): + labels["http.url"] = attributes.get("http.url") + elif attributes.get("http.scheme"): + labels["http.scheme"] = attributes.get("http.scheme") + if attributes.get("http.target"): + labels["http.target"] = attributes.get("http.target") + if attributes.get("http.host"): + labels["http.host"] = attributes.get("http.host") + elif attributes.get("net.host.port"): + labels["net.host.port"] = attributes.get("net.host.port") + if attributes.get("http.server_name"): + labels["http.server_name"] = attributes.get("http.server_name") + elif attributes.get("http.host.name"): + labels["http.host.name"] = attributes.get("http.host.name") + 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 +122,8 @@ def process_request(self, request): if self._excluded_urls.url_disabled(request.build_absolute_uri("?")): return + request.start_time = time.time() + environ = request.META token = attach(extract(get_header_from_environ, environ)) @@ -110,8 +138,10 @@ def process_request(self, request): ), ) + attributes = collect_request_attributes(environ) + request.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 ) @@ -156,6 +186,7 @@ def process_response(self, request, response): "{} {}".format(response.status_code, response.reason_phrase), response, ) + request.labels["http.status_code"] = str(response.status_code) request.META.pop(self._environ_span_key) request.META[self._environ_activation_key].__exit__( @@ -167,4 +198,12 @@ 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: + metric_recorder.record_server_duration_range( + request.start_time, time.time(), request.labels) + except Exception as ex: # pylint: disable=W0703 + _logger.warning("Error recording duration metrics: {}", ex) + return response 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..cf9463f4caa 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,7 @@ 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/opentelemetry-instrumentation/src/opentelemetry/instrumentation/metric.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/metric.py index 74445cbff1b..cb582211d6c 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,59 @@ def __init__( ): super().__init__(meter) self._http_type = http_type + self._client_duration = None + self._server_duration = None 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, - ) + 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]): + if self._client_duration: + start_time = time() + try: + yield start_time + finally: + if self._meter: + elapsed_time = (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: - if self._meter: - elapsed_time = (time() - start_time) * 1000 - self._duration.record(elapsed_time, labels) + 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: + elapsed_time = (end_time - start_time) * 1000 + self._server_duration.record(elapsed_time, labels) From e86964143114a0eba9ecfa060cea22bf14db91ef Mon Sep 17 00:00:00 2001 From: Leighton Date: Mon, 12 Oct 2020 15:35:30 -0400 Subject: [PATCH 02/12] tests --- .../instrumentation/django/__init__.py | 2 +- .../tests/test_middleware.py | 39 +++++++++++++++ .../instrumentation/requests/__init__.py | 2 +- .../tests/test_requests_integration.py | 4 +- .../opentelemetry/instrumentation/metric.py | 23 +++++---- .../tests/test_metric.py | 49 ++++++++++++++++--- .../src/opentelemetry/test/wsgitestutil.py | 5 +- 7 files changed, 100 insertions(+), 24 deletions(-) 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 5b2544512e0..640eaf50f3e 100644 --- a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/__init__.py @@ -54,7 +54,7 @@ def _instrument(self, **kwargs): settings_middleware.insert(0, self._opentelemetry_middleware) self.init_metrics(__name__, __version__,) - metric_recorder = HTTPMetricRecorder(self.meter, HTTPMetricType.BOTH) + metric_recorder = HTTPMetricRecorder(self.meter, HTTPMetricType.SERVER) setattr(settings, "OTEL_METRIC_RECORDER", metric_recorder) setattr(settings, "MIDDLEWARE", settings_middleware) diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py index 4db6c485dec..70c56ef30e7 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py @@ -23,6 +23,7 @@ from opentelemetry.configuration import Configuration from opentelemetry.instrumentation.django import DjangoInstrumentor +from opentelemetry.sdk.util import get_dict_as_key from opentelemetry.test.wsgitestutil import WsgiTestBase from opentelemetry.trace import SpanKind from opentelemetry.trace.status import StatusCanonicalCode @@ -89,6 +90,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() @@ -146,6 +167,24 @@ def test_error(self): span.attributes["http.url"], "http://testserver/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/src/opentelemetry/instrumentation/requests/__init__.py b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py index cf9463f4caa..c2995ab38e2 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py @@ -45,7 +45,7 @@ from opentelemetry.instrumentation.instrumentor import BaseInstrumentor from opentelemetry.instrumentation.metric import ( HTTPMetricRecorder, - HTTPMetricType + HTTPMetricType, MetricMixin, ) from opentelemetry.instrumentation.requests.version import __version__ 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 cb582211d6c..25b24abf9a8 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/metric.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/metric.py @@ -85,15 +85,21 @@ def __init__( # https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/metrics/semantic_conventions/http-metrics.md @contextmanager def record_client_duration(self, labels: Dict[str, str]): - if self._client_duration: - start_time = time() - try: - yield start_time - finally: - if self._meter: - elapsed_time = (time() - start_time) * 1000 - self._client_duration.record(elapsed_time, labels) + start_time = time() + try: + yield start_time + finally: + 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: + elapsed_time = (end_time - start_time) * 1000 + self._client_duration.record(elapsed_time, labels) @contextmanager def record_server_duration(self, labels: Dict[str, str]): @@ -103,7 +109,6 @@ def record_server_duration(self, labels: Dict[str, str]): finally: self.record_server_duration_range(start_time, time(), labels) - def record_server_duration_range( self, start_time, diff --git a/opentelemetry-instrumentation/tests/test_metric.py b/opentelemetry-instrumentation/tests/test_metric.py index ea8724fc889..5d11dbb9b4e 100644 --- a/opentelemetry-instrumentation/tests/test_metric.py +++ b/opentelemetry-instrumentation/tests/test_metric.py @@ -61,26 +61,61 @@ 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_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_duration(labels): + 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) diff --git a/tests/util/src/opentelemetry/test/wsgitestutil.py b/tests/util/src/opentelemetry/test/wsgitestutil.py index 349db8f6944..1396760cf89 100644 --- a/tests/util/src/opentelemetry/test/wsgitestutil.py +++ b/tests/util/src/opentelemetry/test/wsgitestutil.py @@ -15,10 +15,9 @@ import io import wsgiref.util as wsgiref_util -from opentelemetry.test.spantestutil import SpanTestBase +from opentelemetry.test.test_base import TestBase - -class WsgiTestBase(SpanTestBase): +class WsgiTestBase(TestBase): def setUp(self): super().setUp() From 585ca48586f3182929b06cf6e00837dbef3a1dea Mon Sep 17 00:00:00 2001 From: Leighton Date: Mon, 12 Oct 2020 15:36:33 -0400 Subject: [PATCH 03/12] changelog --- .../opentelemetry-instrumentation-django/CHANGELOG.md | 2 ++ .../opentelemetry-instrumentation-requests/CHANGELOG.md | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-django/CHANGELOG.md b/instrumentation/opentelemetry-instrumentation-django/CHANGELOG.md index 30b13c4a223..ae2f50880ff 100644 --- a/instrumentation/opentelemetry-instrumentation-django/CHANGELOG.md +++ b/instrumentation/opentelemetry-instrumentation-django/CHANGELOG.md @@ -4,6 +4,8 @@ - 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)) +- Add support for tracking http metrics + ([#1116](https://github.com/open-telemetry/opentelemetry-python/pull/1116)) ## Version 0.13b0 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 From ad5bf9a68015e195f370f60199ca363ec003f31c Mon Sep 17 00:00:00 2001 From: Leighton Date: Mon, 12 Oct 2020 15:59:58 -0400 Subject: [PATCH 04/12] tests --- .../CHANGELOG.md | 2 +- .../instrumentation/django/__init__.py | 5 ++++- .../instrumentation/django/middleware.py | 14 +++++++----- .../tests/test_middleware.py | 3 ++- .../instrumentation/requests/__init__.py | 4 +++- .../opentelemetry/instrumentation/metric.py | 22 ++++++------------- .../src/opentelemetry/test/wsgitestutil.py | 5 +++-- 7 files changed, 29 insertions(+), 26 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-django/CHANGELOG.md b/instrumentation/opentelemetry-instrumentation-django/CHANGELOG.md index ae2f50880ff..4def10a424e 100644 --- a/instrumentation/opentelemetry-instrumentation-django/CHANGELOG.md +++ b/instrumentation/opentelemetry-instrumentation-django/CHANGELOG.md @@ -5,7 +5,7 @@ - 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)) - Add support for tracking http metrics - ([#1116](https://github.com/open-telemetry/opentelemetry-python/pull/1116)) + ([#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 640eaf50f3e..32e2aadf8ec 100644 --- a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/__init__.py @@ -53,7 +53,10 @@ def _instrument(self, **kwargs): settings_middleware = list(settings_middleware) settings_middleware.insert(0, self._opentelemetry_middleware) - self.init_metrics(__name__, __version__,) + self.init_metrics( + __name__, + __version__, + ) metric_recorder = HTTPMetricRecorder(self.meter, HTTPMetricType.SERVER) setattr(settings, "OTEL_METRIC_RECORDER", metric_recorder) setattr(settings, "MIDDLEWARE", settings_middleware) 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 6201b506e81..bc6e28cbe27 100644 --- a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py @@ -48,8 +48,7 @@ class _DjangoMiddleware(MiddlewareMixin): - """Django Middleware for OpenTelemetry - """ + """Django Middleware for OpenTelemetry""" _environ_activation_key = ( "opentelemetry-instrumentor-django.activation_key" @@ -107,9 +106,13 @@ def _get_metric_labels_from_attributes(attributes): elif attributes.get("net.host.port"): labels["net.host.port"] = attributes.get("net.host.port") if attributes.get("http.server_name"): - labels["http.server_name"] = attributes.get("http.server_name") + labels["http.server_name"] = attributes.get( + "http.server_name" + ) elif attributes.get("http.host.name"): - labels["http.host.name"] = attributes.get("http.host.name") + labels["http.host.name"] = attributes.get( + "http.host.name" + ) if attributes.get("http.flavor"): labels["http.flavor"] = attributes.get("http.flavor") return labels @@ -202,7 +205,8 @@ def process_response(self, request, response): metric_recorder = getattr(settings, "OTEL_METRIC_RECORDER", None) if metric_recorder: metric_recorder.record_server_duration_range( - request.start_time, time.time(), request.labels) + request.start_time, time.time(), request.labels + ) except Exception as ex: # pylint: disable=W0703 _logger.warning("Error recording duration metrics: {}", ex) diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py index 70c56ef30e7..a4289fc1215 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py @@ -24,6 +24,7 @@ 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 @@ -52,7 +53,7 @@ _django_instrumentor = DjangoInstrumentor() -class TestMiddleware(WsgiTestBase): +class TestMiddleware(TestBase, WsgiTestBase): @classmethod def setUpClass(cls): super().setUpClass() 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 c2995ab38e2..770eacf5e27 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py @@ -253,7 +253,9 @@ def _instrument(self, **kwargs): __name__, __version__, ) # pylint: disable=W0201 - self.metric_recorder = HTTPMetricRecorder(self.meter, HTTPMetricType.CLIENT) + self.metric_recorder = HTTPMetricRecorder( + self.meter, HTTPMetricType.CLIENT + ) def _uninstrument(self, **kwargs): _uninstrument() diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/metric.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/metric.py index 25b24abf9a8..a45ada3d9f0 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/metric.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/metric.py @@ -53,7 +53,9 @@ class HTTPMetricRecorder(MetricRecorder): """Metric recorder for http instrumentations. Tracks duration.""" def __init__( - self, meter: Optional[metrics.Meter], http_type: HTTPMetricType, + self, + meter: Optional[metrics.Meter], + http_type: HTTPMetricType, ): super().__init__(meter) self._http_type = http_type @@ -62,9 +64,7 @@ def __init__( if self._meter: if http_type in (HTTPMetricType.CLIENT, HTTPMetricType.BOTH): self._client_duration = self._meter.create_metric( - name="{}.{}.duration".format( - "http", "client" - ), + name="{}.{}.duration".format("http", "client"), description="measures the duration of the outbound HTTP request", unit="ms", value_type=float, @@ -72,9 +72,7 @@ def __init__( ) if http_type is not HTTPMetricType.CLIENT: self._server_duration = self._meter.create_metric( - name="{}.{}.duration".format( - "http", "server" - ), + name="{}.{}.duration".format("http", "server"), description="measures the duration of the inbound HTTP request", unit="ms", value_type=float, @@ -92,10 +90,7 @@ def record_client_duration(self, labels: Dict[str, str]): self.record_client_duration_range(start_time, time(), labels) def record_client_duration_range( - self, - start_time, - end_time, - labels: Dict[str, str] + self, start_time, end_time, labels: Dict[str, str] ): if self._client_duration: elapsed_time = (end_time - start_time) * 1000 @@ -110,10 +105,7 @@ def record_server_duration(self, labels: Dict[str, str]): self.record_server_duration_range(start_time, time(), labels) def record_server_duration_range( - self, - start_time, - end_time, - labels: Dict[str, str] + self, start_time, end_time, labels: Dict[str, str] ): if self._server_duration: elapsed_time = (end_time - start_time) * 1000 diff --git a/tests/util/src/opentelemetry/test/wsgitestutil.py b/tests/util/src/opentelemetry/test/wsgitestutil.py index 1396760cf89..349db8f6944 100644 --- a/tests/util/src/opentelemetry/test/wsgitestutil.py +++ b/tests/util/src/opentelemetry/test/wsgitestutil.py @@ -15,9 +15,10 @@ import io import wsgiref.util as wsgiref_util -from opentelemetry.test.test_base import TestBase +from opentelemetry.test.spantestutil import SpanTestBase -class WsgiTestBase(TestBase): + +class WsgiTestBase(SpanTestBase): def setUp(self): super().setUp() From 1e71ed51e966a738291a50468db438b50128161f Mon Sep 17 00:00:00 2001 From: Leighton Date: Mon, 12 Oct 2020 16:08:44 -0400 Subject: [PATCH 05/12] lint --- .../instrumentation/django/__init__.py | 3 +-- .../tests/test_middleware.py | 1 - .../opentelemetry/instrumentation/metric.py | 4 +--- .../tests/test_metric.py | 24 ++++++++++++++----- 4 files changed, 20 insertions(+), 12 deletions(-) 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 32e2aadf8ec..0d58c83c738 100644 --- a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/__init__.py @@ -54,8 +54,7 @@ def _instrument(self, **kwargs): settings_middleware.insert(0, self._opentelemetry_middleware) self.init_metrics( - __name__, - __version__, + __name__, __version__, ) metric_recorder = HTTPMetricRecorder(self.meter, HTTPMetricType.SERVER) setattr(settings, "OTEL_METRIC_RECORDER", metric_recorder) diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py index a4289fc1215..5621babc888 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py @@ -186,7 +186,6 @@ def test_error(self): self.assertEqual(view_data.labels, key) self.assertEqual(view_data.aggregator.current.count, 1) - @patch( "opentelemetry.instrumentation.django.middleware._DjangoMiddleware._excluded_urls", ExcludeList(["http://testserver/excluded_arg/123", "excluded_noarg"]), diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/metric.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/metric.py index a45ada3d9f0..3a58c6d031d 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/metric.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/metric.py @@ -53,9 +53,7 @@ class HTTPMetricRecorder(MetricRecorder): """Metric recorder for http instrumentations. Tracks duration.""" def __init__( - self, - meter: Optional[metrics.Meter], - http_type: HTTPMetricType, + self, meter: Optional[metrics.Meter], http_type: HTTPMetricType, ): super().__init__(meter) self._http_type = http_type diff --git a/opentelemetry-instrumentation/tests/test_metric.py b/opentelemetry-instrumentation/tests/test_metric.py index 5d11dbb9b4e..14c39d85e09 100644 --- a/opentelemetry-instrumentation/tests/test_metric.py +++ b/opentelemetry-instrumentation/tests/test_metric.py @@ -61,8 +61,12 @@ def test_ctor(self): recorder = HTTPMetricRecorder(meter, HTTPMetricType.CLIENT) # pylint: disable=protected-access self.assertEqual(recorder._http_type, HTTPMetricType.CLIENT) - self.assertTrue(isinstance(recorder._client_duration, metrics.ValueRecorder)) - self.assertEqual(recorder._client_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._client_duration.description, "measures the duration of the outbound HTTP request", @@ -72,18 +76,26 @@ 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.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.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)) + 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__) From ccfba2f8885e6dd094eab5f16e1b9a9bd6f6b923 Mon Sep 17 00:00:00 2001 From: Leighton Date: Mon, 12 Oct 2020 16:12:29 -0400 Subject: [PATCH 06/12] lint --- .../src/opentelemetry/instrumentation/django/middleware.py | 1 - 1 file changed, 1 deletion(-) 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 bc6e28cbe27..6a6c2d200be 100644 --- a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py @@ -13,7 +13,6 @@ # limitations under the License. import time - from logging import getLogger from django.conf import settings From 24cf8ff197ecfb25374ef5f8348607687b651561 Mon Sep 17 00:00:00 2001 From: Leighton Date: Mon, 12 Oct 2020 16:21:45 -0400 Subject: [PATCH 07/12] Update middleware.py --- .../src/opentelemetry/instrumentation/django/middleware.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 6a6c2d200be..47f262515c3 100644 --- a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py @@ -207,6 +207,6 @@ def process_response(self, request, response): request.start_time, time.time(), request.labels ) except Exception as ex: # pylint: disable=W0703 - _logger.warning("Error recording duration metrics: {}", ex) + _logger.warning("Error recording duration metrics: %s", ex) return response From b2bf95765f76f90d6cc8744e087749560277b765 Mon Sep 17 00:00:00 2001 From: Leighton Date: Tue, 13 Oct 2020 10:54:45 -0400 Subject: [PATCH 08/12] address comments --- .../instrumentation/django/middleware.py | 38 ++++++++----------- 1 file changed, 16 insertions(+), 22 deletions(-) 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 47f262515c3..4bd5c677818 100644 --- a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py @@ -90,28 +90,22 @@ def _get_span_name(request): except Resolver404: return "HTTP {}".format(request.method) + _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'] + ] + @staticmethod def _get_metric_labels_from_attributes(attributes): labels = {} labels["http.method"] = attributes.get("http.method", "") - if attributes.get("http.url"): - labels["http.url"] = attributes.get("http.url") - elif attributes.get("http.scheme"): - labels["http.scheme"] = attributes.get("http.scheme") - if attributes.get("http.target"): - labels["http.target"] = attributes.get("http.target") - if attributes.get("http.host"): - labels["http.host"] = attributes.get("http.host") - elif attributes.get("net.host.port"): - labels["net.host.port"] = attributes.get("net.host.port") - if attributes.get("http.server_name"): - labels["http.server_name"] = attributes.get( - "http.server_name" - ) - elif attributes.get("http.host.name"): - labels["http.host.name"] = attributes.get( - "http.host.name" - ) + for attrs in _attributes_by_preference: + labels_from_attributes = {attr: attributes.get(attr, None) for attr in attrs} + if all(labels_from_attributes.values()): + labels.update(labels_from_attributes) + break if attributes.get("http.flavor"): labels["http.flavor"] = attributes.get("http.flavor") return labels @@ -124,7 +118,7 @@ def process_request(self, request): if self._excluded_urls.url_disabled(request.build_absolute_uri("?")): return - request.start_time = time.time() + request._otel_start_time = time.time() environ = request.META @@ -141,7 +135,7 @@ def process_request(self, request): ) attributes = collect_request_attributes(environ) - request.labels = self._get_metric_labels_from_attributes(attributes) + request._otel_labels = self._get_metric_labels_from_attributes(attributes) if span.is_recording(): attributes = extract_attributes_from_object( @@ -188,7 +182,7 @@ def process_response(self, request, response): "{} {}".format(response.status_code, response.reason_phrase), response, ) - request.labels["http.status_code"] = str(response.status_code) + request._otel_labels["http.status_code"] = str(response.status_code) request.META.pop(self._environ_span_key) request.META[self._environ_activation_key].__exit__( @@ -204,7 +198,7 @@ def process_response(self, request, response): metric_recorder = getattr(settings, "OTEL_METRIC_RECORDER", None) if metric_recorder: metric_recorder.record_server_duration_range( - request.start_time, time.time(), request.labels + request._otel_start_time, time.time(), request._otel_labels ) except Exception as ex: # pylint: disable=W0703 _logger.warning("Error recording duration metrics: %s", ex) From 6255c9490f158560fbde12f6e4f89e2f942cd2eb Mon Sep 17 00:00:00 2001 From: Leighton Date: Tue, 13 Oct 2020 11:17:45 -0400 Subject: [PATCH 09/12] fix test --- .../instrumentation/django/middleware.py | 27 +++++++++++-------- 1 file changed, 16 insertions(+), 11 deletions(-) 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 4bd5c677818..badad860805 100644 --- a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py @@ -44,6 +44,12 @@ 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): @@ -90,19 +96,14 @@ def _get_span_name(request): except Resolver404: return "HTTP {}".format(request.method) - _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'] - ] - @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} + for attrs in _ATTRIBUTES_BY_PREFERENCE: + labels_from_attributes = { + attr: attributes.get(attr, None) for attr in attrs + } if all(labels_from_attributes.values()): labels.update(labels_from_attributes) break @@ -135,7 +136,9 @@ def process_request(self, request): ) attributes = collect_request_attributes(environ) - request._otel_labels = self._get_metric_labels_from_attributes(attributes) + request._otel_labels = self._get_metric_labels_from_attributes( + attributes + ) if span.is_recording(): attributes = extract_attributes_from_object( @@ -182,7 +185,9 @@ def process_response(self, request, response): "{} {}".format(response.status_code, response.reason_phrase), response, ) - request._otel_labels["http.status_code"] = str(response.status_code) + request._otel_labels["http.status_code"] = str( + response.status_code + ) request.META.pop(self._environ_span_key) request.META[self._environ_activation_key].__exit__( From 720ed3a1edc90f3368ae96b3dff2eac807749c42 Mon Sep 17 00:00:00 2001 From: Leighton Date: Tue, 13 Oct 2020 11:27:18 -0400 Subject: [PATCH 10/12] Update middleware.py --- .../opentelemetry/instrumentation/django/middleware.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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 badad860805..904551e5eb8 100644 --- a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py @@ -45,10 +45,10 @@ _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'], + ["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"], ] From f25d2d88e972fb8e48935a1985c138f3e241be7d Mon Sep 17 00:00:00 2001 From: Leighton Date: Tue, 13 Oct 2020 11:34:51 -0400 Subject: [PATCH 11/12] Update middleware.py --- .../src/opentelemetry/instrumentation/django/middleware.py | 4 ++++ 1 file changed, 4 insertions(+) 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 904551e5eb8..ea700a0d1f6 100644 --- a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py @@ -119,6 +119,7 @@ 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 @@ -136,6 +137,7 @@ def process_request(self, request): ) attributes = collect_request_attributes(environ) + # pylint:disable=W0212 request._otel_labels = self._get_metric_labels_from_attributes( attributes ) @@ -185,6 +187,7 @@ 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 ) @@ -202,6 +205,7 @@ def process_response(self, request, response): try: metric_recorder = getattr(settings, "OTEL_METRIC_RECORDER", None) if metric_recorder: + # pylint:disable=W0212 metric_recorder.record_server_duration_range( request._otel_start_time, time.time(), request._otel_labels ) From 4790aecae79ef8a4ea3241fcdcdc02c970dc02de Mon Sep 17 00:00:00 2001 From: Leighton Date: Wed, 14 Oct 2020 13:45:46 -0400 Subject: [PATCH 12/12] address comments --- .../opentelemetry/instrumentation/django/middleware.py | 8 ++++---- .../src/opentelemetry/instrumentation/metric.py | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) 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 ea700a0d1f6..b7986fb9a9e 100644 --- a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py @@ -44,7 +44,7 @@ MiddlewareMixin = object _logger = getLogger(__name__) -_ATTRIBUTES_BY_PREFERENCE = [ +_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"], @@ -100,11 +100,11 @@ def _get_span_name(request): def _get_metric_labels_from_attributes(attributes): labels = {} labels["http.method"] = attributes.get("http.method", "") - for attrs in _ATTRIBUTES_BY_PREFERENCE: + for attrs in _attributes_by_preference: labels_from_attributes = { attr: attributes.get(attr, None) for attr in attrs } - if all(labels_from_attributes.values()): + if set(attrs).issubset(attributes.keys()): labels.update(labels_from_attributes) break if attributes.get("http.flavor"): @@ -204,7 +204,7 @@ def process_response(self, request, response): try: metric_recorder = getattr(settings, "OTEL_METRIC_RECORDER", None) - if metric_recorder: + if metric_recorder is not None: # pylint:disable=W0212 metric_recorder.record_server_duration_range( request._otel_start_time, time.time(), request._otel_labels diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/metric.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/metric.py index 3a58c6d031d..6aaca0bcaa0 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/metric.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/metric.py @@ -59,7 +59,7 @@ def __init__( self._http_type = http_type self._client_duration = None self._server_duration = None - if self._meter: + 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"), @@ -90,7 +90,7 @@ def record_client_duration(self, labels: Dict[str, str]): def record_client_duration_range( self, start_time, end_time, labels: Dict[str, str] ): - if self._client_duration: + if self._client_duration is not None: elapsed_time = (end_time - start_time) * 1000 self._client_duration.record(elapsed_time, labels) @@ -105,6 +105,6 @@ def record_server_duration(self, labels: Dict[str, str]): def record_server_duration_range( self, start_time, end_time, labels: Dict[str, str] ): - if self._server_duration: + if self._server_duration is not None: elapsed_time = (end_time - start_time) * 1000 self._server_duration.record(elapsed_time, labels)