From 872975b6be5b8022ee7501221bdb6261624f035e Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Fri, 25 Sep 2020 07:31:35 -0700 Subject: [PATCH] Adding metric collection as part of instrumentations - Requests (#1116) --- docs/examples/basic_meter/http.py | 42 +++++++ docs/instrumentation/instrumentation.rst | 1 + docs/instrumentation/metric.rst | 7 ++ .../CHANGELOG.md | 2 + .../instrumentation/requests/__init__.py | 103 ++++++++++++------ .../tests/test_requests_integration.py | 57 ++++++++++ opentelemetry-instrumentation/CHANGELOG.md | 2 + opentelemetry-instrumentation/setup.cfg | 1 + .../opentelemetry/instrumentation/metric.py | 85 +++++++++++++++ .../tests/test_metric.py | 87 +++++++++++++++ tox.ini | 2 +- 11 files changed, 353 insertions(+), 36 deletions(-) create mode 100644 docs/examples/basic_meter/http.py create mode 100644 docs/instrumentation/metric.rst create mode 100644 opentelemetry-instrumentation/src/opentelemetry/instrumentation/metric.py create mode 100644 opentelemetry-instrumentation/tests/test_metric.py diff --git a/docs/examples/basic_meter/http.py b/docs/examples/basic_meter/http.py new file mode 100644 index 00000000000..8fd6c6294c1 --- /dev/null +++ b/docs/examples/basic_meter/http.py @@ -0,0 +1,42 @@ +# Copyright The OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +""" +This module shows how you can enable collection and exporting of http metrics +related to instrumentations. +""" +import requests + +from opentelemetry import metrics +from opentelemetry.instrumentation.requests import RequestsInstrumentor +from opentelemetry.sdk.metrics import MeterProvider +from opentelemetry.sdk.metrics.export import ConsoleMetricsExporter + +# Sets the global MeterProvider instance +metrics.set_meter_provider(MeterProvider()) + +# Exporter to export metrics to the console +exporter = ConsoleMetricsExporter() + +# Instrument the requests library +RequestsInstrumentor().instrument() + +# Indicate to start collecting and exporting requests related metrics +metrics.get_meter_provider().start_pipeline( + RequestsInstrumentor().meter, exporter, 5 +) + +response = requests.get("http://example.com") + +input("...\n") diff --git a/docs/instrumentation/instrumentation.rst b/docs/instrumentation/instrumentation.rst index 9c01b6b6f4f..16b82a20934 100644 --- a/docs/instrumentation/instrumentation.rst +++ b/docs/instrumentation/instrumentation.rst @@ -13,3 +13,4 @@ Submodules :maxdepth: 1 instrumentor + metric diff --git a/docs/instrumentation/metric.rst b/docs/instrumentation/metric.rst new file mode 100644 index 00000000000..6a69eeeca50 --- /dev/null +++ b/docs/instrumentation/metric.rst @@ -0,0 +1,7 @@ +opentelemetry.instrumentation.metric package +============================================ + +.. automodule:: opentelemetry.instrumentation.metric + :members: + :undoc-members: + :show-inheritance: diff --git a/instrumentation/opentelemetry-instrumentation-requests/CHANGELOG.md b/instrumentation/opentelemetry-instrumentation-requests/CHANGELOG.md index 5b876bd4ad3..c2e4dcdbb92 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/CHANGELOG.md +++ b/instrumentation/opentelemetry-instrumentation-requests/CHANGELOG.md @@ -10,6 +10,8 @@ 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 + ([#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 fef67c5d0da..d0336184ed2 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py @@ -25,7 +25,7 @@ import opentelemetry.instrumentation.requests # You can optionally pass a custom TracerProvider to - RequestInstrumentor.instrument() + # RequestInstrumentor.instrument() opentelemetry.instrumentation.requests.RequestsInstrumentor().instrument() response = requests.get(url="https://www.example.org/") @@ -43,6 +43,10 @@ from opentelemetry import context, propagators from opentelemetry.instrumentation.instrumentor import BaseInstrumentor +from opentelemetry.instrumentation.metric import ( + HTTPMetricRecorder, + MetricMixin, +) from opentelemetry.instrumentation.requests.version import __version__ from opentelemetry.instrumentation.utils import http_status_to_canonical_code from opentelemetry.trace import SpanKind, get_tracer @@ -54,6 +58,7 @@ # pylint: disable=unused-argument +# pylint: disable=R0915 def _instrument(tracer_provider=None, span_callback=None): """Enables tracing of all requests calls that go through :code:`requests.session.Session.request` (this includes @@ -118,43 +123,66 @@ def _instrumented_requests_call( exception = None + recorder = RequestsInstrumentor().metric_recorder + + labels = {} + labels["http.method"] = method + labels["http.url"] = url + with get_tracer( __name__, __version__, tracer_provider ).start_as_current_span(span_name, kind=SpanKind.CLIENT) as span: - if span.is_recording(): - span.set_attribute("component", "http") - span.set_attribute("http.method", method.upper()) - span.set_attribute("http.url", url) - - headers = get_or_create_headers() - propagators.inject(type(headers).__setitem__, headers) - - token = context.attach( - context.set_value(_SUPPRESS_REQUESTS_INSTRUMENTATION_KEY, True) - ) - try: - result = call_wrapped() # *** PROCEED - except Exception as exc: # pylint: disable=W0703 - exception = exc - result = getattr(exc, "response", None) - finally: - context.detach(token) - - if exception is not None and span.is_recording(): - span.set_status( - Status(_exception_to_canonical_code(exception)) + with recorder.record_duration(labels): + if span.is_recording(): + span.set_attribute("component", "http") + span.set_attribute("http.method", method) + span.set_attribute("http.url", url) + + headers = get_or_create_headers() + propagators.inject(type(headers).__setitem__, headers) + + token = context.attach( + context.set_value( + _SUPPRESS_REQUESTS_INSTRUMENTATION_KEY, True + ) ) - span.record_exception(exception) - - if result is not None and span.is_recording(): - span.set_attribute("http.status_code", result.status_code) - span.set_attribute("http.status_text", result.reason) - span.set_status( - Status(http_status_to_canonical_code(result.status_code)) - ) - - if span_callback is not None: - span_callback(span, result) + try: + result = call_wrapped() # *** PROCEED + except Exception as exc: # pylint: disable=W0703 + exception = exc + result = getattr(exc, "response", None) + finally: + context.detach(token) + + if exception is not None and span.is_recording(): + span.set_status( + Status(_exception_to_canonical_code(exception)) + ) + span.record_exception(exception) + + if result is not None: + if span.is_recording(): + span.set_attribute( + "http.status_code", result.status_code + ) + span.set_attribute("http.status_text", result.reason) + span.set_status( + Status( + http_status_to_canonical_code( + result.status_code + ) + ) + ) + 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] + + "." + + str(result.raw.version)[:-1] + ) + if span_callback is not None: + span_callback(span, result) if exception is not None: raise exception.with_traceback(exception.__traceback__) @@ -202,7 +230,7 @@ def _exception_to_canonical_code(exc: Exception) -> StatusCanonicalCode: return StatusCanonicalCode.UNKNOWN -class RequestsInstrumentor(BaseInstrumentor): +class RequestsInstrumentor(BaseInstrumentor, MetricMixin): """An instrumentor for requests See `BaseInstrumentor` """ @@ -219,6 +247,11 @@ def _instrument(self, **kwargs): tracer_provider=kwargs.get("tracer_provider"), span_callback=kwargs.get("span_callback"), ) + self.init_metrics( + __name__, __version__, + ) + # pylint: disable=W0201 + self.metric_recorder = HTTPMetricRecorder(self.meter, SpanKind.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 c3457b73923..2d3636284bb 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py +++ b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py @@ -22,6 +22,7 @@ from opentelemetry import context, propagators, trace from opentelemetry.instrumentation.requests import RequestsInstrumentor from opentelemetry.sdk import resources +from opentelemetry.sdk.util import get_dict_as_key from opentelemetry.test.mock_textmap import MockTextMapPropagator from opentelemetry.test.test_base import TestBase from opentelemetry.trace.status import StatusCanonicalCode @@ -88,6 +89,27 @@ def test_basic(self): span, opentelemetry.instrumentation.requests ) + self.assertIsNotNone(RequestsInstrumentor().meter) + self.assertEqual(len(RequestsInstrumentor().meter.metrics), 1) + recorder = RequestsInstrumentor().meter.metrics.pop() + match_key = get_dict_as_key( + { + "http.flavor": "1.1", + "http.method": "GET", + "http.status_code": "200", + "http.status_text": "OK", + "http.url": "http://httpbin.org/status/200", + } + ) + 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.assertGreater(view_data.aggregator.current.sum, 0) + def test_not_foundbasic(self): url_404 = "http://httpbin.org/status/404" httpretty.register_uri( @@ -246,6 +268,23 @@ def test_requests_exception_without_response(self, *_, **__): span.status.canonical_code, StatusCanonicalCode.UNKNOWN ) + self.assertIsNotNone(RequestsInstrumentor().meter) + self.assertEqual(len(RequestsInstrumentor().meter.metrics), 1) + recorder = RequestsInstrumentor().meter.metrics.pop() + match_key = get_dict_as_key( + { + "http.method": "GET", + "http.url": "http://httpbin.org/status/200", + } + ) + 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) + mocked_response = requests.Response() mocked_response.status_code = 500 mocked_response.reason = "Internal Server Error" @@ -272,6 +311,24 @@ def test_requests_exception_with_response(self, *_, **__): self.assertEqual( span.status.canonical_code, StatusCanonicalCode.INTERNAL ) + self.assertIsNotNone(RequestsInstrumentor().meter) + self.assertEqual(len(RequestsInstrumentor().meter.metrics), 1) + recorder = RequestsInstrumentor().meter.metrics.pop() + match_key = get_dict_as_key( + { + "http.method": "GET", + "http.status_code": "500", + "http.status_text": "Internal Server Error", + "http.url": "http://httpbin.org/status/200", + } + ) + 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) @mock.patch("requests.adapters.HTTPAdapter.send", side_effect=Exception) def test_requests_basic_exception(self, *_, **__): diff --git a/opentelemetry-instrumentation/CHANGELOG.md b/opentelemetry-instrumentation/CHANGELOG.md index 37f50051030..13c01cc32a6 100644 --- a/opentelemetry-instrumentation/CHANGELOG.md +++ b/opentelemetry-instrumentation/CHANGELOG.md @@ -10,6 +10,8 @@ Released 2020-09-17 - Drop support for Python 3.4 ([#1099](https://github.com/open-telemetry/opentelemetry-python/pull/1099)) +- Add support for http metrics + ([#1116](https://github.com/open-telemetry/opentelemetry-python/pull/1116)) ## 0.9b0 diff --git a/opentelemetry-instrumentation/setup.cfg b/opentelemetry-instrumentation/setup.cfg index 6042f043d90..167036238f8 100644 --- a/opentelemetry-instrumentation/setup.cfg +++ b/opentelemetry-instrumentation/setup.cfg @@ -42,6 +42,7 @@ zip_safe = False include_package_data = True install_requires = opentelemetry-api == 0.14.dev0 + opentelemetry-sdk == 0.14.dev0 wrapt >= 1.0.0, < 2.0.0 [options.packages.find] diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/metric.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/metric.py new file mode 100644 index 00000000000..74445cbff1b --- /dev/null +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/metric.py @@ -0,0 +1,85 @@ +# Copyright The OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# type: ignore + +""" +OpenTelemetry Instrumentation Metric mixin +""" +import enum +from contextlib import contextmanager +from time import time +from typing import Dict, Optional + +from opentelemetry import metrics +from opentelemetry.sdk.metrics import ValueRecorder + + +class HTTPMetricType(enum.Enum): + CLIENT = 0 + SERVER = 1 + # TODO: Add both + + +class MetricMixin: + """Used to record metrics related to instrumentations.""" + + def init_metrics(self, name: str, version: str): + self._meter = metrics.get_meter(name, version) + + @property + def meter(self): + return self._meter + + +class MetricRecorder: + """Base class for metric recorders of different types.""" + + def __init__(self, meter: Optional[metrics.Meter] = None): + self._meter = meter + + +class HTTPMetricRecorder(MetricRecorder): + """Metric recorder for http instrumentations. Tracks duration.""" + + def __init__( + self, meter: Optional[metrics.Meter], http_type: HTTPMetricType, + ): + 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, + ) + + # 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]): + start_time = time() + try: + yield start_time + finally: + if self._meter: + elapsed_time = (time() - start_time) * 1000 + self._duration.record(elapsed_time, labels) diff --git a/opentelemetry-instrumentation/tests/test_metric.py b/opentelemetry-instrumentation/tests/test_metric.py new file mode 100644 index 00000000000..ea8724fc889 --- /dev/null +++ b/opentelemetry-instrumentation/tests/test_metric.py @@ -0,0 +1,87 @@ +# Copyright The OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# type: ignore + +from unittest import TestCase, mock + +from opentelemetry import metrics as metrics_api +from opentelemetry.instrumentation.metric import ( + HTTPMetricRecorder, + HTTPMetricType, + MetricMixin, +) +from opentelemetry.metrics import set_meter_provider +from opentelemetry.sdk import metrics +from opentelemetry.sdk.util import get_dict_as_key + + +# pylint: disable=protected-access +class TestMetricMixin(TestCase): + @classmethod + def setUpClass(cls): + metrics_api._METER_PROVIDER = None + set_meter_provider(metrics.MeterProvider()) + + @classmethod + def tearDownClass(cls): + metrics_api._METER_PROVIDER = None + + def test_init(self): + mixin = MetricMixin() + mixin.init_metrics("test", 1.0) + meter = mixin.meter + self.assertTrue(isinstance(meter, metrics.Meter)) + self.assertEqual(meter.instrumentation_info.name, "test") + self.assertEqual(meter.instrumentation_info.version, 1.0) + + +class TestHTTPMetricRecorder(TestCase): + @classmethod + def setUpClass(cls): + metrics_api._METER_PROVIDER = None + set_meter_provider(metrics.MeterProvider()) + + @classmethod + def tearDownClass(cls): + metrics_api._METER_PROVIDER = None + + def test_ctor(self): + meter = metrics_api.get_meter(__name__) + 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.assertEqual( + recorder._duration.description, + "measures the duration of the outbound HTTP request", + ) + + def test_record_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): + labels["test2"] = "asd2" + match_key = get_dict_as_key({"test": "asd", "test2": "asd2"}) + for key in recorder._duration.bound_instruments.keys(): + self.assertEqual(key, match_key) + # pylint: disable=protected-access + bound = recorder._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) diff --git a/tox.ini b/tox.ini index b477d5ba740..806656ced2c 100644 --- a/tox.ini +++ b/tox.ini @@ -396,8 +396,8 @@ deps = commands_pre = pip install -e {toxinidir}/opentelemetry-api \ - -e {toxinidir}/opentelemetry-instrumentation \ -e {toxinidir}/opentelemetry-sdk \ + -e {toxinidir}/opentelemetry-instrumentation \ -e {toxinidir}/instrumentation/opentelemetry-instrumentation-requests \ -e {toxinidir}/instrumentation/opentelemetry-instrumentation-wsgi \ -e {toxinidir}/instrumentation/opentelemetry-instrumentation-flask