From bbbe3ab7abe95b8a1689eafed9880b8d100321ee Mon Sep 17 00:00:00 2001 From: mspillane <37798417+mspillane@users.noreply.github.com> Date: Thu, 11 Jul 2024 17:18:48 +0100 Subject: [PATCH 01/26] Add NoOpTracerProvider test for starlette (#2654) --- .../tests/test_starlette_instrumentation.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/instrumentation/opentelemetry-instrumentation-starlette/tests/test_starlette_instrumentation.py b/instrumentation/opentelemetry-instrumentation-starlette/tests/test_starlette_instrumentation.py index eed1a75c44..3f9f1c7b0f 100644 --- a/instrumentation/opentelemetry-instrumentation-starlette/tests/test_starlette_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-starlette/tests/test_starlette_instrumentation.py @@ -398,6 +398,21 @@ def test_uninstrument(self): spans = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans), 0) + def test_no_op_tracer_provider(self): + self._client.get("/foobar") + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 3) + + self.memory_exporter.clear() + self._instrumentor.uninstrument() + + tracer_provider = NoOpTracerProvider() + self._instrumentor.instrument(tracer_provider=tracer_provider) + + self._client.get("/foobar") + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 0) + def test_sub_app_starlette_call(self): """ !!! Attention: we need to override this testcase for the auto-instrumented variant From 43dfc73c4c7cc1075739f341fbb56044e65ee0ee Mon Sep 17 00:00:00 2001 From: Zhihan Li <54661071+zhihali@users.noreply.github.com> Date: Thu, 11 Jul 2024 17:34:59 +0100 Subject: [PATCH 02/26] add sync and async test guide at contributing.md (#2694) --- CONTRIBUTING.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d7fc509c3a..8ab75ce1c6 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -269,6 +269,9 @@ Below is a checklist of things to be mindful of when implementing a new instrume - ex. - Appropriate error handling - ex. +- Isolate sync and async test + - For synchronous tests, the typical test case class is inherited from `opentelemetry.test.test_base.TestBase`. However, if you want to write asynchronous tests, the test case class should inherit also from `IsolatedAsyncioTestCase`. Adding asynchronous tests to a common test class can lead to tests passing without actually running, which can be misleading. + - ex. ## Expectations from contributors From b697f4ab9a3cf648ae004ef743b6e4564eaae6a2 Mon Sep 17 00:00:00 2001 From: Daniel Hochman Date: Fri, 12 Jul 2024 11:38:40 -0500 Subject: [PATCH 03/26] opentelemetry-instrumentation-asgi: always set status code on duration attributes (#2627) --- CHANGELOG.md | 2 + .../instrumentation/asgi/__init__.py | 50 +++++++-------- .../tests/test_asgi_middleware.py | 63 ++++++++++++++++++- .../opentelemetry/instrumentation/_semconv.py | 32 ++++++---- 4 files changed, 104 insertions(+), 43 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3eddf58cb8..76d77f06e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -61,6 +61,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#2153](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2153)) - `opentelemetry-instrumentation-asgi` Removed `NET_HOST_NAME` AND `NET_HOST_PORT` from active requests count attribute ([#2610](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2610)) +- `opentelemetry-instrumentation-asgi` Bugfix: Middleware did not set status code attribute on duration metrics for non-recording spans. + ([#2627](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2627)) ## Version 1.25.0/0.46b0 (2024-05-31) diff --git a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py index f006f9b0c9..7e51837d4d 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -438,8 +438,6 @@ def set_status_code( sem_conv_opt_in_mode=_HTTPStabilityMode.DEFAULT, ): """Adds HTTP response attributes to span using the status_code argument.""" - if not span.is_recording(): - return status_code_str = str(status_code) try: @@ -836,36 +834,16 @@ async def otel_send(message: dict[str, Any]): ) as send_span: if callable(self.client_response_hook): self.client_response_hook(send_span, scope, message) + + status_code = None + if message["type"] == "http.response.start": + status_code = message["status"] + elif message["type"] == "websocket.send": + status_code = 200 + if send_span.is_recording(): if message["type"] == "http.response.start": - status_code = message["status"] - # We record metrics only once - set_status_code( - server_span, - status_code, - duration_attrs, - self._sem_conv_opt_in_mode, - ) - set_status_code( - send_span, - status_code, - None, - self._sem_conv_opt_in_mode, - ) expecting_trailers = message.get("trailers", False) - elif message["type"] == "websocket.send": - set_status_code( - server_span, - 200, - duration_attrs, - self._sem_conv_opt_in_mode, - ) - set_status_code( - send_span, - 200, - None, - self._sem_conv_opt_in_mode, - ) send_span.set_attribute("asgi.event.type", message["type"]) if ( server_span.is_recording() @@ -886,6 +864,20 @@ async def otel_send(message: dict[str, Any]): server_span.set_attributes( custom_response_attributes ) + if status_code: + # We record metrics only once + set_status_code( + server_span, + status_code, + duration_attrs, + self._sem_conv_opt_in_mode, + ) + set_status_code( + send_span, + status_code, + None, + self._sem_conv_opt_in_mode, + ) propagator = get_global_response_propagator() if propagator: diff --git a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py index ff266cb5bf..5bb04adb25 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py @@ -514,7 +514,9 @@ def test_asgi_not_recording(self): mock_span = mock.Mock() mock_span.is_recording.return_value = False mock_tracer.start_as_current_span.return_value = mock_span - mock_tracer.start_as_current_span.return_value.__enter__ = mock_span + mock_tracer.start_as_current_span.return_value.__enter__ = mock.Mock( + return_value=mock_span + ) mock_tracer.start_as_current_span.return_value.__exit__ = mock_span with mock.patch("opentelemetry.trace.get_tracer") as tracer: tracer.return_value = mock_tracer @@ -1342,6 +1344,65 @@ def test_basic_metric_success(self): ) self.assertEqual(point.value, 0) + def test_basic_metric_success_nonrecording_span(self): + mock_tracer = mock.Mock() + mock_span = mock.Mock() + mock_span.is_recording.return_value = False + mock_tracer.start_as_current_span.return_value = mock_span + mock_tracer.start_as_current_span.return_value.__enter__ = mock.Mock( + return_value=mock_span + ) + mock_tracer.start_as_current_span.return_value.__exit__ = mock_span + with mock.patch("opentelemetry.trace.get_tracer") as tracer: + tracer.return_value = mock_tracer + app = otel_asgi.OpenTelemetryMiddleware(simple_asgi) + self.seed_app(app) + start = default_timer() + self.send_default_request() + duration = max(round((default_timer() - start) * 1000), 0) + expected_duration_attributes = { + "http.method": "GET", + "http.host": "127.0.0.1", + "http.scheme": "http", + "http.flavor": "1.0", + "net.host.port": 80, + "http.status_code": 200, + } + expected_requests_count_attributes = { + "http.method": "GET", + "http.host": "127.0.0.1", + "http.scheme": "http", + "http.flavor": "1.0", + } + metrics_list = self.memory_metrics_reader.get_metrics_data() + # pylint: disable=too-many-nested-blocks + for resource_metric in metrics_list.resource_metrics: + for scope_metrics in resource_metric.scope_metrics: + for metric in scope_metrics.metrics: + for point in list(metric.data.data_points): + if isinstance(point, HistogramDataPoint): + self.assertDictEqual( + expected_duration_attributes, + dict(point.attributes), + ) + self.assertEqual(point.count, 1) + if metric.name == "http.server.duration": + self.assertAlmostEqual( + duration, point.sum, delta=5 + ) + elif ( + metric.name == "http.server.response.size" + ): + self.assertEqual(1024, point.sum) + elif metric.name == "http.server.request.size": + self.assertEqual(128, point.sum) + elif isinstance(point, NumberDataPoint): + self.assertDictEqual( + expected_requests_count_attributes, + dict(point.attributes), + ) + self.assertEqual(point.value, 0) + def test_basic_metric_success_new_semconv(self): app = otel_asgi.OpenTelemetryMiddleware(simple_asgi) self.seed_app(app) diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py index 85b8e2e3ec..7e81692a98 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py @@ -355,29 +355,35 @@ def _set_status( sem_conv_opt_in_mode, ): if status_code < 0: - if _report_new(sem_conv_opt_in_mode): - span.set_attribute(ERROR_TYPE, status_code_str) - metrics_attributes[ERROR_TYPE] = status_code_str - - span.set_status( - Status( - StatusCode.ERROR, - "Non-integer HTTP status: " + status_code_str, + metrics_attributes[ERROR_TYPE] = status_code_str + if span.is_recording(): + if _report_new(sem_conv_opt_in_mode): + span.set_attribute(ERROR_TYPE, status_code_str) + span.set_status( + Status( + StatusCode.ERROR, + "Non-integer HTTP status: " + status_code_str, + ) ) - ) else: status = http_status_to_status_code(status_code, server_span=True) if _report_old(sem_conv_opt_in_mode): - span.set_attribute(SpanAttributes.HTTP_STATUS_CODE, status_code) + if span.is_recording(): + span.set_attribute( + SpanAttributes.HTTP_STATUS_CODE, status_code + ) metrics_attributes[SpanAttributes.HTTP_STATUS_CODE] = status_code if _report_new(sem_conv_opt_in_mode): - span.set_attribute(HTTP_RESPONSE_STATUS_CODE, status_code) + if span.is_recording(): + span.set_attribute(HTTP_RESPONSE_STATUS_CODE, status_code) metrics_attributes[HTTP_RESPONSE_STATUS_CODE] = status_code if status == StatusCode.ERROR: - span.set_attribute(ERROR_TYPE, status_code_str) + if span.is_recording(): + span.set_attribute(ERROR_TYPE, status_code_str) metrics_attributes[ERROR_TYPE] = status_code_str - span.set_status(Status(status)) + if span.is_recording(): + span.set_status(Status(status)) # Get schema version based off of opt-in mode From 5a2794692052deae7b70f66a95dbaafbe6dcba6c Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Fri, 12 Jul 2024 10:49:48 -0700 Subject: [PATCH 04/26] =?UTF-8?q?Add=20Em=C3=ADdio=20Neto=20as=20an=20appr?= =?UTF-8?q?over=20(#2703)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 6c019239ae..dcb2ebbba5 100644 --- a/README.md +++ b/README.md @@ -110,6 +110,7 @@ Meeting notes are available as a public [Google doc](https://docs.google.com/doc Approvers ([@open-telemetry/python-approvers](https://github.com/orgs/open-telemetry/teams/python-approvers)): - [Aaron Abbott](https://github.com/aabmass), Google +- [Emídio Neto](https://github.com/emdneto), Zenvia - [Jeremy Voss](https://github.com/jeremydvoss), Microsoft - [Owais Lone](https://github.com/owais), Splunk - [Pablo Collins](https://github.com/pmcollins), Splunk From 6293d6a991d283b6d754d4736916db2d60ce8e5c Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Fri, 12 Jul 2024 11:48:04 -0700 Subject: [PATCH 05/26] HTTP semantic convention stability migration for fastapi (#2682) --- CHANGELOG.md | 7 +- instrumentation/README.md | 2 +- .../instrumentation/fastapi/__init__.py | 34 +- .../instrumentation/fastapi/package.py | 2 + .../tests/test_fastapi_instrumentation.py | 514 +++++++++++++++++- .../instrumentation/flask/__init__.py | 8 +- 6 files changed, 527 insertions(+), 40 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 76d77f06e0..b6882cbe57 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#2638](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2638)) - `opentelemetry-instrumentation-asgi` Implement new semantic convention opt-in with stable http semantic conventions ([#2610](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2610)) +- `opentelemetry-instrumentation-fastapi` Implement new semantic convention opt-in with stable http semantic conventions + ([#2682](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2682)) - `opentelemetry-instrumentation-httpx` Implement new semantic convention opt-in migration with stable http semantic conventions ([#2631](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2631)) - `opentelemetry-instrumentation-system-metrics` Permit to use psutil 6.0+. @@ -32,9 +34,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `opentelemetry-instrumentation-asgi`, `opentelemetry-instrumentation-fastapi`, `opentelemetry-instrumentation-starlette` Use `tracer` and `meter` of originating components instead of one from `asgi` middleware ([#2580](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2580)) -- Populate `{method}` as `HTTP` on `_OTHER` methods from scope +- Populate `{method}` as `HTTP` on `_OTHER` methods from scope for `asgi` middleware ([#2610](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2610)) - + - Populate `{method}` as `HTTP` on `_OTHER` methods from scope for `fastapi` middleware + ([#2682](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2682)) ### Fixed diff --git a/instrumentation/README.md b/instrumentation/README.md index eb21717843..9add6d0228 100644 --- a/instrumentation/README.md +++ b/instrumentation/README.md @@ -19,7 +19,7 @@ | [opentelemetry-instrumentation-django](./opentelemetry-instrumentation-django) | django >= 1.10 | Yes | experimental | [opentelemetry-instrumentation-elasticsearch](./opentelemetry-instrumentation-elasticsearch) | elasticsearch >= 6.0 | No | experimental | [opentelemetry-instrumentation-falcon](./opentelemetry-instrumentation-falcon) | falcon >= 1.4.1, < 4.0.0 | Yes | experimental -| [opentelemetry-instrumentation-fastapi](./opentelemetry-instrumentation-fastapi) | fastapi ~= 0.58 | Yes | experimental +| [opentelemetry-instrumentation-fastapi](./opentelemetry-instrumentation-fastapi) | fastapi ~= 0.58 | Yes | migration | [opentelemetry-instrumentation-flask](./opentelemetry-instrumentation-flask) | flask >= 1.0 | Yes | migration | [opentelemetry-instrumentation-grpc](./opentelemetry-instrumentation-grpc) | grpcio ~= 1.27 | No | experimental | [opentelemetry-instrumentation-httpx](./opentelemetry-instrumentation-httpx) | httpx >= 0.18.0 | No | migration diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py index 4c673d214a..2f9bbe7314 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py @@ -177,6 +177,12 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A import fastapi from starlette.routing import Match +from opentelemetry.instrumentation._semconv import ( + _get_schema_url, + _HTTPStabilityMode, + _OpenTelemetrySemanticConventionStability, + _OpenTelemetryStabilitySignalType, +) from opentelemetry.instrumentation.asgi import OpenTelemetryMiddleware from opentelemetry.instrumentation.asgi.types import ( ClientRequestHook, @@ -189,7 +195,11 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A from opentelemetry.metrics import get_meter from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.trace import get_tracer -from opentelemetry.util.http import get_excluded_urls, parse_excluded_urls +from opentelemetry.util.http import ( + get_excluded_urls, + parse_excluded_urls, + sanitize_method, +) _excluded_urls_from_env = get_excluded_urls("FASTAPI") _logger = logging.getLogger(__name__) @@ -218,6 +228,11 @@ def instrument_app( app._is_instrumented_by_opentelemetry = False if not getattr(app, "_is_instrumented_by_opentelemetry", False): + # initialize semantic conventions opt-in if needed + _OpenTelemetrySemanticConventionStability._initialize() + sem_conv_opt_in_mode = _OpenTelemetrySemanticConventionStability._get_opentelemetry_stability_opt_in_mode( + _OpenTelemetryStabilitySignalType.HTTP, + ) if excluded_urls is None: excluded_urls = _excluded_urls_from_env else: @@ -226,13 +241,13 @@ def instrument_app( __name__, __version__, tracer_provider, - schema_url="https://opentelemetry.io/schemas/1.11.0", + schema_url=_get_schema_url(sem_conv_opt_in_mode), ) meter = get_meter( __name__, __version__, meter_provider, - schema_url="https://opentelemetry.io/schemas/1.11.0", + schema_url=_get_schema_url(sem_conv_opt_in_mode), ) app.add_middleware( @@ -303,6 +318,7 @@ class _InstrumentedFastAPI(fastapi.FastAPI): _client_request_hook: ClientRequestHook = None _client_response_hook: ClientResponseHook = None _instrumented_fastapi_apps = set() + _sem_conv_opt_in_mode = _HTTPStabilityMode.DEFAULT def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) @@ -310,13 +326,17 @@ def __init__(self, *args, **kwargs): __name__, __version__, _InstrumentedFastAPI._tracer_provider, - schema_url="https://opentelemetry.io/schemas/1.11.0", + schema_url=_get_schema_url( + _InstrumentedFastAPI._sem_conv_opt_in_mode + ), ) meter = get_meter( __name__, __version__, _InstrumentedFastAPI._meter_provider, - schema_url="https://opentelemetry.io/schemas/1.11.0", + schema_url=_get_schema_url( + _InstrumentedFastAPI._sem_conv_opt_in_mode + ), ) self.add_middleware( OpenTelemetryMiddleware, @@ -373,8 +393,10 @@ def _get_default_span_details(scope): A tuple of span name and attributes """ route = _get_route_details(scope) - method = scope.get("method", "") + method = sanitize_method(scope.get("method", "").strip()) attributes = {} + if method == "_OTHER": + method = "HTTP" if route: attributes[SpanAttributes.HTTP_ROUTE] = route if method and route: # http diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/package.py b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/package.py index 8df84fc931..d95a2cf6d5 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/package.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/package.py @@ -16,3 +16,5 @@ _instruments = ("fastapi ~= 0.58",) _supports_metrics = True + +_semconv_status = "migration" diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index 0ad63164d5..69c5d312e5 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -11,6 +11,9 @@ # 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. + +# pylint: disable=too-many-lines + import unittest from timeit import default_timer from unittest.mock import patch @@ -20,39 +23,77 @@ from fastapi.testclient import TestClient import opentelemetry.instrumentation.fastapi as otel_fastapi +from opentelemetry.instrumentation._semconv import ( + OTEL_SEMCONV_STABILITY_OPT_IN, + _OpenTelemetrySemanticConventionStability, + _server_active_requests_count_attrs_new, + _server_active_requests_count_attrs_old, + _server_duration_attrs_new, + _server_duration_attrs_old, +) from opentelemetry.instrumentation.asgi import OpenTelemetryMiddleware from opentelemetry.sdk.metrics.export import ( HistogramDataPoint, NumberDataPoint, ) from opentelemetry.sdk.resources import Resource +from opentelemetry.semconv.attributes.http_attributes import ( + HTTP_REQUEST_METHOD, + HTTP_RESPONSE_STATUS_CODE, + HTTP_ROUTE, +) +from opentelemetry.semconv.attributes.network_attributes import ( + NETWORK_PROTOCOL_VERSION, +) +from opentelemetry.semconv.attributes.url_attributes import URL_SCHEME from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.test.test_base import TestBase -from opentelemetry.util.http import ( - _active_requests_count_attrs, - _duration_attrs, - get_excluded_urls, -) +from opentelemetry.util.http import get_excluded_urls -_expected_metric_names = [ +_expected_metric_names_old = [ "http.server.active_requests", "http.server.duration", "http.server.response.size", "http.server.request.size", ] -_recommended_attrs = { - "http.server.active_requests": _active_requests_count_attrs, - "http.server.duration": {*_duration_attrs, SpanAttributes.HTTP_TARGET}, +_expected_metric_names_new = [ + "http.server.active_requests", + "http.server.request.duration", + "http.server.response.body.size", + "http.server.request.body.size", +] +_expected_metric_names_both = _expected_metric_names_old +_expected_metric_names_both.extend(_expected_metric_names_new) + +_recommended_attrs_old = { + "http.server.active_requests": _server_active_requests_count_attrs_old, + "http.server.duration": { + *_server_duration_attrs_old, + SpanAttributes.HTTP_TARGET, + }, "http.server.response.size": { - *_duration_attrs, + *_server_duration_attrs_old, SpanAttributes.HTTP_TARGET, }, "http.server.request.size": { - *_duration_attrs, + *_server_duration_attrs_old, SpanAttributes.HTTP_TARGET, }, } +_recommended_attrs_new = { + "http.server.active_requests": _server_active_requests_count_attrs_new, + "http.server.request.duration": _server_duration_attrs_new, + "http.server.response.body.size": _server_duration_attrs_new, + "http.server.request.body.size": _server_duration_attrs_new, +} + +_recommended_attrs_both = _recommended_attrs_old.copy() +_recommended_attrs_both.update(_recommended_attrs_new) +_recommended_attrs_both["http.server.active_requests"].extend( + _server_active_requests_count_attrs_old +) + class TestBaseFastAPI(TestBase): def _create_app(self): @@ -88,10 +129,23 @@ def setUpClass(cls): def setUp(self): super().setUp() + + test_name = "" + if hasattr(self, "_testMethodName"): + test_name = self._testMethodName + sem_conv_mode = "default" + if "new_semconv" in test_name: + sem_conv_mode = "http" + elif "both_semconv" in test_name: + sem_conv_mode = "http/dup" self.env_patch = patch.dict( "os.environ", - {"OTEL_PYTHON_FASTAPI_EXCLUDED_URLS": "/exclude/123,healthzz"}, + { + "OTEL_PYTHON_FASTAPI_EXCLUDED_URLS": "/exclude/123,healthzz", + OTEL_SEMCONV_STABILITY_OPT_IN: sem_conv_mode, + }, ) + _OpenTelemetrySemanticConventionStability._initialized = False self.env_patch.start() self.exclude_patch = patch( "opentelemetry.instrumentation.fastapi._excluded_urls_from_env", @@ -142,7 +196,6 @@ async def _(): class TestBaseManualFastAPI(TestBaseFastAPI): - @classmethod def setUpClass(cls): if cls is TestBaseManualFastAPI: @@ -196,7 +249,6 @@ def test_sub_app_fastapi_call(self): class TestBaseAutoFastAPI(TestBaseFastAPI): - @classmethod def setUpClass(cls): if cls is TestBaseAutoFastAPI: @@ -259,6 +311,7 @@ def test_sub_app_fastapi_call(self): ) +# pylint: disable=too-many-public-methods class TestFastAPIManualInstrumentation(TestBaseManualFastAPI): def test_instrument_app_with_instrument(self): if not isinstance(self, TestAutoInstrumentation): @@ -358,7 +411,71 @@ def test_fastapi_metrics(self): ) self.assertTrue(len(scope_metric.metrics) == 3) for metric in scope_metric.metrics: - self.assertIn(metric.name, _expected_metric_names) + self.assertIn(metric.name, _expected_metric_names_old) + data_points = list(metric.data.data_points) + self.assertEqual(len(data_points), 1) + for point in data_points: + if isinstance(point, HistogramDataPoint): + self.assertEqual(point.count, 3) + histogram_data_point_seen = True + if isinstance(point, NumberDataPoint): + number_data_point_seen = True + for attr in point.attributes: + self.assertIn( + attr, _recommended_attrs_old[metric.name] + ) + self.assertTrue(number_data_point_seen and histogram_data_point_seen) + + def test_fastapi_metrics_new_semconv(self): + self._client.get("/foobar") + self._client.get("/foobar") + self._client.get("/foobar") + metrics_list = self.memory_metrics_reader.get_metrics_data() + number_data_point_seen = False + histogram_data_point_seen = False + self.assertTrue(len(metrics_list.resource_metrics) == 1) + for resource_metric in metrics_list.resource_metrics: + self.assertTrue(len(resource_metric.scope_metrics) == 1) + for scope_metric in resource_metric.scope_metrics: + self.assertEqual( + scope_metric.scope.name, + "opentelemetry.instrumentation.fastapi", + ) + self.assertTrue(len(scope_metric.metrics) == 3) + for metric in scope_metric.metrics: + self.assertIn(metric.name, _expected_metric_names_new) + data_points = list(metric.data.data_points) + self.assertEqual(len(data_points), 1) + for point in data_points: + if isinstance(point, HistogramDataPoint): + self.assertEqual(point.count, 3) + histogram_data_point_seen = True + if isinstance(point, NumberDataPoint): + number_data_point_seen = True + for attr in point.attributes: + self.assertIn( + attr, _recommended_attrs_new[metric.name] + ) + self.assertTrue(number_data_point_seen and histogram_data_point_seen) + + def test_fastapi_metrics_both_semconv(self): + self._client.get("/foobar") + self._client.get("/foobar") + self._client.get("/foobar") + metrics_list = self.memory_metrics_reader.get_metrics_data() + number_data_point_seen = False + histogram_data_point_seen = False + self.assertTrue(len(metrics_list.resource_metrics) == 1) + for resource_metric in metrics_list.resource_metrics: + self.assertTrue(len(resource_metric.scope_metrics) == 1) + for scope_metric in resource_metric.scope_metrics: + self.assertEqual( + scope_metric.scope.name, + "opentelemetry.instrumentation.fastapi", + ) + self.assertTrue(len(scope_metric.metrics) == 5) + for metric in scope_metric.metrics: + self.assertIn(metric.name, _expected_metric_names_both) data_points = list(metric.data.data_points) self.assertEqual(len(data_points), 1) for point in data_points: @@ -369,7 +486,7 @@ def test_fastapi_metrics(self): number_data_point_seen = True for attr in point.attributes: self.assertIn( - attr, _recommended_attrs[metric.name] + attr, _recommended_attrs_both[metric.name] ) self.assertTrue(number_data_point_seen and histogram_data_point_seen) @@ -378,21 +495,216 @@ def test_basic_metric_success(self): self._client.get("/foobar") duration = max(round((default_timer() - start) * 1000), 0) expected_duration_attributes = { - "http.method": "GET", - "http.host": "testserver:443", - "http.scheme": "https", - "http.flavor": "1.1", - "http.server_name": "testserver", - "net.host.port": 443, - "http.status_code": 200, - "http.target": "/foobar", + SpanAttributes.HTTP_METHOD: "GET", + SpanAttributes.HTTP_HOST: "testserver:443", + SpanAttributes.HTTP_SCHEME: "https", + SpanAttributes.HTTP_FLAVOR: "1.1", + SpanAttributes.HTTP_SERVER_NAME: "testserver", + SpanAttributes.NET_HOST_PORT: 443, + SpanAttributes.HTTP_STATUS_CODE: 200, + SpanAttributes.HTTP_TARGET: "/foobar", + } + expected_requests_count_attributes = { + SpanAttributes.HTTP_METHOD: "GET", + SpanAttributes.HTTP_HOST: "testserver:443", + SpanAttributes.HTTP_SCHEME: "https", + SpanAttributes.HTTP_FLAVOR: "1.1", + SpanAttributes.HTTP_SERVER_NAME: "testserver", + } + metrics_list = self.memory_metrics_reader.get_metrics_data() + for metric in ( + metrics_list.resource_metrics[0].scope_metrics[0].metrics + ): + for point in list(metric.data.data_points): + if isinstance(point, HistogramDataPoint): + self.assertDictEqual( + expected_duration_attributes, + dict(point.attributes), + ) + self.assertEqual(point.count, 1) + self.assertAlmostEqual(duration, point.sum, delta=40) + if isinstance(point, NumberDataPoint): + self.assertDictEqual( + expected_requests_count_attributes, + dict(point.attributes), + ) + self.assertEqual(point.value, 0) + + def test_basic_metric_success_new_semconv(self): + start = default_timer() + self._client.get("/foobar") + duration_s = max(default_timer() - start, 0) + expected_duration_attributes = { + HTTP_REQUEST_METHOD: "GET", + URL_SCHEME: "https", + NETWORK_PROTOCOL_VERSION: "1.1", + HTTP_RESPONSE_STATUS_CODE: 200, + HTTP_ROUTE: "/foobar", + } + expected_requests_count_attributes = { + HTTP_REQUEST_METHOD: "GET", + URL_SCHEME: "https", + } + metrics_list = self.memory_metrics_reader.get_metrics_data() + for metric in ( + metrics_list.resource_metrics[0].scope_metrics[0].metrics + ): + for point in list(metric.data.data_points): + if isinstance(point, HistogramDataPoint): + self.assertDictEqual( + expected_duration_attributes, + dict(point.attributes), + ) + self.assertEqual(point.count, 1) + if metric.name == "http.server.request.duration": + self.assertAlmostEqual(duration_s, point.sum, places=1) + elif metric.name == "http.server.response.body.size": + self.assertEqual(25, point.sum) + elif metric.name == "http.server.request.body.size": + self.assertEqual(25, point.sum) + if isinstance(point, NumberDataPoint): + self.assertDictEqual( + expected_requests_count_attributes, + dict(point.attributes), + ) + self.assertEqual(point.value, 0) + + def test_basic_metric_success_both_semconv(self): + start = default_timer() + self._client.get("/foobar") + duration = max(round((default_timer() - start) * 1000), 0) + duration_s = max(default_timer() - start, 0) + expected_duration_attributes_old = { + SpanAttributes.HTTP_METHOD: "GET", + SpanAttributes.HTTP_HOST: "testserver:443", + SpanAttributes.HTTP_SCHEME: "https", + SpanAttributes.HTTP_FLAVOR: "1.1", + SpanAttributes.HTTP_SERVER_NAME: "testserver", + SpanAttributes.NET_HOST_PORT: 443, + SpanAttributes.HTTP_STATUS_CODE: 200, + SpanAttributes.HTTP_TARGET: "/foobar", + } + expected_duration_attributes_new = { + HTTP_REQUEST_METHOD: "GET", + URL_SCHEME: "https", + NETWORK_PROTOCOL_VERSION: "1.1", + HTTP_RESPONSE_STATUS_CODE: 200, + HTTP_ROUTE: "/foobar", + } + expected_requests_count_attributes = { + SpanAttributes.HTTP_METHOD: "GET", + SpanAttributes.HTTP_HOST: "testserver:443", + SpanAttributes.HTTP_SCHEME: "https", + SpanAttributes.HTTP_FLAVOR: "1.1", + SpanAttributes.HTTP_SERVER_NAME: "testserver", + HTTP_REQUEST_METHOD: "GET", + URL_SCHEME: "https", + } + metrics_list = self.memory_metrics_reader.get_metrics_data() + for metric in ( + metrics_list.resource_metrics[0].scope_metrics[0].metrics + ): + for point in list(metric.data.data_points): + if isinstance(point, HistogramDataPoint): + self.assertEqual(point.count, 1) + self.assertAlmostEqual(duration, point.sum, delta=40) + if metric.name == "http.server.request.duration": + self.assertAlmostEqual(duration_s, point.sum, places=1) + self.assertDictEqual( + expected_duration_attributes_new, + dict(point.attributes), + ) + elif metric.name == "http.server.response.body.size": + self.assertEqual(25, point.sum) + self.assertDictEqual( + expected_duration_attributes_new, + dict(point.attributes), + ) + elif metric.name == "http.server.request.body.size": + self.assertEqual(25, point.sum) + self.assertDictEqual( + expected_duration_attributes_new, + dict(point.attributes), + ) + elif metric.name == "http.server.duration": + self.assertAlmostEqual(duration, point.sum, delta=40) + self.assertDictEqual( + expected_duration_attributes_old, + dict(point.attributes), + ) + elif metric.name == "http.server.response.size": + self.assertEqual(25, point.sum) + self.assertDictEqual( + expected_duration_attributes_old, + dict(point.attributes), + ) + elif metric.name == "http.server.request.size": + self.assertEqual(25, point.sum) + self.assertDictEqual( + expected_duration_attributes_old, + dict(point.attributes), + ) + if isinstance(point, NumberDataPoint): + self.assertDictEqual( + expected_requests_count_attributes, + dict(point.attributes), + ) + self.assertEqual(point.value, 0) + + def test_basic_metric_nonstandard_http_method_success(self): + start = default_timer() + self._client.request("NONSTANDARD", "/foobar") + duration = max(round((default_timer() - start) * 1000), 0) + expected_duration_attributes = { + SpanAttributes.HTTP_METHOD: "_OTHER", + SpanAttributes.HTTP_HOST: "testserver:443", + SpanAttributes.HTTP_SCHEME: "https", + SpanAttributes.HTTP_FLAVOR: "1.1", + SpanAttributes.HTTP_SERVER_NAME: "testserver", + SpanAttributes.NET_HOST_PORT: 443, + SpanAttributes.HTTP_STATUS_CODE: 405, + SpanAttributes.HTTP_TARGET: "/foobar", + } + expected_requests_count_attributes = { + SpanAttributes.HTTP_METHOD: "_OTHER", + SpanAttributes.HTTP_HOST: "testserver:443", + SpanAttributes.HTTP_SCHEME: "https", + SpanAttributes.HTTP_FLAVOR: "1.1", + SpanAttributes.HTTP_SERVER_NAME: "testserver", + } + metrics_list = self.memory_metrics_reader.get_metrics_data() + for metric in ( + metrics_list.resource_metrics[0].scope_metrics[0].metrics + ): + for point in list(metric.data.data_points): + if isinstance(point, HistogramDataPoint): + self.assertDictEqual( + expected_duration_attributes, + dict(point.attributes), + ) + self.assertEqual(point.count, 1) + self.assertAlmostEqual(duration, point.sum, delta=40) + if isinstance(point, NumberDataPoint): + self.assertDictEqual( + expected_requests_count_attributes, + dict(point.attributes), + ) + self.assertEqual(point.value, 0) + + def test_basic_metric_nonstandard_http_method_success_new_semconv(self): + start = default_timer() + self._client.request("NONSTANDARD", "/foobar") + duration_s = max(default_timer() - start, 0) + expected_duration_attributes = { + HTTP_REQUEST_METHOD: "_OTHER", + URL_SCHEME: "https", + NETWORK_PROTOCOL_VERSION: "1.1", + HTTP_RESPONSE_STATUS_CODE: 405, + HTTP_ROUTE: "/foobar", } expected_requests_count_attributes = { - "http.method": "GET", - "http.host": "testserver:443", - "http.scheme": "https", - "http.flavor": "1.1", - "http.server_name": "testserver", + HTTP_REQUEST_METHOD: "_OTHER", + URL_SCHEME: "https", } metrics_list = self.memory_metrics_reader.get_metrics_data() for metric in ( @@ -404,8 +716,95 @@ def test_basic_metric_success(self): expected_duration_attributes, dict(point.attributes), ) + self.assertEqual(point.count, 1) + if metric.name == "http.server.request.duration": + self.assertAlmostEqual(duration_s, point.sum, places=1) + elif metric.name == "http.server.response.body.size": + self.assertEqual(31, point.sum) + elif metric.name == "http.server.request.body.size": + self.assertEqual(25, point.sum) + if isinstance(point, NumberDataPoint): + self.assertDictEqual( + expected_requests_count_attributes, + dict(point.attributes), + ) + self.assertEqual(point.value, 0) + + def test_basic_metric_nonstandard_http_method_success_both_semconv(self): + start = default_timer() + self._client.request("NONSTANDARD", "/foobar") + duration = max(round((default_timer() - start) * 1000), 0) + duration_s = max(default_timer() - start, 0) + expected_duration_attributes_old = { + SpanAttributes.HTTP_METHOD: "_OTHER", + SpanAttributes.HTTP_HOST: "testserver:443", + SpanAttributes.HTTP_SCHEME: "https", + SpanAttributes.HTTP_FLAVOR: "1.1", + SpanAttributes.HTTP_SERVER_NAME: "testserver", + SpanAttributes.NET_HOST_PORT: 443, + SpanAttributes.HTTP_STATUS_CODE: 405, + SpanAttributes.HTTP_TARGET: "/foobar", + } + expected_duration_attributes_new = { + HTTP_REQUEST_METHOD: "_OTHER", + URL_SCHEME: "https", + NETWORK_PROTOCOL_VERSION: "1.1", + HTTP_RESPONSE_STATUS_CODE: 405, + HTTP_ROUTE: "/foobar", + } + expected_requests_count_attributes = { + SpanAttributes.HTTP_METHOD: "_OTHER", + SpanAttributes.HTTP_HOST: "testserver:443", + SpanAttributes.HTTP_SCHEME: "https", + SpanAttributes.HTTP_FLAVOR: "1.1", + SpanAttributes.HTTP_SERVER_NAME: "testserver", + HTTP_REQUEST_METHOD: "_OTHER", + URL_SCHEME: "https", + } + metrics_list = self.memory_metrics_reader.get_metrics_data() + for metric in ( + metrics_list.resource_metrics[0].scope_metrics[0].metrics + ): + for point in list(metric.data.data_points): + if isinstance(point, HistogramDataPoint): self.assertEqual(point.count, 1) self.assertAlmostEqual(duration, point.sum, delta=40) + if metric.name == "http.server.request.duration": + self.assertAlmostEqual(duration_s, point.sum, places=1) + self.assertDictEqual( + expected_duration_attributes_new, + dict(point.attributes), + ) + elif metric.name == "http.server.response.body.size": + self.assertEqual(31, point.sum) + self.assertDictEqual( + expected_duration_attributes_new, + dict(point.attributes), + ) + elif metric.name == "http.server.request.body.size": + self.assertEqual(25, point.sum) + self.assertDictEqual( + expected_duration_attributes_new, + dict(point.attributes), + ) + elif metric.name == "http.server.duration": + self.assertAlmostEqual(duration, point.sum, delta=40) + self.assertDictEqual( + expected_duration_attributes_old, + dict(point.attributes), + ) + elif metric.name == "http.server.response.size": + self.assertEqual(31, point.sum) + self.assertDictEqual( + expected_duration_attributes_old, + dict(point.attributes), + ) + elif metric.name == "http.server.request.size": + self.assertEqual(25, point.sum) + self.assertDictEqual( + expected_duration_attributes_old, + dict(point.attributes), + ) if isinstance(point, NumberDataPoint): self.assertDictEqual( expected_requests_count_attributes, @@ -438,6 +837,63 @@ def test_basic_post_request_metric_success(self): if isinstance(point, NumberDataPoint): self.assertEqual(point.value, 0) + def test_basic_post_request_metric_success_new_semconv(self): + start = default_timer() + response = self._client.post( + "/foobar", + json={"foo": "bar"}, + ) + duration_s = max(default_timer() - start, 0) + response_size = int(response.headers.get("content-length")) + request_size = int(response.request.headers.get("content-length")) + metrics_list = self.memory_metrics_reader.get_metrics_data() + for metric in ( + metrics_list.resource_metrics[0].scope_metrics[0].metrics + ): + for point in list(metric.data.data_points): + if isinstance(point, HistogramDataPoint): + self.assertEqual(point.count, 1) + if metric.name == "http.server.request.duration": + self.assertAlmostEqual(duration_s, point.sum, places=1) + elif metric.name == "http.server.response.body.size": + self.assertEqual(response_size, point.sum) + elif metric.name == "http.server.request.body.size": + self.assertEqual(request_size, point.sum) + if isinstance(point, NumberDataPoint): + self.assertEqual(point.value, 0) + + def test_basic_post_request_metric_success_both_semconv(self): + start = default_timer() + response = self._client.post( + "/foobar", + json={"foo": "bar"}, + ) + duration = max(round((default_timer() - start) * 1000), 0) + duration_s = max(default_timer() - start, 0) + response_size = int(response.headers.get("content-length")) + request_size = int(response.request.headers.get("content-length")) + metrics_list = self.memory_metrics_reader.get_metrics_data() + for metric in ( + metrics_list.resource_metrics[0].scope_metrics[0].metrics + ): + for point in list(metric.data.data_points): + if isinstance(point, HistogramDataPoint): + self.assertEqual(point.count, 1) + if metric.name == "http.server.request.duration": + self.assertAlmostEqual(duration_s, point.sum, places=1) + elif metric.name == "http.server.response.body.size": + self.assertEqual(response_size, point.sum) + elif metric.name == "http.server.request.body.size": + self.assertEqual(request_size, point.sum) + elif metric.name == "http.server.duration": + self.assertAlmostEqual(duration, point.sum, delta=40) + elif metric.name == "http.server.response.size": + self.assertEqual(response_size, point.sum) + elif metric.name == "http.server.request.size": + self.assertEqual(request_size, point.sum) + if isinstance(point, NumberDataPoint): + self.assertEqual(point.value, 0) + def test_metric_uninstrument_app(self): self._client.get("/foobar") self._instrumentor.uninstrument_app(self._app) diff --git a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py index 34e9b5ea50..eaf6c79506 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py @@ -543,7 +543,9 @@ def __init__(self, *args, **kwargs): __name__, __version__, _InstrumentedFlask._meter_provider, - schema_url="https://opentelemetry.io/schemas/1.11.0", + schema_url=_get_schema_url( + _InstrumentedFlask._sem_conv_opt_in_mode + ), ) duration_histogram_old = None if _report_old(_InstrumentedFlask._sem_conv_opt_in_mode): @@ -579,7 +581,9 @@ def __init__(self, *args, **kwargs): __name__, __version__, _InstrumentedFlask._tracer_provider, - schema_url="https://opentelemetry.io/schemas/1.11.0", + schema_url=_get_schema_url( + _InstrumentedFlask._sem_conv_opt_in_mode + ), ) _before_request = _wrapped_before_request( From 15f3b97d5a49a3c616e3691af2b31d3f0013bb65 Mon Sep 17 00:00:00 2001 From: Gustavo Carvalho <99373133+gustavosett@users.noreply.github.com> Date: Fri, 12 Jul 2024 19:44:44 -0300 Subject: [PATCH 06/26] Improved Test Coverage for HTTP Utility's IP Setting Functionality (#2693) --- .../tests/test_try_set_ip.py | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 util/opentelemetry-util-http/tests/test_try_set_ip.py diff --git a/util/opentelemetry-util-http/tests/test_try_set_ip.py b/util/opentelemetry-util-http/tests/test_try_set_ip.py new file mode 100644 index 0000000000..c87bf990a6 --- /dev/null +++ b/util/opentelemetry-util-http/tests/test_try_set_ip.py @@ -0,0 +1,67 @@ +# 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. + +import logging +import socket +import unittest +from http.client import HTTPConnection +from unittest.mock import MagicMock, patch + +from opentelemetry.util.http.httplib import trysetip + + +class TestTrySetIP(unittest.TestCase): + def setUp(self): + # Setup a mock HTTPConnection + self.conn = MagicMock(spec=HTTPConnection) + self.conn.sock = MagicMock(spec=socket.socket) + + # Mock state function and Span class + self.mock_state = {"need_ip": [MagicMock()]} + self.mock_getstate = patch( + "opentelemetry.util.http.httplib._getstate", + return_value=self.mock_state, + ) + self.mock_getstate.start() + + def test_ip_set_successfully(self): + self.conn.sock.getpeername.return_value = ("192.168.1.1", 8080) + + success = trysetip(self.conn, loglevel=logging.DEBUG) + + # Verify that the IP was set correctly + for span in self.mock_state["need_ip"]: + span.set_attribute.assert_called_once_with( + "net.peer.ip", "192.168.1.1" + ) + self.assertTrue(success) + + def test_no_socket_connection(self): + # Setup the connection with no socket + self.conn.sock = None + + success = trysetip(self.conn, loglevel=logging.DEBUG) + + self.assertFalse(success) + + def test_exception_during_ip_retrieval(self): + self.conn.sock.getpeername.side_effect = Exception("Test Exception") + + with self.assertLogs(level=logging.WARNING) as warning: + success = trysetip(self.conn, loglevel=logging.WARNING) + self.assertEqual(len(warning.records), 1) + self.assertIn( + "Failed to get peer address", warning.records[0].message + ) + self.assertTrue(success) From 6bc48be45af45769ed071b14d684d1d40b3d155e Mon Sep 17 00:00:00 2001 From: Riccardo Magliocchetti Date: Sat, 13 Jul 2024 01:34:34 +0200 Subject: [PATCH 07/26] exporter/prometheus-remote-write: bump certifi to latest (#2701) Co-authored-by: Leighton Chen --- .../test-requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exporter/opentelemetry-exporter-prometheus-remote-write/test-requirements.txt b/exporter/opentelemetry-exporter-prometheus-remote-write/test-requirements.txt index ec8f90336d..f7e66ddd13 100644 --- a/exporter/opentelemetry-exporter-prometheus-remote-write/test-requirements.txt +++ b/exporter/opentelemetry-exporter-prometheus-remote-write/test-requirements.txt @@ -1,5 +1,5 @@ asgiref==3.7.2 -certifi==2024.2.2 +certifi==2024.7.4 charset-normalizer==3.3.2 # We can drop this after bumping baseline to pypy-39 cramjam==2.1.0; platform_python_implementation == "PyPy" From 432d6f570cbe28599a00cef5e369da0f860ffd7f Mon Sep 17 00:00:00 2001 From: Riccardo Magliocchetti Date: Mon, 15 Jul 2024 18:54:14 +0200 Subject: [PATCH 08/26] instrumentation/django: bump django 4.2 to 4.2.14 (#2700) --- .../test-requirements-2.txt | 2 +- .../test-requirements-3.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-django/test-requirements-2.txt b/instrumentation/opentelemetry-instrumentation-django/test-requirements-2.txt index 9c78f42e46..2ed199fbf3 100644 --- a/instrumentation/opentelemetry-instrumentation-django/test-requirements-2.txt +++ b/instrumentation/opentelemetry-instrumentation-django/test-requirements-2.txt @@ -1,7 +1,7 @@ asgiref==3.7.2 backports.zoneinfo==0.2.1 Deprecated==1.2.14 -Django==4.2.11 +Django==4.2.14 importlib-metadata==6.11.0 iniconfig==2.0.0 packaging==24.0 diff --git a/instrumentation/opentelemetry-instrumentation-django/test-requirements-3.txt b/instrumentation/opentelemetry-instrumentation-django/test-requirements-3.txt index 77c049d87b..c3e65be730 100644 --- a/instrumentation/opentelemetry-instrumentation-django/test-requirements-3.txt +++ b/instrumentation/opentelemetry-instrumentation-django/test-requirements-3.txt @@ -1,6 +1,6 @@ asgiref==3.7.2 Deprecated==1.2.14 -Django==4.2.11 +Django==4.2.14 importlib-metadata==6.11.0 iniconfig==2.0.0 packaging==24.0 From 7567efa341450a73c06580c0e12b53dec9ca0b10 Mon Sep 17 00:00:00 2001 From: Zhihan Li <54661071+zhihali@users.noreply.github.com> Date: Mon, 15 Jul 2024 18:09:36 +0100 Subject: [PATCH 09/26] Handle redis.exceptions.WatchError as a non-error event in instrumentation (#2668) --- CHANGELOG.md | 3 +- .../instrumentation/redis/__init__.py | 19 ++- .../test-requirements.txt | 1 + .../tests/test_redis.py | 117 +++++++++++++++++- 4 files changed, 135 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b6882cbe57..7bf02de0e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,7 +40,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#2682](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2682)) ### Fixed - +- Handle `redis.exceptions.WatchError` as a non-error event in redis instrumentation + ([#2668](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2668)) - `opentelemetry-instrumentation-httpx` Ensure httpx.get or httpx.request like methods are instrumented ([#2538](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2538)) - Add Python 3.12 support diff --git a/instrumentation/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py b/instrumentation/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py index 1d61e8cfd3..c5f19fc736 100644 --- a/instrumentation/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py @@ -106,7 +106,7 @@ def response_hook(span, instance, response): from opentelemetry.instrumentation.redis.version import __version__ from opentelemetry.instrumentation.utils import unwrap from opentelemetry.semconv.trace import SpanAttributes -from opentelemetry.trace import Span +from opentelemetry.trace import Span, StatusCode _DEFAULT_SERVICE = "redis" @@ -212,9 +212,16 @@ def _traced_execute_pipeline(func, instance, args, kwargs): span.set_attribute( "db.redis.pipeline_length", len(command_stack) ) - response = func(*args, **kwargs) + + response = None + try: + response = func(*args, **kwargs) + except redis.WatchError: + span.set_status(StatusCode.UNSET) + if callable(response_hook): response_hook(span, instance, response) + return response pipeline_class = ( @@ -281,7 +288,13 @@ async def _async_traced_execute_pipeline(func, instance, args, kwargs): span.set_attribute( "db.redis.pipeline_length", len(command_stack) ) - response = await func(*args, **kwargs) + + response = None + try: + response = await func(*args, **kwargs) + except redis.WatchError: + span.set_status(StatusCode.UNSET) + if callable(response_hook): response_hook(span, instance, response) return response diff --git a/instrumentation/opentelemetry-instrumentation-redis/test-requirements.txt b/instrumentation/opentelemetry-instrumentation-redis/test-requirements.txt index 1731de082a..43d4bd9788 100644 --- a/instrumentation/opentelemetry-instrumentation-redis/test-requirements.txt +++ b/instrumentation/opentelemetry-instrumentation-redis/test-requirements.txt @@ -1,6 +1,7 @@ asgiref==3.7.2 async-timeout==4.0.3 Deprecated==1.2.14 +fakeredis==2.23.3 importlib-metadata==6.11.0 iniconfig==2.0.0 packaging==24.0 diff --git a/instrumentation/opentelemetry-instrumentation-redis/tests/test_redis.py b/instrumentation/opentelemetry-instrumentation-redis/tests/test_redis.py index 4a2fce5026..23d21b6e5a 100644 --- a/instrumentation/opentelemetry-instrumentation-redis/tests/test_redis.py +++ b/instrumentation/opentelemetry-instrumentation-redis/tests/test_redis.py @@ -12,11 +12,16 @@ # See the License for the specific language governing permissions and # limitations under the License. import asyncio -from unittest import mock +from unittest import IsolatedAsyncioTestCase, mock from unittest.mock import AsyncMock +import fakeredis +import pytest import redis import redis.asyncio +from fakeredis.aioredis import FakeRedis +from redis.exceptions import ConnectionError as redis_ConnectionError +from redis.exceptions import WatchError from opentelemetry import trace from opentelemetry.instrumentation.redis import RedisInstrumentor @@ -311,3 +316,113 @@ def test_attributes_unix_socket(self): span.attributes[SpanAttributes.NET_TRANSPORT], NetTransportValues.OTHER.value, ) + + def test_connection_error(self): + server = fakeredis.FakeServer() + server.connected = False + redis_client = fakeredis.FakeStrictRedis(server=server) + try: + redis_client.set("foo", "bar") + except redis_ConnectionError: + pass + + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 1) + span = spans[0] + + self.assertEqual(span.name, "SET") + self.assertEqual(span.kind, SpanKind.CLIENT) + self.assertEqual(span.status.status_code, trace.StatusCode.ERROR) + + def test_response_error(self): + redis_client = fakeredis.FakeStrictRedis() + redis_client.lpush("mylist", "value") + try: + redis_client.incr( + "mylist" + ) # Trying to increment a list, which is invalid + except redis.ResponseError: + pass + + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 2) + + span = spans[0] + self.assertEqual(span.name, "LPUSH") + self.assertEqual(span.kind, SpanKind.CLIENT) + self.assertEqual(span.status.status_code, trace.StatusCode.UNSET) + + span = spans[1] + self.assertEqual(span.name, "INCRBY") + self.assertEqual(span.kind, SpanKind.CLIENT) + self.assertEqual(span.status.status_code, trace.StatusCode.ERROR) + + def test_watch_error_sync(self): + def redis_operations(): + try: + redis_client = fakeredis.FakeStrictRedis() + pipe = redis_client.pipeline(transaction=True) + pipe.watch("a") + redis_client.set("a", "bad") # This will cause the WatchError + pipe.multi() + pipe.set("a", "1") + pipe.execute() + except WatchError: + pass + + redis_operations() + + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 3) + + # there should be 3 tests, we start watch operation and have 2 set operation on same key + self.assertEqual(len(spans), 3) + + self.assertEqual(spans[0].attributes.get("db.statement"), "WATCH ?") + self.assertEqual(spans[0].kind, SpanKind.CLIENT) + self.assertEqual(spans[0].status.status_code, trace.StatusCode.UNSET) + + for span in spans[1:]: + self.assertEqual(span.attributes.get("db.statement"), "SET ? ?") + self.assertEqual(span.kind, SpanKind.CLIENT) + self.assertEqual(span.status.status_code, trace.StatusCode.UNSET) + + +class TestRedisAsync(TestBase, IsolatedAsyncioTestCase): + def setUp(self): + super().setUp() + RedisInstrumentor().instrument(tracer_provider=self.tracer_provider) + + def tearDown(self): + super().tearDown() + RedisInstrumentor().uninstrument() + + @pytest.mark.asyncio + async def test_watch_error_async(self): + async def redis_operations(): + try: + redis_client = FakeRedis() + async with redis_client.pipeline(transaction=False) as pipe: + await pipe.watch("a") + await redis_client.set("a", "bad") + pipe.multi() + await pipe.set("a", "1") + await pipe.execute() + except WatchError: + pass + + await redis_operations() + + spans = self.memory_exporter.get_finished_spans() + + # there should be 3 tests, we start watch operation and have 2 set operation on same key + self.assertEqual(len(spans), 3) + + self.assertEqual(spans[0].attributes.get("db.statement"), "WATCH ?") + self.assertEqual(spans[0].kind, SpanKind.CLIENT) + self.assertEqual(spans[0].status.status_code, trace.StatusCode.UNSET) + + for span in spans[1:]: + self.assertEqual(span.attributes.get("db.statement"), "SET ? ?") + self.assertEqual(span.kind, SpanKind.CLIENT) + self.assertEqual(span.status.status_code, trace.StatusCode.UNSET) From 7c7a2a4312060e4ff55a718a8efb289b16b10803 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Artur=20Sm=C4=99t?= <1754812+artursmet@users.noreply.github.com> Date: Mon, 15 Jul 2024 19:24:16 +0200 Subject: [PATCH 10/26] Add documentation for request/response hooks for requests instrumentor (#2704) --- .../instrumentation/requests/__init__.py | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) 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 18cc3e767c..0a55564386 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py @@ -31,6 +31,30 @@ Configuration ------------- +Request/Response hooks +********************** + +The requests instrumentation supports extending tracing behavior with the help of +request and response hooks. These are functions that are called back by the instrumentation +right after a Span is created for a request and right before the span is finished processing a response respectively. +The hooks can be configured as follows: + +.. code:: python + + # `request_obj` is an instance of requests.PreparedRequest + def request_hook(span, request_obj): + pass + + # `request_obj` is an instance of requests.PreparedRequest + # `response` is an instance of requests.Response + def response_hook(span, request_obj, response) + pass + + RequestsInstrumentor().instrument( + request_hook=request_hook, response_hook=response_hook) + ) + + Exclude lists ************* To exclude certain URLs from being tracked, set the environment variable ``OTEL_PYTHON_REQUESTS_EXCLUDED_URLS`` From 7e48ee7254067f68af547e44cdffa7699c0b8778 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Em=C3=ADdio=20Neto?= <9735060+emdneto@users.noreply.github.com> Date: Mon, 15 Jul 2024 17:49:35 -0300 Subject: [PATCH 11/26] HTTP semantic convention stability migration for aiohttp-client (#2673) --- CHANGELOG.md | 2 + instrumentation/README.md | 2 +- .../aiohttp_client/__init__.py | 99 +++++-- .../instrumentation/aiohttp_client/package.py | 4 + .../tests/test_aiohttp_client_integration.py | 278 ++++++++++++++++-- .../instrumentation/asgi/__init__.py | 3 +- .../instrumentation/wsgi/__init__.py | 3 +- .../opentelemetry/instrumentation/_semconv.py | 13 +- 8 files changed, 348 insertions(+), 56 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7bf02de0e3..f4573e4b44 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#2630](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2630)) - `opentelemetry-instrumentation-system-metrics` Add support for capture open file descriptors ([#2652](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2652)) +- `opentelemetry-instrumentation-aiohttp-client` Implement new semantic convention opt-in migration + ([#2673](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2673)) ### Breaking changes diff --git a/instrumentation/README.md b/instrumentation/README.md index 9add6d0228..bbc5d8d75c 100644 --- a/instrumentation/README.md +++ b/instrumentation/README.md @@ -2,7 +2,7 @@ | Instrumentation | Supported Packages | Metrics support | Semconv status | | --------------- | ------------------ | --------------- | -------------- | | [opentelemetry-instrumentation-aio-pika](./opentelemetry-instrumentation-aio-pika) | aio_pika >= 7.2.0, < 10.0.0 | No | experimental -| [opentelemetry-instrumentation-aiohttp-client](./opentelemetry-instrumentation-aiohttp-client) | aiohttp ~= 3.0 | No | experimental +| [opentelemetry-instrumentation-aiohttp-client](./opentelemetry-instrumentation-aiohttp-client) | aiohttp ~= 3.0 | No | migration | [opentelemetry-instrumentation-aiohttp-server](./opentelemetry-instrumentation-aiohttp-server) | aiohttp ~= 3.0 | No | experimental | [opentelemetry-instrumentation-aiopg](./opentelemetry-instrumentation-aiopg) | aiopg >= 0.13.0, < 2.0.0 | No | experimental | [opentelemetry-instrumentation-asgi](./opentelemetry-instrumentation-asgi) | asgiref ~= 3.0 | Yes | migration diff --git a/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py b/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py index 9f842bde79..459e3121b9 100644 --- a/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py @@ -90,19 +90,28 @@ def response_hook(span: Span, params: typing.Union[ from opentelemetry import context as context_api from opentelemetry import trace +from opentelemetry.instrumentation._semconv import ( + _get_schema_url, + _HTTPStabilityMode, + _OpenTelemetrySemanticConventionStability, + _OpenTelemetryStabilitySignalType, + _report_new, + _set_http_method, + _set_http_url, + _set_status, +) from opentelemetry.instrumentation.aiohttp_client.package import _instruments from opentelemetry.instrumentation.aiohttp_client.version import __version__ from opentelemetry.instrumentation.instrumentor import BaseInstrumentor from opentelemetry.instrumentation.utils import ( - http_status_to_status_code, is_instrumentation_enabled, unwrap, ) from opentelemetry.propagate import inject -from opentelemetry.semconv.trace import SpanAttributes +from opentelemetry.semconv.attributes.error_attributes import ERROR_TYPE from opentelemetry.trace import Span, SpanKind, TracerProvider, get_tracer from opentelemetry.trace.status import Status, StatusCode -from opentelemetry.util.http import remove_url_credentials +from opentelemetry.util.http import remove_url_credentials, sanitize_method _UrlFilterT = typing.Optional[typing.Callable[[yarl.URL], str]] _RequestHookT = typing.Optional[ @@ -122,11 +131,46 @@ def response_hook(span: Span, params: typing.Union[ ] +def _get_span_name(method: str) -> str: + method = sanitize_method(method.upper().strip()) + if method == "_OTHER": + method = "HTTP" + + return method + + +def _set_http_status_code_attribute( + span, + status_code, + metric_attributes=None, + sem_conv_opt_in_mode=_HTTPStabilityMode.DEFAULT, +): + status_code_str = str(status_code) + try: + status_code = int(status_code) + except ValueError: + status_code = -1 + if metric_attributes is None: + metric_attributes = {} + # When we have durations we should set metrics only once + # Also the decision to include status code on a histogram should + # not be dependent on tracing decisions. + _set_status( + span, + metric_attributes, + status_code, + status_code_str, + server_span=False, + sem_conv_opt_in_mode=sem_conv_opt_in_mode, + ) + + def create_trace_config( url_filter: _UrlFilterT = None, request_hook: _RequestHookT = None, response_hook: _ResponseHookT = None, tracer_provider: TracerProvider = None, + sem_conv_opt_in_mode: _HTTPStabilityMode = _HTTPStabilityMode.DEFAULT, ) -> aiohttp.TraceConfig: """Create an aiohttp-compatible trace configuration. @@ -167,9 +211,12 @@ def create_trace_config( __name__, __version__, tracer_provider, - schema_url="https://opentelemetry.io/schemas/1.11.0", + schema_url=_get_schema_url(sem_conv_opt_in_mode), ) + # TODO: Use this when we have durations for aiohttp-client + metric_attributes = {} + def _end_trace(trace_config_ctx: types.SimpleNamespace): context_api.detach(trace_config_ctx.token) trace_config_ctx.span.end() @@ -183,18 +230,22 @@ async def on_request_start( trace_config_ctx.span = None return - http_method = params.method.upper() - request_span_name = f"{http_method}" + http_method = params.method + request_span_name = _get_span_name(http_method) request_url = ( remove_url_credentials(trace_config_ctx.url_filter(params.url)) if callable(trace_config_ctx.url_filter) else remove_url_credentials(str(params.url)) ) - span_attributes = { - SpanAttributes.HTTP_METHOD: http_method, - SpanAttributes.HTTP_URL: request_url, - } + span_attributes = {} + _set_http_method( + span_attributes, + http_method, + request_span_name, + sem_conv_opt_in_mode, + ) + _set_http_url(span_attributes, request_url, sem_conv_opt_in_mode) trace_config_ctx.span = trace_config_ctx.tracer.start_span( request_span_name, kind=SpanKind.CLIENT, attributes=span_attributes @@ -219,14 +270,13 @@ async def on_request_end( if callable(response_hook): response_hook(trace_config_ctx.span, params) + _set_http_status_code_attribute( + trace_config_ctx.span, + params.response.status, + metric_attributes, + sem_conv_opt_in_mode, + ) - if trace_config_ctx.span.is_recording(): - trace_config_ctx.span.set_status( - Status(http_status_to_status_code(int(params.response.status))) - ) - trace_config_ctx.span.set_attribute( - SpanAttributes.HTTP_STATUS_CODE, params.response.status - ) _end_trace(trace_config_ctx) async def on_request_exception( @@ -238,7 +288,13 @@ async def on_request_exception( return if trace_config_ctx.span.is_recording() and params.exception: - trace_config_ctx.span.set_status(Status(StatusCode.ERROR)) + exc_type = type(params.exception).__qualname__ + if _report_new(sem_conv_opt_in_mode): + trace_config_ctx.span.set_attribute(ERROR_TYPE, exc_type) + + trace_config_ctx.span.set_status( + Status(StatusCode.ERROR, exc_type) + ) trace_config_ctx.span.record_exception(params.exception) if callable(response_hook): @@ -271,6 +327,7 @@ def _instrument( trace_configs: typing.Optional[ typing.Sequence[aiohttp.TraceConfig] ] = None, + sem_conv_opt_in_mode: _HTTPStabilityMode = _HTTPStabilityMode.DEFAULT, ): """Enables tracing of all ClientSessions @@ -293,6 +350,7 @@ def instrumented_init(wrapped, instance, args, kwargs): request_hook=request_hook, response_hook=response_hook, tracer_provider=tracer_provider, + sem_conv_opt_in_mode=sem_conv_opt_in_mode, ) trace_config._is_instrumented_by_opentelemetry = True client_trace_configs.append(trace_config) @@ -344,12 +402,17 @@ def _instrument(self, **kwargs): ``trace_configs``: An optional list of aiohttp.TraceConfig items, allowing customize enrichment of spans based on aiohttp events (see specification: https://docs.aiohttp.org/en/stable/tracing_reference.html) """ + _OpenTelemetrySemanticConventionStability._initialize() + _sem_conv_opt_in_mode = _OpenTelemetrySemanticConventionStability._get_opentelemetry_stability_opt_in_mode( + _OpenTelemetryStabilitySignalType.HTTP, + ) _instrument( tracer_provider=kwargs.get("tracer_provider"), url_filter=kwargs.get("url_filter"), request_hook=kwargs.get("request_hook"), response_hook=kwargs.get("response_hook"), trace_configs=kwargs.get("trace_configs"), + sem_conv_opt_in_mode=_sem_conv_opt_in_mode, ) def _uninstrument(self, **kwargs): diff --git a/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/package.py b/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/package.py index c9b6b6fe27..98ae4b0874 100644 --- a/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/package.py +++ b/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/package.py @@ -14,3 +14,7 @@ _instruments = ("aiohttp ~= 3.0",) + +_supports_metrics = False + +_semconv_status = "migration" diff --git a/instrumentation/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py b/instrumentation/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py index b6fe1eb57a..538421dd6a 100644 --- a/instrumentation/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py +++ b/instrumentation/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py @@ -28,10 +28,21 @@ from opentelemetry import trace as trace_api from opentelemetry.instrumentation import aiohttp_client +from opentelemetry.instrumentation._semconv import ( + OTEL_SEMCONV_STABILITY_OPT_IN, + _HTTPStabilityMode, + _OpenTelemetrySemanticConventionStability, +) from opentelemetry.instrumentation.aiohttp_client import ( AioHttpClientInstrumentor, ) from opentelemetry.instrumentation.utils import suppress_instrumentation +from opentelemetry.semconv.attributes.error_attributes import ERROR_TYPE +from opentelemetry.semconv.attributes.http_attributes import ( + HTTP_REQUEST_METHOD, + HTTP_RESPONSE_STATUS_CODE, +) +from opentelemetry.semconv.attributes.url_attributes import URL_FULL from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.test.test_base import TestBase from opentelemetry.trace import Span, StatusCode @@ -59,7 +70,23 @@ async def do_request(): class TestAioHttpIntegration(TestBase): - def assert_spans(self, spans): + + _test_status_codes = ( + (HTTPStatus.OK, StatusCode.UNSET), + (HTTPStatus.TEMPORARY_REDIRECT, StatusCode.UNSET), + (HTTPStatus.NOT_FOUND, StatusCode.ERROR), + (HTTPStatus.BAD_REQUEST, StatusCode.ERROR), + (HTTPStatus.SERVICE_UNAVAILABLE, StatusCode.ERROR), + (HTTPStatus.GATEWAY_TIMEOUT, StatusCode.ERROR), + ) + + def setUp(self): + super().setUp() + _OpenTelemetrySemanticConventionStability._initialized = False + + def assert_spans(self, spans, num_spans=1): + finished_spans = self.memory_exporter.get_finished_spans() + self.assertEqual(num_spans, len(finished_spans)) self.assertEqual( [ ( @@ -67,7 +94,7 @@ def assert_spans(self, spans): (span.status.status_code, span.status.description), dict(span.attributes), ) - for span in self.memory_exporter.get_finished_spans() + for span in finished_spans ], spans, ) @@ -99,39 +126,72 @@ async def client_request(server: aiohttp.test_utils.TestServer): return run_with_test_server(client_request, url, handler) def test_status_codes(self): - for status_code, span_status in ( - (HTTPStatus.OK, StatusCode.UNSET), - (HTTPStatus.TEMPORARY_REDIRECT, StatusCode.UNSET), - (HTTPStatus.SERVICE_UNAVAILABLE, StatusCode.ERROR), - ( - HTTPStatus.GATEWAY_TIMEOUT, - StatusCode.ERROR, - ), - ): + for status_code, span_status in self._test_status_codes: with self.subTest(status_code=status_code): + path = "test-path?query=param#foobar" host, port = self._http_request( trace_config=aiohttp_client.create_trace_config(), - url="/test-path?query=param#foobar", + url=f"/{path}", status_code=status_code, ) + url = f"http://{host}:{port}/{path}" + attributes = { + SpanAttributes.HTTP_METHOD: "GET", + SpanAttributes.HTTP_URL: url, + SpanAttributes.HTTP_STATUS_CODE: status_code, + } + spans = [("GET", (span_status, None), attributes)] + self.assert_spans(spans) + self.memory_exporter.clear() - url = f"http://{host}:{port}/test-path?query=param#foobar" - self.assert_spans( - [ - ( - "GET", - (span_status, None), - { - SpanAttributes.HTTP_METHOD: "GET", - SpanAttributes.HTTP_URL: url, - SpanAttributes.HTTP_STATUS_CODE: int( - status_code - ), - }, - ) - ] + def test_status_codes_new_semconv(self): + for status_code, span_status in self._test_status_codes: + with self.subTest(status_code=status_code): + path = "test-path?query=param#foobar" + host, port = self._http_request( + trace_config=aiohttp_client.create_trace_config( + sem_conv_opt_in_mode=_HTTPStabilityMode.HTTP + ), + url=f"/{path}", + status_code=status_code, ) + url = f"http://{host}:{port}/{path}" + attributes = { + HTTP_REQUEST_METHOD: "GET", + URL_FULL: url, + HTTP_RESPONSE_STATUS_CODE: status_code, + } + if status_code >= 400: + attributes[ERROR_TYPE] = str(status_code.value) + spans = [("GET", (span_status, None), attributes)] + self.assert_spans(spans) + self.memory_exporter.clear() + def test_status_codes_both_semconv(self): + for status_code, span_status in self._test_status_codes: + with self.subTest(status_code=status_code): + path = "test-path?query=param#foobar" + host, port = self._http_request( + trace_config=aiohttp_client.create_trace_config( + sem_conv_opt_in_mode=_HTTPStabilityMode.HTTP_DUP + ), + url=f"/{path}", + status_code=status_code, + ) + url = f"http://{host}:{port}/{path}" + attributes = { + HTTP_REQUEST_METHOD: "GET", + SpanAttributes.HTTP_METHOD: "GET", + URL_FULL: url, + SpanAttributes.HTTP_URL: url, + HTTP_RESPONSE_STATUS_CODE: status_code, + SpanAttributes.HTTP_STATUS_CODE: status_code, + } + if status_code >= 400: + attributes[ERROR_TYPE] = str(status_code.value) + + spans = [("GET", (span_status, None), attributes)] + self.assert_spans(spans, 1) self.memory_exporter.clear() def test_schema_url(self): @@ -149,6 +209,40 @@ def test_schema_url(self): ) self.memory_exporter.clear() + def test_schema_url_new_semconv(self): + with self.subTest(status_code=200): + self._http_request( + trace_config=aiohttp_client.create_trace_config( + sem_conv_opt_in_mode=_HTTPStabilityMode.HTTP + ), + url="/test-path?query=param#foobar", + status_code=200, + ) + + span = self.memory_exporter.get_finished_spans()[0] + self.assertEqual( + span.instrumentation_info.schema_url, + "https://opentelemetry.io/schemas/v1.21.0", + ) + self.memory_exporter.clear() + + def test_schema_url_both_semconv(self): + with self.subTest(status_code=200): + self._http_request( + trace_config=aiohttp_client.create_trace_config( + sem_conv_opt_in_mode=_HTTPStabilityMode.HTTP_DUP + ), + url="/test-path?query=param#foobar", + status_code=200, + ) + + span = self.memory_exporter.get_finished_spans()[0] + self.assertEqual( + span.instrumentation_info.schema_url, + "https://opentelemetry.io/schemas/v1.21.0", + ) + self.memory_exporter.clear() + def test_not_recording(self): mock_tracer = mock.Mock() mock_span = mock.Mock() @@ -263,7 +357,7 @@ async def do_request(url): [ ( "GET", - (expected_status, None), + (expected_status, "ClientConnectorError"), { SpanAttributes.HTTP_METHOD: "GET", SpanAttributes.HTTP_URL: url, @@ -273,6 +367,89 @@ async def do_request(url): ) self.memory_exporter.clear() + def test_basic_exception(self): + async def request_handler(request): + assert "traceparent" in request.headers + + host, port = self._http_request( + trace_config=aiohttp_client.create_trace_config(), + url="/test", + request_handler=request_handler, + ) + span = self.memory_exporter.get_finished_spans()[0] + self.assertEqual(len(span.events), 1) + self.assertEqual(span.events[0].name, "exception") + self.assert_spans( + [ + ( + "GET", + (StatusCode.ERROR, "ServerDisconnectedError"), + { + SpanAttributes.HTTP_METHOD: "GET", + SpanAttributes.HTTP_URL: f"http://{host}:{port}/test", + }, + ) + ] + ) + + def test_basic_exception_new_semconv(self): + async def request_handler(request): + assert "traceparent" in request.headers + + host, port = self._http_request( + trace_config=aiohttp_client.create_trace_config( + sem_conv_opt_in_mode=_HTTPStabilityMode.HTTP + ), + url="/test", + request_handler=request_handler, + ) + span = self.memory_exporter.get_finished_spans()[0] + self.assertEqual(len(span.events), 1) + self.assertEqual(span.events[0].name, "exception") + self.assert_spans( + [ + ( + "GET", + (StatusCode.ERROR, "ServerDisconnectedError"), + { + HTTP_REQUEST_METHOD: "GET", + URL_FULL: f"http://{host}:{port}/test", + ERROR_TYPE: "ServerDisconnectedError", + }, + ) + ] + ) + + def test_basic_exception_both_semconv(self): + async def request_handler(request): + assert "traceparent" in request.headers + + host, port = self._http_request( + trace_config=aiohttp_client.create_trace_config( + sem_conv_opt_in_mode=_HTTPStabilityMode.HTTP_DUP + ), + url="/test", + request_handler=request_handler, + ) + span = self.memory_exporter.get_finished_spans()[0] + self.assertEqual(len(span.events), 1) + self.assertEqual(span.events[0].name, "exception") + self.assert_spans( + [ + ( + "GET", + (StatusCode.ERROR, "ServerDisconnectedError"), + { + HTTP_REQUEST_METHOD: "GET", + URL_FULL: f"http://{host}:{port}/test", + ERROR_TYPE: "ServerDisconnectedError", + SpanAttributes.HTTP_METHOD: "GET", + SpanAttributes.HTTP_URL: f"http://{host}:{port}/test", + }, + ) + ] + ) + def test_timeout(self): async def request_handler(request): await asyncio.sleep(1) @@ -290,7 +467,7 @@ async def request_handler(request): [ ( "GET", - (StatusCode.ERROR, None), + (StatusCode.ERROR, "ServerTimeoutError"), { SpanAttributes.HTTP_METHOD: "GET", SpanAttributes.HTTP_URL: f"http://{host}:{port}/test_timeout", @@ -317,7 +494,7 @@ async def request_handler(request): [ ( "GET", - (StatusCode.ERROR, None), + (StatusCode.ERROR, "TooManyRedirects"), { SpanAttributes.HTTP_METHOD: "GET", SpanAttributes.HTTP_URL: f"http://{host}:{port}/test_too_many_redirects", @@ -374,6 +551,7 @@ class TestAioHttpClientInstrumentor(TestBase): def setUp(self): super().setUp() AioHttpClientInstrumentor().instrument() + _OpenTelemetrySemanticConventionStability._initialized = False def tearDown(self): super().tearDown() @@ -414,6 +592,46 @@ def test_instrument(self): ) self.assertEqual(200, span.attributes[SpanAttributes.HTTP_STATUS_CODE]) + def test_instrument_new_semconv(self): + AioHttpClientInstrumentor().uninstrument() + with mock.patch.dict( + "os.environ", {OTEL_SEMCONV_STABILITY_OPT_IN: "http"} + ): + AioHttpClientInstrumentor().instrument() + host, port = run_with_test_server( + self.get_default_request(), self.URL, self.default_handler + ) + span = self.assert_spans(1) + self.assertEqual("GET", span.name) + self.assertEqual("GET", span.attributes[HTTP_REQUEST_METHOD]) + self.assertEqual( + f"http://{host}:{port}/test-path", + span.attributes[URL_FULL], + ) + self.assertEqual(200, span.attributes[HTTP_RESPONSE_STATUS_CODE]) + + def test_instrument_both_semconv(self): + AioHttpClientInstrumentor().uninstrument() + with mock.patch.dict( + "os.environ", {OTEL_SEMCONV_STABILITY_OPT_IN: "http/dup"} + ): + AioHttpClientInstrumentor().instrument() + host, port = run_with_test_server( + self.get_default_request(), self.URL, self.default_handler + ) + url = f"http://{host}:{port}/test-path" + attributes = { + HTTP_REQUEST_METHOD: "GET", + SpanAttributes.HTTP_METHOD: "GET", + URL_FULL: url, + SpanAttributes.HTTP_URL: url, + HTTP_RESPONSE_STATUS_CODE: 200, + SpanAttributes.HTTP_STATUS_CODE: 200, + } + span = self.assert_spans(1) + self.assertEqual("GET", span.name) + self.assertEqual(span.attributes, attributes) + def test_instrument_with_custom_trace_config(self): trace_config = aiohttp.TraceConfig() diff --git a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py index 7e51837d4d..0525181eac 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -451,7 +451,8 @@ def set_status_code( metric_attributes, status_code, status_code_str, - sem_conv_opt_in_mode, + server_span=True, + sem_conv_opt_in_mode=sem_conv_opt_in_mode, ) diff --git a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py index d75147d6aa..355b1d7458 100644 --- a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py @@ -496,7 +496,8 @@ def add_response_attributes( duration_attrs, status_code, status_code_str, - sem_conv_opt_in_mode, + server_span=True, + sem_conv_opt_in_mode=sem_conv_opt_in_mode, ) diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py index 7e81692a98..33668333ce 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py @@ -349,10 +349,11 @@ def _set_http_flavor_version(result, version, sem_conv_opt_in_mode): def _set_status( span, - metrics_attributes, - status_code, - status_code_str, - sem_conv_opt_in_mode, + metrics_attributes: dict, + status_code: int, + status_code_str: str, + server_span: bool = True, + sem_conv_opt_in_mode: _HTTPStabilityMode = _HTTPStabilityMode.DEFAULT, ): if status_code < 0: metrics_attributes[ERROR_TYPE] = status_code_str @@ -366,7 +367,9 @@ def _set_status( ) ) else: - status = http_status_to_status_code(status_code, server_span=True) + status = http_status_to_status_code( + status_code, server_span=server_span + ) if _report_old(sem_conv_opt_in_mode): if span.is_recording(): From f8f58ee411ac2e318512717378cd365ba4aa7a66 Mon Sep 17 00:00:00 2001 From: Alex Hall Date: Tue, 16 Jul 2024 20:06:59 +0200 Subject: [PATCH 12/26] Add `http.target` to Django duration metric attributes (#2624) --- CHANGELOG.md | 2 ++ .../instrumentation/django/middleware/otel_middleware.py | 6 ++++++ .../tests/test_middleware.py | 3 ++- 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f4573e4b44..57447ccf97 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#2652](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2652)) - `opentelemetry-instrumentation-aiohttp-client` Implement new semantic convention opt-in migration ([#2673](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2673)) +- `opentelemetry-instrumentation-django` Add `http.target` to Django duration metric attributes + ([#2624](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2624)) ### Breaking changes diff --git a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware/otel_middleware.py b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware/otel_middleware.py index 6b64865ef7..11e92c9757 100644 --- a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware/otel_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware/otel_middleware.py @@ -315,6 +315,12 @@ def process_view(self, request, view_func, *args, **kwargs): route = getattr(match, "route", None) if route: span.set_attribute(SpanAttributes.HTTP_ROUTE, route) + duration_attrs = request.META[ + self._environ_duration_attr_key + ] + # Metrics currently use the 1.11.0 schema, which puts the route in `http.target`. + # TODO: use `http.route` when the user sets `OTEL_SEMCONV_STABILITY_OPT_IN`. + duration_attrs[SpanAttributes.HTTP_TARGET] = route def process_exception(self, request, exception): if self._excluded_urls.url_disabled(request.build_absolute_uri("?")): diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py index c6b0568ef8..4945f05455 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py @@ -480,7 +480,8 @@ def test_wsgi_metrics(self): ] _recommended_attrs = { "http.server.active_requests": _active_requests_count_attrs, - "http.server.duration": _duration_attrs, + "http.server.duration": _duration_attrs + | {SpanAttributes.HTTP_TARGET}, } start = default_timer() for _ in range(3): From 24bc71eb2ef1a66a6563be3155000d91cdd48501 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Em=C3=ADdio=20Neto?= <9735060+emdneto@users.noreply.github.com> Date: Tue, 16 Jul 2024 16:57:38 -0300 Subject: [PATCH 13/26] Remove package install from aiopg instrumentation (#2712) Co-authored-by: Diego Hurtado --- .../opentelemetry-instrumentation-aiopg/test-requirements.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-aiopg/test-requirements.txt b/instrumentation/opentelemetry-instrumentation-aiopg/test-requirements.txt index 8a465a3326..bc61c3d6fc 100644 --- a/instrumentation/opentelemetry-instrumentation-aiopg/test-requirements.txt +++ b/instrumentation/opentelemetry-instrumentation-aiopg/test-requirements.txt @@ -4,7 +4,6 @@ async-timeout==4.0.3 Deprecated==1.2.14 importlib-metadata==6.11.0 iniconfig==2.0.0 -install==1.3.5 packaging==24.0 pluggy==1.5.0 psycopg2-binary==2.9.9 From 5a7935ff1fa66cee2bffd850b1f4062667fa3391 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Tue, 16 Jul 2024 14:20:02 -0600 Subject: [PATCH 14/26] Deprecate sklearn instrumentation (#2708) Fixes #2176 --- .github/workflows/instrumentations_0.yml | 11 - .github/workflows/lint.yml | 28 - CHANGELOG.md | 2 + eachdist.ini | 1 - instrumentation/README.md | 1 - .../LICENSE | 201 ----- .../README.rst | 24 - .../pyproject.toml | 49 -- .../instrumentation/sklearn/__init__.py | 792 ------------------ .../instrumentation/sklearn/package.py | 16 - .../instrumentation/sklearn/version.py | 15 - .../test-requirements.txt | 19 - .../tests/__init__.py | 0 .../tests/fixtures.py | 54 -- .../tests/test_sklearn.py | 190 ----- .../pyproject.toml | 1 - .../instrumentation/bootstrap_gen.py | 4 - tox.ini | 16 - 18 files changed, 2 insertions(+), 1422 deletions(-) delete mode 100644 instrumentation/opentelemetry-instrumentation-sklearn/LICENSE delete mode 100644 instrumentation/opentelemetry-instrumentation-sklearn/README.rst delete mode 100644 instrumentation/opentelemetry-instrumentation-sklearn/pyproject.toml delete mode 100644 instrumentation/opentelemetry-instrumentation-sklearn/src/opentelemetry/instrumentation/sklearn/__init__.py delete mode 100644 instrumentation/opentelemetry-instrumentation-sklearn/src/opentelemetry/instrumentation/sklearn/package.py delete mode 100644 instrumentation/opentelemetry-instrumentation-sklearn/src/opentelemetry/instrumentation/sklearn/version.py delete mode 100644 instrumentation/opentelemetry-instrumentation-sklearn/test-requirements.txt delete mode 100644 instrumentation/opentelemetry-instrumentation-sklearn/tests/__init__.py delete mode 100644 instrumentation/opentelemetry-instrumentation-sklearn/tests/fixtures.py delete mode 100644 instrumentation/opentelemetry-instrumentation-sklearn/tests/test_sklearn.py diff --git a/.github/workflows/instrumentations_0.yml b/.github/workflows/instrumentations_0.yml index d54cb50119..69c07bfacc 100644 --- a/.github/workflows/instrumentations_0.yml +++ b/.github/workflows/instrumentations_0.yml @@ -66,7 +66,6 @@ jobs: - "redis" - "remoulade" - "requests" - - "sklearn" - "sqlalchemy" - "sqlite3" - "starlette" @@ -75,14 +74,6 @@ jobs: - "tortoiseorm" os: [ubuntu-20.04] exclude: - - python-version: py39 - package: "sklearn" - - python-version: py310 - package: "sklearn" - - python-version: py311 - package: "sklearn" - - python-version: py312 - package: "sklearn" - python-version: py312 package: "boto" - python-version: py312 @@ -103,8 +94,6 @@ jobs: package: "remoulade" - python-version: pypy3 package: "requests" - - python-version: pypy3 - package: "sklearn" - python-version: pypy3 package: "confluent-kafka" - python-version: pypy3 diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index c156f7d942..29cd5cac16 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -93,31 +93,3 @@ jobs: key: v7-build-tox-cache-${{ matrix.package }}-${{ hashFiles('tox.ini', 'gen-requirements.txt', 'dev-requirements.txt') }} - name: run tox run: tox -e lint-${{ matrix.package }} - - lint-3_8: - strategy: - fail-fast: false # ensures the entire test matrix is run, even if one permutation fails - matrix: - package: - - "instrumentation-sklearn" - os: [ubuntu-20.04] - runs-on: ubuntu-20.04 - steps: - - name: Checkout Contrib Repo @ SHA - ${{ github.sha }} - uses: actions/checkout@v4 - - name: Set up Python 3.8 - uses: actions/setup-python@v5 - with: - python-version: 3.8 - - name: Install tox - run: pip install tox - - name: Cache tox environment - # Preserves .tox directory between runs for faster installs - uses: actions/cache@v4 - with: - path: | - .tox - ~/.cache/pip - key: v7-build-tox-cache-${{ matrix.package }}-${{ hashFiles('tox.ini', 'gen-requirements.txt', 'dev-requirements.txt') }} - - name: run tox - run: tox -e lint-${{ matrix.package }} diff --git a/CHANGELOG.md b/CHANGELOG.md index 57447ccf97..2de76b2736 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- `opentelemetry-instrumentation-sklearn` Deprecated the sklearn instrumentation + ([#2708](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2708)) - `opentelemetry-instrumentation-pyramid` Record exceptions raised when serving a request ([#2622](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2622)) - `opentelemetry-sdk-extension-aws` Add AwsXrayLambdaPropagator diff --git a/eachdist.ini b/eachdist.ini index 7f170e4947..b698eaf299 100644 --- a/eachdist.ini +++ b/eachdist.ini @@ -54,7 +54,6 @@ packages= [lintroots] extraroots=examples/*,scripts/ subglob=*.py,tests/,test/,src/*,examples/* -ignore=sklearn [testroots] extraroots=examples/*,tests/ diff --git a/instrumentation/README.md b/instrumentation/README.md index bbc5d8d75c..989ba529cb 100644 --- a/instrumentation/README.md +++ b/instrumentation/README.md @@ -38,7 +38,6 @@ | [opentelemetry-instrumentation-redis](./opentelemetry-instrumentation-redis) | redis >= 2.6 | No | experimental | [opentelemetry-instrumentation-remoulade](./opentelemetry-instrumentation-remoulade) | remoulade >= 0.50 | No | experimental | [opentelemetry-instrumentation-requests](./opentelemetry-instrumentation-requests) | requests ~= 2.0 | Yes | migration -| [opentelemetry-instrumentation-sklearn](./opentelemetry-instrumentation-sklearn) | scikit-learn ~= 0.24.0 | No | experimental | [opentelemetry-instrumentation-sqlalchemy](./opentelemetry-instrumentation-sqlalchemy) | sqlalchemy | Yes | experimental | [opentelemetry-instrumentation-sqlite3](./opentelemetry-instrumentation-sqlite3) | sqlite3 | No | experimental | [opentelemetry-instrumentation-starlette](./opentelemetry-instrumentation-starlette) | starlette ~= 0.13.0 | Yes | experimental diff --git a/instrumentation/opentelemetry-instrumentation-sklearn/LICENSE b/instrumentation/opentelemetry-instrumentation-sklearn/LICENSE deleted file mode 100644 index 261eeb9e9f..0000000000 --- a/instrumentation/opentelemetry-instrumentation-sklearn/LICENSE +++ /dev/null @@ -1,201 +0,0 @@ - Apache License - Version 2.0, January 2004 - http://www.apache.org/licenses/ - - TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION - - 1. Definitions. - - "License" shall mean the terms and conditions for use, reproduction, - and distribution as defined by Sections 1 through 9 of this document. - - "Licensor" shall mean the copyright owner or entity authorized by - the copyright owner that is granting the License. - - "Legal Entity" shall mean the union of the acting entity and all - other entities that control, are controlled by, or are under common - control with that entity. For the purposes of this definition, - "control" means (i) the power, direct or indirect, to cause the - direction or management of such entity, whether by contract or - otherwise, or (ii) ownership of fifty percent (50%) or more of the - outstanding shares, or (iii) beneficial ownership of such entity. - - "You" (or "Your") shall mean an individual or Legal Entity - exercising permissions granted by this License. - - "Source" form shall mean the preferred form for making modifications, - including but not limited to software source code, documentation - source, and configuration files. - - "Object" form shall mean any form resulting from mechanical - transformation or translation of a Source form, including but - not limited to compiled object code, generated documentation, - and conversions to other media types. - - "Work" shall mean the work of authorship, whether in Source or - Object form, made available under the License, as indicated by a - copyright notice that is included in or attached to the work - (an example is provided in the Appendix below). - - "Derivative Works" shall mean any work, whether in Source or Object - form, that is based on (or derived from) the Work and for which the - editorial revisions, annotations, elaborations, or other modifications - represent, as a whole, an original work of authorship. For the purposes - of this License, Derivative Works shall not include works that remain - separable from, or merely link (or bind by name) to the interfaces of, - the Work and Derivative Works thereof. - - "Contribution" shall mean any work of authorship, including - the original version of the Work and any modifications or additions - to that Work or Derivative Works thereof, that is intentionally - submitted to Licensor for inclusion in the Work by the copyright owner - or by an individual or Legal Entity authorized to submit on behalf of - the copyright owner. For the purposes of this definition, "submitted" - means any form of electronic, verbal, or written communication sent - to the Licensor or its representatives, including but not limited to - communication on electronic mailing lists, source code control systems, - and issue tracking systems that are managed by, or on behalf of, the - Licensor for the purpose of discussing and improving the Work, but - excluding communication that is conspicuously marked or otherwise - designated in writing by the copyright owner as "Not a Contribution." - - "Contributor" shall mean Licensor and any individual or Legal Entity - on behalf of whom a Contribution has been received by Licensor and - subsequently incorporated within the Work. - - 2. Grant of Copyright License. Subject to the terms and conditions of - this License, each Contributor hereby grants to You a perpetual, - worldwide, non-exclusive, no-charge, royalty-free, irrevocable - copyright license to reproduce, prepare Derivative Works of, - publicly display, publicly perform, sublicense, and distribute the - Work and such Derivative Works in Source or Object form. - - 3. Grant of Patent License. Subject to the terms and conditions of - this License, each Contributor hereby grants to You a perpetual, - worldwide, non-exclusive, no-charge, royalty-free, irrevocable - (except as stated in this section) patent license to make, have made, - use, offer to sell, sell, import, and otherwise transfer the Work, - where such license applies only to those patent claims licensable - by such Contributor that are necessarily infringed by their - Contribution(s) alone or by combination of their Contribution(s) - with the Work to which such Contribution(s) was submitted. If You - institute patent litigation against any entity (including a - cross-claim or counterclaim in a lawsuit) alleging that the Work - or a Contribution incorporated within the Work constitutes direct - or contributory patent infringement, then any patent licenses - granted to You under this License for that Work shall terminate - as of the date such litigation is filed. - - 4. Redistribution. You may reproduce and distribute copies of the - Work or Derivative Works thereof in any medium, with or without - modifications, and in Source or Object form, provided that You - meet the following conditions: - - (a) You must give any other recipients of the Work or - Derivative Works a copy of this License; and - - (b) You must cause any modified files to carry prominent notices - stating that You changed the files; and - - (c) You must retain, in the Source form of any Derivative Works - that You distribute, all copyright, patent, trademark, and - attribution notices from the Source form of the Work, - excluding those notices that do not pertain to any part of - the Derivative Works; and - - (d) If the Work includes a "NOTICE" text file as part of its - distribution, then any Derivative Works that You distribute must - include a readable copy of the attribution notices contained - within such NOTICE file, excluding those notices that do not - pertain to any part of the Derivative Works, in at least one - of the following places: within a NOTICE text file distributed - as part of the Derivative Works; within the Source form or - documentation, if provided along with the Derivative Works; or, - within a display generated by the Derivative Works, if and - wherever such third-party notices normally appear. The contents - of the NOTICE file are for informational purposes only and - do not modify the License. You may add Your own attribution - notices within Derivative Works that You distribute, alongside - or as an addendum to the NOTICE text from the Work, provided - that such additional attribution notices cannot be construed - as modifying the License. - - You may add Your own copyright statement to Your modifications and - may provide additional or different license terms and conditions - for use, reproduction, or distribution of Your modifications, or - for any such Derivative Works as a whole, provided Your use, - reproduction, and distribution of the Work otherwise complies with - the conditions stated in this License. - - 5. Submission of Contributions. Unless You explicitly state otherwise, - any Contribution intentionally submitted for inclusion in the Work - by You to the Licensor shall be under the terms and conditions of - this License, without any additional terms or conditions. - Notwithstanding the above, nothing herein shall supersede or modify - the terms of any separate license agreement you may have executed - with Licensor regarding such Contributions. - - 6. Trademarks. This License does not grant permission to use the trade - names, trademarks, service marks, or product names of the Licensor, - except as required for reasonable and customary use in describing the - origin of the Work and reproducing the content of the NOTICE file. - - 7. Disclaimer of Warranty. Unless required by applicable law or - agreed to in writing, Licensor provides the Work (and each - Contributor provides its Contributions) on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or - implied, including, without limitation, any warranties or conditions - of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A - PARTICULAR PURPOSE. You are solely responsible for determining the - appropriateness of using or redistributing the Work and assume any - risks associated with Your exercise of permissions under this License. - - 8. Limitation of Liability. In no event and under no legal theory, - whether in tort (including negligence), contract, or otherwise, - unless required by applicable law (such as deliberate and grossly - negligent acts) or agreed to in writing, shall any Contributor be - liable to You for damages, including any direct, indirect, special, - incidental, or consequential damages of any character arising as a - result of this License or out of the use or inability to use the - Work (including but not limited to damages for loss of goodwill, - work stoppage, computer failure or malfunction, or any and all - other commercial damages or losses), even if such Contributor - has been advised of the possibility of such damages. - - 9. Accepting Warranty or Additional Liability. While redistributing - the Work or Derivative Works thereof, You may choose to offer, - and charge a fee for, acceptance of support, warranty, indemnity, - or other liability obligations and/or rights consistent with this - License. However, in accepting such obligations, You may act only - on Your own behalf and on Your sole responsibility, not on behalf - of any other Contributor, and only if You agree to indemnify, - defend, and hold each Contributor harmless for any liability - incurred by, or claims asserted against, such Contributor by reason - of your accepting any such warranty or additional liability. - - END OF TERMS AND CONDITIONS - - APPENDIX: How to apply the Apache License to your work. - - To apply the Apache License to your work, attach the following - boilerplate notice, with the fields enclosed by brackets "[]" - replaced with your own identifying information. (Don't include - the brackets!) The text should be enclosed in the appropriate - comment syntax for the file format. We also recommend that a - file or class name and description of purpose be included on the - same "printed page" as the copyright notice for easier - identification within third-party archives. - - Copyright [yyyy] [name of copyright owner] - - 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. diff --git a/instrumentation/opentelemetry-instrumentation-sklearn/README.rst b/instrumentation/opentelemetry-instrumentation-sklearn/README.rst deleted file mode 100644 index 9a3300c4f2..0000000000 --- a/instrumentation/opentelemetry-instrumentation-sklearn/README.rst +++ /dev/null @@ -1,24 +0,0 @@ -OpenTelemetry Scikit-Learn Instrumentation -========================================== - -|pypi| - -.. |pypi| image:: https://badge.fury.io/py/opentelemetry-instrumentation-sklearn.svg - :target: https://pypi.org/project/opentelemetry-instrumentation-sklearn/ - -This library allows tracing HTTP requests made by the -`scikit-learn `_ library. - -Installation ------------- - -:: - - pip install opentelemetry-instrumentation-sklearn - -References ----------- - -* `OpenTelemetry sklearn Instrumentation `_ -* `OpenTelemetry Project `_ -* `OpenTelemetry Python Examples `_ diff --git a/instrumentation/opentelemetry-instrumentation-sklearn/pyproject.toml b/instrumentation/opentelemetry-instrumentation-sklearn/pyproject.toml deleted file mode 100644 index 6e57529ed1..0000000000 --- a/instrumentation/opentelemetry-instrumentation-sklearn/pyproject.toml +++ /dev/null @@ -1,49 +0,0 @@ -[build-system] -requires = ["hatchling"] -build-backend = "hatchling.build" - -[project] -name = "opentelemetry-instrumentation-sklearn" -dynamic = ["version"] -description = "OpenTelemetry sklearn instrumentation" -readme = "README.rst" -license = "Apache-2.0" -requires-python = ">=3.8" -authors = [ - { name = "OpenTelemetry Authors", email = "cncf-opentelemetry-contributors@lists.cncf.io" }, -] -classifiers = [ - "Development Status :: 4 - Beta", - "Intended Audience :: Developers", - "License :: OSI Approved :: Apache Software License", - "Programming Language :: Python", - "Programming Language :: Python :: 3", - "Programming Language :: Python :: 3.8", -] -dependencies = [ - "opentelemetry-api ~= 1.12", - "opentelemetry-instrumentation == 0.47b0.dev", -] - -[project.optional-dependencies] -instruments = [ - "scikit-learn ~= 0.24.0", -] - -[project.entry-points.opentelemetry_instrumentor] -sklearn = "opentelemetry.instrumentation.sklearn:SklearnInstrumentor" - -[project.urls] -Homepage = "https://github.com/open-telemetry/opentelemetry-python-contrib/tree/main/instrumentation/opentelemetry-instrumentation-sklearn" - -[tool.hatch.version] -path = "src/opentelemetry/instrumentation/sklearn/version.py" - -[tool.hatch.build.targets.sdist] -include = [ - "/src", - "/tests", -] - -[tool.hatch.build.targets.wheel] -packages = ["src/opentelemetry"] diff --git a/instrumentation/opentelemetry-instrumentation-sklearn/src/opentelemetry/instrumentation/sklearn/__init__.py b/instrumentation/opentelemetry-instrumentation-sklearn/src/opentelemetry/instrumentation/sklearn/__init__.py deleted file mode 100644 index a67bfa6ef4..0000000000 --- a/instrumentation/opentelemetry-instrumentation-sklearn/src/opentelemetry/instrumentation/sklearn/__init__.py +++ /dev/null @@ -1,792 +0,0 @@ -# Copyright 2020, 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. -""" -The integration with sklearn supports the scikit-learn compatible libraries, -it can be enabled by using ``SklearnInstrumentor``. - -.. sklearn: https://github.com/scikit-learn/scikit-learn - -Usage ------ - -Package instrumentation example: - -.. code-block:: python - - from opentelemetry.instrumentation.sklearn import SklearnInstrumentor - - # instrument the sklearn library - SklearnInstrumentor().instrument() - - # instrument sklearn and other libraries - SklearnInstrumentor( - packages=["sklearn", "lightgbm", "xgboost"] - ).instrument() - - -Model instrumentation example: - -.. code-block:: python - - from opentelemetry.instrumentation.sklearn import SklearnInstrumentor - from sklearn.datasets import load_iris - from sklearn.ensemble import RandomForestClassifier - from sklearn.model_selection import train_test_split - from sklearn.pipeline import Pipeline - - X, y = load_iris(return_X_y=True) - X_train, X_test, y_train, y_test = train_test_split(X, y) - - model = Pipeline( - [ - ("class", RandomForestClassifier(n_estimators=10)), - ] - ) - - model.fit(X_train, y_train) - - SklearnInstrumentor().instrument_estimator(model) - -""" -import logging -import os -from functools import wraps -from importlib import import_module -from inspect import isclass -from pkgutil import iter_modules -from typing import ( - Callable, - Collection, - Dict, - List, - MutableMapping, - Sequence, - Type, - Union, -) - -from sklearn.base import BaseEstimator -from sklearn.pipeline import FeatureUnion, Pipeline -from sklearn.tree import BaseDecisionTree -from sklearn.utils.metaestimators import _IffHasAttrDescriptor - -from opentelemetry.instrumentation.instrumentor import BaseInstrumentor - -# pylint: disable=no-name-in-module -from opentelemetry.instrumentation.sklearn.package import _instruments -from opentelemetry.instrumentation.sklearn.version import __version__ -from opentelemetry.trace import get_tracer -from opentelemetry.util.types import Attributes - -logger = logging.getLogger(__name__) - - -def implement_span_estimator( - func: Callable, - estimator: Union[BaseEstimator, Type[BaseEstimator]], - attributes: Attributes = None, -): - """Wrap the method call with a span. - - Args: - func: A callable to be wrapped in a span - estimator: An instance or class of an estimator - attributes: Attributes to apply to the span - - Returns: - The passed function wrapped in a span. - """ - if isclass(estimator): - name = estimator.__name__ - else: - name = estimator.__class__.__name__ - logger.debug("Instrumenting: %s.%s", name, func.__name__) - attributes = attributes or {} - name = f"{name}.{func.__name__}" - return implement_span_function(func, name, attributes) - - -def implement_span_function(func: Callable, name: str, attributes: Attributes): - """Wrap the function with a span. - - Args: - func: A callable to be wrapped in a span - name: The name of the span - attributes: Attributes to apply to the span - - Returns: - The passed function wrapped in a span. - """ - - @wraps(func) - def wrapper(*args, **kwargs): - with get_tracer( - __name__, - __version__, - schema_url="https://opentelemetry.io/schemas/1.11.0", - ).start_as_current_span(name=name) as span: - if span.is_recording(): - for key, val in attributes.items(): - span.set_attribute(key, val) - return func(*args, **kwargs) - - return wrapper - - -def implement_span_delegator( - obj: _IffHasAttrDescriptor, attributes: Attributes = None -): - """Wrap the descriptor's fn with a span. - - Args: - obj: An instance of _IffHasAttrDescriptor - attributes: Attributes to apply to the span - """ - # Don't instrument inherited delegators - if hasattr(obj, "_otel_original_fn"): - logger.debug("Already instrumented: %s", obj.fn.__qualname__) - return - logger.debug("Instrumenting: %s", obj.fn.__qualname__) - attributes = attributes or {} - setattr(obj, "_otel_original_fn", getattr(obj, "fn")) - setattr( - obj, - "fn", - implement_span_function(obj.fn, obj.fn.__qualname__, attributes), - ) - - -def get_delegator( - estimator: Type[BaseEstimator], method_name: str -) -> Union[_IffHasAttrDescriptor, None]: - """Get the delegator from a class method or None. - - Args: - estimator: A class derived from ``sklearn``'s ``BaseEstimator``. - method_name (str): The method name of the estimator on which to - check for delegation. - - Returns: - The delegator, if one exists, otherwise None. - """ - class_attr = getattr(estimator, method_name) - if getattr(class_attr, "__closure__", None) is not None: - for cell in class_attr.__closure__: - if isinstance(cell.cell_contents, _IffHasAttrDescriptor): - return cell.cell_contents - return None - - -def get_base_estimators(packages: List[str]) -> Dict[str, Type[BaseEstimator]]: - """Walk package hierarchies to get BaseEstimator-derived classes. - - Args: - packages (list(str)): A list of package names to instrument. - - Returns: - A dictionary of qualnames and classes inheriting from - ``BaseEstimator``. - """ - klasses = {} - for package_name in packages: - lib = import_module(package_name) - package_dir = os.path.dirname(lib.__file__) - for _, module_name, _ in iter_modules([package_dir]): - # import the module and iterate through its attributes - try: - module = import_module(package_name + "." + module_name) - except ImportError: - logger.warning( - "Unable to import %s.%s", package_name, module_name - ) - continue - for attribute_name in dir(module): - attrib = getattr(module, attribute_name) - if isclass(attrib) and issubclass(attrib, BaseEstimator): - klasses[ - ".".join([package_name, module_name, attribute_name]) - ] = attrib - return klasses - - -# Methods on which spans should be applied. -DEFAULT_METHODS = [ - "fit", - "transform", - "predict", - "predict_proba", - "_fit", - "_transform", - "_predict", - "_predict_proba", -] - -# Classes and their attributes which contain a list of tupled estimators -# through which we should walk recursively for estimators. -DEFAULT_NAMEDTUPLE_ATTRIBS = { - Pipeline: ["steps"], - FeatureUnion: ["transformer_list"], -} - -# Classes and their attributes which contain an estimator or sequence of -# estimators through which we should walk recursively for estimators. -DEFAULT_ATTRIBS = {} - -# Classes (including children) explicitly excluded from autoinstrumentation -DEFAULT_EXCLUDE_CLASSES = [BaseDecisionTree] - -# Default packages for autoinstrumentation -DEFAULT_PACKAGES = ["sklearn"] - - -class SklearnInstrumentor(BaseInstrumentor): - """Instrument a fitted sklearn model with opentelemetry spans. - - Instrument methods of ``BaseEstimator``-derived components in a sklearn - model. The assumption is that a machine learning model ``Pipeline`` (or - class descendent) is being instrumented with opentelemetry. Within a - ``Pipeline`` is some hierarchy of estimators and transformers. - - The ``instrument_estimator`` method walks this hierarchy of estimators, - implementing each of the defined methods with its own span. - - Certain estimators in the sklearn ecosystem contain other estimators as - instance attributes. Support for walking this embedded sub-hierarchy is - supported with ``recurse_attribs``. This argument is a dictionary - with classes as keys, and a list of attributes representing embedded - estimators as values. By default, ``recurse_attribs`` is empty. - - Similar to Pipelines, there are also estimators which have class attributes - as a list of 2-tuples; for instance, the ``FeatureUnion`` and its attribute - ``transformer_list``. Instrumenting estimators like this is also - supported through the ``recurse_namedtuple_attribs`` argument. This - argument is a dictionary with classes as keys, and a list of attribute - names representing the namedtuple list(s). By default, the - ``recurse_namedtuple_attribs`` dictionary supports - ``Pipeline`` with ``steps``, and ``FeatureUnion`` with - ``transformer_list``. - - Note that spans will not be generated for any child transformer whose - parent transformer has ``n_jobs`` parameter set to anything besides - ``None`` or ``1``. - - Package instrumentation example: - - .. code-block:: python - - from opentelemetry.instrumentation.sklearn import SklearnInstrumentor - - # instrument the sklearn library - SklearnInstrumentor().instrument() - - # instrument several sklearn-compatible libraries - packages = ["sklearn", "lightgbm", "xgboost"] - SklearnInstrumentor(packages=packages).instrument() - - - Model instrumentation example: - - .. code-block:: python - - from opentelemetry.instrumentation.sklearn import SklearnInstrumentor - from sklearn.datasets import load_iris - from sklearn.ensemble import RandomForestClassifier - from sklearn.model_selection import train_test_split - from sklearn.pipeline import Pipeline - - X, y = load_iris(return_X_y=True) - X_train, X_test, y_train, y_test = train_test_split(X, y) - - model = Pipeline( - [ - ("class", RandomForestClassifier(n_estimators=10)), - ] - ) - - model.fit(X_train, y_train) - - SklearnInstrumentor().instrument_estimator(model) - - Args: - methods (list): A list of method names on which to instrument a span. - This list of methods will be checked on all estimators in the model - hierarchy. Used in package and model instrumentation - recurse_attribs (dict): A dictionary of ``BaseEstimator``-derived - sklearn classes as keys, with values being a list of attributes. Each - attribute represents either an estimator or list of estimators on - which to also implement spans. An example is - ``RandomForestClassifier`` and its attribute ``estimators_``. Used - in model instrumentation only. - recurse_namedtuple_attribs (dict): A dictionary of ``BaseEstimator``- - derived sklearn types as keys, with values being a list of - attribute names. Each attribute represents a list of 2-tuples in - which the first element is the estimator name, and the second - element is the estimator. Defaults include sklearn's ``Pipeline`` - and its attribute ``steps``, and the ``FeatureUnion`` and its - attribute ``transformer_list``. Used in model instrumentation only. - packages: A list of sklearn-compatible packages to - instrument. Used with package instrumentation only. - exclude_classes: A list of classes to exclude from instrumentation. - Child classes are also excluded. Default is sklearn's - ``[BaseDecisionTree]``. - """ - - def __new__(cls, *args, **kwargs): - """Override new. - - The base class' new method passes args and kwargs. We override because - we init the class with configuration and Python raises TypeError when - additional arguments are passed to the object.__new__() method. - """ - if cls._instance is None: - cls._instance = object.__new__(cls) - - return cls._instance - - def __init__( - self, - methods: List[str] = None, - recurse_attribs: Dict[Type[BaseEstimator], List[str]] = None, - recurse_namedtuple_attribs: Dict[ - Type[BaseEstimator], List[str] - ] = None, - packages: List[str] = None, - exclude_classes: List[Type] = None, - ): - self.methods = methods or DEFAULT_METHODS - self.recurse_attribs = recurse_attribs or DEFAULT_ATTRIBS - self.recurse_namedtuple_attribs = ( - recurse_namedtuple_attribs or DEFAULT_NAMEDTUPLE_ATTRIBS - ) - self.packages = packages or DEFAULT_PACKAGES - if exclude_classes is None: - self.exclude_classes = tuple(DEFAULT_EXCLUDE_CLASSES) - else: - self.exclude_classes = tuple(exclude_classes) - - def instrumentation_dependencies(self) -> Collection[str]: - return _instruments - - def _instrument(self, **kwargs): - """Instrument the library, and any additional specified on init.""" - klasses = get_base_estimators(packages=self.packages) - attributes = kwargs.get("attributes") - for _, klass in klasses.items(): - if issubclass(klass, self.exclude_classes): - logger.debug("Not instrumenting (excluded): %s", str(klass)) - else: - logger.debug("Instrumenting: %s", str(klass)) - for method_name in self.methods: - if hasattr(klass, method_name): - self._instrument_class_method( - estimator=klass, - method_name=method_name, - attributes=attributes, - ) - - def _uninstrument(self, **kwargs): - """Uninstrument the library""" - klasses = get_base_estimators(packages=self.packages) - for _, klass in klasses.items(): - logger.debug("Uninstrumenting: %s", str(klass)) - for method_name in self.methods: - if hasattr(klass, method_name): - self._uninstrument_class_method( - estimator=klass, method_name=method_name - ) - - def instrument_estimator( - self, estimator: BaseEstimator, attributes: Attributes = None - ): - """Instrument a fitted estimator and its hierarchy where configured. - - Args: - estimator (sklearn.base.BaseEstimator): A fitted ``sklearn`` - estimator, typically a ``Pipeline`` instance. - attributes (dict): Attributes to attach to the spans. - """ - if isinstance(estimator, self.exclude_classes): - logger.debug( - "Not instrumenting (excluded): %s", - estimator.__class__.__name__, - ) - return - - if isinstance( - estimator, tuple(self.recurse_namedtuple_attribs.keys()) - ): - self._instrument_estimator_namedtuple( - estimator=estimator, attributes=attributes - ) - - if isinstance(estimator, tuple(self.recurse_attribs.keys())): - self._instrument_estimator_attribute( - estimator=estimator, attributes=attributes - ) - - for method_name in self.methods: - if hasattr(estimator, method_name): - self._instrument_instance_method( - estimator=estimator, - method_name=method_name, - attributes=attributes, - ) - - def uninstrument_estimator(self, estimator: BaseEstimator): - """Uninstrument a fitted estimator and its hierarchy where configured. - - Args: - estimator (sklearn.base.BaseEstimator): A fitted ``sklearn`` - estimator, typically a ``Pipeline`` instance. - """ - if isinstance(estimator, self.exclude_classes): - logger.debug( - "Not uninstrumenting (excluded): %s", - estimator.__class__.__name__, - ) - return - - if isinstance( - estimator, tuple(self.recurse_namedtuple_attribs.keys()) - ): - self._uninstrument_estimator_namedtuple(estimator=estimator) - - if isinstance(estimator, tuple(self.recurse_attribs.keys())): - self._uninstrument_estimator_attribute(estimator=estimator) - - for method_name in self.methods: - if hasattr(estimator, method_name): - self._uninstrument_instance_method( - estimator=estimator, method_name=method_name - ) - - def _check_instrumented( - self, - estimator: Union[BaseEstimator, Type[BaseEstimator]], - method_name: str, - ) -> bool: - """Check an estimator-method is instrumented. - - Args: - estimator (BaseEstimator): A class or instance of an ``sklearn`` - estimator. - method_name (str): The method name of the estimator on which to - check for instrumentation. - """ - orig_method_name = "_otel_original_" + method_name - has_original = hasattr(estimator, orig_method_name) - orig_class, orig_method = getattr( - estimator, orig_method_name, (None, None) - ) - same_class = orig_class == estimator - if has_original and same_class: - class_method = self._unwrap_function( - getattr(estimator, method_name) - ) - # if they match then the subclass doesn't override - # if they don't then the overridden method needs instrumentation - if class_method.__name__ == orig_method.__name__: - return True - return False - - def _uninstrument_class_method( - self, estimator: Type[BaseEstimator], method_name: str - ): - """Uninstrument a class method. - - Replaces the patched method with the original, and deletes the - attribute which stored the original method. - - Args: - estimator (BaseEstimator): A class or instance of an ``sklearn`` - estimator. - method_name (str): The method name of the estimator on which to - apply a span. - """ - orig_method_name = "_otel_original_" + method_name - if isclass(estimator): - qualname = estimator.__qualname__ - else: - qualname = estimator.__class__.__qualname__ - delegator = get_delegator(estimator, method_name) - if self._check_instrumented(estimator, method_name): - logger.debug( - "Uninstrumenting: %s.%s", - qualname, - method_name, - ) - _, orig_method = getattr(estimator, orig_method_name) - setattr( - estimator, - method_name, - orig_method, - ) - delattr(estimator, orig_method_name) - elif delegator is not None: - if not hasattr(delegator, "_otel_original_fn"): - logger.debug( - "Already uninstrumented: %s.%s", - qualname, - method_name, - ) - return - setattr( - delegator, - "fn", - getattr(delegator, "_otel_original_fn"), - ) - delattr(delegator, "_otel_original_fn") - else: - logger.debug( - "Already uninstrumented: %s.%s", - qualname, - method_name, - ) - - def _uninstrument_instance_method( - self, estimator: BaseEstimator, method_name: str - ): - """Uninstrument an instance method. - - Replaces the patched method with the original, and deletes the - attribute which stored the original method. - - Args: - estimator (BaseEstimator): A class or instance of an ``sklearn`` - estimator. - method_name (str): The method name of the estimator on which to - apply a span. - """ - orig_method_name = "_otel_original_" + method_name - if isclass(estimator): - qualname = estimator.__qualname__ - else: - qualname = estimator.__class__.__qualname__ - if self._check_instrumented(estimator, method_name): - logger.debug( - "Uninstrumenting: %s.%s", - qualname, - method_name, - ) - _, orig_method = getattr(estimator, orig_method_name) - setattr( - estimator, - method_name, - orig_method, - ) - delattr(estimator, orig_method_name) - else: - logger.debug( - "Already uninstrumented: %s.%s", - qualname, - method_name, - ) - - def _instrument_class_method( - self, - estimator: Type[BaseEstimator], - method_name: str, - attributes: Attributes = None, - ): - """Instrument an estimator method with a span. - - When instrumenting we attach a tuple of (Class, method) to the - attribute ``_otel_original_`` for each method. This allows - us to replace the patched with the original in uninstrumentation, but - also allows proper instrumentation of child classes without - instrumenting inherited methods twice. - - Args: - estimator (BaseEstimator): A ``BaseEstimator``-derived - class - method_name (str): The method name of the estimator on which to - apply a span. - attributes (dict): Attributes to attach to the spans. - """ - if self._check_instrumented(estimator, method_name): - logger.debug( - "Already instrumented: %s.%s", - estimator.__qualname__, - method_name, - ) - return - class_attr = getattr(estimator, method_name) - delegator = get_delegator(estimator, method_name) - if isinstance(class_attr, property): - logger.debug( - "Not instrumenting found property: %s.%s", - estimator.__qualname__, - method_name, - ) - elif delegator is not None: - implement_span_delegator(delegator) - else: - setattr( - estimator, - "_otel_original_" + method_name, - (estimator, class_attr), - ) - setattr( - estimator, - method_name, - implement_span_estimator(class_attr, estimator, attributes), - ) - - def _unwrap_function(self, function): - """Fetch the function underlying any decorators""" - if hasattr(function, "__wrapped__"): - return self._unwrap_function(function.__wrapped__) - return function - - def _instrument_instance_method( - self, - estimator: BaseEstimator, - method_name: str, - attributes: Attributes = None, - ): - """Instrument an estimator instance method with a span. - - When instrumenting we attach a tuple of (Class, method) to the - attribute ``_otel_original_`` for each method. This allows - us to replace the patched with the original in unstrumentation. - - Args: - estimator (BaseEstimator): A fitted ``sklearn`` estimator. - method_name (str): The method name of the estimator on which to - apply a span. - attributes (dict): Attributes to attach to the spans. - """ - if self._check_instrumented(estimator, method_name): - logger.debug( - "Already instrumented: %s.%s", - estimator.__class__.__qualname__, - method_name, - ) - return - - class_attr = getattr(type(estimator), method_name, None) - if isinstance(class_attr, property): - logger.debug( - "Not instrumenting found property: %s.%s", - estimator.__class__.__qualname__, - method_name, - ) - else: - method = getattr(estimator, method_name) - setattr( - estimator, "_otel_original_" + method_name, (estimator, method) - ) - setattr( - estimator, - method_name, - implement_span_estimator(method, estimator, attributes), - ) - - def _instrument_estimator_attribute( - self, estimator: BaseEstimator, attributes: Attributes = None - ): - """Instrument instance attributes which also contain estimators. - - Handle instance attributes which are also estimators, are a list - (Sequence) of estimators, or are mappings (dictionary) in which - the values are estimators. - - Examples include ``RandomForestClassifier`` and - ``MultiOutputRegressor`` instances which have attributes - ``estimators_`` attributes. - - Args: - estimator (BaseEstimator): A fitted ``sklearn`` estimator, with an - attribute which also contains an estimator or collection of - estimators. - attributes (dict): Attributes to attach to the spans. - """ - attribs = self.recurse_attribs.get(estimator.__class__, []) - for attrib in attribs: - attrib_value = getattr(estimator, attrib) - if isinstance(attrib_value, Sequence): - for value in attrib_value: - self.instrument_estimator( - estimator=value, attributes=attributes - ) - elif isinstance(attrib_value, MutableMapping): - for value in attrib_value.values(): - self.instrument_estimator( - estimator=value, attributes=attributes - ) - else: - self.instrument_estimator( - estimator=attrib_value, attributes=attributes - ) - - def _instrument_estimator_namedtuple( - self, estimator: BaseEstimator, attributes: Attributes = None - ): - """Instrument attributes with (name, estimator) tupled components. - - Examples include Pipeline and FeatureUnion instances which - have attributes steps and transformer_list, respectively. - - Args: - estimator: A fitted sklearn estimator, with an attribute which also - contains an estimator or collection of estimators. - attributes (dict): Attributes to attach to the spans. - """ - attribs = self.recurse_namedtuple_attribs.get(estimator.__class__, []) - for attrib in attribs: - for _, est in getattr(estimator, attrib): - self.instrument_estimator(estimator=est, attributes=attributes) - - def _uninstrument_estimator_attribute(self, estimator: BaseEstimator): - """Uninstrument instance attributes which also contain estimators. - - Handle instance attributes which are also estimators, are a list - (Sequence) of estimators, or are mappings (dictionary) in which - the values are estimators. - - Examples include ``RandomForestClassifier`` and - ``MultiOutputRegressor`` instances which have attributes - ``estimators_`` attributes. - - Args: - estimator (BaseEstimator): A fitted ``sklearn`` estimator, with an - attribute which also contains an estimator or collection of - estimators. - """ - attribs = self.recurse_attribs.get(estimator.__class__, []) - for attrib in attribs: - attrib_value = getattr(estimator, attrib) - if isinstance(attrib_value, Sequence): - for value in attrib_value: - self.uninstrument_estimator(estimator=value) - elif isinstance(attrib_value, MutableMapping): - for value in attrib_value.values(): - self.uninstrument_estimator(estimator=value) - else: - self.uninstrument_estimator(estimator=attrib_value) - - def _uninstrument_estimator_namedtuple(self, estimator: BaseEstimator): - """Uninstrument attributes with (name, estimator) tupled components. - - Examples include Pipeline and FeatureUnion instances which - have attributes steps and transformer_list, respectively. - - Args: - estimator: A fitted sklearn estimator, with an attribute which also - contains an estimator or collection of estimators. - """ - attribs = self.recurse_namedtuple_attribs.get(estimator.__class__, []) - for attrib in attribs: - for _, est in getattr(estimator, attrib): - self.uninstrument_estimator(estimator=est) diff --git a/instrumentation/opentelemetry-instrumentation-sklearn/src/opentelemetry/instrumentation/sklearn/package.py b/instrumentation/opentelemetry-instrumentation-sklearn/src/opentelemetry/instrumentation/sklearn/package.py deleted file mode 100644 index 41db461453..0000000000 --- a/instrumentation/opentelemetry-instrumentation-sklearn/src/opentelemetry/instrumentation/sklearn/package.py +++ /dev/null @@ -1,16 +0,0 @@ -# Copyright 2020, 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. - - -_instruments = ("scikit-learn ~= 0.24.0",) diff --git a/instrumentation/opentelemetry-instrumentation-sklearn/src/opentelemetry/instrumentation/sklearn/version.py b/instrumentation/opentelemetry-instrumentation-sklearn/src/opentelemetry/instrumentation/sklearn/version.py deleted file mode 100644 index deef26c62f..0000000000 --- a/instrumentation/opentelemetry-instrumentation-sklearn/src/opentelemetry/instrumentation/sklearn/version.py +++ /dev/null @@ -1,15 +0,0 @@ -# Copyright 2020, 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. - -__version__ = "0.47b0.dev" diff --git a/instrumentation/opentelemetry-instrumentation-sklearn/test-requirements.txt b/instrumentation/opentelemetry-instrumentation-sklearn/test-requirements.txt deleted file mode 100644 index af341e16ed..0000000000 --- a/instrumentation/opentelemetry-instrumentation-sklearn/test-requirements.txt +++ /dev/null @@ -1,19 +0,0 @@ -asgiref==3.7.2 -Deprecated==1.2.14 -importlib-metadata==6.11.0 -iniconfig==2.0.0 -joblib==1.3.2 -numpy==1.24.4 -packaging==24.0 -pluggy==1.5.0 -py-cpuinfo==9.0.0 -pytest==7.4.4 -scikit-learn==0.24.2 -scipy==1.10.1 -threadpoolctl==3.3.0 -tomli==2.0.1 -typing_extensions==4.10.0 -wrapt==1.16.0 -zipp==3.19.2 --e opentelemetry-instrumentation --e instrumentation/opentelemetry-instrumentation-sklearn diff --git a/instrumentation/opentelemetry-instrumentation-sklearn/tests/__init__.py b/instrumentation/opentelemetry-instrumentation-sklearn/tests/__init__.py deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/instrumentation/opentelemetry-instrumentation-sklearn/tests/fixtures.py b/instrumentation/opentelemetry-instrumentation-sklearn/tests/fixtures.py deleted file mode 100644 index cf26c0fcf2..0000000000 --- a/instrumentation/opentelemetry-instrumentation-sklearn/tests/fixtures.py +++ /dev/null @@ -1,54 +0,0 @@ -# Copyright 2020, 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. - -import numpy as np -from sklearn.datasets import load_iris -from sklearn.decomposition import PCA, TruncatedSVD -from sklearn.ensemble import RandomForestClassifier -from sklearn.model_selection import train_test_split -from sklearn.pipeline import FeatureUnion, Pipeline -from sklearn.preprocessing import Normalizer, StandardScaler - -X, y = load_iris(return_X_y=True) -X_train, X_test, y_train, y_test = train_test_split(X, y) - - -def pipeline(): - """A dummy model that has a bunch of components that we can test.""" - model = Pipeline( - [ - ("scaler", StandardScaler()), - ("normal", Normalizer()), - ( - "union", - FeatureUnion( - [ - ("pca", PCA(n_components=1)), - ("svd", TruncatedSVD(n_components=2)), - ], - n_jobs=1, # parallelized components won't generate spans - ), - ), - ("class", RandomForestClassifier(n_estimators=10)), - ] - ) - model.fit(X_train, y_train) - return model - - -def random_input(): - """A random record from the feature set.""" - rows = X.shape[0] - random_row = np.random.choice(rows, size=1) - return X[random_row, :] diff --git a/instrumentation/opentelemetry-instrumentation-sklearn/tests/test_sklearn.py b/instrumentation/opentelemetry-instrumentation-sklearn/tests/test_sklearn.py deleted file mode 100644 index db69761ece..0000000000 --- a/instrumentation/opentelemetry-instrumentation-sklearn/tests/test_sklearn.py +++ /dev/null @@ -1,190 +0,0 @@ -# Copyright 2020, 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. - -from sklearn.ensemble import RandomForestClassifier - -# pylint: disable=no-name-in-module -from opentelemetry.instrumentation.sklearn import ( - DEFAULT_EXCLUDE_CLASSES, - DEFAULT_METHODS, - SklearnInstrumentor, - get_base_estimators, - get_delegator, -) -from opentelemetry.test.test_base import TestBase -from opentelemetry.trace import SpanKind - -from .fixtures import pipeline, random_input - - -def assert_instrumented(base_estimators): - for _, estimator in base_estimators.items(): - for method_name in DEFAULT_METHODS: - original_method_name = "_otel_original_" + method_name - if issubclass(estimator, tuple(DEFAULT_EXCLUDE_CLASSES)): - assert not hasattr(estimator, original_method_name) - continue - class_attr = getattr(estimator, method_name, None) - if isinstance(class_attr, property): - assert not hasattr(estimator, original_method_name) - continue - delegator = None - if hasattr(estimator, method_name): - delegator = get_delegator(estimator, method_name) - if delegator is not None: - assert hasattr(delegator, "_otel_original_fn") - elif hasattr(estimator, method_name): - assert hasattr(estimator, original_method_name) - - -def assert_uninstrumented(base_estimators): - for _, estimator in base_estimators.items(): - for method_name in DEFAULT_METHODS: - original_method_name = "_otel_original_" + method_name - if issubclass(estimator, tuple(DEFAULT_EXCLUDE_CLASSES)): - assert not hasattr(estimator, original_method_name) - continue - class_attr = getattr(estimator, method_name, None) - if isinstance(class_attr, property): - assert not hasattr(estimator, original_method_name) - continue - delegator = None - if hasattr(estimator, method_name): - delegator = get_delegator(estimator, method_name) - if delegator is not None: - assert not hasattr(delegator, "_otel_original_fn") - elif hasattr(estimator, method_name): - assert not hasattr(estimator, original_method_name) - - -class TestSklearn(TestBase): - def test_package_instrumentation(self): - ski = SklearnInstrumentor() - - base_estimators = get_base_estimators(packages=["sklearn"]) - - model = pipeline() - - ski.instrument() - assert_instrumented(base_estimators) - - x_test = random_input() - - model.predict(x_test) - - spans = self.memory_exporter.get_finished_spans() - self.assertEqual(len(spans), 8) - self.memory_exporter.clear() - - ski.uninstrument() - assert_uninstrumented(base_estimators) - - model = pipeline() - x_test = random_input() - - model.predict(x_test) - - spans = self.memory_exporter.get_finished_spans() - self.assertEqual(len(spans), 0) - - def test_span_properties(self): - """Test that we get all of the spans we expect.""" - model = pipeline() - ski = SklearnInstrumentor() - ski.instrument_estimator(estimator=model) - - x_test = random_input() - - model.predict(x_test) - - spans = self.memory_exporter.get_finished_spans() - self.assertEqual(len(spans), 8) - span = spans[0] - self.assertEqual(span.name, "StandardScaler.transform") - self.assertEqual(span.kind, SpanKind.INTERNAL) - self.assertEqual(span.parent.span_id, spans[-1].context.span_id) - span = spans[1] - self.assertEqual(span.name, "Normalizer.transform") - self.assertEqual(span.kind, SpanKind.INTERNAL) - self.assertEqual(span.parent.span_id, spans[-1].context.span_id) - span = spans[2] - self.assertEqual(span.name, "PCA.transform") - self.assertEqual(span.kind, SpanKind.INTERNAL) - self.assertEqual(span.parent.span_id, spans[4].context.span_id) - span = spans[3] - self.assertEqual(span.name, "TruncatedSVD.transform") - self.assertEqual(span.kind, SpanKind.INTERNAL) - self.assertEqual(span.parent.span_id, spans[4].context.span_id) - span = spans[4] - self.assertEqual(span.name, "FeatureUnion.transform") - self.assertEqual(span.kind, SpanKind.INTERNAL) - self.assertEqual(span.parent.span_id, spans[-1].context.span_id) - span = spans[5] - self.assertEqual(span.name, "RandomForestClassifier.predict_proba") - self.assertEqual(span.kind, SpanKind.INTERNAL) - self.assertEqual(span.parent.span_id, spans[6].context.span_id) - span = spans[6] - self.assertEqual(span.name, "RandomForestClassifier.predict") - self.assertEqual(span.kind, SpanKind.INTERNAL) - self.assertEqual(span.parent.span_id, spans[-1].context.span_id) - span = spans[7] - self.assertEqual(span.name, "Pipeline.predict") - self.assertEqual(span.kind, SpanKind.INTERNAL) - - self.memory_exporter.clear() - - # uninstrument - ski.uninstrument_estimator(estimator=model) - x_test = random_input() - model.predict(x_test) - spans = self.memory_exporter.get_finished_spans() - self.assertEqual(len(spans), 0) - - def test_attrib_config(self): - """Test that the attribute config makes spans on the decision trees.""" - model = pipeline() - attrib_config = {RandomForestClassifier: ["estimators_"]} - ski = SklearnInstrumentor( - recurse_attribs=attrib_config, - exclude_classes=[], # decision trees excluded by default - ) - ski.instrument_estimator(estimator=model) - - x_test = random_input() - model.predict(x_test) - - spans = self.memory_exporter.get_finished_spans() - self.assertEqual(len(spans), 8 + model.steps[-1][-1].n_estimators) - - self.memory_exporter.clear() - - ski.uninstrument_estimator(estimator=model) - x_test = random_input() - model.predict(x_test) - spans = self.memory_exporter.get_finished_spans() - self.assertEqual(len(spans), 0) - - def test_span_attributes(self): - model = pipeline() - attributes = {"model_name": "random_forest_model"} - ski = SklearnInstrumentor() - ski.instrument_estimator(estimator=model, attributes=attributes) - - x_test = random_input() - - model.predict(x_test) - - spans = self.memory_exporter.get_finished_spans() - for span in spans: - assert span.attributes["model_name"] == "random_forest_model" diff --git a/opentelemetry-contrib-instrumentations/pyproject.toml b/opentelemetry-contrib-instrumentations/pyproject.toml index 74c28f38cf..f83b9d5ee5 100644 --- a/opentelemetry-contrib-instrumentations/pyproject.toml +++ b/opentelemetry-contrib-instrumentations/pyproject.toml @@ -66,7 +66,6 @@ dependencies = [ "opentelemetry-instrumentation-redis==0.47b0.dev", "opentelemetry-instrumentation-remoulade==0.47b0.dev", "opentelemetry-instrumentation-requests==0.47b0.dev", - "opentelemetry-instrumentation-sklearn==0.47b0.dev", "opentelemetry-instrumentation-sqlalchemy==0.47b0.dev", "opentelemetry-instrumentation-sqlite3==0.47b0.dev", "opentelemetry-instrumentation-starlette==0.47b0.dev", diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/bootstrap_gen.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/bootstrap_gen.py index 3dfd97e0b2..3f4c78862f 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/bootstrap_gen.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/bootstrap_gen.py @@ -152,10 +152,6 @@ "library": "requests ~= 2.0", "instrumentation": "opentelemetry-instrumentation-requests==0.47b0.dev", }, - { - "library": "scikit-learn ~= 0.24.0", - "instrumentation": "opentelemetry-instrumentation-sklearn==0.47b0.dev", - }, { "library": "sqlalchemy", "instrumentation": "opentelemetry-instrumentation-sqlalchemy==0.47b0.dev", diff --git a/tox.ini b/tox.ini index 529e2a76c0..aac890ed7c 100644 --- a/tox.ini +++ b/tox.ini @@ -273,10 +273,6 @@ envlist = pypy3-test-instrumentation-celery lint-instrumentation-celery - ; opentelemetry-instrumentation-sklearn - py3{8}-test-instrumentation-sklearn - lint-instrumentation-sklearn - ; opentelemetry-instrumentation-system-metrics py3{8,9,10,11,12}-test-instrumentation-system-metrics pypy3-test-instrumentation-system-metrics @@ -701,12 +697,6 @@ commands_pre = prometheus: pip install opentelemetry-test-utils@{env:CORE_REPO}\#egg=opentelemetry-test-utils&subdirectory=tests/opentelemetry-test-utils prometheus: pip install -r {toxinidir}/exporter/opentelemetry-exporter-prometheus-remote-write/test-requirements.txt - sklearn: pip install opentelemetry-api@{env:CORE_REPO}\#egg=opentelemetry-api&subdirectory=opentelemetry-api - sklearn: pip install opentelemetry-semantic-conventions@{env:CORE_REPO}\#egg=opentelemetry-semantic-conventions&subdirectory=opentelemetry-semantic-conventions - sklearn: pip install opentelemetry-sdk@{env:CORE_REPO}\#egg=opentelemetry-sdk&subdirectory=opentelemetry-sdk - sklearn: pip install opentelemetry-test-utils@{env:CORE_REPO}\#egg=opentelemetry-test-utils&subdirectory=tests/opentelemetry-test-utils - sklearn: pip install -r {toxinidir}/instrumentation/opentelemetry-instrumentation-sklearn/test-requirements.txt - sqlalchemy: pip install opentelemetry-api@{env:CORE_REPO}\#egg=opentelemetry-api&subdirectory=opentelemetry-api sqlalchemy: pip install opentelemetry-semantic-conventions@{env:CORE_REPO}\#egg=opentelemetry-semantic-conventions&subdirectory=opentelemetry-semantic-conventions sqlalchemy: pip install opentelemetry-sdk@{env:CORE_REPO}\#egg=opentelemetry-sdk&subdirectory=opentelemetry-sdk @@ -1021,12 +1011,6 @@ commands = lint-instrumentation-requests: flake8 --config {toxinidir}/.flake8 {toxinidir}/instrumentation/opentelemetry-instrumentation-requests lint-instrumentation-requests: sh -c "cd instrumentation && pylint --rcfile ../.pylintrc opentelemetry-instrumentation-requests" - test-instrumentation-sklearn: pytest {toxinidir}/instrumentation/opentelemetry-instrumentation-sklearn/tests {posargs} - lint-instrumentation-sklearn: black --diff --check --config {toxinidir}/pyproject.toml {toxinidir}/instrumentation/opentelemetry-instrumentation-sklearn - lint-instrumentation-sklearn: isort --diff --check-only --settings-path {toxinidir}/.isort.cfg {toxinidir}/instrumentation/opentelemetry-instrumentation-sklearn - lint-instrumentation-sklearn: flake8 --config {toxinidir}/.flake8 {toxinidir}/instrumentation/opentelemetry-instrumentation-sklearn - lint-instrumentation-sklearn: sh -c "cd instrumentation && pylint --rcfile ../.pylintrc opentelemetry-instrumentation-sklearn" - test-instrumentation-sqlalchemy: pytest {toxinidir}/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests {posargs} lint-instrumentation-sqlalchemy: black --diff --check --config {toxinidir}/pyproject.toml {toxinidir}/instrumentation/opentelemetry-instrumentation-sqlalchemy lint-instrumentation-sqlalchemy: isort --diff --check-only --settings-path {toxinidir}/.isort.cfg {toxinidir}/instrumentation/opentelemetry-instrumentation-sqlalchemy From e6c27e08007c4acf2a3fa9f903c52c4656b9ea62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Em=C3=ADdio=20Neto?= <9735060+emdneto@users.noreply.github.com> Date: Wed, 17 Jul 2024 14:46:31 -0300 Subject: [PATCH 15/26] Add support to instrument httpx when using proxy (#2664) --- CHANGELOG.md | 4 +- .../instrumentation/httpx/__init__.py | 69 +++++++++ .../tests/test_httpx_integration.py | 139 +++++++++++++++++- 3 files changed, 205 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2de76b2736..98bbd3a0bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#2630](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2630)) - `opentelemetry-instrumentation-system-metrics` Add support for capture open file descriptors ([#2652](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2652)) +- `opentelemetry-instrumentation-httpx` Add support for instrument client with proxy + ([#2664](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2664)) - `opentelemetry-instrumentation-aiohttp-client` Implement new semantic convention opt-in migration ([#2673](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2673)) - `opentelemetry-instrumentation-django` Add `http.target` to Django duration metric attributes @@ -63,7 +65,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `opentelemetry-instrumentation-asgi` Fix generation of `http.target` and `http.url` attributes for ASGI apps using sub apps ([#2477](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2477)) -- `opentelemetry-instrumentation-aws-lambda` Bugfix: AWS Lambda event source key incorrect for SNS in instrumentation library. +- `opentelemetry-instrumentation-aws-lambda` Bugfix: AWS Lambda event source key incorrect for SNS in instrumentation library. ([#2612](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2612)) - `opentelemetry-instrumentation-asyncio` instrumented `asyncio.wait_for` properly raises `asyncio.TimeoutError` as expected ([#2637](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2637)) diff --git a/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py b/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py index d2ff0be292..e3ce383d7e 100644 --- a/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py @@ -640,6 +640,7 @@ def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) self._original_transport = self._transport + self._original_mounts = self._mounts.copy() self._is_instrumented_by_opentelemetry = True self._transport = SyncOpenTelemetryTransport( @@ -648,6 +649,21 @@ def __init__(self, *args, **kwargs): request_hook=_InstrumentedClient._request_hook, response_hook=_InstrumentedClient._response_hook, ) + self._mounts.update( + { + url_pattern: ( + SyncOpenTelemetryTransport( + transport, + tracer_provider=_InstrumentedClient._tracer_provider, + request_hook=_InstrumentedClient._request_hook, + response_hook=_InstrumentedClient._response_hook, + ) + if transport is not None + else transport + ) + for url_pattern, transport in self._original_mounts.items() + } + ) class _InstrumentedAsyncClient(httpx.AsyncClient): @@ -659,6 +675,7 @@ def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) self._original_transport = self._transport + self._original_mounts = self._mounts.copy() self._is_instrumented_by_opentelemetry = True self._transport = AsyncOpenTelemetryTransport( @@ -668,6 +685,22 @@ def __init__(self, *args, **kwargs): response_hook=_InstrumentedAsyncClient._response_hook, ) + self._mounts.update( + { + url_pattern: ( + AsyncOpenTelemetryTransport( + transport, + tracer_provider=_InstrumentedAsyncClient._tracer_provider, + request_hook=_InstrumentedAsyncClient._request_hook, + response_hook=_InstrumentedAsyncClient._response_hook, + ) + if transport is not None + else transport + ) + for url_pattern, transport in self._original_mounts.items() + } + ) + class HTTPXClientInstrumentor(BaseInstrumentor): # pylint: disable=protected-access,attribute-defined-outside-init @@ -752,6 +785,7 @@ def instrument_client( if not client._is_instrumented_by_opentelemetry: if isinstance(client, httpx.Client): client._original_transport = client._transport + client._original_mounts = client._mounts.copy() transport = client._transport or httpx.HTTPTransport() client._transport = SyncOpenTelemetryTransport( transport, @@ -760,8 +794,25 @@ def instrument_client( response_hook=response_hook, ) client._is_instrumented_by_opentelemetry = True + client._mounts.update( + { + url_pattern: ( + SyncOpenTelemetryTransport( + transport, + tracer_provider=tracer_provider, + request_hook=request_hook, + response_hook=response_hook, + ) + if transport is not None + else transport + ) + for url_pattern, transport in client._original_mounts.items() + } + ) + if isinstance(client, httpx.AsyncClient): transport = client._transport or httpx.AsyncHTTPTransport() + client._original_mounts = client._mounts.copy() client._transport = AsyncOpenTelemetryTransport( transport, tracer_provider=tracer_provider, @@ -769,6 +820,21 @@ def instrument_client( response_hook=response_hook, ) client._is_instrumented_by_opentelemetry = True + client._mounts.update( + { + url_pattern: ( + AsyncOpenTelemetryTransport( + transport, + tracer_provider=tracer_provider, + request_hook=request_hook, + response_hook=response_hook, + ) + if transport is not None + else transport + ) + for url_pattern, transport in client._original_mounts.items() + } + ) else: _logger.warning( "Attempting to instrument Httpx client while already instrumented" @@ -787,6 +853,9 @@ def uninstrument_client( client._transport = client._original_transport del client._original_transport client._is_instrumented_by_opentelemetry = False + if hasattr(client, "_original_mounts"): + client._mounts = client._original_mounts.copy() + del client._original_mounts else: _logger.warning( "Attempting to uninstrument Httpx " diff --git a/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py b/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py index 84bab598e6..03141e61b5 100644 --- a/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py +++ b/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py @@ -530,6 +530,7 @@ def create_transport( tracer_provider: typing.Optional["TracerProvider"] = None, request_hook: typing.Optional["RequestHook"] = None, response_hook: typing.Optional["ResponseHook"] = None, + **kwargs, ): pass @@ -539,6 +540,7 @@ def create_client( transport: typing.Union[ SyncOpenTelemetryTransport, AsyncOpenTelemetryTransport, None ] = None, + **kwargs, ): pass @@ -643,6 +645,30 @@ def test_not_recording_not_set_attribute_in_exception_new_semconv( self.assertFalse(mock_span.set_attribute.called) self.assertFalse(mock_span.set_status.called) + @respx.mock + def test_client_mounts_with_instrumented_transport(self): + https_url = "https://mock/status/200" + respx.get(https_url).mock(httpx.Response(200)) + proxy_mounts = { + "http://": self.create_transport( + proxy=httpx.Proxy("http://localhost:8080") + ), + "https://": self.create_transport( + proxy=httpx.Proxy("http://localhost:8443") + ), + } + client1 = self.create_client(mounts=proxy_mounts) + client2 = self.create_client(mounts=proxy_mounts) + self.perform_request(self.URL, client=client1) + self.perform_request(https_url, client=client2) + spans = self.assert_span(num_spans=2) + self.assertEqual( + spans[0].attributes[SpanAttributes.HTTP_URL], self.URL + ) + self.assertEqual( + spans[1].attributes[SpanAttributes.HTTP_URL], https_url + ) + class BaseInstrumentorTest(BaseTest, metaclass=abc.ABCMeta): @abc.abstractmethod def create_client( @@ -650,15 +676,39 @@ def create_client( transport: typing.Union[ SyncOpenTelemetryTransport, AsyncOpenTelemetryTransport, None ] = None, + **kwargs, ): pass + @abc.abstractmethod + def create_proxy_transport(self, url: str): + pass + def setUp(self): super().setUp() HTTPXClientInstrumentor().instrument() self.client = self.create_client() HTTPXClientInstrumentor().uninstrument() + def create_proxy_mounts(self): + return { + "http://": self.create_proxy_transport( + "http://localhost:8080" + ), + "https://": self.create_proxy_transport( + "http://localhost:8080" + ), + } + + def assert_proxy_mounts(self, mounts, num_mounts, transport_type): + self.assertEqual(len(mounts), num_mounts) + for transport in mounts: + with self.subTest(transport): + self.assertIsInstance( + transport, + transport_type, + ) + def test_custom_tracer_provider(self): resource = resources.Resource.create({}) result = self.create_tracer_provider(resource=resource) @@ -855,6 +905,71 @@ def test_uninstrument_new_client(self): self.assertEqual(result.text, "Hello!") self.assert_span() + def test_instrument_proxy(self): + proxy_mounts = self.create_proxy_mounts() + HTTPXClientInstrumentor().instrument() + client = self.create_client(mounts=proxy_mounts) + self.perform_request(self.URL, client=client) + self.assert_span(num_spans=1) + self.assert_proxy_mounts( + client._mounts.values(), + 2, + (SyncOpenTelemetryTransport, AsyncOpenTelemetryTransport), + ) + HTTPXClientInstrumentor().uninstrument() + + def test_instrument_client_with_proxy(self): + proxy_mounts = self.create_proxy_mounts() + client = self.create_client(mounts=proxy_mounts) + self.assert_proxy_mounts( + client._mounts.values(), + 2, + (httpx.HTTPTransport, httpx.AsyncHTTPTransport), + ) + HTTPXClientInstrumentor().instrument_client(client) + result = self.perform_request(self.URL, client=client) + self.assertEqual(result.text, "Hello!") + self.assert_span(num_spans=1) + self.assert_proxy_mounts( + client._mounts.values(), + 2, + (SyncOpenTelemetryTransport, AsyncOpenTelemetryTransport), + ) + HTTPXClientInstrumentor().uninstrument_client(client) + + def test_uninstrument_client_with_proxy(self): + proxy_mounts = self.create_proxy_mounts() + HTTPXClientInstrumentor().instrument() + client = self.create_client(mounts=proxy_mounts) + self.assert_proxy_mounts( + client._mounts.values(), + 2, + (SyncOpenTelemetryTransport, AsyncOpenTelemetryTransport), + ) + + HTTPXClientInstrumentor().uninstrument_client(client) + result = self.perform_request(self.URL, client=client) + + self.assertEqual(result.text, "Hello!") + self.assert_span(num_spans=0) + self.assert_proxy_mounts( + client._mounts.values(), + 2, + (httpx.HTTPTransport, httpx.AsyncHTTPTransport), + ) + # Test that other clients as well as instance client is still + # instrumented + client2 = self.create_client() + result = self.perform_request(self.URL, client=client2) + self.assertEqual(result.text, "Hello!") + self.assert_span() + + self.memory_exporter.clear() + + result = self.perform_request(self.URL) + self.assertEqual(result.text, "Hello!") + self.assert_span() + class TestSyncIntegration(BaseTestCases.BaseManualTest): def setUp(self): @@ -871,8 +986,9 @@ def create_transport( tracer_provider: typing.Optional["TracerProvider"] = None, request_hook: typing.Optional["RequestHook"] = None, response_hook: typing.Optional["ResponseHook"] = None, + **kwargs, ): - transport = httpx.HTTPTransport() + transport = httpx.HTTPTransport(**kwargs) telemetry_transport = SyncOpenTelemetryTransport( transport, tracer_provider=tracer_provider, @@ -884,8 +1000,9 @@ def create_transport( def create_client( self, transport: typing.Optional[SyncOpenTelemetryTransport] = None, + **kwargs, ): - return httpx.Client(transport=transport) + return httpx.Client(transport=transport, **kwargs) def perform_request( self, @@ -921,8 +1038,9 @@ def create_transport( tracer_provider: typing.Optional["TracerProvider"] = None, request_hook: typing.Optional["AsyncRequestHook"] = None, response_hook: typing.Optional["AsyncResponseHook"] = None, + **kwargs, ): - transport = httpx.AsyncHTTPTransport() + transport = httpx.AsyncHTTPTransport(**kwargs) telemetry_transport = AsyncOpenTelemetryTransport( transport, tracer_provider=tracer_provider, @@ -934,8 +1052,9 @@ def create_transport( def create_client( self, transport: typing.Optional[AsyncOpenTelemetryTransport] = None, + **kwargs, ): - return httpx.AsyncClient(transport=transport) + return httpx.AsyncClient(transport=transport, **kwargs) def perform_request( self, @@ -977,8 +1096,9 @@ class TestSyncInstrumentationIntegration(BaseTestCases.BaseInstrumentorTest): def create_client( self, transport: typing.Optional[SyncOpenTelemetryTransport] = None, + **kwargs, ): - return httpx.Client() + return httpx.Client(**kwargs) def perform_request( self, @@ -991,6 +1111,9 @@ def perform_request( return self.client.request(method, url, headers=headers) return client.request(method, url, headers=headers) + def create_proxy_transport(self, url): + return httpx.HTTPTransport(proxy=httpx.Proxy(url)) + class TestAsyncInstrumentationIntegration(BaseTestCases.BaseInstrumentorTest): response_hook = staticmethod(_async_response_hook) @@ -1007,8 +1130,9 @@ def setUp(self): def create_client( self, transport: typing.Optional[AsyncOpenTelemetryTransport] = None, + **kwargs, ): - return httpx.AsyncClient() + return httpx.AsyncClient(**kwargs) def perform_request( self, @@ -1027,6 +1151,9 @@ async def _perform_request(): return _async_call(_perform_request()) + def create_proxy_transport(self, url): + return httpx.AsyncHTTPTransport(proxy=httpx.Proxy(url)) + def test_basic_multiple(self): # We need to create separate clients because in httpx >= 0.19, # closing the client after "with" means the second http call fails From 05073008a3644087721c7a61e3776fbc1173fe09 Mon Sep 17 00:00:00 2001 From: Chris Guidry Date: Thu, 18 Jul 2024 13:11:04 -0400 Subject: [PATCH 16/26] Re-raise `redis.WatchError`s when they occur (#2721) --- .../instrumentation/redis/__init__.py | 21 +++++++++++++++---- .../tests/test_redis.py | 8 ++----- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py b/instrumentation/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py index c5f19fc736..08337c2d4a 100644 --- a/instrumentation/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py @@ -203,6 +203,8 @@ def _traced_execute_pipeline(func, instance, args, kwargs): span_name, ) = _build_span_meta_data_for_pipeline(instance) + exception = None + with tracer.start_as_current_span( span_name, kind=trace.SpanKind.CLIENT ) as span: @@ -216,13 +218,17 @@ def _traced_execute_pipeline(func, instance, args, kwargs): response = None try: response = func(*args, **kwargs) - except redis.WatchError: + except redis.WatchError as watch_exception: span.set_status(StatusCode.UNSET) + exception = watch_exception if callable(response_hook): response_hook(span, instance, response) - return response + if exception: + raise exception + + return response pipeline_class = ( "BasePipeline" if redis.VERSION < (3, 0, 0) else "Pipeline" @@ -279,6 +285,8 @@ async def _async_traced_execute_pipeline(func, instance, args, kwargs): span_name, ) = _build_span_meta_data_for_pipeline(instance) + exception = None + with tracer.start_as_current_span( span_name, kind=trace.SpanKind.CLIENT ) as span: @@ -292,12 +300,17 @@ async def _async_traced_execute_pipeline(func, instance, args, kwargs): response = None try: response = await func(*args, **kwargs) - except redis.WatchError: + except redis.WatchError as watch_exception: span.set_status(StatusCode.UNSET) + exception = watch_exception if callable(response_hook): response_hook(span, instance, response) - return response + + if exception: + raise exception + + return response if redis.VERSION >= _REDIS_ASYNCIO_VERSION: wrap_function_wrapper( diff --git a/instrumentation/opentelemetry-instrumentation-redis/tests/test_redis.py b/instrumentation/opentelemetry-instrumentation-redis/tests/test_redis.py index 23d21b6e5a..c436589adb 100644 --- a/instrumentation/opentelemetry-instrumentation-redis/tests/test_redis.py +++ b/instrumentation/opentelemetry-instrumentation-redis/tests/test_redis.py @@ -359,7 +359,7 @@ def test_response_error(self): def test_watch_error_sync(self): def redis_operations(): - try: + with pytest.raises(WatchError): redis_client = fakeredis.FakeStrictRedis() pipe = redis_client.pipeline(transaction=True) pipe.watch("a") @@ -367,8 +367,6 @@ def redis_operations(): pipe.multi() pipe.set("a", "1") pipe.execute() - except WatchError: - pass redis_operations() @@ -400,7 +398,7 @@ def tearDown(self): @pytest.mark.asyncio async def test_watch_error_async(self): async def redis_operations(): - try: + with pytest.raises(WatchError): redis_client = FakeRedis() async with redis_client.pipeline(transaction=False) as pipe: await pipe.watch("a") @@ -408,8 +406,6 @@ async def redis_operations(): pipe.multi() await pipe.set("a", "1") await pipe.execute() - except WatchError: - pass await redis_operations() From fa23d8a7a057db2548bb9d0d9f1a5ca618dbd5fe Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Thu, 18 Jul 2024 16:47:07 -0700 Subject: [PATCH 17/26] Re-add `opentelemetry-instrumentation-aiohttp-server` to release script (#2722) * Update .pylintrc * Update build.sh --- scripts/build.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/build.sh b/scripts/build.sh index 93dc0edce1..69df4f7748 100755 --- a/scripts/build.sh +++ b/scripts/build.sh @@ -32,7 +32,7 @@ DISTDIR=dist cd $DISTDIR for x in * ; do # FIXME: Remove this logic once these packages are available in Pypi - if echo "$x" | grep -Eq "^opentelemetry_(instrumentation_aiohttp_server|resource_detector_container).*(\.tar\.gz|\.whl)$"; then + if echo "$x" | grep -Eq "^opentelemetry_resource_detector_container.*(\.tar\.gz|\.whl)$"; then echo "Skipping $x because of erroneous uploads. See: https://github.com/open-telemetry/opentelemetry-python-contrib/issues/2053" rm $x # FIXME: Remove this once opentelemetry-resource-detector-azure package goes 1.X From 6594cdf0538ad0bedf77f602870134cc88a8fc12 Mon Sep 17 00:00:00 2001 From: Riccardo Magliocchetti Date: Mon, 22 Jul 2024 18:16:46 +0200 Subject: [PATCH 18/26] resource/azure: make the version of opentelemetry-instrument more relaxed (#2725) --- resource/opentelemetry-resource-detector-azure/pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/resource/opentelemetry-resource-detector-azure/pyproject.toml b/resource/opentelemetry-resource-detector-azure/pyproject.toml index fec99ef157..14952b751c 100644 --- a/resource/opentelemetry-resource-detector-azure/pyproject.toml +++ b/resource/opentelemetry-resource-detector-azure/pyproject.toml @@ -26,7 +26,7 @@ classifiers = [ ] dependencies = [ "opentelemetry-sdk ~= 1.21", - "opentelemetry-instrumentation == 0.47b0.dev", + "opentelemetry-instrumentation ~= 0.43b0", ] [project.entry-points.opentelemetry_resource_detector] From a322a0a26b8c62f5c2c58fab2eefc2aac2b11391 Mon Sep 17 00:00:00 2001 From: GonzaloGuasch Date: Mon, 22 Jul 2024 13:33:40 -0300 Subject: [PATCH 19/26] flask: add `http.target` and `http.route` to metric attributes (#2621) --- CHANGELOG.md | 2 ++ .../instrumentation/flask/__init__.py | 15 +++++++++++++++ .../tests/test_programmatic.py | 16 ++++++++++++---- 3 files changed, 29 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 98bbd3a0bc..5ed29f8ad3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- `opentelemetry-instrumentation-flask` Add `http.route` and `http.target` to metric attributes + ([#2621](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2621)) - `opentelemetry-instrumentation-sklearn` Deprecated the sklearn instrumentation ([#2708](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2708)) - `opentelemetry-instrumentation-pyramid` Record exceptions raised when serving a request diff --git a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py index eaf6c79506..9bc5e85a12 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py @@ -266,6 +266,7 @@ def response_hook(span: Span, status: str, response_headers: List): ) from opentelemetry.instrumentation.utils import _start_internal_or_server_span from opentelemetry.metrics import get_meter +from opentelemetry.semconv.attributes.http_attributes import HTTP_ROUTE from opentelemetry.semconv.metrics import MetricInstruments from opentelemetry.semconv.metrics.http_metrics import ( HTTP_SERVER_REQUEST_DURATION, @@ -340,12 +341,16 @@ def _wrapped_app(wrapped_app_environ, start_response): ) active_requests_counter.add(1, active_requests_count_attrs) + request_route = None def _start_response(status, response_headers, *args, **kwargs): if flask.request and ( excluded_urls is None or not excluded_urls.url_disabled(flask.request.url) ): + nonlocal request_route + request_route = flask.request.url_rule + span = flask.request.environ.get(_ENVIRON_SPAN_KEY) propagator = get_global_response_propagator() @@ -388,6 +393,12 @@ def _start_response(status, response_headers, *args, **kwargs): duration_attrs_old = otel_wsgi._parse_duration_attrs( attributes, _HTTPStabilityMode.DEFAULT ) + + if request_route: + duration_attrs_old[SpanAttributes.HTTP_TARGET] = str( + request_route + ) + duration_histogram_old.record( max(round(duration_s * 1000), 0), duration_attrs_old ) @@ -395,6 +406,10 @@ def _start_response(status, response_headers, *args, **kwargs): duration_attrs_new = otel_wsgi._parse_duration_attrs( attributes, _HTTPStabilityMode.HTTP ) + + if request_route: + duration_attrs_new[HTTP_ROUTE] = str(request_route) + duration_histogram_new.record( max(duration_s, 0), duration_attrs_new ) diff --git a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py index 94437bbfd2..4458daae21 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py +++ b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py @@ -85,6 +85,12 @@ def expected_attributes_new(override_attributes): return default_attributes +_server_duration_attrs_old_copy = _server_duration_attrs_old.copy() +_server_duration_attrs_old_copy.append("http.target") + +_server_duration_attrs_new_copy = _server_duration_attrs_new.copy() +_server_duration_attrs_new_copy.append("http.route") + _expected_metric_names_old = [ "http.server.active_requests", "http.server.duration", @@ -95,11 +101,11 @@ def expected_attributes_new(override_attributes): ] _recommended_metrics_attrs_old = { "http.server.active_requests": _server_active_requests_count_attrs_old, - "http.server.duration": _server_duration_attrs_old, + "http.server.duration": _server_duration_attrs_old_copy, } _recommended_metrics_attrs_new = { "http.server.active_requests": _server_active_requests_count_attrs_new, - "http.server.request.duration": _server_duration_attrs_new, + "http.server.request.duration": _server_duration_attrs_new_copy, } _server_active_requests_count_attrs_both = ( _server_active_requests_count_attrs_old @@ -109,8 +115,8 @@ def expected_attributes_new(override_attributes): ) _recommended_metrics_attrs_both = { "http.server.active_requests": _server_active_requests_count_attrs_both, - "http.server.duration": _server_duration_attrs_old, - "http.server.request.duration": _server_duration_attrs_new, + "http.server.duration": _server_duration_attrs_old_copy, + "http.server.request.duration": _server_duration_attrs_new_copy, } @@ -570,6 +576,7 @@ def test_basic_metric_success(self): self.client.get("/hello/756") expected_duration_attributes = { "http.method": "GET", + "http.target": "/hello/", "http.host": "localhost", "http.scheme": "http", "http.flavor": "1.1", @@ -595,6 +602,7 @@ def test_basic_metric_success_new_semconv(self): expected_duration_attributes = { "http.request.method": "GET", "url.scheme": "http", + "http.route": "/hello/", "network.protocol.version": "1.1", "http.response.status_code": 200, } From 910d5ec2810294b79e5686af1a13862acc2609d5 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Mon, 22 Jul 2024 12:02:35 -0700 Subject: [PATCH 20/26] HTTP semantic convention stability migration for django (#2714) --- CHANGELOG.md | 6 +- .../aiohttp_server/__init__.py | 2 +- .../instrumentation/django/__init__.py | 49 ++- .../django/middleware/otel_middleware.py | 134 ++++--- .../tests/test_middleware.py | 343 +++++++++++++++++- .../tests/test_middleware_asgi.py | 267 +++++++++++++- .../instrumentation/falcon/__init__.py | 2 +- .../instrumentation/pyramid/callbacks.py | 2 +- .../instrumentation/tornado/__init__.py | 2 +- 9 files changed, 736 insertions(+), 71 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ed29f8ad3..a320a1d4fc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#2673](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2673)) - `opentelemetry-instrumentation-django` Add `http.target` to Django duration metric attributes ([#2624](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2624)) +- `opentelemetry-instrumentation-django` Implement new semantic convention opt-in with stable http semantic conventions + ([#2714](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2714)) ### Breaking changes @@ -46,8 +48,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#2580](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2580)) - Populate `{method}` as `HTTP` on `_OTHER` methods from scope for `asgi` middleware ([#2610](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2610)) - - Populate `{method}` as `HTTP` on `_OTHER` methods from scope for `fastapi` middleware +- Populate `{method}` as `HTTP` on `_OTHER` methods from scope for `fastapi` instrumentation ([#2682](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2682)) +- Populate `{method}` as `HTTP` on `_OTHER` methods from scope for `django` middleware + ([#2714](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2714)) ### Fixed - Handle `redis.exceptions.WatchError` as a non-error event in redis instrumentation diff --git a/instrumentation/opentelemetry-instrumentation-aiohttp-server/src/opentelemetry/instrumentation/aiohttp_server/__init__.py b/instrumentation/opentelemetry-instrumentation-aiohttp-server/src/opentelemetry/instrumentation/aiohttp_server/__init__.py index 2e519ac1c5..659ff24af6 100644 --- a/instrumentation/opentelemetry-instrumentation-aiohttp-server/src/opentelemetry/instrumentation/aiohttp_server/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-aiohttp-server/src/opentelemetry/instrumentation/aiohttp_server/__init__.py @@ -207,7 +207,7 @@ async def middleware(request, handler): duration_histogram = meter.create_histogram( name=MetricInstruments.HTTP_SERVER_DURATION, unit="ms", - description="Duration of HTTP client requests.", + description="Duration of HTTP server requests.", ) active_requests_counter = meter.create_up_down_counter( 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 37ac760283..651df12043 100644 --- a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/__init__.py @@ -243,6 +243,13 @@ def response_hook(span, request, response): from django.conf import settings from django.core.exceptions import ImproperlyConfigured +from opentelemetry.instrumentation._semconv import ( + _get_schema_url, + _OpenTelemetrySemanticConventionStability, + _OpenTelemetryStabilitySignalType, + _report_new, + _report_old, +) from opentelemetry.instrumentation.django.environment_variables import ( OTEL_PYTHON_DJANGO_INSTRUMENT, ) @@ -253,7 +260,13 @@ def response_hook(span, request, response): from opentelemetry.instrumentation.django.version import __version__ from opentelemetry.instrumentation.instrumentor import BaseInstrumentor from opentelemetry.metrics import get_meter +from opentelemetry.semconv._incubating.metrics.http_metrics import ( + create_http_server_active_requests, +) from opentelemetry.semconv.metrics import MetricInstruments +from opentelemetry.semconv.metrics.http_metrics import ( + HTTP_SERVER_REQUEST_DURATION, +) from opentelemetry.trace import get_tracer from opentelemetry.util.http import get_excluded_urls, parse_excluded_urls @@ -293,6 +306,12 @@ def _instrument(self, **kwargs): if environ.get(OTEL_PYTHON_DJANGO_INSTRUMENT) == "False": return + # initialize semantic conventions opt-in if needed + _OpenTelemetrySemanticConventionStability._initialize() + sem_conv_opt_in_mode = _OpenTelemetrySemanticConventionStability._get_opentelemetry_stability_opt_in_mode( + _OpenTelemetryStabilitySignalType.HTTP, + ) + tracer_provider = kwargs.get("tracer_provider") meter_provider = kwargs.get("meter_provider") _excluded_urls = kwargs.get("excluded_urls") @@ -300,14 +319,15 @@ def _instrument(self, **kwargs): __name__, __version__, tracer_provider=tracer_provider, - schema_url="https://opentelemetry.io/schemas/1.11.0", + schema_url=_get_schema_url(sem_conv_opt_in_mode), ) meter = get_meter( __name__, __version__, meter_provider=meter_provider, - schema_url="https://opentelemetry.io/schemas/1.11.0", + schema_url=_get_schema_url(sem_conv_opt_in_mode), ) + _DjangoMiddleware._sem_conv_opt_in_mode = sem_conv_opt_in_mode _DjangoMiddleware._tracer = tracer _DjangoMiddleware._meter = meter _DjangoMiddleware._excluded_urls = ( @@ -319,15 +339,22 @@ def _instrument(self, **kwargs): _DjangoMiddleware._otel_response_hook = kwargs.pop( "response_hook", None ) - _DjangoMiddleware._duration_histogram = meter.create_histogram( - name=MetricInstruments.HTTP_SERVER_DURATION, - unit="ms", - description="Duration of HTTP client requests.", - ) - _DjangoMiddleware._active_request_counter = meter.create_up_down_counter( - name=MetricInstruments.HTTP_SERVER_ACTIVE_REQUESTS, - unit="requests", - description="measures the number of concurrent HTTP requests those are currently in flight", + _DjangoMiddleware._duration_histogram_old = None + if _report_old(sem_conv_opt_in_mode): + _DjangoMiddleware._duration_histogram_old = meter.create_histogram( + name=MetricInstruments.HTTP_SERVER_DURATION, + unit="ms", + description="Duration of HTTP server requests.", + ) + _DjangoMiddleware._duration_histogram_new = None + if _report_new(sem_conv_opt_in_mode): + _DjangoMiddleware._duration_histogram_new = meter.create_histogram( + name=HTTP_SERVER_REQUEST_DURATION, + description="Duration of HTTP server requests.", + unit="s", + ) + _DjangoMiddleware._active_request_counter = ( + create_http_server_active_requests(meter) ) # 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 diff --git a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware/otel_middleware.py b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware/otel_middleware.py index 11e92c9757..4530a16506 100644 --- a/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware/otel_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware/otel_middleware.py @@ -22,6 +22,17 @@ from django.http import HttpRequest, HttpResponse from opentelemetry.context import detach +from opentelemetry.instrumentation._semconv import ( + _filter_semconv_active_request_count_attr, + _filter_semconv_duration_attrs, + _HTTPStabilityMode, + _report_new, + _report_old, + _server_active_requests_count_attrs_new, + _server_active_requests_count_attrs_old, + _server_duration_attrs_new, + _server_duration_attrs_old, +) from opentelemetry.instrumentation.propagators import ( get_global_response_propagator, ) @@ -40,6 +51,7 @@ collect_request_attributes as wsgi_collect_request_attributes, ) from opentelemetry.instrumentation.wsgi import wsgi_getter +from opentelemetry.semconv.attributes.http_attributes import HTTP_ROUTE from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.trace import Span, SpanKind, use_span from opentelemetry.util.http import ( @@ -47,13 +59,12 @@ OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SERVER_REQUEST, OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SERVER_RESPONSE, SanitizeValue, - _parse_active_request_count_attrs, - _parse_duration_attrs, get_custom_headers, get_excluded_urls, get_traced_request_attrs, normalise_request_header_name, normalise_response_header_name, + sanitize_method, ) try: @@ -113,26 +124,6 @@ def __call__(self, request): _is_asgi_supported = False _logger = getLogger(__name__) -_attributes_by_preference = [ - [ - SpanAttributes.HTTP_SCHEME, - SpanAttributes.HTTP_HOST, - SpanAttributes.HTTP_TARGET, - ], - [ - SpanAttributes.HTTP_SCHEME, - SpanAttributes.HTTP_SERVER_NAME, - SpanAttributes.NET_HOST_PORT, - SpanAttributes.HTTP_TARGET, - ], - [ - SpanAttributes.HTTP_SCHEME, - SpanAttributes.NET_HOST_NAME, - SpanAttributes.NET_HOST_PORT, - SpanAttributes.HTTP_TARGET, - ], - [SpanAttributes.HTTP_URL], -] def _is_asgi_request(request: HttpRequest) -> bool: @@ -159,8 +150,10 @@ class _DjangoMiddleware(MiddlewareMixin): _excluded_urls = get_excluded_urls("DJANGO") _tracer = None _meter = None - _duration_histogram = None + _duration_histogram_old = None + _duration_histogram_new = None _active_request_counter = None + _sem_conv_opt_in_mode = _HTTPStabilityMode.DEFAULT _otel_request_hook: Callable[[Span, HttpRequest], None] = None _otel_response_hook: Callable[[Span, HttpRequest, HttpResponse], None] = ( @@ -169,6 +162,9 @@ class _DjangoMiddleware(MiddlewareMixin): @staticmethod def _get_span_name(request): + method = sanitize_method(request.method.strip()) + if method == "_OTHER": + return "HTTP" try: if getattr(request, "resolver_match"): match = request.resolver_match @@ -176,10 +172,10 @@ def _get_span_name(request): match = resolve(request.path) if hasattr(match, "route") and match.route: - return f"{request.method} {match.route}" + return f"{method} {match.route}" if hasattr(match, "url_name") and match.url_name: - return f"{request.method} {match.url_name}" + return f"{method} {match.url_name}" return request.method @@ -213,7 +209,10 @@ def process_request(self, request): carrier_getter = wsgi_getter collect_request_attributes = wsgi_collect_request_attributes - attributes = collect_request_attributes(carrier) + attributes = collect_request_attributes( + carrier, + self._sem_conv_opt_in_mode, + ) span, token = _start_internal_or_server_span( tracer=self._tracer, span_name=self._get_span_name(request), @@ -226,14 +225,15 @@ def process_request(self, request): ) active_requests_count_attrs = _parse_active_request_count_attrs( - attributes + attributes, + self._sem_conv_opt_in_mode, ) - duration_attrs = _parse_duration_attrs(attributes) request.META[self._environ_active_request_attr_key] = ( active_requests_count_attrs ) - request.META[self._environ_duration_attr_key] = duration_attrs + # Pass all of attributes to duration key because we will filter during response + request.META[self._environ_duration_attr_key] = attributes self._active_request_counter.add(1, active_requests_count_attrs) if span.is_recording(): attributes = extract_attributes_from_object( @@ -309,18 +309,20 @@ def process_view(self, request, view_func, *args, **kwargs): ): span = request.META[self._environ_span_key] - if span.is_recording(): - match = getattr(request, "resolver_match", None) - if match: - route = getattr(match, "route", None) - if route: + match = getattr(request, "resolver_match", None) + if match: + route = getattr(match, "route", None) + if route: + if span.is_recording(): + # http.route is present for both old and new semconv span.set_attribute(SpanAttributes.HTTP_ROUTE, route) - duration_attrs = request.META[ - self._environ_duration_attr_key - ] - # Metrics currently use the 1.11.0 schema, which puts the route in `http.target`. - # TODO: use `http.route` when the user sets `OTEL_SEMCONV_STABILITY_OPT_IN`. + duration_attrs = request.META[ + self._environ_duration_attr_key + ] + if _report_old(self._sem_conv_opt_in_mode): duration_attrs[SpanAttributes.HTTP_TARGET] = route + if _report_new(self._sem_conv_opt_in_mode): + duration_attrs[HTTP_ROUTE] = route def process_exception(self, request, exception): if self._excluded_urls.url_disabled(request.build_absolute_uri("?")): @@ -347,15 +349,16 @@ def process_response(self, request, response): duration_attrs = request.META.pop( self._environ_duration_attr_key, None ) - if duration_attrs: - duration_attrs[SpanAttributes.HTTP_STATUS_CODE] = ( - response.status_code - ) request_start_time = request.META.pop(self._environ_timer_key, None) if activation and span: if is_asgi_request: - set_status_code(span, response.status_code) + set_status_code( + span, + response.status_code, + metric_attributes=duration_attrs, + sem_conv_opt_in_mode=self._sem_conv_opt_in_mode, + ) if span.is_recording() and span.kind == SpanKind.SERVER: custom_headers = {} @@ -381,6 +384,8 @@ def process_response(self, request, response): span, f"{response.status_code} {response.reason_phrase}", response.items(), + duration_attrs=duration_attrs, + sem_conv_opt_in_mode=self._sem_conv_opt_in_mode, ) if span.is_recording() and span.kind == SpanKind.SERVER: custom_attributes = ( @@ -416,13 +421,46 @@ def process_response(self, request, response): activation.__exit__(None, None, None) if request_start_time is not None: - duration = max( - round((default_timer() - request_start_time) * 1000), 0 - ) - self._duration_histogram.record(duration, duration_attrs) + duration_s = default_timer() - request_start_time + if self._duration_histogram_old: + duration_attrs_old = _parse_duration_attrs( + duration_attrs, _HTTPStabilityMode.DEFAULT + ) + self._duration_histogram_old.record( + max(round(duration_s * 1000), 0), duration_attrs_old + ) + if self._duration_histogram_new: + duration_attrs_new = _parse_duration_attrs( + duration_attrs, _HTTPStabilityMode.HTTP + ) + self._duration_histogram_new.record( + max(duration_s, 0), duration_attrs_new + ) self._active_request_counter.add(-1, active_requests_count_attrs) if request.META.get(self._environ_token, None) is not None: detach(request.META.get(self._environ_token)) request.META.pop(self._environ_token) return response + + +def _parse_duration_attrs( + req_attrs, sem_conv_opt_in_mode=_HTTPStabilityMode.DEFAULT +): + return _filter_semconv_duration_attrs( + req_attrs, + _server_duration_attrs_old, + _server_duration_attrs_new, + sem_conv_opt_in_mode, + ) + + +def _parse_active_request_count_attrs( + req_attrs, sem_conv_opt_in_mode=_HTTPStabilityMode.DEFAULT +): + return _filter_semconv_active_request_count_attr( + req_attrs, + _server_active_requests_count_attrs_old, + _server_active_requests_count_attrs_new, + sem_conv_opt_in_mode, + ) diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py index 4945f05455..1b9a904ce1 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py @@ -13,6 +13,7 @@ # limitations under the License. # pylint: disable=E0611 +# pylint: disable=too-many-lines from sys import modules from timeit import default_timer @@ -24,6 +25,14 @@ from django.test.utils import setup_test_environment, teardown_test_environment from opentelemetry import trace +from opentelemetry.instrumentation._semconv import ( + OTEL_SEMCONV_STABILITY_OPT_IN, + _OpenTelemetrySemanticConventionStability, + _server_active_requests_count_attrs_new, + _server_active_requests_count_attrs_old, + _server_duration_attrs_new, + _server_duration_attrs_old, +) from opentelemetry.instrumentation.django import ( DjangoInstrumentor, _DjangoMiddleware, @@ -39,6 +48,23 @@ ) from opentelemetry.sdk.trace import Span from opentelemetry.sdk.trace.id_generator import RandomIdGenerator +from opentelemetry.semconv.attributes.client_attributes import CLIENT_ADDRESS +from opentelemetry.semconv.attributes.error_attributes import ERROR_TYPE +from opentelemetry.semconv.attributes.exception_attributes import ( + EXCEPTION_MESSAGE, + EXCEPTION_TYPE, +) +from opentelemetry.semconv.attributes.http_attributes import ( + HTTP_REQUEST_METHOD, + HTTP_REQUEST_METHOD_ORIGINAL, + HTTP_RESPONSE_STATUS_CODE, + HTTP_ROUTE, +) +from opentelemetry.semconv.attributes.network_attributes import ( + NETWORK_PROTOCOL_VERSION, +) +from opentelemetry.semconv.attributes.server_attributes import SERVER_PORT +from opentelemetry.semconv.attributes.url_attributes import URL_SCHEME from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.test.wsgitestutil import WsgiTestBase from opentelemetry.trace import ( @@ -51,8 +77,6 @@ OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SANITIZE_FIELDS, OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SERVER_REQUEST, OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SERVER_RESPONSE, - _active_requests_count_attrs, - _duration_attrs, get_excluded_urls, get_traced_request_attrs, ) @@ -112,15 +136,25 @@ def setUpClass(cls): def setUp(self): super().setUp() setup_test_environment() - _django_instrumentor.instrument() + test_name = "" + if hasattr(self, "_testMethodName"): + test_name = self._testMethodName + sem_conv_mode = "default" + if "new_semconv" in test_name: + sem_conv_mode = "http" + elif "both_semconv" in test_name: + sem_conv_mode = "http/dup" self.env_patch = patch.dict( "os.environ", { "OTEL_PYTHON_DJANGO_EXCLUDED_URLS": "http://testserver/excluded_arg/123,excluded_noarg", "OTEL_PYTHON_DJANGO_TRACED_REQUEST_ATTRS": "path_info,content_type,non_existing_variable", + OTEL_SEMCONV_STABILITY_OPT_IN: sem_conv_mode, }, ) + _OpenTelemetrySemanticConventionStability._initialized = False self.env_patch.start() + _django_instrumentor.instrument() self.exclude_patch = patch( "opentelemetry.instrumentation.django.middleware.otel_middleware._DjangoMiddleware._excluded_urls", get_excluded_urls("DJANGO"), @@ -199,6 +233,57 @@ def test_traced_get(self): self.assertEqual(span.attributes[SpanAttributes.HTTP_SCHEME], "http") self.assertEqual(span.attributes[SpanAttributes.HTTP_STATUS_CODE], 200) + def test_traced_get_new_semconv(self): + Client().get("/traced/") + + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 1) + + span = spans[0] + + self.assertEqual(span.name, "GET ^traced/" if DJANGO_2_2 else "GET") + self.assertEqual(span.kind, SpanKind.SERVER) + self.assertEqual(span.status.status_code, StatusCode.UNSET) + self.assertEqual(span.attributes[HTTP_REQUEST_METHOD], "GET") + self.assertEqual(span.attributes[URL_SCHEME], "http") + self.assertEqual(span.attributes[SERVER_PORT], 80) + self.assertEqual(span.attributes[CLIENT_ADDRESS], "127.0.0.1") + self.assertEqual(span.attributes[NETWORK_PROTOCOL_VERSION], "1.1") + if DJANGO_2_2: + self.assertEqual(span.attributes[HTTP_ROUTE], "^traced/") + self.assertEqual(span.attributes[HTTP_RESPONSE_STATUS_CODE], 200) + + def test_traced_get_both_semconv(self): + Client().get("/traced/") + + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 1) + + span = spans[0] + + self.assertEqual(span.name, "GET ^traced/" if DJANGO_2_2 else "GET") + self.assertEqual(span.kind, SpanKind.SERVER) + self.assertEqual(span.status.status_code, StatusCode.UNSET) + self.assertEqual(span.attributes[SpanAttributes.HTTP_METHOD], "GET") + self.assertEqual( + span.attributes[SpanAttributes.HTTP_URL], + "http://testserver/traced/", + ) + if DJANGO_2_2: + self.assertEqual( + span.attributes[SpanAttributes.HTTP_ROUTE], "^traced/" + ) + self.assertEqual(span.attributes[SpanAttributes.HTTP_SCHEME], "http") + self.assertEqual(span.attributes[SpanAttributes.HTTP_STATUS_CODE], 200) + self.assertEqual(span.attributes[HTTP_REQUEST_METHOD], "GET") + self.assertEqual(span.attributes[URL_SCHEME], "http") + self.assertEqual(span.attributes[SERVER_PORT], 80) + self.assertEqual(span.attributes[CLIENT_ADDRESS], "127.0.0.1") + self.assertEqual(span.attributes[NETWORK_PROTOCOL_VERSION], "1.1") + if DJANGO_2_2: + self.assertEqual(span.attributes[HTTP_ROUTE], "^traced/") + self.assertEqual(span.attributes[HTTP_RESPONSE_STATUS_CODE], 200) + def test_not_recording(self): mock_tracer = Mock() mock_span = Mock() @@ -245,6 +330,53 @@ def test_traced_post(self): self.assertEqual(span.attributes[SpanAttributes.HTTP_SCHEME], "http") self.assertEqual(span.attributes[SpanAttributes.HTTP_STATUS_CODE], 200) + def test_traced_post_new_semconv(self): + Client().post("/traced/") + + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 1) + + span = spans[0] + + self.assertEqual(span.name, "POST ^traced/" if DJANGO_2_2 else "POST") + self.assertEqual(span.kind, SpanKind.SERVER) + self.assertEqual(span.status.status_code, StatusCode.UNSET) + self.assertEqual(span.attributes[HTTP_REQUEST_METHOD], "POST") + self.assertEqual(span.attributes[URL_SCHEME], "http") + self.assertEqual(span.attributes[SERVER_PORT], 80) + self.assertEqual(span.attributes[CLIENT_ADDRESS], "127.0.0.1") + self.assertEqual(span.attributes[NETWORK_PROTOCOL_VERSION], "1.1") + if DJANGO_2_2: + self.assertEqual(span.attributes[HTTP_ROUTE], "^traced/") + self.assertEqual(span.attributes[HTTP_RESPONSE_STATUS_CODE], 200) + + def test_traced_post_both_semconv(self): + Client().post("/traced/") + + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 1) + + span = spans[0] + + self.assertEqual(span.name, "POST ^traced/" if DJANGO_2_2 else "POST") + self.assertEqual(span.kind, SpanKind.SERVER) + self.assertEqual(span.status.status_code, StatusCode.UNSET) + self.assertEqual(span.attributes[SpanAttributes.HTTP_METHOD], "POST") + self.assertEqual( + span.attributes[SpanAttributes.HTTP_URL], + "http://testserver/traced/", + ) + self.assertEqual(span.attributes[SpanAttributes.HTTP_SCHEME], "http") + self.assertEqual(span.attributes[SpanAttributes.HTTP_STATUS_CODE], 200) + self.assertEqual(span.attributes[HTTP_REQUEST_METHOD], "POST") + self.assertEqual(span.attributes[URL_SCHEME], "http") + self.assertEqual(span.attributes[SERVER_PORT], 80) + self.assertEqual(span.attributes[CLIENT_ADDRESS], "127.0.0.1") + self.assertEqual(span.attributes[NETWORK_PROTOCOL_VERSION], "1.1") + if DJANGO_2_2: + self.assertEqual(span.attributes[HTTP_ROUTE], "^traced/") + self.assertEqual(span.attributes[HTTP_RESPONSE_STATUS_CODE], 200) + def test_error(self): with self.assertRaises(ValueError): Client().get("/error/") @@ -279,6 +411,67 @@ def test_error(self): event.attributes[SpanAttributes.EXCEPTION_MESSAGE], "error" ) + def test_error_new_semconv(self): + with self.assertRaises(ValueError): + Client().get("/error/") + + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 1) + + span = spans[0] + + self.assertEqual(span.name, "GET ^error/" if DJANGO_2_2 else "GET") + self.assertEqual(span.kind, SpanKind.SERVER) + self.assertEqual(span.status.status_code, StatusCode.ERROR) + self.assertEqual(span.attributes[HTTP_REQUEST_METHOD], "GET") + if DJANGO_2_2: + self.assertEqual(span.attributes[HTTP_ROUTE], "^error/") + self.assertEqual(span.attributes[URL_SCHEME], "http") + self.assertEqual(span.attributes[HTTP_RESPONSE_STATUS_CODE], 500) + + self.assertEqual(len(span.events), 1) + event = span.events[0] + self.assertEqual(event.name, "exception") + self.assertEqual(event.attributes[EXCEPTION_TYPE], "ValueError") + self.assertEqual(event.attributes[EXCEPTION_MESSAGE], "error") + self.assertEqual(span.attributes[ERROR_TYPE], "500") + + def test_error_both_semconv(self): + with self.assertRaises(ValueError): + Client().get("/error/") + + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 1) + + span = spans[0] + + self.assertEqual(span.name, "GET ^error/" if DJANGO_2_2 else "GET") + self.assertEqual(span.kind, SpanKind.SERVER) + self.assertEqual(span.status.status_code, StatusCode.ERROR) + self.assertEqual(span.attributes[SpanAttributes.HTTP_METHOD], "GET") + self.assertEqual( + span.attributes[SpanAttributes.HTTP_URL], + "http://testserver/error/", + ) + if DJANGO_2_2: + self.assertEqual( + span.attributes[SpanAttributes.HTTP_ROUTE], "^error/" + ) + self.assertEqual(span.attributes[SpanAttributes.HTTP_SCHEME], "http") + self.assertEqual(span.attributes[SpanAttributes.HTTP_STATUS_CODE], 500) + self.assertEqual(span.attributes[HTTP_REQUEST_METHOD], "GET") + if DJANGO_2_2: + self.assertEqual(span.attributes[HTTP_ROUTE], "^error/") + self.assertEqual(span.attributes[URL_SCHEME], "http") + self.assertEqual(span.attributes[HTTP_RESPONSE_STATUS_CODE], 500) + + self.assertEqual(len(span.events), 1) + event = span.events[0] + self.assertEqual(event.name, "exception") + self.assertEqual(event.attributes[EXCEPTION_TYPE], "ValueError") + self.assertEqual(event.attributes[EXCEPTION_MESSAGE], "error") + self.assertEqual(span.attributes[ERROR_TYPE], "500") + def test_exclude_lists(self): client = Client() client.get("/excluded_arg/123") @@ -343,6 +536,46 @@ def test_span_name_404(self): span = span_list[0] self.assertEqual(span.name, "GET") + def test_nonstandard_http_method_span_name(self): + Client().request( + REQUEST_METHOD="NONSTANDARD", PATH_INFO="/span_name/1234/" + ) + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 1) + + span = span_list[0] + self.assertEqual(span.name, "HTTP") + self.assertEqual(span.attributes[SpanAttributes.HTTP_METHOD], "_OTHER") + + def test_nonstandard_http_method_span_name_new_semconv(self): + Client().request( + REQUEST_METHOD="NONSTANDARD", PATH_INFO="/span_name/1234/" + ) + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 1) + + span = span_list[0] + self.assertEqual(span.name, "HTTP") + self.assertEqual(span.attributes[HTTP_REQUEST_METHOD], "_OTHER") + self.assertEqual( + span.attributes[HTTP_REQUEST_METHOD_ORIGINAL], "NONSTANDARD" + ) + + def test_nonstandard_http_method_span_name_both_semconv(self): + Client().request( + REQUEST_METHOD="NONSTANDARD", PATH_INFO="/span_name/1234/" + ) + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 1) + + span = span_list[0] + self.assertEqual(span.name, "HTTP") + self.assertEqual(span.attributes[SpanAttributes.HTTP_METHOD], "_OTHER") + self.assertEqual(span.attributes[HTTP_REQUEST_METHOD], "_OTHER") + self.assertEqual( + span.attributes[HTTP_REQUEST_METHOD_ORIGINAL], "NONSTANDARD" + ) + def test_traced_request_attrs(self): Client().get("/span_name/1234/", CONTENT_TYPE="test/ct") span_list = self.memory_exporter.get_finished_spans() @@ -479,9 +712,8 @@ def test_wsgi_metrics(self): "http.server.duration", ] _recommended_attrs = { - "http.server.active_requests": _active_requests_count_attrs, - "http.server.duration": _duration_attrs - | {SpanAttributes.HTTP_TARGET}, + "http.server.active_requests": _server_active_requests_count_attrs_old, + "http.server.duration": _server_duration_attrs_old, } start = default_timer() for _ in range(3): @@ -517,6 +749,105 @@ def test_wsgi_metrics(self): ) self.assertTrue(histrogram_data_point_seen and number_data_point_seen) + # pylint: disable=too-many-locals + def test_wsgi_metrics_new_semconv(self): + _expected_metric_names = [ + "http.server.active_requests", + "http.server.request.duration", + ] + _recommended_attrs = { + "http.server.active_requests": _server_active_requests_count_attrs_new, + "http.server.request.duration": _server_duration_attrs_new, + } + start = default_timer() + for _ in range(3): + response = Client().get("/span_name/1234/") + self.assertEqual(response.status_code, 200) + duration_s = default_timer() - start + metrics_list = self.memory_metrics_reader.get_metrics_data() + number_data_point_seen = False + histrogram_data_point_seen = False + + self.assertTrue(len(metrics_list.resource_metrics) != 0) + for resource_metric in metrics_list.resource_metrics: + self.assertTrue(len(resource_metric.scope_metrics) != 0) + for scope_metric in resource_metric.scope_metrics: + self.assertTrue(len(scope_metric.metrics) != 0) + for metric in scope_metric.metrics: + self.assertIn(metric.name, _expected_metric_names) + data_points = list(metric.data.data_points) + self.assertEqual(len(data_points), 1) + for point in data_points: + if isinstance(point, HistogramDataPoint): + self.assertEqual(point.count, 3) + histrogram_data_point_seen = True + self.assertAlmostEqual( + duration_s, point.sum, places=1 + ) + if isinstance(point, NumberDataPoint): + number_data_point_seen = True + self.assertEqual(point.value, 0) + for attr in point.attributes: + self.assertIn( + attr, _recommended_attrs[metric.name] + ) + self.assertTrue(histrogram_data_point_seen and number_data_point_seen) + + # pylint: disable=too-many-locals + # pylint: disable=too-many-nested-blocks + def test_wsgi_metrics_both_semconv(self): + _expected_metric_names = [ + "http.server.duration", + "http.server.active_requests", + "http.server.request.duration", + ] + active_count_both_attrs = list(_server_active_requests_count_attrs_new) + active_count_both_attrs.extend(_server_active_requests_count_attrs_old) + _recommended_attrs = { + "http.server.active_requests": active_count_both_attrs, + "http.server.request.duration": _server_duration_attrs_new, + "http.server.duration": _server_duration_attrs_old, + } + start = default_timer() + for _ in range(3): + response = Client().get("/span_name/1234/") + self.assertEqual(response.status_code, 200) + duration_s = max(default_timer() - start, 0) + duration = max(round(duration_s * 1000), 0) + metrics_list = self.memory_metrics_reader.get_metrics_data() + number_data_point_seen = False + histrogram_data_point_seen = False + + self.assertTrue(len(metrics_list.resource_metrics) != 0) + for resource_metric in metrics_list.resource_metrics: + self.assertTrue(len(resource_metric.scope_metrics) != 0) + for scope_metric in resource_metric.scope_metrics: + self.assertTrue(len(scope_metric.metrics) != 0) + for metric in scope_metric.metrics: + self.assertIn(metric.name, _expected_metric_names) + data_points = list(metric.data.data_points) + self.assertEqual(len(data_points), 1) + for point in data_points: + if isinstance(point, HistogramDataPoint): + self.assertEqual(point.count, 3) + histrogram_data_point_seen = True + if metric.name == "http.server.request.duration": + self.assertAlmostEqual( + duration_s, point.sum, places=1 + ) + elif metric.name == "http.server.duration": + self.assertAlmostEqual( + duration, point.sum, delta=100 + ) + if isinstance(point, NumberDataPoint): + number_data_point_seen = True + self.assertEqual(point.value, 0) + for attr in point.attributes: + self.assertIn( + attr, _recommended_attrs[metric.name] + ) + self.assertTrue(histrogram_data_point_seen and number_data_point_seen) + def test_wsgi_metrics_unistrument(self): Client().get("/span_name/1234/") _django_instrumentor.uninstrument() diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware_asgi.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware_asgi.py index 0e2472d15e..d06c9c635c 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware_asgi.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware_asgi.py @@ -24,6 +24,10 @@ from django.test.utils import setup_test_environment, teardown_test_environment from opentelemetry import trace as trace_api +from opentelemetry.instrumentation._semconv import ( + OTEL_SEMCONV_STABILITY_OPT_IN, + _OpenTelemetrySemanticConventionStability, +) from opentelemetry.instrumentation.django import ( DjangoInstrumentor, _DjangoMiddleware, @@ -35,6 +39,22 @@ from opentelemetry.sdk import resources from opentelemetry.sdk.trace import Span from opentelemetry.sdk.trace.id_generator import RandomIdGenerator +from opentelemetry.semconv.attributes.client_attributes import CLIENT_ADDRESS +from opentelemetry.semconv.attributes.exception_attributes import ( + EXCEPTION_MESSAGE, + EXCEPTION_TYPE, +) +from opentelemetry.semconv.attributes.http_attributes import ( + HTTP_REQUEST_METHOD, + HTTP_REQUEST_METHOD_ORIGINAL, + HTTP_RESPONSE_STATUS_CODE, + HTTP_ROUTE, +) +from opentelemetry.semconv.attributes.network_attributes import ( + NETWORK_PROTOCOL_VERSION, +) +from opentelemetry.semconv.attributes.server_attributes import SERVER_PORT +from opentelemetry.semconv.attributes.url_attributes import URL_SCHEME from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.test.test_base import TestBase from opentelemetry.trace import ( @@ -87,6 +107,7 @@ @pytest.mark.skipif( not DJANGO_3_1, reason="AsyncClient implemented since Django 3.1" ) +# pylint: disable=too-many-public-methods class TestMiddlewareAsgi(SimpleTestCase, TestBase): @classmethod def setUpClass(cls): @@ -96,15 +117,25 @@ def setUpClass(cls): def setUp(self): super().setUp() setup_test_environment() - _django_instrumentor.instrument() + test_name = "" + if hasattr(self, "_testMethodName"): + test_name = self._testMethodName + sem_conv_mode = "default" + if "new_semconv" in test_name: + sem_conv_mode = "http" + elif "both_semconv" in test_name: + sem_conv_mode = "http/dup" self.env_patch = patch.dict( "os.environ", { "OTEL_PYTHON_DJANGO_EXCLUDED_URLS": "http://testserver/excluded_arg/123,excluded_noarg", "OTEL_PYTHON_DJANGO_TRACED_REQUEST_ATTRS": "path_info,content_type,non_existing_variable", + OTEL_SEMCONV_STABILITY_OPT_IN: sem_conv_mode, }, ) + _OpenTelemetrySemanticConventionStability._initialized = False self.env_patch.start() + _django_instrumentor.instrument() self.exclude_patch = patch( "opentelemetry.instrumentation.django.middleware.otel_middleware._DjangoMiddleware._excluded_urls", get_excluded_urls("DJANGO"), @@ -152,6 +183,57 @@ async def test_templated_route_get(self): self.assertEqual(span.attributes[SpanAttributes.HTTP_SCHEME], "http") self.assertEqual(span.attributes[SpanAttributes.HTTP_STATUS_CODE], 200) + async def test_templated_route_get_new_semconv(self): + await self.async_client.get("/route/2020/template/") + + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 1) + + span = spans[0] + + self.assertEqual(span.name, "GET ^route/(?P[0-9]{4})/template/$") + self.assertEqual(span.kind, SpanKind.SERVER) + self.assertEqual(span.status.status_code, StatusCode.UNSET) + self.assertEqual(span.attributes[HTTP_REQUEST_METHOD], "GET") + self.assertEqual(span.attributes[SERVER_PORT], 80) + self.assertEqual(span.attributes[CLIENT_ADDRESS], "127.0.0.1") + self.assertEqual(span.attributes[NETWORK_PROTOCOL_VERSION], "1.1") + self.assertEqual( + span.attributes[HTTP_ROUTE], + "^route/(?P[0-9]{4})/template/$", + ) + self.assertEqual(span.attributes[URL_SCHEME], "http") + self.assertEqual(span.attributes[HTTP_RESPONSE_STATUS_CODE], 200) + + async def test_templated_route_get_both_semconv(self): + await self.async_client.get("/route/2020/template/") + + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 1) + + span = spans[0] + + self.assertEqual(span.name, "GET ^route/(?P[0-9]{4})/template/$") + self.assertEqual(span.kind, SpanKind.SERVER) + self.assertEqual(span.status.status_code, StatusCode.UNSET) + self.assertEqual(span.attributes[SpanAttributes.HTTP_METHOD], "GET") + self.assertEqual( + span.attributes[SpanAttributes.HTTP_URL], + "http://127.0.0.1/route/2020/template/", + ) + self.assertEqual(span.attributes[SpanAttributes.HTTP_SCHEME], "http") + self.assertEqual(span.attributes[SpanAttributes.HTTP_STATUS_CODE], 200) + self.assertEqual(span.attributes[HTTP_REQUEST_METHOD], "GET") + self.assertEqual(span.attributes[SERVER_PORT], 80) + self.assertEqual(span.attributes[CLIENT_ADDRESS], "127.0.0.1") + self.assertEqual(span.attributes[NETWORK_PROTOCOL_VERSION], "1.1") + self.assertEqual( + span.attributes[HTTP_ROUTE], + "^route/(?P[0-9]{4})/template/$", + ) + self.assertEqual(span.attributes[URL_SCHEME], "http") + self.assertEqual(span.attributes[HTTP_RESPONSE_STATUS_CODE], 200) + async def test_traced_get(self): await self.async_client.get("/traced/") @@ -174,6 +256,51 @@ async def test_traced_get(self): self.assertEqual(span.attributes[SpanAttributes.HTTP_SCHEME], "http") self.assertEqual(span.attributes[SpanAttributes.HTTP_STATUS_CODE], 200) + async def test_traced_get_new_semconv(self): + await self.async_client.get("/traced/") + + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 1) + + span = spans[0] + + self.assertEqual(span.name, "GET ^traced/") + self.assertEqual(span.kind, SpanKind.SERVER) + self.assertEqual(span.status.status_code, StatusCode.UNSET) + self.assertEqual(span.attributes[HTTP_REQUEST_METHOD], "GET") + self.assertEqual(span.attributes[URL_SCHEME], "http") + self.assertEqual(span.attributes[SERVER_PORT], 80) + self.assertEqual(span.attributes[CLIENT_ADDRESS], "127.0.0.1") + self.assertEqual(span.attributes[NETWORK_PROTOCOL_VERSION], "1.1") + self.assertEqual(span.attributes[HTTP_ROUTE], "^traced/") + self.assertEqual(span.attributes[HTTP_RESPONSE_STATUS_CODE], 200) + + async def test_traced_get_both_semconv(self): + await self.async_client.get("/traced/") + + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 1) + + span = spans[0] + + self.assertEqual(span.name, "GET ^traced/") + self.assertEqual(span.kind, SpanKind.SERVER) + self.assertEqual(span.status.status_code, StatusCode.UNSET) + self.assertEqual(span.attributes[SpanAttributes.HTTP_METHOD], "GET") + self.assertEqual( + span.attributes[SpanAttributes.HTTP_URL], + "http://127.0.0.1/traced/", + ) + self.assertEqual(span.attributes[SpanAttributes.HTTP_SCHEME], "http") + self.assertEqual(span.attributes[SpanAttributes.HTTP_STATUS_CODE], 200) + self.assertEqual(span.attributes[HTTP_REQUEST_METHOD], "GET") + self.assertEqual(span.attributes[URL_SCHEME], "http") + self.assertEqual(span.attributes[SERVER_PORT], 80) + self.assertEqual(span.attributes[CLIENT_ADDRESS], "127.0.0.1") + self.assertEqual(span.attributes[NETWORK_PROTOCOL_VERSION], "1.1") + self.assertEqual(span.attributes[HTTP_ROUTE], "^traced/") + self.assertEqual(span.attributes[HTTP_RESPONSE_STATUS_CODE], 200) + async def test_not_recording(self): mock_tracer = Mock() mock_span = Mock() @@ -209,6 +336,51 @@ async def test_traced_post(self): self.assertEqual(span.attributes[SpanAttributes.HTTP_SCHEME], "http") self.assertEqual(span.attributes[SpanAttributes.HTTP_STATUS_CODE], 200) + async def test_traced_post_new_semconv(self): + await self.async_client.post("/traced/") + + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 1) + + span = spans[0] + + self.assertEqual(span.name, "POST ^traced/") + self.assertEqual(span.kind, SpanKind.SERVER) + self.assertEqual(span.status.status_code, StatusCode.UNSET) + self.assertEqual(span.attributes[HTTP_REQUEST_METHOD], "POST") + self.assertEqual(span.attributes[URL_SCHEME], "http") + self.assertEqual(span.attributes[SERVER_PORT], 80) + self.assertEqual(span.attributes[CLIENT_ADDRESS], "127.0.0.1") + self.assertEqual(span.attributes[NETWORK_PROTOCOL_VERSION], "1.1") + self.assertEqual(span.attributes[HTTP_ROUTE], "^traced/") + self.assertEqual(span.attributes[HTTP_RESPONSE_STATUS_CODE], 200) + + async def test_traced_post_both_semconv(self): + await self.async_client.post("/traced/") + + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 1) + + span = spans[0] + + self.assertEqual(span.name, "POST ^traced/") + self.assertEqual(span.kind, SpanKind.SERVER) + self.assertEqual(span.status.status_code, StatusCode.UNSET) + self.assertEqual(span.attributes[SpanAttributes.HTTP_METHOD], "POST") + self.assertEqual( + span.attributes[SpanAttributes.HTTP_URL], + "http://127.0.0.1/traced/", + ) + self.assertEqual(span.attributes[SpanAttributes.HTTP_SCHEME], "http") + self.assertEqual(span.attributes[SpanAttributes.HTTP_STATUS_CODE], 200) + self.assertEqual(span.attributes[HTTP_REQUEST_METHOD], "POST") + self.assertEqual(span.attributes[URL_SCHEME], "http") + self.assertEqual(span.attributes[SERVER_PORT], 80) + self.assertEqual(span.attributes[CLIENT_ADDRESS], "127.0.0.1") + self.assertEqual(span.attributes[NETWORK_PROTOCOL_VERSION], "1.1") + self.assertEqual(span.attributes[HTTP_ROUTE], "^traced/") + self.assertEqual(span.attributes[HTTP_RESPONSE_STATUS_CODE], 200) + async def test_error(self): with self.assertRaises(ValueError): await self.async_client.get("/error/") @@ -240,6 +412,60 @@ async def test_error(self): event.attributes[SpanAttributes.EXCEPTION_MESSAGE], "error" ) + async def test_error_new_semconv(self): + with self.assertRaises(ValueError): + await self.async_client.get("/error/") + + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 1) + + span = spans[0] + + self.assertEqual(span.name, "GET ^error/") + self.assertEqual(span.kind, SpanKind.SERVER) + self.assertEqual(span.status.status_code, StatusCode.ERROR) + self.assertEqual(span.attributes[HTTP_REQUEST_METHOD], "GET") + self.assertEqual(span.attributes[HTTP_ROUTE], "^error/") + self.assertEqual(span.attributes[URL_SCHEME], "http") + self.assertEqual(span.attributes[HTTP_RESPONSE_STATUS_CODE], 500) + + self.assertEqual(len(span.events), 1) + event = span.events[0] + self.assertEqual(event.name, "exception") + self.assertEqual(event.attributes[EXCEPTION_TYPE], "ValueError") + self.assertEqual(event.attributes[EXCEPTION_MESSAGE], "error") + + async def test_error_both_semconv(self): + with self.assertRaises(ValueError): + await self.async_client.get("/error/") + + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 1) + + span = spans[0] + + self.assertEqual(span.name, "GET ^error/") + self.assertEqual(span.kind, SpanKind.SERVER) + self.assertEqual(span.status.status_code, StatusCode.ERROR) + self.assertEqual(span.attributes[SpanAttributes.HTTP_METHOD], "GET") + self.assertEqual( + span.attributes[SpanAttributes.HTTP_URL], + "http://127.0.0.1/error/", + ) + self.assertEqual(span.attributes[SpanAttributes.HTTP_ROUTE], "^error/") + self.assertEqual(span.attributes[SpanAttributes.HTTP_SCHEME], "http") + self.assertEqual(span.attributes[SpanAttributes.HTTP_STATUS_CODE], 500) + self.assertEqual(span.attributes[HTTP_REQUEST_METHOD], "GET") + self.assertEqual(span.attributes[HTTP_ROUTE], "^error/") + self.assertEqual(span.attributes[URL_SCHEME], "http") + self.assertEqual(span.attributes[HTTP_RESPONSE_STATUS_CODE], 500) + + self.assertEqual(len(span.events), 1) + event = span.events[0] + self.assertEqual(event.name, "exception") + self.assertEqual(event.attributes[EXCEPTION_TYPE], "ValueError") + self.assertEqual(event.attributes[EXCEPTION_MESSAGE], "error") + async def test_exclude_lists(self): await self.async_client.get("/excluded_arg/123") span_list = self.memory_exporter.get_finished_spans() @@ -285,6 +511,45 @@ async def test_span_name_404(self): span = span_list[0] self.assertEqual(span.name, "GET") + async def test_nonstandard_http_method_span_name(self): + await self.async_client.request( + method="NONSTANDARD", path="/span_name/1234/" + ) + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 1) + + span = span_list[0] + self.assertEqual(span.name, "HTTP") + + async def test_nonstandard_http_method_span_name_new_semconv(self): + await self.async_client.request( + method="NONSTANDARD", path="/span_name/1234/" + ) + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 1) + + span = span_list[0] + self.assertEqual(span.name, "HTTP") + self.assertEqual(span.attributes[HTTP_REQUEST_METHOD], "_OTHER") + self.assertEqual( + span.attributes[HTTP_REQUEST_METHOD_ORIGINAL], "NONSTANDARD" + ) + + async def test_nonstandard_http_method_span_name_both_semconv(self): + await self.async_client.request( + method="NONSTANDARD", path="/span_name/1234/" + ) + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 1) + + span = span_list[0] + self.assertEqual(span.name, "HTTP") + self.assertEqual(span.attributes[SpanAttributes.HTTP_METHOD], "_OTHER") + self.assertEqual(span.attributes[HTTP_REQUEST_METHOD], "_OTHER") + self.assertEqual( + span.attributes[HTTP_REQUEST_METHOD_ORIGINAL], "NONSTANDARD" + ) + async def test_traced_request_attrs(self): await self.async_client.get("/span_name/1234/", CONTENT_TYPE="test/ct") span_list = self.memory_exporter.get_finished_spans() diff --git a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py index 79c9a0cf0f..1a252b9a16 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py @@ -268,7 +268,7 @@ def __init__(self, *args, **kwargs): self.duration_histogram = self._otel_meter.create_histogram( name=MetricInstruments.HTTP_SERVER_DURATION, unit="ms", - description="Duration of HTTP client requests.", + description="Duration of HTTP server requests.", ) self.active_requests_counter = self._otel_meter.create_up_down_counter( name=MetricInstruments.HTTP_SERVER_ACTIVE_REQUESTS, diff --git a/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py b/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py index d0010ed8d0..09f1645384 100644 --- a/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py +++ b/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py @@ -141,7 +141,7 @@ def trace_tween_factory(handler, registry): duration_histogram = meter.create_histogram( name=MetricInstruments.HTTP_SERVER_DURATION, unit="ms", - description="Duration of HTTP client requests.", + description="Duration of HTTP server requests.", ) active_requests_counter = meter.create_up_down_counter( name=MetricInstruments.HTTP_SERVER_ACTIVE_REQUESTS, diff --git a/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py b/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py index be9129bda0..1b56db3876 100644 --- a/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py @@ -296,7 +296,7 @@ def _create_server_histograms(meter) -> Dict[str, Histogram]: MetricInstruments.HTTP_SERVER_DURATION: meter.create_histogram( name=MetricInstruments.HTTP_SERVER_DURATION, unit="ms", - description="Duration of HTTP client requests.", + description="Duration of HTTP server requests.", ), MetricInstruments.HTTP_SERVER_REQUEST_SIZE: meter.create_histogram( name=MetricInstruments.HTTP_SERVER_REQUEST_SIZE, From cc52bd27299ff767fcdcdf536b418ac33f4b29dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Em=C3=ADdio=20Neto?= <9735060+emdneto@users.noreply.github.com> Date: Mon, 22 Jul 2024 16:56:22 -0300 Subject: [PATCH 21/26] Fix http clients method attribute in case of non standard http methods (#2726) --- CHANGELOG.md | 5 +- .../aiohttp_client/__init__.py | 11 ++- .../tests/test_aiohttp_client_integration.py | 87 +++++++++++++++++++ .../instrumentation/httpx/__init__.py | 12 +-- .../tests/test_httpx_integration.py | 54 ++++++++++++ .../instrumentation/requests/__init__.py | 12 ++- .../tests/test_requests_integration.py | 43 +++++++++ 7 files changed, 209 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a320a1d4fc..4228bf1254 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#2682](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2682)) - Populate `{method}` as `HTTP` on `_OTHER` methods from scope for `django` middleware ([#2714](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2714)) +- `opentelemetry-instrumentation-httpx`, `opentelemetry-instrumentation-aiohttp-client`, + `opentelemetry-instrumentation-requests` Populate `{method}` as `HTTP` on `_OTHER` methods + ([#2726](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2726)) ### Fixed - Handle `redis.exceptions.WatchError` as a non-error event in redis instrumentation @@ -79,7 +82,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#2153](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2153)) - `opentelemetry-instrumentation-asgi` Removed `NET_HOST_NAME` AND `NET_HOST_PORT` from active requests count attribute ([#2610](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2610)) -- `opentelemetry-instrumentation-asgi` Bugfix: Middleware did not set status code attribute on duration metrics for non-recording spans. +- `opentelemetry-instrumentation-asgi` Bugfix: Middleware did not set status code attribute on duration metrics for non-recording spans. ([#2627](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2627)) diff --git a/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py b/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py index 459e3121b9..de60fa6379 100644 --- a/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py @@ -132,10 +132,9 @@ def response_hook(span: Span, params: typing.Union[ def _get_span_name(method: str) -> str: - method = sanitize_method(method.upper().strip()) + method = sanitize_method(method.strip()) if method == "_OTHER": method = "HTTP" - return method @@ -230,8 +229,8 @@ async def on_request_start( trace_config_ctx.span = None return - http_method = params.method - request_span_name = _get_span_name(http_method) + method = params.method + request_span_name = _get_span_name(method) request_url = ( remove_url_credentials(trace_config_ctx.url_filter(params.url)) if callable(trace_config_ctx.url_filter) @@ -241,8 +240,8 @@ async def on_request_start( span_attributes = {} _set_http_method( span_attributes, - http_method, - request_span_name, + method, + sanitize_method(method), sem_conv_opt_in_mode, ) _set_http_url(span_attributes, request_url, sem_conv_opt_in_mode) diff --git a/instrumentation/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py b/instrumentation/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py index 538421dd6a..3873a6f094 100644 --- a/instrumentation/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py +++ b/instrumentation/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py @@ -40,6 +40,7 @@ from opentelemetry.semconv.attributes.error_attributes import ERROR_TYPE from opentelemetry.semconv.attributes.http_attributes import ( HTTP_REQUEST_METHOD, + HTTP_REQUEST_METHOD_ORIGINAL, HTTP_RESPONSE_STATUS_CODE, ) from opentelemetry.semconv.attributes.url_attributes import URL_FULL @@ -503,6 +504,92 @@ async def request_handler(request): ] ) + def test_nonstandard_http_method(self): + trace_configs = [aiohttp_client.create_trace_config()] + app = HttpServerMock("nonstandard_method") + + @app.route("/status/200", methods=["NONSTANDARD"]) + def index(): + return ("", 405, {}) + + url = "http://localhost:5000/status/200" + + with app.run("localhost", 5000): + with self.subTest(url=url): + + async def do_request(url): + async with aiohttp.ClientSession( + trace_configs=trace_configs, + ) as session: + async with session.request("NONSTANDARD", url): + pass + + loop = asyncio.get_event_loop() + loop.run_until_complete(do_request(url)) + + self.assert_spans( + [ + ( + "HTTP", + (StatusCode.ERROR, None), + { + SpanAttributes.HTTP_METHOD: "_OTHER", + SpanAttributes.HTTP_URL: url, + SpanAttributes.HTTP_STATUS_CODE: int( + HTTPStatus.METHOD_NOT_ALLOWED + ), + }, + ) + ] + ) + self.memory_exporter.clear() + + def test_nonstandard_http_method_new_semconv(self): + trace_configs = [ + aiohttp_client.create_trace_config( + sem_conv_opt_in_mode=_HTTPStabilityMode.HTTP + ) + ] + app = HttpServerMock("nonstandard_method") + + @app.route("/status/200", methods=["NONSTANDARD"]) + def index(): + return ("", 405, {}) + + url = "http://localhost:5000/status/200" + + with app.run("localhost", 5000): + with self.subTest(url=url): + + async def do_request(url): + async with aiohttp.ClientSession( + trace_configs=trace_configs, + ) as session: + async with session.request("NONSTANDARD", url): + pass + + loop = asyncio.get_event_loop() + loop.run_until_complete(do_request(url)) + + self.assert_spans( + [ + ( + "HTTP", + (StatusCode.ERROR, None), + { + HTTP_REQUEST_METHOD: "_OTHER", + URL_FULL: url, + HTTP_RESPONSE_STATUS_CODE: int( + HTTPStatus.METHOD_NOT_ALLOWED + ), + HTTP_REQUEST_METHOD_ORIGINAL: "NONSTANDARD", + ERROR_TYPE: "405", + }, + ) + ] + ) + self.memory_exporter.clear() + def test_credential_removal(self): trace_configs = [aiohttp_client.create_trace_config()] diff --git a/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py b/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py index e3ce383d7e..f2a18a2770 100644 --- a/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py @@ -259,7 +259,7 @@ class ResponseInfo(typing.NamedTuple): def _get_default_span_name(method: str) -> str: - method = sanitize_method(method.upper().strip()) + method = sanitize_method(method.strip()) if method == "_OTHER": method = "HTTP" @@ -326,12 +326,16 @@ def _apply_request_client_attributes_to_span( span_attributes: dict, url: typing.Union[str, URL, httpx.URL], method_original: str, - span_name: str, semconv: _HTTPStabilityMode, ): url = httpx.URL(url) # http semconv transition: http.method -> http.request.method - _set_http_method(span_attributes, method_original, span_name, semconv) + _set_http_method( + span_attributes, + method_original, + sanitize_method(method_original), + semconv, + ) # http semconv transition: http.url -> url.full _set_http_url(span_attributes, str(url), semconv) @@ -450,7 +454,6 @@ def handle_request( span_attributes, url, method_original, - span_name, self._sem_conv_opt_in_mode, ) @@ -572,7 +575,6 @@ async def handle_async_request(self, *args, **kwargs) -> typing.Union[ span_attributes, url, method_original, - span_name, self._sem_conv_opt_in_mode, ) diff --git a/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py b/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py index 03141e61b5..011b5e57d2 100644 --- a/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py +++ b/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py @@ -39,6 +39,7 @@ from opentelemetry.semconv.attributes.error_attributes import ERROR_TYPE from opentelemetry.semconv.attributes.http_attributes import ( HTTP_REQUEST_METHOD, + HTTP_REQUEST_METHOD_ORIGINAL, HTTP_RESPONSE_STATUS_CODE, ) from opentelemetry.semconv.attributes.network_attributes import ( @@ -217,6 +218,59 @@ def test_basic(self): span, opentelemetry.instrumentation.httpx ) + def test_nonstandard_http_method(self): + respx.route(method="NONSTANDARD").mock( + return_value=httpx.Response(405) + ) + self.perform_request(self.URL, method="NONSTANDARD") + span = self.assert_span() + + self.assertIs(span.kind, trace.SpanKind.CLIENT) + self.assertEqual(span.name, "HTTP") + self.assertEqual( + span.attributes, + { + SpanAttributes.HTTP_METHOD: "_OTHER", + SpanAttributes.HTTP_URL: self.URL, + SpanAttributes.HTTP_STATUS_CODE: 405, + }, + ) + + self.assertIs(span.status.status_code, trace.StatusCode.ERROR) + + self.assertEqualSpanInstrumentationInfo( + span, opentelemetry.instrumentation.httpx + ) + + def test_nonstandard_http_method_new_semconv(self): + respx.route(method="NONSTANDARD").mock( + return_value=httpx.Response(405) + ) + self.perform_request(self.URL, method="NONSTANDARD") + span = self.assert_span() + + self.assertIs(span.kind, trace.SpanKind.CLIENT) + self.assertEqual(span.name, "HTTP") + self.assertEqual( + span.attributes, + { + HTTP_REQUEST_METHOD: "_OTHER", + URL_FULL: self.URL, + SERVER_ADDRESS: "mock", + NETWORK_PEER_ADDRESS: "mock", + HTTP_RESPONSE_STATUS_CODE: 405, + NETWORK_PROTOCOL_VERSION: "1.1", + ERROR_TYPE: "405", + HTTP_REQUEST_METHOD_ORIGINAL: "NONSTANDARD", + }, + ) + + self.assertIs(span.status.status_code, trace.StatusCode.ERROR) + + self.assertEqualSpanInstrumentationInfo( + span, opentelemetry.instrumentation.httpx + ) + def test_basic_new_semconv(self): url = "http://mock:8080/status/200" respx.get(url).mock( 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 0a55564386..3aa1b476f5 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py @@ -188,13 +188,19 @@ def get_or_create_headers(): span_attributes = {} _set_http_method( - span_attributes, method, span_name, sem_conv_opt_in_mode + span_attributes, + method, + sanitize_method(method), + sem_conv_opt_in_mode, ) _set_http_url(span_attributes, url, sem_conv_opt_in_mode) metric_labels = {} _set_http_method( - metric_labels, method, span_name, sem_conv_opt_in_mode + metric_labels, + method, + sanitize_method(method), + sem_conv_opt_in_mode, ) try: @@ -365,7 +371,7 @@ def get_default_span_name(method): Returns: span name """ - method = sanitize_method(method.upper().strip()) + method = sanitize_method(method.strip()) if method == "_OTHER": return "HTTP" return method diff --git a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py index 75518fc8d3..a5cb8927ae 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py +++ b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py @@ -36,6 +36,7 @@ from opentelemetry.semconv.attributes.error_attributes import ERROR_TYPE from opentelemetry.semconv.attributes.http_attributes import ( HTTP_REQUEST_METHOD, + HTTP_REQUEST_METHOD_ORIGINAL, HTTP_RESPONSE_STATUS_CODE, ) from opentelemetry.semconv.attributes.network_attributes import ( @@ -247,6 +248,48 @@ def test_basic_both_semconv(self): span, opentelemetry.instrumentation.requests ) + @mock.patch("httpretty.http.HttpBaseClass.METHODS", ("NONSTANDARD",)) + def test_nonstandard_http_method(self): + httpretty.register_uri("NONSTANDARD", self.URL, status=405) + session = requests.Session() + session.request("NONSTANDARD", self.URL) + span = self.assert_span() + self.assertIs(span.kind, trace.SpanKind.CLIENT) + self.assertEqual(span.name, "HTTP") + self.assertEqual( + span.attributes, + { + SpanAttributes.HTTP_METHOD: "_OTHER", + SpanAttributes.HTTP_URL: self.URL, + SpanAttributes.HTTP_STATUS_CODE: 405, + }, + ) + + self.assertIs(span.status.status_code, trace.StatusCode.ERROR) + + @mock.patch("httpretty.http.HttpBaseClass.METHODS", ("NONSTANDARD",)) + def test_nonstandard_http_method_new_semconv(self): + httpretty.register_uri("NONSTANDARD", self.URL, status=405) + session = requests.Session() + session.request("NONSTANDARD", self.URL) + span = self.assert_span() + self.assertIs(span.kind, trace.SpanKind.CLIENT) + self.assertEqual(span.name, "HTTP") + self.assertEqual( + span.attributes, + { + HTTP_REQUEST_METHOD: "_OTHER", + URL_FULL: self.URL, + SERVER_ADDRESS: "mock", + NETWORK_PEER_ADDRESS: "mock", + HTTP_RESPONSE_STATUS_CODE: 405, + NETWORK_PROTOCOL_VERSION: "1.1", + ERROR_TYPE: "405", + HTTP_REQUEST_METHOD_ORIGINAL: "NONSTANDARD", + }, + ) + self.assertIs(span.status.status_code, trace.StatusCode.ERROR) + def test_hooks(self): def request_hook(span, request_obj): span.update_name("name set from hook") From 948b47d43c750c703b5b36d04e3d55b110ca9dac Mon Sep 17 00:00:00 2001 From: CC Date: Tue, 23 Jul 2024 21:24:01 +0530 Subject: [PATCH 22/26] Add NoOpTraceProvider test for opentelemetry-instrumentation-logging (#2649) --- .../tests/test_logging.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-logging/tests/test_logging.py b/instrumentation/opentelemetry-instrumentation-logging/tests/test_logging.py index a5a0d5adff..c8b8744cf3 100644 --- a/instrumentation/opentelemetry-instrumentation-logging/tests/test_logging.py +++ b/instrumentation/opentelemetry-instrumentation-logging/tests/test_logging.py @@ -23,7 +23,7 @@ LoggingInstrumentor, ) from opentelemetry.test.test_base import TestBase -from opentelemetry.trace import ProxyTracer, get_tracer +from opentelemetry.trace import NoOpTracerProvider, ProxyTracer, get_tracer class FakeTracerProvider: @@ -207,3 +207,18 @@ def test_uninstrumented(self): self.assertFalse(hasattr(record, "otelTraceID")) self.assertFalse(hasattr(record, "otelServiceName")) self.assertFalse(hasattr(record, "otelTraceSampled")) + + def test_no_op_tracer_provider(self): + LoggingInstrumentor().uninstrument() + LoggingInstrumentor().instrument(tracer_provider=NoOpTracerProvider()) + + with self.caplog.at_level(level=logging.INFO): + logger = logging.getLogger("test logger") + logger.info("hello") + + self.assertEqual(len(self.caplog.records), 1) + record = self.caplog.records[0] + self.assertEqual(record.otelSpanID, "0") + self.assertEqual(record.otelTraceID, "0") + self.assertEqual(record.otelServiceName, "") + self.assertEqual(record.otelTraceSampled, False) From 38e4ea4a2400e8560c36f2fcdf1c61987fb4ebf8 Mon Sep 17 00:00:00 2001 From: Zhihan Li <54661071+zhihali@users.noreply.github.com> Date: Tue, 23 Jul 2024 17:31:33 +0100 Subject: [PATCH 23/26] Teach fastapi instrumentation about fastapi-slim (#2702) --- .github/workflows/instrumentations_1.yml | 1 + CHANGELOG.md | 3 ++ instrumentation/README.md | 2 +- .../pyproject.toml | 3 +- .../instrumentation/fastapi/__init__.py | 12 ++++++- .../instrumentation/fastapi/package.py | 5 ++- .../test-requirements-slim.txt | 32 +++++++++++++++++++ .../instrumentation/bootstrap_gen.py | 4 +++ tox.ini | 7 ++++ 9 files changed, 65 insertions(+), 4 deletions(-) create mode 100644 instrumentation/opentelemetry-instrumentation-fastapi/test-requirements-slim.txt diff --git a/.github/workflows/instrumentations_1.yml b/.github/workflows/instrumentations_1.yml index 8c99eb0572..a658796ef8 100644 --- a/.github/workflows/instrumentations_1.yml +++ b/.github/workflows/instrumentations_1.yml @@ -38,6 +38,7 @@ jobs: - "resource-detector-azure" - "resource-detector-container" - "util-http" + - "fastapi-slim" os: [ubuntu-20.04] exclude: - python-version: pypy3 diff --git a/CHANGELOG.md b/CHANGELOG.md index 4228bf1254..7caff5e940 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,8 +55,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `opentelemetry-instrumentation-httpx`, `opentelemetry-instrumentation-aiohttp-client`, `opentelemetry-instrumentation-requests` Populate `{method}` as `HTTP` on `_OTHER` methods ([#2726](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2726)) +- `opentelemetry-instrumentation-fastapi` Add dependency support for fastapi-slim + ([#2702](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2702)) ### Fixed + - Handle `redis.exceptions.WatchError` as a non-error event in redis instrumentation ([#2668](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2668)) - `opentelemetry-instrumentation-httpx` Ensure httpx.get or httpx.request like methods are instrumented diff --git a/instrumentation/README.md b/instrumentation/README.md index 989ba529cb..900ee6ea74 100644 --- a/instrumentation/README.md +++ b/instrumentation/README.md @@ -19,7 +19,7 @@ | [opentelemetry-instrumentation-django](./opentelemetry-instrumentation-django) | django >= 1.10 | Yes | experimental | [opentelemetry-instrumentation-elasticsearch](./opentelemetry-instrumentation-elasticsearch) | elasticsearch >= 6.0 | No | experimental | [opentelemetry-instrumentation-falcon](./opentelemetry-instrumentation-falcon) | falcon >= 1.4.1, < 4.0.0 | Yes | experimental -| [opentelemetry-instrumentation-fastapi](./opentelemetry-instrumentation-fastapi) | fastapi ~= 0.58 | Yes | migration +| [opentelemetry-instrumentation-fastapi](./opentelemetry-instrumentation-fastapi) | fastapi ~= 0.58,fastapi-slim ~= 0.111.0 | Yes | migration | [opentelemetry-instrumentation-flask](./opentelemetry-instrumentation-flask) | flask >= 1.0 | Yes | migration | [opentelemetry-instrumentation-grpc](./opentelemetry-instrumentation-grpc) | grpcio ~= 1.27 | No | experimental | [opentelemetry-instrumentation-httpx](./opentelemetry-instrumentation-httpx) | httpx >= 0.18.0 | No | migration diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/pyproject.toml b/instrumentation/opentelemetry-instrumentation-fastapi/pyproject.toml index 7bae75494e..1ef5507504 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/pyproject.toml +++ b/instrumentation/opentelemetry-instrumentation-fastapi/pyproject.toml @@ -34,7 +34,8 @@ dependencies = [ [project.optional-dependencies] instruments = [ - "fastapi ~= 0.58", + "fastapi ~= 0.58", + "fastapi-slim ~= 0.111.0", ] [project.entry-points.opentelemetry_instrumentor] diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py index 2f9bbe7314..9356093a45 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py @@ -172,6 +172,7 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A --- """ import logging +from importlib.util import find_spec from typing import Collection import fastapi @@ -189,7 +190,11 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A ClientResponseHook, ServerRequestHook, ) -from opentelemetry.instrumentation.fastapi.package import _instruments +from opentelemetry.instrumentation.fastapi.package import ( + _fastapi, + _fastapi_slim, + _instruments, +) from opentelemetry.instrumentation.fastapi.version import __version__ from opentelemetry.instrumentation.instrumentor import BaseInstrumentor from opentelemetry.metrics import get_meter @@ -280,6 +285,11 @@ def uninstrument_app(app: fastapi.FastAPI): app._is_instrumented_by_opentelemetry = False def instrumentation_dependencies(self) -> Collection[str]: + if find_spec("fastapi") is not None: + return (_fastapi,) + if find_spec("fastapi_slim") is not None: + return (_fastapi_slim,) + # If neither is installed, return both as potential dependencies return _instruments def _instrument(self, **kwargs): diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/package.py b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/package.py index d95a2cf6d5..55e1059d7a 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/package.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/package.py @@ -13,7 +13,10 @@ # limitations under the License. -_instruments = ("fastapi ~= 0.58",) +_fastapi = "fastapi ~= 0.58" +_fastapi_slim = "fastapi-slim ~= 0.111.0" + +_instruments = (_fastapi, _fastapi_slim) _supports_metrics = True diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/test-requirements-slim.txt b/instrumentation/opentelemetry-instrumentation-fastapi/test-requirements-slim.txt new file mode 100644 index 0000000000..2ea55f1b91 --- /dev/null +++ b/instrumentation/opentelemetry-instrumentation-fastapi/test-requirements-slim.txt @@ -0,0 +1,32 @@ +annotated-types==0.6.0 +anyio==4.3.0 +asgiref==3.7.2 +certifi==2024.7.4 +charset-normalizer==3.3.2 +Deprecated==1.2.14 +exceptiongroup==1.2.0 +fastapi-slim==0.111.0 +h11==0.14.0 +httpcore==1.0.4 +httpx==0.27.0 +idna==3.7 +importlib-metadata==6.11.0 +iniconfig==2.0.0 +packaging==24.0 +pluggy==1.5.0 +py-cpuinfo==9.0.0 +pydantic==2.6.2 +pydantic_core==2.16.3 +pytest==7.4.4 +requests==2.32.3 +sniffio==1.3.0 +starlette==0.37.2 +tomli==2.0.1 +typing_extensions==4.9.0 +urllib3==2.2.2 +wrapt==1.16.0 +zipp==3.19.2 +-e opentelemetry-instrumentation +-e instrumentation/opentelemetry-instrumentation-asgi +-e util/opentelemetry-util-http +-e instrumentation/opentelemetry-instrumentation-fastapi diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/bootstrap_gen.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/bootstrap_gen.py index 3f4c78862f..9b71a8bdff 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/bootstrap_gen.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/bootstrap_gen.py @@ -84,6 +84,10 @@ "library": "fastapi ~= 0.58", "instrumentation": "opentelemetry-instrumentation-fastapi==0.47b0.dev", }, + { + "library": "fastapi-slim ~= 0.111.0", + "instrumentation": "opentelemetry-instrumentation-fastapi==0.47b0.dev", + }, { "library": "flask >= 1.0", "instrumentation": "opentelemetry-instrumentation-flask==0.47b0.dev", diff --git a/tox.ini b/tox.ini index aac890ed7c..4ba434b29b 100644 --- a/tox.ini +++ b/tox.ini @@ -116,7 +116,9 @@ envlist = ; opentelemetry-instrumentation-fastapi py3{8,9,10,11,12}-test-instrumentation-fastapi + py3{8,9,10,11,12}-test-instrumentation-fastapi-slim pypy3-test-instrumentation-fastapi + pypy3-test-instrumentation-fastapi-slim lint-instrumentation-fastapi ; opentelemetry-instrumentation-flask @@ -544,6 +546,11 @@ commands_pre = fastapi: pip install opentelemetry-sdk@{env:CORE_REPO}\#egg=opentelemetry-sdk&subdirectory=opentelemetry-sdk fastapi: pip install opentelemetry-test-utils@{env:CORE_REPO}\#egg=opentelemetry-test-utils&subdirectory=tests/opentelemetry-test-utils fastapi: pip install -r {toxinidir}/instrumentation/opentelemetry-instrumentation-fastapi/test-requirements.txt + fastapi-slim: pip install opentelemetry-api@{env:CORE_REPO}\#egg=opentelemetry-api&subdirectory=opentelemetry-api + fastapi-slim: pip install opentelemetry-semantic-conventions@{env:CORE_REPO}\#egg=opentelemetry-semantic-conventions&subdirectory=opentelemetry-semantic-conventions + fastapi-slim: pip install opentelemetry-sdk@{env:CORE_REPO}\#egg=opentelemetry-sdk&subdirectory=opentelemetry-sdk + fastapi-slim: pip install opentelemetry-test-utils@{env:CORE_REPO}\#egg=opentelemetry-test-utils&subdirectory=tests/opentelemetry-test-utils + fastapi-slim: pip install -r {toxinidir}/instrumentation/opentelemetry-instrumentation-fastapi/test-requirements-slim.txt mysql: pip install opentelemetry-api@{env:CORE_REPO}\#egg=opentelemetry-api&subdirectory=opentelemetry-api mysql: pip install opentelemetry-semantic-conventions@{env:CORE_REPO}\#egg=opentelemetry-semantic-conventions&subdirectory=opentelemetry-semantic-conventions From 92da527977390dc55807ab2547f31f8788255e9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Em=C3=ADdio=20Neto?= <9735060+emdneto@users.noreply.github.com> Date: Tue, 23 Jul 2024 14:09:11 -0300 Subject: [PATCH 24/26] HTTP semantic convention stability migration for urllib3 (#2715) --- CHANGELOG.md | 6 + instrumentation/README.md | 2 +- .../instrumentation/urllib3/__init__.py | 313 ++++++++++++++---- .../instrumentation/urllib3/package.py | 2 + .../tests/test_urllib3_integration.py | 230 ++++++++++++- .../tests/test_urllib3_metrics.py | 305 +++++++++++++++-- 6 files changed, 748 insertions(+), 110 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7caff5e940..bc400865a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#2673](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2673)) - `opentelemetry-instrumentation-django` Add `http.target` to Django duration metric attributes ([#2624](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2624)) +- `opentelemetry-instrumentation-urllib3` Implement new semantic convention opt-in migration + ([#2715](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2715)) - `opentelemetry-instrumentation-django` Implement new semantic convention opt-in with stable http semantic conventions ([#2714](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2714)) @@ -48,6 +50,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#2580](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2580)) - Populate `{method}` as `HTTP` on `_OTHER` methods from scope for `asgi` middleware ([#2610](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2610)) +- Populate `{method}` as `HTTP` on `_OTHER` methods from scope for `fastapi` middleware + ([#2682](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2682)) +- `opentelemetry-instrumentation-urllib3` Populate `{method}` as `HTTP` on `_OTHER` methods for span name + ([#2715](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2715)) - Populate `{method}` as `HTTP` on `_OTHER` methods from scope for `fastapi` instrumentation ([#2682](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2682)) - Populate `{method}` as `HTTP` on `_OTHER` methods from scope for `django` middleware diff --git a/instrumentation/README.md b/instrumentation/README.md index 900ee6ea74..555e0bcd2a 100644 --- a/instrumentation/README.md +++ b/instrumentation/README.md @@ -46,5 +46,5 @@ | [opentelemetry-instrumentation-tornado](./opentelemetry-instrumentation-tornado) | tornado >= 5.1.1 | Yes | experimental | [opentelemetry-instrumentation-tortoiseorm](./opentelemetry-instrumentation-tortoiseorm) | tortoise-orm >= 0.17.0 | No | experimental | [opentelemetry-instrumentation-urllib](./opentelemetry-instrumentation-urllib) | urllib | Yes | experimental -| [opentelemetry-instrumentation-urllib3](./opentelemetry-instrumentation-urllib3) | urllib3 >= 1.0.0, < 3.0.0 | Yes | experimental +| [opentelemetry-instrumentation-urllib3](./opentelemetry-instrumentation-urllib3) | urllib3 >= 1.0.0, < 3.0.0 | Yes | migration | [opentelemetry-instrumentation-wsgi](./opentelemetry-instrumentation-wsgi) | wsgi | Yes | migration \ No newline at end of file diff --git a/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/__init__.py b/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/__init__.py index add5db8f19..a05084725d 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/__init__.py @@ -87,25 +87,49 @@ def response_hook(span, request, response): import urllib3.connectionpool import wrapt +from opentelemetry.instrumentation._semconv import ( + _client_duration_attrs_new, + _client_duration_attrs_old, + _filter_semconv_duration_attrs, + _get_schema_url, + _HTTPStabilityMode, + _OpenTelemetrySemanticConventionStability, + _OpenTelemetryStabilitySignalType, + _report_new, + _report_old, + _set_http_host, + _set_http_method, + _set_http_net_peer_name_client, + _set_http_network_protocol_version, + _set_http_peer_port_client, + _set_http_scheme, + _set_http_url, + _set_status, +) from opentelemetry.instrumentation.instrumentor import BaseInstrumentor from opentelemetry.instrumentation.urllib3.package import _instruments from opentelemetry.instrumentation.urllib3.version import __version__ from opentelemetry.instrumentation.utils import ( - http_status_to_status_code, is_http_instrumentation_enabled, suppress_http_instrumentation, unwrap, ) from opentelemetry.metrics import Histogram, get_meter from opentelemetry.propagate import inject +from opentelemetry.semconv._incubating.metrics.http_metrics import ( + create_http_client_request_body_size, + create_http_client_response_body_size, +) from opentelemetry.semconv.metrics import MetricInstruments -from opentelemetry.semconv.trace import SpanAttributes +from opentelemetry.semconv.metrics.http_metrics import ( + HTTP_CLIENT_REQUEST_DURATION, +) from opentelemetry.trace import Span, SpanKind, Tracer, get_tracer -from opentelemetry.trace.status import Status from opentelemetry.util.http import ( ExcludeList, get_excluded_urls, parse_excluded_urls, + sanitize_method, ) from opentelemetry.util.http.httplib import set_ip_on_next_http_connection @@ -158,12 +182,18 @@ def _instrument(self, **kwargs): ``excluded_urls``: A string containing a comma-delimited list of regexes used to exclude URLs from tracking """ + # initialize semantic conventions opt-in if needed + _OpenTelemetrySemanticConventionStability._initialize() + sem_conv_opt_in_mode = _OpenTelemetrySemanticConventionStability._get_opentelemetry_stability_opt_in_mode( + _OpenTelemetryStabilitySignalType.HTTP, + ) + schema_url = _get_schema_url(sem_conv_opt_in_mode) tracer_provider = kwargs.get("tracer_provider") tracer = get_tracer( __name__, __version__, tracer_provider, - schema_url="https://opentelemetry.io/schemas/1.11.0", + schema_url=schema_url, ) excluded_urls = kwargs.get("excluded_urls") @@ -173,30 +203,57 @@ def _instrument(self, **kwargs): __name__, __version__, meter_provider, - schema_url="https://opentelemetry.io/schemas/1.11.0", - ) - - duration_histogram = meter.create_histogram( - name=MetricInstruments.HTTP_CLIENT_DURATION, - unit="ms", - description="Measures the duration of outbound HTTP requests.", - ) - request_size_histogram = meter.create_histogram( - name=MetricInstruments.HTTP_CLIENT_REQUEST_SIZE, - unit="By", - description="Measures the size of HTTP request messages.", - ) - response_size_histogram = meter.create_histogram( - name=MetricInstruments.HTTP_CLIENT_RESPONSE_SIZE, - unit="By", - description="Measures the size of HTTP response messages.", + schema_url=schema_url, ) + duration_histogram_old = None + request_size_histogram_old = None + response_size_histogram_old = None + if _report_old(sem_conv_opt_in_mode): + # http.client.duration histogram + duration_histogram_old = meter.create_histogram( + name=MetricInstruments.HTTP_CLIENT_DURATION, + unit="ms", + description="Measures the duration of the outbound HTTP request", + ) + # http.client.request.size histogram + request_size_histogram_old = meter.create_histogram( + name=MetricInstruments.HTTP_CLIENT_REQUEST_SIZE, + unit="By", + description="Measures the size of HTTP request messages.", + ) + # http.client.response.size histogram + response_size_histogram_old = meter.create_histogram( + name=MetricInstruments.HTTP_CLIENT_RESPONSE_SIZE, + unit="By", + description="Measures the size of HTTP response messages.", + ) + duration_histogram_new = None + request_size_histogram_new = None + response_size_histogram_new = None + if _report_new(sem_conv_opt_in_mode): + # http.client.request.duration histogram + duration_histogram_new = meter.create_histogram( + name=HTTP_CLIENT_REQUEST_DURATION, + unit="s", + description="Duration of HTTP client requests.", + ) + # http.client.request.body.size histogram + request_size_histogram_new = create_http_client_request_body_size( + meter + ) + # http.client.response.body.size histogram + response_size_histogram_new = ( + create_http_client_response_body_size(meter) + ) _instrument( tracer, - duration_histogram, - request_size_histogram, - response_size_histogram, + duration_histogram_old, + duration_histogram_new, + request_size_histogram_old, + request_size_histogram_new, + response_size_histogram_old, + response_size_histogram_new, request_hook=kwargs.get("request_hook"), response_hook=kwargs.get("response_hook"), url_filter=kwargs.get("url_filter"), @@ -205,21 +262,33 @@ def _instrument(self, **kwargs): if excluded_urls is None else parse_excluded_urls(excluded_urls) ), + sem_conv_opt_in_mode=sem_conv_opt_in_mode, ) def _uninstrument(self, **kwargs): _uninstrument() +def _get_span_name(method: str) -> str: + method = sanitize_method(method.strip()) + if method == "_OTHER": + method = "HTTP" + return method + + def _instrument( tracer: Tracer, - duration_histogram: Histogram, - request_size_histogram: Histogram, - response_size_histogram: Histogram, + duration_histogram_old: Histogram, + duration_histogram_new: Histogram, + request_size_histogram_old: Histogram, + request_size_histogram_new: Histogram, + response_size_histogram_old: Histogram, + response_size_histogram_new: Histogram, request_hook: _RequestHookT = None, response_hook: _ResponseHookT = None, url_filter: _UrlFilterT = None, excluded_urls: ExcludeList = None, + sem_conv_opt_in_mode: _HTTPStabilityMode = _HTTPStabilityMode.DEFAULT, ): def instrumented_urlopen(wrapped, instance, args, kwargs): if not is_http_instrumentation_enabled(): @@ -233,11 +302,16 @@ def instrumented_urlopen(wrapped, instance, args, kwargs): headers = _prepare_headers(kwargs) body = _get_url_open_arg("body", args, kwargs) - span_name = method.strip() - span_attributes = { - SpanAttributes.HTTP_METHOD: method, - SpanAttributes.HTTP_URL: url, - } + span_name = _get_span_name(method) + span_attributes = {} + + _set_http_method( + span_attributes, + method, + sanitize_method(method), + sem_conv_opt_in_mode, + ) + _set_http_url(span_attributes, url, sem_conv_opt_in_mode) with tracer.start_as_current_span( span_name, kind=SpanKind.CLIENT, attributes=span_attributes @@ -245,32 +319,43 @@ def instrumented_urlopen(wrapped, instance, args, kwargs): if callable(request_hook): request_hook(span, instance, headers, body) inject(headers) - + # TODO: add error handling to also set exception `error.type` in new semconv with suppress_http_instrumentation(): start_time = default_timer() response = wrapped(*args, **kwargs) - elapsed_time = round((default_timer() - start_time) * 1000) + duration_s = default_timer() - start_time + # set http status code based on semconv + metric_attributes = {} + _set_status_code_attribute( + span, response.status, metric_attributes, sem_conv_opt_in_mode + ) - _apply_response(span, response) if callable(response_hook): response_hook(span, instance, response) request_size = _get_body_size(body) response_size = int(response.headers.get("Content-Length", 0)) - metric_attributes = _create_metric_attributes( - instance, response, method + _set_metric_attributes( + metric_attributes, + instance, + response, + method, + sem_conv_opt_in_mode, ) - duration_histogram.record( - elapsed_time, attributes=metric_attributes - ) - if request_size is not None: - request_size_histogram.record( - request_size, attributes=metric_attributes - ) - response_size_histogram.record( - response_size, attributes=metric_attributes + _record_metrics( + metric_attributes, + duration_histogram_old, + duration_histogram_new, + request_size_histogram_old, + request_size_histogram_new, + response_size_histogram_old, + response_size_histogram_new, + duration_s, + request_size, + response_size, + sem_conv_opt_in_mode, ) return response @@ -342,35 +427,133 @@ def _prepare_headers(urlopen_kwargs: typing.Dict) -> typing.Dict: return headers -def _apply_response(span: Span, response: urllib3.response.HTTPResponse): - if not span.is_recording(): - return - - span.set_attribute(SpanAttributes.HTTP_STATUS_CODE, response.status) - span.set_status(Status(http_status_to_status_code(response.status))) +def _set_status_code_attribute( + span: Span, + status_code: int, + metric_attributes: dict = None, + sem_conv_opt_in_mode: _HTTPStabilityMode = _HTTPStabilityMode.DEFAULT, +) -> None: + + status_code_str = str(status_code) + try: + status_code = int(status_code) + except ValueError: + status_code = -1 + + if metric_attributes is None: + metric_attributes = {} + + _set_status( + span, + metric_attributes, + status_code, + status_code_str, + server_span=False, + sem_conv_opt_in_mode=sem_conv_opt_in_mode, + ) -def _create_metric_attributes( +def _set_metric_attributes( + metric_attributes: dict, instance: urllib3.connectionpool.HTTPConnectionPool, response: urllib3.response.HTTPResponse, method: str, -) -> dict: - metric_attributes = { - SpanAttributes.HTTP_METHOD: method, - SpanAttributes.HTTP_HOST: instance.host, - SpanAttributes.HTTP_SCHEME: instance.scheme, - SpanAttributes.HTTP_STATUS_CODE: response.status, - SpanAttributes.NET_PEER_NAME: instance.host, - SpanAttributes.NET_PEER_PORT: instance.port, - } + sem_conv_opt_in_mode: _HTTPStabilityMode = _HTTPStabilityMode.DEFAULT, +) -> None: + + _set_http_host(metric_attributes, instance.host, sem_conv_opt_in_mode) + _set_http_scheme(metric_attributes, instance.scheme, sem_conv_opt_in_mode) + _set_http_method( + metric_attributes, + method, + sanitize_method(method), + sem_conv_opt_in_mode, + ) + _set_http_net_peer_name_client( + metric_attributes, instance.host, sem_conv_opt_in_mode + ) + _set_http_peer_port_client( + metric_attributes, instance.port, sem_conv_opt_in_mode + ) version = getattr(response, "version") if version: - metric_attributes[SpanAttributes.HTTP_FLAVOR] = ( - "1.1" if version == 11 else "1.0" + http_version = "1.1" if version == 11 else "1.0" + _set_http_network_protocol_version( + metric_attributes, http_version, sem_conv_opt_in_mode ) - return metric_attributes + +def _filter_attributes_semconv( + metric_attributes, + sem_conv_opt_in_mode: _HTTPStabilityMode = _HTTPStabilityMode.DEFAULT, +): + duration_attrs_old = None + duration_attrs_new = None + if _report_old(sem_conv_opt_in_mode): + duration_attrs_old = _filter_semconv_duration_attrs( + metric_attributes, + _client_duration_attrs_old, + _client_duration_attrs_new, + _HTTPStabilityMode.DEFAULT, + ) + if _report_new(sem_conv_opt_in_mode): + duration_attrs_new = _filter_semconv_duration_attrs( + metric_attributes, + _client_duration_attrs_old, + _client_duration_attrs_new, + _HTTPStabilityMode.HTTP, + ) + + return (duration_attrs_old, duration_attrs_new) + + +def _record_metrics( + metric_attributes: dict, + duration_histogram_old: Histogram, + duration_histogram_new: Histogram, + request_size_histogram_old: Histogram, + request_size_histogram_new: Histogram, + response_size_histogram_old: Histogram, + response_size_histogram_new: Histogram, + duration_s: float, + request_size: typing.Optional[int], + response_size: int, + sem_conv_opt_in_mode: _HTTPStabilityMode = _HTTPStabilityMode.DEFAULT, +): + attrs_old, attrs_new = _filter_attributes_semconv( + metric_attributes, sem_conv_opt_in_mode + ) + if duration_histogram_old: + # Default behavior is to record the duration in milliseconds + duration_histogram_old.record( + max(round(duration_s * 1000), 0), + attributes=attrs_old, + ) + + if duration_histogram_new: + # New semconv record the duration in seconds + duration_histogram_new.record( + duration_s, + attributes=attrs_new, + ) + + if request_size is not None: + if request_size_histogram_old: + request_size_histogram_old.record( + request_size, attributes=attrs_old + ) + + if request_size_histogram_new: + request_size_histogram_new.record( + request_size, attributes=attrs_new + ) + + if response_size_histogram_old: + response_size_histogram_old.record(response_size, attributes=attrs_old) + + if response_size_histogram_new: + response_size_histogram_new.record(response_size, attributes=attrs_new) def _uninstrument(): diff --git a/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/package.py b/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/package.py index 9d52db0a1f..568120c44d 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/package.py +++ b/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/package.py @@ -16,3 +16,5 @@ _instruments = ("urllib3 >= 1.0.0, < 3.0.0",) _supports_metrics = True + +_semconv_status = "migration" diff --git a/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py b/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py index 23124ea590..7dc899691f 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py +++ b/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py @@ -16,16 +16,29 @@ from unittest import mock import httpretty +import httpretty.core +import httpretty.http import urllib3 import urllib3.exceptions from opentelemetry import trace +from opentelemetry.instrumentation._semconv import ( + OTEL_SEMCONV_STABILITY_OPT_IN, + _HTTPStabilityMode, + _OpenTelemetrySemanticConventionStability, +) from opentelemetry.instrumentation.urllib3 import URLLib3Instrumentor from opentelemetry.instrumentation.utils import ( suppress_http_instrumentation, suppress_instrumentation, ) from opentelemetry.propagate import get_global_textmap, set_global_textmap +from opentelemetry.semconv.attributes.http_attributes import ( + HTTP_REQUEST_METHOD, + HTTP_REQUEST_METHOD_ORIGINAL, + HTTP_RESPONSE_STATUS_CODE, +) +from opentelemetry.semconv.attributes.url_attributes import URL_FULL from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.test.mock_textmap import MockTextMapPropagator from opentelemetry.test.test_base import TestBase @@ -41,12 +54,23 @@ class TestURLLib3Instrumentor(TestBase): def setUp(self): super().setUp() + test_name = "" + if hasattr(self, "_testMethodName"): + test_name = self._testMethodName + sem_conv_mode = "default" + if "new_semconv" in test_name: + sem_conv_mode = "http" + elif "both_semconv" in test_name: + sem_conv_mode = "http/dup" + self.env_patch = mock.patch.dict( "os.environ", { - "OTEL_PYTHON_URLLIB3_EXCLUDED_URLS": "http://localhost/env_excluded_arg/123,env_excluded_noarg" + "OTEL_PYTHON_URLLIB3_EXCLUDED_URLS": "http://localhost/env_excluded_arg/123,env_excluded_noarg", + OTEL_SEMCONV_STABILITY_OPT_IN: sem_conv_mode, }, ) + _OpenTelemetrySemanticConventionStability._initialized = False self.env_patch.start() self.exclude_patch = mock.patch( @@ -64,6 +88,7 @@ def setUp(self): def tearDown(self): super().tearDown() + self.env_patch.stop() URLLib3Instrumentor().uninstrument() httpretty.disable() @@ -81,46 +106,103 @@ def assert_span(self, exporter=None, num_spans=1): return span_list def assert_success_span( - self, response: urllib3.response.HTTPResponse, url: str + self, + response: urllib3.response.HTTPResponse, + url: str, + sem_conv_opt_in_mode: _HTTPStabilityMode = _HTTPStabilityMode.DEFAULT, ): self.assertEqual(b"Hello!", response.data) span = self.assert_span() self.assertIs(trace.SpanKind.CLIENT, span.kind) self.assertEqual("GET", span.name) - - attributes = { + self.assertEqual( + span.status.status_code, trace.status.StatusCode.UNSET + ) + attr_old = { SpanAttributes.HTTP_METHOD: "GET", SpanAttributes.HTTP_URL: url, SpanAttributes.HTTP_STATUS_CODE: 200, } - self.assertEqual(attributes, span.attributes) - def assert_exception_span(self, url: str): - span = self.assert_span() + attr_new = { + HTTP_REQUEST_METHOD: "GET", + URL_FULL: url, + HTTP_RESPONSE_STATUS_CODE: 200, + } attributes = { + _HTTPStabilityMode.DEFAULT: attr_old, + _HTTPStabilityMode.HTTP: attr_new, + _HTTPStabilityMode.HTTP_DUP: {**attr_new, **attr_old}, + } + self.assertEqual(span.attributes, attributes.get(sem_conv_opt_in_mode)) + + def assert_exception_span( + self, + url: str, + sem_conv_opt_in_mode: _HTTPStabilityMode = _HTTPStabilityMode.DEFAULT, + ): + span = self.assert_span() + + attr_old = { SpanAttributes.HTTP_METHOD: "GET", SpanAttributes.HTTP_URL: url, } - self.assertEqual(attributes, span.attributes) + + attr_new = { + HTTP_REQUEST_METHOD: "GET", + URL_FULL: url, + # TODO: Add `error.type` attribute when supported + } + + attributes = { + _HTTPStabilityMode.DEFAULT: attr_old, + _HTTPStabilityMode.HTTP: attr_new, + _HTTPStabilityMode.HTTP_DUP: {**attr_new, **attr_old}, + } + + self.assertEqual(span.attributes, attributes.get(sem_conv_opt_in_mode)) self.assertEqual( trace.status.StatusCode.ERROR, span.status.status_code ) @staticmethod def perform_request( - url: str, headers: typing.Mapping = None, retries: urllib3.Retry = None + url: str, + headers: typing.Mapping = None, + retries: urllib3.Retry = None, + method: str = "GET", ) -> urllib3.response.HTTPResponse: if retries is None: retries = urllib3.Retry.from_int(0) pool = urllib3.PoolManager() - return pool.request("GET", url, headers=headers, retries=retries) + return pool.request(method, url, headers=headers, retries=retries) def test_basic_http_success(self): response = self.perform_request(self.HTTP_URL) - self.assert_success_span(response, self.HTTP_URL) + self.assert_success_span( + response, + self.HTTP_URL, + sem_conv_opt_in_mode=_HTTPStabilityMode.DEFAULT, + ) + + def test_basic_http_success_new_semconv(self): + response = self.perform_request(self.HTTP_URL) + self.assert_success_span( + response, + self.HTTP_URL, + sem_conv_opt_in_mode=_HTTPStabilityMode.HTTP, + ) + + def test_basic_http_success_both_semconv(self): + response = self.perform_request(self.HTTP_URL) + self.assert_success_span( + response, + self.HTTP_URL, + sem_conv_opt_in_mode=_HTTPStabilityMode.HTTP_DUP, + ) def test_basic_http_success_using_connection_pool(self): pool = urllib3.HTTPConnectionPool("mock") @@ -145,10 +227,32 @@ def test_schema_url(self): self.assertEqual(b"Hello!", response.data) span = self.assert_span() self.assertEqual( - span.instrumentation_info.schema_url, + span.instrumentation_scope.schema_url, "https://opentelemetry.io/schemas/1.11.0", ) + def test_schema_url_new_semconv(self): + pool = urllib3.HTTPSConnectionPool("mock") + response = pool.request("GET", "/status/200") + + self.assertEqual(b"Hello!", response.data) + span = self.assert_span() + self.assertEqual( + span.instrumentation_scope.schema_url, + "https://opentelemetry.io/schemas/v1.21.0", + ) + + def test_schema_url_both_semconv(self): + pool = urllib3.HTTPSConnectionPool("mock") + response = pool.request("GET", "/status/200") + + self.assertEqual(b"Hello!", response.data) + span = self.assert_span() + self.assertEqual( + span.instrumentation_scope.schema_url, + "https://opentelemetry.io/schemas/v1.21.0", + ) + def test_basic_not_found(self): url_404 = "http://mock/status/404" httpretty.register_uri(httpretty.GET, url_404, status=404) @@ -162,6 +266,80 @@ def test_basic_not_found(self): ) self.assertIs(trace.status.StatusCode.ERROR, span.status.status_code) + def test_basic_not_found_new_semconv(self): + url_404 = "http://mock/status/404" + httpretty.register_uri(httpretty.GET, url_404, status=404) + + response = self.perform_request(url_404) + self.assertEqual(404, response.status) + + span = self.assert_span() + self.assertEqual(404, span.attributes.get(HTTP_RESPONSE_STATUS_CODE)) + self.assertIs(trace.status.StatusCode.ERROR, span.status.status_code) + + def test_basic_not_found_both_semconv(self): + url_404 = "http://mock/status/404" + httpretty.register_uri(httpretty.GET, url_404, status=404) + + response = self.perform_request(url_404) + self.assertEqual(404, response.status) + + span = self.assert_span() + self.assertEqual(404, span.attributes.get(HTTP_RESPONSE_STATUS_CODE)) + self.assertEqual( + 404, span.attributes.get(SpanAttributes.HTTP_STATUS_CODE) + ) + self.assertIs(trace.status.StatusCode.ERROR, span.status.status_code) + + @mock.patch("httpretty.http.HttpBaseClass.METHODS", ("NONSTANDARD",)) + def test_nonstandard_http_method(self): + httpretty.register_uri( + "NONSTANDARD", self.HTTP_URL, body="Hello!", status=405 + ) + self.perform_request(self.HTTP_URL, method="NONSTANDARD") + span = self.assert_span() + self.assertEqual("HTTP", span.name) + self.assertEqual( + span.attributes.get(SpanAttributes.HTTP_METHOD), "_OTHER" + ) + self.assertEqual( + span.attributes.get(SpanAttributes.HTTP_STATUS_CODE), 405 + ) + + @mock.patch("httpretty.http.HttpBaseClass.METHODS", ("NONSTANDARD",)) + def test_nonstandard_http_method_new_semconv(self): + httpretty.register_uri( + "NONSTANDARD", self.HTTP_URL, body="Hello!", status=405 + ) + self.perform_request(self.HTTP_URL, method="NONSTANDARD") + span = self.assert_span() + self.assertEqual("HTTP", span.name) + self.assertEqual(span.attributes.get(HTTP_REQUEST_METHOD), "_OTHER") + self.assertEqual( + span.attributes.get(HTTP_REQUEST_METHOD_ORIGINAL), "NONSTANDARD" + ) + self.assertEqual(span.attributes.get(HTTP_RESPONSE_STATUS_CODE), 405) + + @mock.patch("httpretty.http.HttpBaseClass.METHODS", ("NONSTANDARD",)) + def test_nonstandard_http_method_both_semconv(self): + httpretty.register_uri( + "NONSTANDARD", self.HTTP_URL, body="Hello!", status=405 + ) + self.perform_request(self.HTTP_URL, method="NONSTANDARD") + span = self.assert_span() + self.assertEqual("HTTP", span.name) + self.assertEqual( + span.attributes.get(SpanAttributes.HTTP_METHOD), "_OTHER" + ) + self.assertEqual( + span.attributes.get(SpanAttributes.HTTP_STATUS_CODE), 405 + ) + self.assertEqual(span.attributes.get(HTTP_REQUEST_METHOD), "_OTHER") + self.assertEqual( + span.attributes.get(HTTP_REQUEST_METHOD_ORIGINAL), "NONSTANDARD" + ) + self.assertEqual(span.attributes.get(HTTP_RESPONSE_STATUS_CODE), 405) + def test_basic_http_non_default_port(self): url = "http://mock:666/status/200" httpretty.register_uri(httpretty.GET, url, body="Hello!") @@ -287,6 +465,34 @@ def test_request_exception(self, _): self.assert_exception_span(self.HTTP_URL) + @mock.patch( + "urllib3.connectionpool.HTTPConnectionPool._make_request", + side_effect=urllib3.exceptions.ConnectTimeoutError, + ) + def test_request_exception_new_semconv(self, _): + with self.assertRaises(urllib3.exceptions.ConnectTimeoutError): + self.perform_request( + self.HTTP_URL, retries=urllib3.Retry(connect=False) + ) + + self.assert_exception_span( + self.HTTP_URL, sem_conv_opt_in_mode=_HTTPStabilityMode.HTTP + ) + + @mock.patch( + "urllib3.connectionpool.HTTPConnectionPool._make_request", + side_effect=urllib3.exceptions.ConnectTimeoutError, + ) + def test_request_exception_both_semconv(self, _): + with self.assertRaises(urllib3.exceptions.ConnectTimeoutError): + self.perform_request( + self.HTTP_URL, retries=urllib3.Retry(connect=False) + ) + + self.assert_exception_span( + self.HTTP_URL, sem_conv_opt_in_mode=_HTTPStabilityMode.HTTP_DUP + ) + @mock.patch( "urllib3.connectionpool.HTTPConnectionPool._make_request", side_effect=urllib3.exceptions.ProtocolError, diff --git a/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_metrics.py b/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_metrics.py index 787b920d7c..c6e9011351 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_metrics.py +++ b/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_metrics.py @@ -14,13 +14,30 @@ import io from timeit import default_timer +from unittest import mock import httpretty import urllib3 import urllib3.exceptions from urllib3 import encode_multipart_formdata +from opentelemetry.instrumentation._semconv import ( + OTEL_SEMCONV_STABILITY_OPT_IN, + _OpenTelemetrySemanticConventionStability, +) from opentelemetry.instrumentation.urllib3 import URLLib3Instrumentor +from opentelemetry.semconv.attributes.http_attributes import ( + HTTP_REQUEST_METHOD, + HTTP_RESPONSE_STATUS_CODE, +) +from opentelemetry.semconv.attributes.network_attributes import ( + NETWORK_PROTOCOL_VERSION, +) +from opentelemetry.semconv.attributes.server_attributes import ( + SERVER_ADDRESS, + SERVER_PORT, +) +from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.test.httptest import HttpTestBase from opentelemetry.test.test_base import TestBase @@ -30,6 +47,24 @@ class TestURLLib3InstrumentorMetric(HttpTestBase, TestBase): def setUp(self): super().setUp() + + test_name = "" + if hasattr(self, "_testMethodName"): + test_name = self._testMethodName + sem_conv_mode = "default" + if "new_semconv" in test_name: + sem_conv_mode = "http" + elif "both_semconv" in test_name: + sem_conv_mode = "http/dup" + + self.env_patch = mock.patch.dict( + "os.environ", + { + OTEL_SEMCONV_STABILITY_OPT_IN: sem_conv_mode, + }, + ) + _OpenTelemetrySemanticConventionStability._initialized = False + self.env_patch.start() URLLib3Instrumentor().instrument() httpretty.enable(allow_net_connect=False) httpretty.register_uri(httpretty.GET, self.HTTP_URL, body="Hello!") @@ -38,6 +73,7 @@ def setUp(self): def tearDown(self): super().tearDown() + self.env_patch.stop() self.pool.clear() URLLib3Instrumentor().uninstrument() @@ -47,7 +83,149 @@ def tearDown(self): def test_basic_metrics(self): start_time = default_timer() response = self.pool.request("GET", self.HTTP_URL) - client_duration_estimated = (default_timer() - start_time) * 1000 + duration_ms = max(round((default_timer() - start_time) * 1000), 0) + metrics = self.get_sorted_metrics() + + ( + client_duration, + client_request_size, + client_response_size, + ) = metrics + + attrs_old = { + SpanAttributes.HTTP_STATUS_CODE: 200, + SpanAttributes.HTTP_HOST: "mock", + SpanAttributes.NET_PEER_PORT: 80, + SpanAttributes.NET_PEER_NAME: "mock", + SpanAttributes.HTTP_METHOD: "GET", + SpanAttributes.HTTP_FLAVOR: "1.1", + SpanAttributes.HTTP_SCHEME: "http", + } + + self.assertEqual(client_duration.name, "http.client.duration") + self.assert_metric_expected( + client_duration, + [ + self.create_histogram_data_point( + count=1, + sum_data_point=duration_ms, + max_data_point=duration_ms, + min_data_point=duration_ms, + attributes=attrs_old, + ) + ], + est_value_delta=40, + ) + + self.assertEqual(client_request_size.name, "http.client.request.size") + self.assert_metric_expected( + client_request_size, + [ + self.create_histogram_data_point( + count=1, + sum_data_point=0, + max_data_point=0, + min_data_point=0, + attributes=attrs_old, + ) + ], + ) + + expected_size = len(response.data) + self.assertEqual( + client_response_size.name, "http.client.response.size" + ) + self.assert_metric_expected( + client_response_size, + [ + self.create_histogram_data_point( + count=1, + sum_data_point=expected_size, + max_data_point=expected_size, + min_data_point=expected_size, + attributes=attrs_old, + ) + ], + ) + + def test_basic_metrics_new_semconv(self): + start_time = default_timer() + response = self.pool.request("GET", self.HTTP_URL) + duration_s = max(default_timer() - start_time, 0) + + metrics = self.get_sorted_metrics() + ( + client_request_size, + client_duration, + client_response_size, + ) = metrics + + attrs_new = { + NETWORK_PROTOCOL_VERSION: "1.1", + SERVER_ADDRESS: "mock", + SERVER_PORT: 80, + HTTP_REQUEST_METHOD: "GET", + HTTP_RESPONSE_STATUS_CODE: 200, + # TODO: add URL_SCHEME to tests when supported in the implementation + } + + self.assertEqual(client_duration.name, "http.client.request.duration") + self.assert_metric_expected( + client_duration, + [ + self.create_histogram_data_point( + count=1, + sum_data_point=duration_s, + max_data_point=duration_s, + min_data_point=duration_s, + attributes=attrs_new, + ) + ], + est_value_delta=40 / 1000, + ) + + self.assertEqual( + client_request_size.name, "http.client.request.body.size" + ) + self.assert_metric_expected( + client_request_size, + [ + self.create_histogram_data_point( + count=1, + sum_data_point=0, + max_data_point=0, + min_data_point=0, + attributes=attrs_new, + ) + ], + ) + + expected_size = len(response.data) + self.assertEqual( + client_response_size.name, "http.client.response.body.size" + ) + self.assert_metric_expected( + client_response_size, + [ + self.create_histogram_data_point( + count=1, + sum_data_point=expected_size, + max_data_point=expected_size, + min_data_point=expected_size, + attributes=attrs_new, + ) + ], + ) + + @mock.patch("httpretty.http.HttpBaseClass.METHODS", ("NONSTANDARD",)) + def test_basic_metrics_nonstandard_http_method(self): + httpretty.register_uri( + "NONSTANDARD", self.HTTP_URL, body="", status=405 + ) + + start_time = default_timer() + response = self.pool.request("NONSTANDARD", self.HTTP_URL) + duration_ms = max(round((default_timer() - start_time) * 1000), 0) metrics = self.get_sorted_metrics() @@ -57,27 +235,29 @@ def test_basic_metrics(self): client_response_size, ) = metrics + attrs_old = { + SpanAttributes.HTTP_STATUS_CODE: 405, + SpanAttributes.HTTP_HOST: "mock", + SpanAttributes.NET_PEER_PORT: 80, + SpanAttributes.NET_PEER_NAME: "mock", + SpanAttributes.HTTP_METHOD: "_OTHER", + SpanAttributes.HTTP_FLAVOR: "1.1", + SpanAttributes.HTTP_SCHEME: "http", + } + self.assertEqual(client_duration.name, "http.client.duration") self.assert_metric_expected( client_duration, [ self.create_histogram_data_point( count=1, - sum_data_point=client_duration_estimated, - max_data_point=client_duration_estimated, - min_data_point=client_duration_estimated, - attributes={ - "http.flavor": "1.1", - "http.host": "mock", - "http.method": "GET", - "http.scheme": "http", - "http.status_code": 200, - "net.peer.name": "mock", - "net.peer.port": 80, - }, + sum_data_point=duration_ms, + max_data_point=duration_ms, + min_data_point=duration_ms, + attributes=attrs_old, ) ], - est_value_delta=200, + est_value_delta=40, ) self.assertEqual(client_request_size.name, "http.client.request.size") @@ -89,15 +269,7 @@ def test_basic_metrics(self): sum_data_point=0, max_data_point=0, min_data_point=0, - attributes={ - "http.flavor": "1.1", - "http.host": "mock", - "http.method": "GET", - "http.scheme": "http", - "http.status_code": 200, - "net.peer.name": "mock", - "net.peer.port": 80, - }, + attributes=attrs_old, ) ], ) @@ -114,15 +286,82 @@ def test_basic_metrics(self): sum_data_point=expected_size, max_data_point=expected_size, min_data_point=expected_size, - attributes={ - "http.flavor": "1.1", - "http.host": "mock", - "http.method": "GET", - "http.scheme": "http", - "http.status_code": 200, - "net.peer.name": "mock", - "net.peer.port": 80, - }, + attributes=attrs_old, + ) + ], + ) + + @mock.patch("httpretty.http.HttpBaseClass.METHODS", ("NONSTANDARD",)) + def test_basic_metrics_nonstandard_http_method_new_semconv(self): + httpretty.register_uri( + "NONSTANDARD", self.HTTP_URL, body="", status=405 + ) + start_time = default_timer() + response = self.pool.request("NONSTANDARD", self.HTTP_URL) + duration_s = max(default_timer() - start_time, 0) + + metrics = self.get_sorted_metrics() + + ( + client_request_size, + client_duration, + client_response_size, + ) = metrics + + attrs_new = { + NETWORK_PROTOCOL_VERSION: "1.1", + SERVER_ADDRESS: "mock", + SERVER_PORT: 80, + HTTP_REQUEST_METHOD: "_OTHER", + HTTP_RESPONSE_STATUS_CODE: 405, + "error.type": "405", + # TODO: add URL_SCHEME to tests when supported in the implementation + } + + self.assertEqual(client_duration.name, "http.client.request.duration") + self.assert_metric_expected( + client_duration, + [ + self.create_histogram_data_point( + count=1, + sum_data_point=duration_s, + max_data_point=duration_s, + min_data_point=duration_s, + attributes=attrs_new, + ) + ], + est_value_delta=40 / 1000, + ) + + self.assertEqual( + client_request_size.name, "http.client.request.body.size" + ) + self.assert_metric_expected( + client_request_size, + [ + self.create_histogram_data_point( + count=1, + sum_data_point=0, + max_data_point=0, + min_data_point=0, + attributes=attrs_new, + ) + ], + ) + + expected_size = len(response.data) + self.assertEqual( + client_response_size.name, "http.client.response.body.size" + ) + self.assert_metric_expected( + client_response_size, + [ + self.create_histogram_data_point( + count=1, + sum_data_point=expected_size, + max_data_point=expected_size, + min_data_point=expected_size, + attributes=attrs_new, ) ], ) @@ -274,3 +513,5 @@ def test_metric_uninstrument(self): for metric in metrics: for point in list(metric.data.data_points): self.assertEqual(point.count, 1) + # instrument again to avoid warning message on tearDown + URLLib3Instrumentor().instrument() From e799a74bbe62b103bdb5649e4c5849fc977ee35f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Em=C3=ADdio=20Neto?= <9735060+emdneto@users.noreply.github.com> Date: Tue, 23 Jul 2024 20:09:30 -0300 Subject: [PATCH 25/26] fix schema url in instrumentation tests (#2730) * fix schema url in instrumentation tests * change core repo sha for test * Update CORE_REPO_SHA * Try removing cache * Clear cache here as well --------- Co-authored-by: Diego Hurtado --- .github/workflows/instrumentations_0.yml | 4 ++-- .github/workflows/instrumentations_1.yml | 4 ++-- .github/workflows/lint.yml | 2 +- .github/workflows/test.yml | 2 +- .../tests/test_aiohttp_client_integration.py | 4 ++-- .../tests/test_urllib3_integration.py | 4 ++-- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/.github/workflows/instrumentations_0.yml b/.github/workflows/instrumentations_0.yml index 69c07bfacc..382284d204 100644 --- a/.github/workflows/instrumentations_0.yml +++ b/.github/workflows/instrumentations_0.yml @@ -6,7 +6,7 @@ on: - 'release/*' pull_request: env: - CORE_REPO_SHA: 141a6a2e473ef7f0ec4915dfb71e3c0fa595283e + CORE_REPO_SHA: main jobs: instrumentations-0: @@ -114,6 +114,6 @@ jobs: path: | .tox ~/.cache/pip - key: v7-build-tox-cache-${{ env.RUN_MATRIX_COMBINATION }}-${{ hashFiles('tox.ini', 'gen-requirements.txt', 'dev-requirements.txt') }} + key: v7-build-tox-cache-${{ env.RUN_MATRIX_COMBINATION }}-${{ hashFiles('gen-requirements.txt', 'dev-requirements.txt') }} - name: run tox run: tox -f ${{ matrix.python-version }}-${{ matrix.package }} -- -ra diff --git a/.github/workflows/instrumentations_1.yml b/.github/workflows/instrumentations_1.yml index a658796ef8..f3e63116f8 100644 --- a/.github/workflows/instrumentations_1.yml +++ b/.github/workflows/instrumentations_1.yml @@ -6,7 +6,7 @@ on: - 'release/*' pull_request: env: - CORE_REPO_SHA: 141a6a2e473ef7f0ec4915dfb71e3c0fa595283e + CORE_REPO_SHA: main jobs: instrumentations-1: @@ -59,6 +59,6 @@ jobs: path: | .tox ~/.cache/pip - key: v7-build-tox-cache-${{ env.RUN_MATRIX_COMBINATION }}-${{ hashFiles('tox.ini', 'gen-requirements.txt', 'dev-requirements.txt') }} + key: v7-build-tox-cache-${{ env.RUN_MATRIX_COMBINATION }}-${{ hashFiles('gen-requirements.txt', 'dev-requirements.txt') }} - name: run tox run: tox -f ${{ matrix.python-version }}-${{ matrix.package }} -- -ra diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 29cd5cac16..b9f7a41c17 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -6,7 +6,7 @@ on: - 'release/*' pull_request: env: - CORE_REPO_SHA: 141a6a2e473ef7f0ec4915dfb71e3c0fa595283e + CORE_REPO_SHA: main jobs: lint-3_12: diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index ee66efac64..2714942c21 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -6,7 +6,7 @@ on: - 'release/*' pull_request: env: - CORE_REPO_SHA: 141a6a2e473ef7f0ec4915dfb71e3c0fa595283e + CORE_REPO_SHA: main jobs: misc: diff --git a/instrumentation/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py b/instrumentation/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py index 3873a6f094..adf54c295b 100644 --- a/instrumentation/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py +++ b/instrumentation/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py @@ -223,7 +223,7 @@ def test_schema_url_new_semconv(self): span = self.memory_exporter.get_finished_spans()[0] self.assertEqual( span.instrumentation_info.schema_url, - "https://opentelemetry.io/schemas/v1.21.0", + "https://opentelemetry.io/schemas/1.21.0", ) self.memory_exporter.clear() @@ -240,7 +240,7 @@ def test_schema_url_both_semconv(self): span = self.memory_exporter.get_finished_spans()[0] self.assertEqual( span.instrumentation_info.schema_url, - "https://opentelemetry.io/schemas/v1.21.0", + "https://opentelemetry.io/schemas/1.21.0", ) self.memory_exporter.clear() diff --git a/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py b/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py index 7dc899691f..5ad4e40ca3 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py +++ b/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py @@ -239,7 +239,7 @@ def test_schema_url_new_semconv(self): span = self.assert_span() self.assertEqual( span.instrumentation_scope.schema_url, - "https://opentelemetry.io/schemas/v1.21.0", + "https://opentelemetry.io/schemas/1.21.0", ) def test_schema_url_both_semconv(self): @@ -250,7 +250,7 @@ def test_schema_url_both_semconv(self): span = self.assert_span() self.assertEqual( span.instrumentation_scope.schema_url, - "https://opentelemetry.io/schemas/v1.21.0", + "https://opentelemetry.io/schemas/1.21.0", ) def test_basic_not_found(self): From a47810c2a203f3f6345b18b5f1c47de446bf864e Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 24 Jul 2024 14:55:21 -0600 Subject: [PATCH 26/26] Enable global propagator for AWS instrumentation (#2599) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Enable global propagator for AWS instrumentation Fixes #2598 * Add entry point test case * Update instrumentation/opentelemetry-instrumentation-aws-lambda/pyproject.toml Co-authored-by: Riccardo Magliocchetti * Add test for propagator * Fix entry point name * Update instrumentation/opentelemetry-instrumentation-aws-lambda/tests/test_aws_lambda_instrumentation_manual.py Co-authored-by: Emídio Neto <9735060+emdneto@users.noreply.github.com> * Try with ubuntu latest * Try with 24.04 * Fix propagator key * Fix lint * Revert ununtuns --------- Co-authored-by: Riccardo Magliocchetti Co-authored-by: Emídio Neto <9735060+emdneto@users.noreply.github.com> --- CHANGELOG.md | 2 + .../pyproject.toml | 3 + .../instrumentation/aws_lambda/__init__.py | 55 ++----------------- .../tests/test_aws_lambda_instrumentation.py | 13 ----- .../test_aws_lambda_instrumentation_manual.py | 43 ++++++++++----- .../pyproject.toml | 2 +- .../opentelemetry/propagators/aws/__init__.py | 7 ++- .../propagators/aws/aws_xray_propagator.py | 4 +- .../tests/test_aws_xray_lambda_propagator.py | 23 ++++++-- 9 files changed, 66 insertions(+), 86 deletions(-) delete mode 100644 instrumentation/opentelemetry-instrumentation-aws-lambda/tests/test_aws_lambda_instrumentation.py diff --git a/CHANGELOG.md b/CHANGELOG.md index bc400865a2..c759549027 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `opentelemetry-instrumentation-flask` Add `http.route` and `http.target` to metric attributes ([#2621](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2621)) +- `opentelemetry-instrumentation-aws-lambda` Enable global propagator for AWS instrumentation + ([#2708](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2708)) - `opentelemetry-instrumentation-sklearn` Deprecated the sklearn instrumentation ([#2708](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2708)) - `opentelemetry-instrumentation-pyramid` Record exceptions raised when serving a request diff --git a/instrumentation/opentelemetry-instrumentation-aws-lambda/pyproject.toml b/instrumentation/opentelemetry-instrumentation-aws-lambda/pyproject.toml index cbed1edb9e..c10df60c48 100644 --- a/instrumentation/opentelemetry-instrumentation-aws-lambda/pyproject.toml +++ b/instrumentation/opentelemetry-instrumentation-aws-lambda/pyproject.toml @@ -33,6 +33,9 @@ dependencies = [ [project.optional-dependencies] instruments = [] +[project.entry-points.opentelemetry_instrumentor] +aws-lambda = "opentelemetry.instrumentation.aws_lambda:AwsLambdaInstrumentor" + [project.urls] Homepage = "https://github.com/open-telemetry/opentelemetry-python-contrib/tree/main/instrumentation/opentelemetry-instrumentation-aws-lambda" diff --git a/instrumentation/opentelemetry-instrumentation-aws-lambda/src/opentelemetry/instrumentation/aws_lambda/__init__.py b/instrumentation/opentelemetry-instrumentation-aws-lambda/src/opentelemetry/instrumentation/aws_lambda/__init__.py index 4acf4dea90..c320c12bde 100644 --- a/instrumentation/opentelemetry-instrumentation-aws-lambda/src/opentelemetry/instrumentation/aws_lambda/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-aws-lambda/src/opentelemetry/instrumentation/aws_lambda/__init__.py @@ -83,10 +83,6 @@ def custom_event_context_extractor(lambda_event): from opentelemetry.instrumentation.utils import unwrap from opentelemetry.metrics import MeterProvider, get_meter_provider from opentelemetry.propagate import get_global_textmap -from opentelemetry.propagators.aws.aws_xray_propagator import ( - TRACE_HEADER_KEY, - AwsXRayPropagator, -) from opentelemetry.semconv.resource import ResourceAttributes from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.trace import ( @@ -96,7 +92,6 @@ def custom_event_context_extractor(lambda_event): get_tracer, get_tracer_provider, ) -from opentelemetry.trace.propagation import get_current_span from opentelemetry.trace.status import Status, StatusCode logger = logging.getLogger(__name__) @@ -107,9 +102,6 @@ def custom_event_context_extractor(lambda_event): OTEL_INSTRUMENTATION_AWS_LAMBDA_FLUSH_TIMEOUT = ( "OTEL_INSTRUMENTATION_AWS_LAMBDA_FLUSH_TIMEOUT" ) -OTEL_LAMBDA_DISABLE_AWS_CONTEXT_PROPAGATION = ( - "OTEL_LAMBDA_DISABLE_AWS_CONTEXT_PROPAGATION" -) def _default_event_context_extractor(lambda_event: Any) -> Context: @@ -145,7 +137,6 @@ def _default_event_context_extractor(lambda_event: Any) -> Context: def _determine_parent_context( lambda_event: Any, event_context_extractor: Callable[[Any], Context], - disable_aws_context_propagation: bool = False, ) -> Context: """Determine the parent context for the current Lambda invocation. @@ -159,36 +150,14 @@ def _determine_parent_context( Event as input and extracts an OTel Context from it. By default, the context is extracted from the HTTP headers of an API Gateway request. - disable_aws_context_propagation: By default, this instrumentation - will try to read the context from the `_X_AMZN_TRACE_ID` environment - variable set by Lambda, set this to `True` to disable this behavior. Returns: A Context with configuration found in the carrier. """ - parent_context = None - - if not disable_aws_context_propagation: - xray_env_var = os.environ.get(_X_AMZN_TRACE_ID) - if xray_env_var: - parent_context = AwsXRayPropagator().extract( - {TRACE_HEADER_KEY: xray_env_var} - ) - - if ( - parent_context - and get_current_span(parent_context) - .get_span_context() - .trace_flags.sampled - ): - return parent_context + if event_context_extractor is None: + return _default_event_context_extractor(lambda_event) - if event_context_extractor: - parent_context = event_context_extractor(lambda_event) - else: - parent_context = _default_event_context_extractor(lambda_event) - - return parent_context + return event_context_extractor(lambda_event) def _set_api_gateway_v1_proxy_attributes( @@ -286,14 +255,15 @@ def _instrument( flush_timeout, event_context_extractor: Callable[[Any], Context], tracer_provider: TracerProvider = None, - disable_aws_context_propagation: bool = False, meter_provider: MeterProvider = None, ): + # pylint: disable=too-many-locals # pylint: disable=too-many-statements def _instrumented_lambda_handler_call( # noqa pylint: disable=too-many-branches call_wrapped, instance, args, kwargs ): + orig_handler_name = ".".join( [wrapped_module_name, wrapped_function_name] ) @@ -303,7 +273,6 @@ def _instrumented_lambda_handler_call( # noqa pylint: disable=too-many-branches parent_context = _determine_parent_context( lambda_event, event_context_extractor, - disable_aws_context_propagation, ) try: @@ -451,9 +420,6 @@ def _instrument(self, **kwargs): Event as input and extracts an OTel Context from it. By default, the context is extracted from the HTTP headers of an API Gateway request. - ``disable_aws_context_propagation``: By default, this instrumentation - will try to read the context from the `_X_AMZN_TRACE_ID` environment - variable set by Lambda, set this to `True` to disable this behavior. """ lambda_handler = os.environ.get(ORIG_HANDLER, os.environ.get(_HANDLER)) # pylint: disable=attribute-defined-outside-init @@ -475,16 +441,6 @@ def _instrument(self, **kwargs): flush_timeout_env, ) - disable_aws_context_propagation = kwargs.get( - "disable_aws_context_propagation", False - ) or os.getenv( - OTEL_LAMBDA_DISABLE_AWS_CONTEXT_PROPAGATION, "False" - ).strip().lower() in ( - "true", - "1", - "t", - ) - _instrument( self._wrapped_module_name, self._wrapped_function_name, @@ -493,7 +449,6 @@ def _instrument(self, **kwargs): "event_context_extractor", _default_event_context_extractor ), tracer_provider=kwargs.get("tracer_provider"), - disable_aws_context_propagation=disable_aws_context_propagation, meter_provider=kwargs.get("meter_provider"), ) diff --git a/instrumentation/opentelemetry-instrumentation-aws-lambda/tests/test_aws_lambda_instrumentation.py b/instrumentation/opentelemetry-instrumentation-aws-lambda/tests/test_aws_lambda_instrumentation.py deleted file mode 100644 index b0a6f42841..0000000000 --- a/instrumentation/opentelemetry-instrumentation-aws-lambda/tests/test_aws_lambda_instrumentation.py +++ /dev/null @@ -1,13 +0,0 @@ -# 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. diff --git a/instrumentation/opentelemetry-instrumentation-aws-lambda/tests/test_aws_lambda_instrumentation_manual.py b/instrumentation/opentelemetry-instrumentation-aws-lambda/tests/test_aws_lambda_instrumentation_manual.py index ecce9ea12c..9f25524e43 100644 --- a/instrumentation/opentelemetry-instrumentation-aws-lambda/tests/test_aws_lambda_instrumentation_manual.py +++ b/instrumentation/opentelemetry-instrumentation-aws-lambda/tests/test_aws_lambda_instrumentation_manual.py @@ -11,18 +11,19 @@ # 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. + import os from dataclasses import dataclass -from importlib import import_module +from importlib import import_module, reload from typing import Any, Callable, Dict from unittest import mock +from opentelemetry import propagate from opentelemetry.environment_variables import OTEL_PROPAGATORS from opentelemetry.instrumentation.aws_lambda import ( _HANDLER, _X_AMZN_TRACE_ID, OTEL_INSTRUMENTATION_AWS_LAMBDA_FLUSH_TIMEOUT, - OTEL_LAMBDA_DISABLE_AWS_CONTEXT_PROPAGATION, AwsLambdaInstrumentor, ) from opentelemetry.propagate import get_global_textmap @@ -37,6 +38,7 @@ from opentelemetry.trace.propagation.tracecontext import ( TraceContextTextMapPropagator, ) +from opentelemetry.util._importlib_metadata import entry_points from .mocks.api_gateway_http_api_event import ( MOCK_LAMBDA_API_GATEWAY_HTTP_API_EVENT, @@ -56,6 +58,7 @@ def __init__(self, aws_request_id, invoked_function_arn): ) MOCK_XRAY_TRACE_ID = 0x5FB7331105E8BB83207FA31D4D9CDB4C + MOCK_XRAY_TRACE_ID_STR = f"{MOCK_XRAY_TRACE_ID:x}" MOCK_XRAY_PARENT_SPAN_ID = 0x3328B8445A6DBAD2 MOCK_XRAY_TRACE_CONTEXT_COMMON = f"Root={TRACE_ID_VERSION}-{MOCK_XRAY_TRACE_ID_STR[:TRACE_ID_FIRST_PART_LENGTH]}-{MOCK_XRAY_TRACE_ID_STR[TRACE_ID_FIRST_PART_LENGTH:]};Parent={MOCK_XRAY_PARENT_SPAN_ID:x}" @@ -81,6 +84,7 @@ def mock_execute_lambda(event=None): """Mocks the AWS Lambda execution. NOTE: We don't use `moto`'s `mock_lambda` because we are not instrumenting + calls to AWS Lambda using the AWS SDK. Instead, we are instrumenting AWS Lambda itself. @@ -122,10 +126,13 @@ def test_active_tracing(self): { **os.environ, # Using Active tracing + OTEL_PROPAGATORS: "xray-lambda", _X_AMZN_TRACE_ID: MOCK_XRAY_TRACE_CONTEXT_SAMPLED, }, ) + test_env_patch.start() + reload(propagate) AwsLambdaInstrumentor().instrument() @@ -173,8 +180,7 @@ class TestCase: xray_traceid: str expected_state_value: str = None expected_trace_state_len: int = 0 - disable_aws_context_propagation: bool = False - disable_aws_context_propagation_envvar: str = "" + propagators: str = "tracecontext" def custom_event_context_extractor(lambda_event): return get_global_textmap().extract(lambda_event["foo"]["headers"]) @@ -226,9 +232,10 @@ def custom_event_context_extractor(lambda_event): expected_traceid=MOCK_XRAY_TRACE_ID, expected_parentid=MOCK_XRAY_PARENT_SPAN_ID, xray_traceid=MOCK_XRAY_TRACE_CONTEXT_SAMPLED, + propagators="xray-lambda", ), TestCase( - name="custom_extractor_sampled_xray_disable_aws_propagation", + name="custom_extractor_sampled_xray", custom_extractor=custom_event_context_extractor, context={ "foo": { @@ -238,7 +245,6 @@ def custom_event_context_extractor(lambda_event): } } }, - disable_aws_context_propagation=True, expected_traceid=MOCK_W3C_TRACE_ID, expected_parentid=MOCK_W3C_PARENT_SPAN_ID, expected_trace_state_len=3, @@ -246,7 +252,7 @@ def custom_event_context_extractor(lambda_event): xray_traceid=MOCK_XRAY_TRACE_CONTEXT_SAMPLED, ), TestCase( - name="no_custom_extractor_xray_disable_aws_propagation_via_env_var", + name="no_custom_extractor_xray", custom_extractor=None, context={ "headers": { @@ -254,8 +260,6 @@ def custom_event_context_extractor(lambda_event): TraceContextTextMapPropagator._TRACESTATE_HEADER_NAME: f"{MOCK_W3C_TRACE_STATE_KEY}={MOCK_W3C_TRACE_STATE_VALUE},foo=1,bar=2", } }, - disable_aws_context_propagation=False, - disable_aws_context_propagation_envvar="true", expected_traceid=MOCK_W3C_TRACE_ID, expected_parentid=MOCK_W3C_PARENT_SPAN_ID, expected_trace_state_len=3, @@ -264,21 +268,21 @@ def custom_event_context_extractor(lambda_event): ), ] for test in tests: + test_env_patch = mock.patch.dict( "os.environ", { **os.environ, # NOT Active Tracing _X_AMZN_TRACE_ID: test.xray_traceid, - OTEL_LAMBDA_DISABLE_AWS_CONTEXT_PROPAGATION: test.disable_aws_context_propagation_envvar, - # NOT using the X-Ray Propagator - OTEL_PROPAGATORS: "tracecontext", + OTEL_PROPAGATORS: test.propagators, }, ) test_env_patch.start() + reload(propagate) + AwsLambdaInstrumentor().instrument( event_context_extractor=test.custom_extractor, - disable_aws_context_propagation=test.disable_aws_context_propagation, ) mock_execute_lambda(test.context) spans = self.memory_exporter.get_finished_spans() @@ -374,6 +378,7 @@ def test_lambda_handles_invalid_event_source(self): }, ) test_env_patch.start() + reload(propagate) AwsLambdaInstrumentor().instrument() @@ -513,3 +518,15 @@ def test_no_op_tracer_provider(self): spans = self.memory_exporter.get_finished_spans() assert spans is not None self.assertEqual(len(spans), 0) + + def test_load_entry_point(self): + self.assertIs( + next( + iter( + entry_points( + group="opentelemetry_instrumentor", name="aws-lambda" + ) + ) + ).load(), + AwsLambdaInstrumentor, + ) diff --git a/propagator/opentelemetry-propagator-aws-xray/pyproject.toml b/propagator/opentelemetry-propagator-aws-xray/pyproject.toml index 4a3e22269a..546c0790a2 100644 --- a/propagator/opentelemetry-propagator-aws-xray/pyproject.toml +++ b/propagator/opentelemetry-propagator-aws-xray/pyproject.toml @@ -30,7 +30,7 @@ dependencies = [ [project.entry-points.opentelemetry_propagator] xray = "opentelemetry.propagators.aws:AwsXRayPropagator" -xray_lambda = "opentelemetry.propagators.aws:AwsXRayLambdaPropagator" +xray-lambda = "opentelemetry.propagators.aws:AwsXRayLambdaPropagator" [project.urls] Homepage = "https://github.com/open-telemetry/opentelemetry-python-contrib/tree/main/propagator/opentelemetry-propagator-aws-xray" diff --git a/propagator/opentelemetry-propagator-aws-xray/src/opentelemetry/propagators/aws/__init__.py b/propagator/opentelemetry-propagator-aws-xray/src/opentelemetry/propagators/aws/__init__.py index f28f1c8b15..5520086559 100644 --- a/propagator/opentelemetry-propagator-aws-xray/src/opentelemetry/propagators/aws/__init__.py +++ b/propagator/opentelemetry-propagator-aws-xray/src/opentelemetry/propagators/aws/__init__.py @@ -12,6 +12,9 @@ # See the License for the specific language governing permissions and # limitations under the License. -from opentelemetry.propagators.aws.aws_xray_propagator import AwsXRayPropagator +from opentelemetry.propagators.aws.aws_xray_propagator import ( + AwsXRayLambdaPropagator, + AwsXRayPropagator, +) -__all__ = ["AwsXRayPropagator"] +__all__ = ["AwsXRayPropagator", "AwsXRayLambdaPropagator"] diff --git a/propagator/opentelemetry-propagator-aws-xray/src/opentelemetry/propagators/aws/aws_xray_propagator.py b/propagator/opentelemetry-propagator-aws-xray/src/opentelemetry/propagators/aws/aws_xray_propagator.py index 4966218211..295a5def9b 100644 --- a/propagator/opentelemetry-propagator-aws-xray/src/opentelemetry/propagators/aws/aws_xray_propagator.py +++ b/propagator/opentelemetry-propagator-aws-xray/src/opentelemetry/propagators/aws/aws_xray_propagator.py @@ -328,9 +328,9 @@ def fields(self): return {TRACE_HEADER_KEY} -class AwsXrayLambdaPropagator(AwsXRayPropagator): +class AwsXRayLambdaPropagator(AwsXRayPropagator): """Implementation of the AWS X-Ray Trace Header propagation protocol but - with special handling for Lambda's ``_X_AMZN_TRACE_ID` environment + with special handling for Lambda's ``_X_AMZN_TRACE_ID`` environment variable. """ diff --git a/propagator/opentelemetry-propagator-aws-xray/tests/test_aws_xray_lambda_propagator.py b/propagator/opentelemetry-propagator-aws-xray/tests/test_aws_xray_lambda_propagator.py index a0432d1457..2d8937e1b3 100644 --- a/propagator/opentelemetry-propagator-aws-xray/tests/test_aws_xray_lambda_propagator.py +++ b/propagator/opentelemetry-propagator-aws-xray/tests/test_aws_xray_lambda_propagator.py @@ -21,7 +21,7 @@ from opentelemetry.context import get_current from opentelemetry.propagators.aws.aws_xray_propagator import ( TRACE_HEADER_KEY, - AwsXrayLambdaPropagator, + AwsXRayLambdaPropagator, ) from opentelemetry.propagators.textmap import DefaultGetter from opentelemetry.sdk.trace import ReadableSpan @@ -33,6 +33,7 @@ get_current_span, use_span, ) +from opentelemetry.util._importlib_metadata import entry_points class AwsXRayLambdaPropagatorTest(TestCase): @@ -40,7 +41,7 @@ class AwsXRayLambdaPropagatorTest(TestCase): def test_extract_no_environment_variable(self): actual_context = get_current_span( - AwsXrayLambdaPropagator().extract( + AwsXRayLambdaPropagator().extract( {}, context=get_current(), getter=DefaultGetter() ) ).get_span_context() @@ -57,7 +58,7 @@ def test_extract_no_environment_variable_valid_context(self): with use_span(NonRecordingSpan(SpanContext(1, 2, False))): actual_context = get_current_span( - AwsXrayLambdaPropagator().extract( + AwsXRayLambdaPropagator().extract( {}, context=get_current(), getter=DefaultGetter() ) ).get_span_context() @@ -83,7 +84,7 @@ def test_extract_no_environment_variable_valid_context(self): def test_extract_from_environment_variable(self): actual_context = get_current_span( - AwsXrayLambdaPropagator().extract( + AwsXRayLambdaPropagator().extract( {}, context=get_current(), getter=DefaultGetter() ) ).get_span_context() @@ -108,7 +109,7 @@ def test_extract_from_environment_variable(self): ) def test_add_link_from_environment_variable(self): - propagator = AwsXrayLambdaPropagator() + propagator = AwsXRayLambdaPropagator() default_getter = DefaultGetter() @@ -162,3 +163,15 @@ def test_add_link_from_environment_variable(self): self.assertEqual( span_link_context.trace_state, TraceState.get_default() ) + + def test_load_entry_point(self): + self.assertIs( + next( + iter( + entry_points( + group="opentelemetry_propagator", name="xray-lambda" + ) + ) + ).load(), + AwsXRayLambdaPropagator, + )