diff --git a/.github/component_owners.yml b/.github/component_owners.yml index efd15a6775..b092d5d2c3 100644 --- a/.github/component_owners.yml +++ b/.github/component_owners.yml @@ -35,6 +35,7 @@ components: sdk-extension/opentelemetry-sdk-extension-aws: - NathanielRN - Kausik-A + - srprash instrumentation/opentelemetry-instrumentation-tortoiseorm: - tonybaloney diff --git a/.github/workflows/instrumentations_0.yml b/.github/workflows/instrumentations_0.yml index 972a33532c..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: f31903a03721ce90c338be33131222d4cba37325 + CORE_REPO_SHA: main jobs: instrumentations-0: diff --git a/.github/workflows/instrumentations_1.yml b/.github/workflows/instrumentations_1.yml index 1698a49be8..40689bdc13 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: f31903a03721ce90c338be33131222d4cba37325 + CORE_REPO_SHA: main jobs: instrumentations-1: @@ -39,6 +39,7 @@ jobs: - "resource-detector-container" - "util-http" - "fastapi-slim" + - "processor-baggage" os: [ubuntu-20.04] exclude: - python-version: pypy3 diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 0e2dc7110d..b9f7a41c17 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -6,7 +6,7 @@ on: - 'release/*' pull_request: env: - CORE_REPO_SHA: f31903a03721ce90c338be33131222d4cba37325 + CORE_REPO_SHA: main jobs: lint-3_12: diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index d749788204..2714942c21 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -6,7 +6,7 @@ on: - 'release/*' pull_request: env: - CORE_REPO_SHA: f31903a03721ce90c338be33131222d4cba37325 + CORE_REPO_SHA: main jobs: misc: diff --git a/CHANGELOG.md b/CHANGELOG.md index b339f8a21e..1b09c75064 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +## Fixed + +- `opentelemetry-instrumentation-aws-lambda` Avoid exception when a handler is not present. + ([#2750](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2750)) +- `opentelemetry-instrumentation-django` Fix regression - `http.target` re-added back to old semconv duration metrics + ([#2746](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2746)) +- `opentelemetry-instrumentation-grpc` Fixes the issue with the gRPC instrumentation not working with the 1.63.0 and higher version of gRPC + ([#2483](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2484)) + ## Version 1.26.0/0.47b0 (2024-07-23) ### Added @@ -47,6 +56,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#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)) +- `opentelemetry-instrumentation-urllib` Implement new semantic convention opt-in migration + ([#2736](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2736)) ### Breaking changes @@ -62,11 +73,14 @@ 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)) +- Populate `{method}` as `HTTP` on `_OTHER` methods from scope for `urllib` instrumentation + ([#2736](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2736)) - `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)) +- `opentelemetry-instrumentation-urllib3` improve request_hook, replacing `headers` and `body` parameters with a single `request_info: RequestInfo` parameter that now contains the `method` and `url` ([#2711](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2711)) ### Fixed @@ -98,7 +112,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `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) ### Breaking changes diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8ab75ce1c6..302e2e7481 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -250,7 +250,8 @@ The continuous integration overrides that environment variable with as per the c Below is a checklist of things to be mindful of when implementing a new instrumentation or working on a specific instrumentation. It is one of our goals as a community to keep the implementation specific details of instrumentations as similar across the board as possible for ease of testing and feature parity. It is also good to abstract as much common functionality as possible. - Follow semantic conventions - - The instrumentation should follow the semantic conventions defined [here](https://github.com/open-telemetry/opentelemetry-specification/tree/main/specification/semantic-conventions.md) + - The instrumentation should follow the semantic conventions defined [here](https://github.com/open-telemetry/semantic-conventions/tree/main/docs) +- Contains a name that is not already claimed in [Pypi](https://pypi.org/). Contact a maintainer, bring the issue up in the weekly Python SIG or create a ticket in Pypi if a desired name has already been taken. - Extends from [BaseInstrumentor](https://github.com/open-telemetry/opentelemetry-python-contrib/blob/2518a4ac07cb62ad6587dd8f6cbb5f8663a7e179/opentelemetry-instrumentation/src/opentelemetry/instrumentation/instrumentor.py#L35) - Supports auto-instrumentation - Add an entry point (ex. ) diff --git a/RELEASING.md b/RELEASING.md index 256674d1b8..5a359159fb 100644 --- a/RELEASING.md +++ b/RELEASING.md @@ -90,6 +90,10 @@ . If the build has not run automatically, it can be manually trigger via the readthedocs interface. +## Releasing dev version of new packages to claim namespace + +When a contribution introduces a new package, in order to mitigate name-squatting incidents, release the current development version of the new package under the `opentelemetry` user to simply claim the namespace. This should be done shortly after the PR that introduced this package has been merged into `main`. + ## Troubleshooting ### Publish failed diff --git a/instrumentation/README.md b/instrumentation/README.md index 555e0bcd2a..20438e0a12 100644 --- a/instrumentation/README.md +++ b/instrumentation/README.md @@ -45,6 +45,6 @@ | [opentelemetry-instrumentation-threading](./opentelemetry-instrumentation-threading) | threading | No | experimental | [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-urllib](./opentelemetry-instrumentation-urllib) | urllib | Yes | migration | [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-asyncio/pyproject.toml b/instrumentation/opentelemetry-instrumentation-asyncio/pyproject.toml index bac1400a38..cda5403ef5 100644 --- a/instrumentation/opentelemetry-instrumentation-asyncio/pyproject.toml +++ b/instrumentation/opentelemetry-instrumentation-asyncio/pyproject.toml @@ -28,7 +28,6 @@ dependencies = [ "opentelemetry-api ~= 1.14", "opentelemetry-instrumentation == 0.48b0.dev", "opentelemetry-semantic-conventions == 0.48b0.dev", - "opentelemetry-test-utils == 0.48b0.dev", "wrapt >= 1.0.0, < 2.0.0", ] 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 c320c12bde..883296d85b 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 @@ -410,7 +410,7 @@ def _instrument(self, **kwargs): """Instruments Lambda Handlers on AWS Lambda. See more: - https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/instrumentation/aws-lambda.md#instrumenting-aws-lambda + https://github.com/open-telemetry/semantic-conventions/blob/main/docs/faas/aws-lambda.md Args: **kwargs: Optional arguments @@ -422,6 +422,14 @@ def _instrument(self, **kwargs): request. """ lambda_handler = os.environ.get(ORIG_HANDLER, os.environ.get(_HANDLER)) + if not lambda_handler: + logger.warning( + ( + "Could not find the ORIG_HANDLER or _HANDLER in the environment variables. ", + "This instrumentation requires the OpenTelemetry Lambda extension installed.", + ) + ) + return # pylint: disable=attribute-defined-outside-init ( self._wrapped_module_name, 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 9f25524e43..00940547ea 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 @@ -495,6 +495,20 @@ def test_lambda_handles_handler_exception_with_api_gateway_proxy_event( exc_env_patch.stop() + def test_lambda_handles_should_do_nothing_when_environment_variables_not_present( + self, + ): + exc_env_patch = mock.patch.dict( + "os.environ", + {_HANDLER: ""}, + ) + exc_env_patch.start() + AwsLambdaInstrumentor().instrument() + + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 0) + exc_env_patch.stop() + def test_uninstrument(self): AwsLambdaInstrumentor().instrument() 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 4530a16506..667d6f1091 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 @@ -333,6 +333,7 @@ def process_exception(self, request, exception): # pylint: disable=too-many-branches # pylint: disable=too-many-locals + # pylint: disable=too-many-statements def process_response(self, request, response): if self._excluded_urls.url_disabled(request.build_absolute_uri("?")): return response @@ -426,6 +427,10 @@ def process_response(self, request, response): duration_attrs_old = _parse_duration_attrs( duration_attrs, _HTTPStabilityMode.DEFAULT ) + # http.target to be included in old semantic conventions + target = duration_attrs.get(SpanAttributes.HTTP_TARGET) + if target: + duration_attrs_old[SpanAttributes.HTTP_TARGET] = target self._duration_histogram_old.record( max(round(duration_s * 1000), 0), duration_attrs_old ) diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py index 1b9a904ce1..85ebbd747f 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py @@ -28,10 +28,6 @@ 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, @@ -48,24 +44,6 @@ ) 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 ( SpanKind, @@ -197,18 +175,18 @@ def test_templated_route_get(self): ) 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["http.method"], "GET") self.assertEqual( - span.attributes[SpanAttributes.HTTP_URL], + span.attributes["http.url"], "http://testserver/route/2020/template/", ) if DJANGO_2_2: self.assertEqual( - span.attributes[SpanAttributes.HTTP_ROUTE], + span.attributes["http.route"], "^route/(?P[0-9]{4})/template/$", ) - self.assertEqual(span.attributes[SpanAttributes.HTTP_SCHEME], "http") - self.assertEqual(span.attributes[SpanAttributes.HTTP_STATUS_CODE], 200) + self.assertEqual(span.attributes["http.scheme"], "http") + self.assertEqual(span.attributes["http.status_code"], 200) def test_traced_get(self): Client().get("/traced/") @@ -221,17 +199,15 @@ def test_traced_get(self): 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["http.method"], "GET") self.assertEqual( - span.attributes[SpanAttributes.HTTP_URL], + span.attributes["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.route"], "^traced/") + self.assertEqual(span.attributes["http.scheme"], "http") + self.assertEqual(span.attributes["http.status_code"], 200) def test_traced_get_new_semconv(self): Client().get("/traced/") @@ -244,14 +220,14 @@ def test_traced_get_new_semconv(self): 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") + 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) + 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/") @@ -264,25 +240,23 @@ def test_traced_get_both_semconv(self): 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["http.method"], "GET") self.assertEqual( - span.attributes[SpanAttributes.HTTP_URL], + span.attributes["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") + self.assertEqual(span.attributes["http.route"], "^traced/") + self.assertEqual(span.attributes["http.scheme"], "http") + self.assertEqual(span.attributes["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) + self.assertEqual(span.attributes["http.route"], "^traced/") + self.assertEqual(span.attributes["http.response.status_code"], 200) def test_not_recording(self): mock_tracer = Mock() @@ -318,17 +292,15 @@ def test_traced_post(self): 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["http.method"], "POST") self.assertEqual( - span.attributes[SpanAttributes.HTTP_URL], + span.attributes["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.route"], "^traced/") + self.assertEqual(span.attributes["http.scheme"], "http") + self.assertEqual(span.attributes["http.status_code"], 200) def test_traced_post_new_semconv(self): Client().post("/traced/") @@ -341,14 +313,14 @@ def test_traced_post_new_semconv(self): 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") + 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) + 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/") @@ -361,21 +333,21 @@ def test_traced_post_both_semconv(self): 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["http.method"], "POST") self.assertEqual( - span.attributes[SpanAttributes.HTTP_URL], + span.attributes["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") + self.assertEqual(span.attributes["http.scheme"], "http") + self.assertEqual(span.attributes["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) + self.assertEqual(span.attributes["http.route"], "^traced/") + self.assertEqual(span.attributes["http.response.status_code"], 200) def test_error(self): with self.assertRaises(ValueError): @@ -389,27 +361,21 @@ def test_error(self): 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["http.method"], "GET") self.assertEqual( - span.attributes[SpanAttributes.HTTP_URL], + span.attributes["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.route"], "^error/") + self.assertEqual(span.attributes["http.scheme"], "http") + self.assertEqual(span.attributes["http.status_code"], 500) self.assertEqual(len(span.events), 1) event = span.events[0] self.assertEqual(event.name, "exception") - self.assertEqual( - event.attributes[SpanAttributes.EXCEPTION_TYPE], "ValueError" - ) - self.assertEqual( - event.attributes[SpanAttributes.EXCEPTION_MESSAGE], "error" - ) + self.assertEqual(event.attributes["exception.type"], "ValueError") + self.assertEqual(event.attributes["exception.message"], "error") def test_error_new_semconv(self): with self.assertRaises(ValueError): @@ -423,18 +389,18 @@ def test_error_new_semconv(self): 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") + 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(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") + 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): @@ -448,29 +414,27 @@ def test_error_both_semconv(self): 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["http.method"], "GET") self.assertEqual( - span.attributes[SpanAttributes.HTTP_URL], + span.attributes["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") + self.assertEqual(span.attributes["http.route"], "^error/") + self.assertEqual(span.attributes["http.scheme"], "http") + self.assertEqual(span.attributes["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(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") + 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() @@ -545,7 +509,7 @@ def test_nonstandard_http_method_span_name(self): span = span_list[0] self.assertEqual(span.name, "HTTP") - self.assertEqual(span.attributes[SpanAttributes.HTTP_METHOD], "_OTHER") + self.assertEqual(span.attributes["http.method"], "_OTHER") def test_nonstandard_http_method_span_name_new_semconv(self): Client().request( @@ -556,9 +520,9 @@ def test_nonstandard_http_method_span_name_new_semconv(self): span = span_list[0] self.assertEqual(span.name, "HTTP") - self.assertEqual(span.attributes[HTTP_REQUEST_METHOD], "_OTHER") + self.assertEqual(span.attributes["http.request.method"], "_OTHER") self.assertEqual( - span.attributes[HTTP_REQUEST_METHOD_ORIGINAL], "NONSTANDARD" + span.attributes["http.request.method_original"], "NONSTANDARD" ) def test_nonstandard_http_method_span_name_both_semconv(self): @@ -570,10 +534,10 @@ def test_nonstandard_http_method_span_name_both_semconv(self): 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.method"], "_OTHER") + self.assertEqual(span.attributes["http.request.method"], "_OTHER") self.assertEqual( - span.attributes[HTTP_REQUEST_METHOD_ORIGINAL], "NONSTANDARD" + span.attributes["http.request.method_original"], "NONSTANDARD" ) def test_traced_request_attrs(self): @@ -711,9 +675,20 @@ def test_wsgi_metrics(self): "http.server.active_requests", "http.server.duration", ] - _recommended_attrs = { - "http.server.active_requests": _server_active_requests_count_attrs_old, - "http.server.duration": _server_duration_attrs_old, + expected_duration_attributes = { + "http.method": "GET", + "http.scheme": "http", + "http.flavor": "1.1", + "http.server_name": "testserver", + "net.host.port": 80, + "http.status_code": 200, + "http.target": "^span_name/([0-9]{4})/$", + } + expected_requests_count_attributes = { + "http.method": "GET", + "http.scheme": "http", + "http.flavor": "1.1", + "http.server_name": "testserver", } start = default_timer() for _ in range(3): @@ -740,12 +715,16 @@ def test_wsgi_metrics(self): self.assertAlmostEqual( duration, point.sum, delta=100 ) + self.assertDictEqual( + expected_duration_attributes, + dict(point.attributes), + ) 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.assertDictEqual( + expected_requests_count_attributes, + dict(point.attributes), ) self.assertTrue(histrogram_data_point_seen and number_data_point_seen) @@ -755,9 +734,16 @@ def test_wsgi_metrics_new_semconv(self): "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, + expected_duration_attributes = { + "http.request.method": "GET", + "url.scheme": "http", + "network.protocol.version": "1.1", + "http.response.status_code": 200, + "http.route": "^span_name/([0-9]{4})/$", + } + expected_requests_count_attributes = { + "http.request.method": "GET", + "url.scheme": "http", } start = default_timer() for _ in range(3): @@ -784,12 +770,16 @@ def test_wsgi_metrics_new_semconv(self): self.assertAlmostEqual( duration_s, point.sum, places=1 ) + self.assertDictEqual( + expected_duration_attributes, + dict(point.attributes), + ) 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.assertDictEqual( + expected_requests_count_attributes, + dict(point.attributes), ) self.assertTrue(histrogram_data_point_seen and number_data_point_seen) @@ -801,12 +791,29 @@ def test_wsgi_metrics_both_semconv(self): "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, + expected_duration_attributes_old = { + "http.method": "GET", + "http.scheme": "http", + "http.flavor": "1.1", + "http.server_name": "testserver", + "net.host.port": 80, + "http.status_code": 200, + "http.target": "^span_name/([0-9]{4})/$", + } + expected_duration_attributes_new = { + "http.request.method": "GET", + "url.scheme": "http", + "network.protocol.version": "1.1", + "http.response.status_code": 200, + "http.route": "^span_name/([0-9]{4})/$", + } + expected_requests_count_attributes = { + "http.method": "GET", + "http.scheme": "http", + "http.flavor": "1.1", + "http.server_name": "testserver", + "http.request.method": "GET", + "url.scheme": "http", } start = default_timer() for _ in range(3): @@ -835,16 +842,24 @@ def test_wsgi_metrics_both_semconv(self): self.assertAlmostEqual( duration_s, point.sum, places=1 ) + self.assertDictEqual( + expected_duration_attributes_new, + dict(point.attributes), + ) elif metric.name == "http.server.duration": self.assertAlmostEqual( duration, point.sum, delta=100 ) + self.assertDictEqual( + expected_duration_attributes_old, + dict(point.attributes), + ) 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.assertDictEqual( + expected_requests_count_attributes, + dict(point.attributes), ) self.assertTrue(histrogram_data_point_seen and number_data_point_seen) @@ -911,9 +926,7 @@ def test_django_with_wsgi_instrumented(self): Client().get("/span_name/1234/") span_list = self.exporter.get_finished_spans() print(span_list) - self.assertEqual( - span_list[0].attributes[SpanAttributes.HTTP_STATUS_CODE], 200 - ) + self.assertEqual(span_list[0].attributes["http.status_code"], 200) self.assertEqual(trace.SpanKind.INTERNAL, span_list[0].kind) self.assertEqual( parent_span.get_span_context().span_id, diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index 69c5d312e5..d233331283 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -768,7 +768,6 @@ def test_basic_metric_nonstandard_http_method_success_both_semconv(self): 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( 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 9bc5e85a12..0093715ae1 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py @@ -395,6 +395,7 @@ def _start_response(status, response_headers, *args, **kwargs): ) if request_route: + # http.target to be included in old semantic conventions duration_attrs_old[SpanAttributes.HTTP_TARGET] = str( request_route ) diff --git a/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/grpcext/_interceptor.py b/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/grpcext/_interceptor.py index 53ee46a20d..32cec6dee0 100644 --- a/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/grpcext/_interceptor.py +++ b/instrumentation/opentelemetry-instrumentation-grpc/src/opentelemetry/instrumentation/grpc/grpcext/_interceptor.py @@ -272,11 +272,23 @@ def unsubscribe(self, *args, **kwargs): self._channel.unsubscribe(*args, **kwargs) def unary_unary( - self, method, request_serializer=None, response_deserializer=None + self, + method, + request_serializer=None, + response_deserializer=None, + _registered_method=False, ): - base_callable = self._channel.unary_unary( - method, request_serializer, response_deserializer - ) + if _registered_method: + base_callable = self._channel.unary_unary( + method, + request_serializer, + response_deserializer, + _registered_method, + ) + else: + base_callable = self._channel.unary_unary( + method, request_serializer, response_deserializer + ) if isinstance(self._interceptor, grpcext.UnaryClientInterceptor): return _InterceptorUnaryUnaryMultiCallable( method, base_callable, self._interceptor @@ -284,11 +296,23 @@ def unary_unary( return base_callable def unary_stream( - self, method, request_serializer=None, response_deserializer=None + self, + method, + request_serializer=None, + response_deserializer=None, + _registered_method=False, ): - base_callable = self._channel.unary_stream( - method, request_serializer, response_deserializer - ) + if _registered_method: + base_callable = self._channel.unary_stream( + method, + request_serializer, + response_deserializer, + _registered_method, + ) + else: + base_callable = self._channel.unary_stream( + method, request_serializer, response_deserializer + ) if isinstance(self._interceptor, grpcext.StreamClientInterceptor): return _InterceptorUnaryStreamMultiCallable( method, base_callable, self._interceptor @@ -296,11 +320,23 @@ def unary_stream( return base_callable def stream_unary( - self, method, request_serializer=None, response_deserializer=None + self, + method, + request_serializer=None, + response_deserializer=None, + _registered_method=False, ): - base_callable = self._channel.stream_unary( - method, request_serializer, response_deserializer - ) + if _registered_method: + base_callable = self._channel.stream_unary( + method, + request_serializer, + response_deserializer, + _registered_method, + ) + else: + base_callable = self._channel.stream_unary( + method, request_serializer, response_deserializer + ) if isinstance(self._interceptor, grpcext.StreamClientInterceptor): return _InterceptorStreamUnaryMultiCallable( method, base_callable, self._interceptor @@ -308,11 +344,23 @@ def stream_unary( return base_callable def stream_stream( - self, method, request_serializer=None, response_deserializer=None + self, + method, + request_serializer=None, + response_deserializer=None, + _registered_method=False, ): - base_callable = self._channel.stream_stream( - method, request_serializer, response_deserializer - ) + if _registered_method: + base_callable = self._channel.stream_stream( + method, + request_serializer, + response_deserializer, + _registered_method, + ) + else: + base_callable = self._channel.stream_stream( + method, request_serializer, response_deserializer + ) if isinstance(self._interceptor, grpcext.StreamClientInterceptor): return _InterceptorStreamStreamMultiCallable( method, base_callable, self._interceptor diff --git a/instrumentation/opentelemetry-instrumentation-grpc/test-requirements.txt b/instrumentation/opentelemetry-instrumentation-grpc/test-requirements-0.txt similarity index 100% rename from instrumentation/opentelemetry-instrumentation-grpc/test-requirements.txt rename to instrumentation/opentelemetry-instrumentation-grpc/test-requirements-0.txt diff --git a/instrumentation/opentelemetry-instrumentation-grpc/test-requirements-1.txt b/instrumentation/opentelemetry-instrumentation-grpc/test-requirements-1.txt new file mode 100644 index 0000000000..6d9531cd5e --- /dev/null +++ b/instrumentation/opentelemetry-instrumentation-grpc/test-requirements-1.txt @@ -0,0 +1,20 @@ +asgiref==3.7.2 +attrs==23.2.0 +Deprecated==1.2.14 +grpcio==1.63.0 +importlib-metadata==6.11.0 +iniconfig==2.0.0 +packaging==23.2 +pluggy==1.4.0 +protobuf==3.20.3 +py==1.11.0 +py-cpuinfo==9.0.0 +pytest==7.1.3 +pytest-asyncio==0.23.5 +pytest-benchmark==4.0.0 +tomli==2.0.1 +typing_extensions==4.9.0 +wrapt==1.16.0 +zipp==3.17.0 +-e opentelemetry-instrumentation +-e instrumentation/opentelemetry-instrumentation-grpc diff --git a/instrumentation/opentelemetry-instrumentation-sqlalchemy/test-requirements-0.txt b/instrumentation/opentelemetry-instrumentation-sqlalchemy/test-requirements-0.txt index af0e495a39..407222e8f6 100644 --- a/instrumentation/opentelemetry-instrumentation-sqlalchemy/test-requirements-0.txt +++ b/instrumentation/opentelemetry-instrumentation-sqlalchemy/test-requirements-0.txt @@ -1,15 +1,12 @@ asgiref==3.7.2 cffi==1.15.1 Deprecated==1.2.14 -greenlet==0.4.13 -hpy==0.0.4.dev179+g9b5d200 importlib-metadata==6.11.0 iniconfig==2.0.0 packaging==24.0 pluggy==1.5.0 py-cpuinfo==9.0.0 pytest==7.4.4 -readline==6.2.4.1 SQLAlchemy==1.1.18 tomli==2.0.1 typing_extensions==4.10.0 diff --git a/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py b/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py index befc022b35..d9072ba727 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py @@ -85,25 +85,49 @@ def response_hook(span, request_obj, response) Request, ) +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_method, + _set_http_network_protocol_version, + _set_http_url, + _set_status, +) from opentelemetry.instrumentation.instrumentor import BaseInstrumentor from opentelemetry.instrumentation.urllib.package import _instruments from opentelemetry.instrumentation.urllib.version import __version__ from opentelemetry.instrumentation.utils import ( - http_status_to_status_code, is_http_instrumentation_enabled, suppress_http_instrumentation, ) from opentelemetry.metrics import Histogram, get_meter from opentelemetry.propagate import inject +from opentelemetry.semconv._incubating.metrics.http_metrics import ( + HTTP_CLIENT_REQUEST_BODY_SIZE, + HTTP_CLIENT_RESPONSE_BODY_SIZE, + create_http_client_request_body_size, + create_http_client_response_body_size, +) +from opentelemetry.semconv.attributes.error_attributes import ERROR_TYPE from opentelemetry.semconv.metrics import MetricInstruments +from opentelemetry.semconv.metrics.http_metrics import ( + HTTP_CLIENT_REQUEST_DURATION, +) from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.trace import Span, SpanKind, get_tracer -from opentelemetry.trace.status import Status from opentelemetry.util.http import ( ExcludeList, get_excluded_urls, parse_excluded_urls, remove_url_credentials, + sanitize_method, ) _excluded_urls_from_env = get_excluded_urls("URLLIB") @@ -133,12 +157,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") meter_provider = kwargs.get("meter_provider") @@ -146,10 +176,10 @@ def _instrument(self, **kwargs): __name__, __version__, meter_provider, - schema_url="https://opentelemetry.io/schemas/1.11.0", + schema_url=schema_url, ) - histograms = _create_client_histograms(meter) + histograms = _create_client_histograms(meter, sem_conv_opt_in_mode) _instrument( tracer, @@ -161,6 +191,7 @@ 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): @@ -173,12 +204,14 @@ def uninstrument_opener( _uninstrument_from(opener, restore_as_bound_func=True) +# pylint: disable=too-many-statements def _instrument( tracer, histograms: Dict[str, Histogram], request_hook: _RequestHookT = None, response_hook: _ResponseHookT = None, excluded_urls: ExcludeList = None, + sem_conv_opt_in_mode: _HTTPStabilityMode = _HTTPStabilityMode.DEFAULT, ): """Enables tracing of all requests calls that go through :code:`urllib.Client._make_request`""" @@ -214,14 +247,22 @@ def _instrumented_open_call( method = request.get_method().upper() - span_name = method.strip() + span_name = _get_span_name(method) url = remove_url_credentials(url) - labels = { - SpanAttributes.HTTP_METHOD: method, - SpanAttributes.HTTP_URL: url, - } + data = getattr(request, "data", None) + request_size = 0 if data is None else len(data) + + labels = {} + + _set_http_method( + labels, + method, + sanitize_method(method), + sem_conv_opt_in_mode, + ) + _set_http_url(labels, url, sem_conv_opt_in_mode) with tracer.start_as_current_span( span_name, kind=SpanKind.CLIENT, attributes=labels @@ -241,24 +282,50 @@ def _instrumented_open_call( exception = exc result = getattr(exc, "file", None) finally: - elapsed_time = round((default_timer() - start_time) * 1000) - + duration_s = default_timer() - start_time + response_size = 0 if result is not None: + response_size = int(result.headers.get("Content-Length", 0)) code_ = result.getcode() - labels[SpanAttributes.HTTP_STATUS_CODE] = str(code_) - - if span.is_recording() and code_ is not None: - span.set_attribute(SpanAttributes.HTTP_STATUS_CODE, code_) - span.set_status(Status(http_status_to_status_code(code_))) + # set http status code based on semconv + if code_: + _set_status_code_attribute( + span, code_, labels, sem_conv_opt_in_mode + ) ver_ = str(getattr(result, "version", "")) if ver_: - labels[SpanAttributes.HTTP_FLAVOR] = ( - f"{ver_[:1]}.{ver_[:-1]}" + _set_http_network_protocol_version( + labels, f"{ver_[:1]}.{ver_[:-1]}", sem_conv_opt_in_mode ) + if exception is not None and _report_new(sem_conv_opt_in_mode): + span.set_attribute(ERROR_TYPE, type(exception).__qualname__) + labels[ERROR_TYPE] = type(exception).__qualname__ + + duration_attrs_old = _filter_semconv_duration_attrs( + labels, + _client_duration_attrs_old, + _client_duration_attrs_new, + sem_conv_opt_in_mode=_HTTPStabilityMode.DEFAULT, + ) + duration_attrs_new = _filter_semconv_duration_attrs( + labels, + _client_duration_attrs_old, + _client_duration_attrs_new, + sem_conv_opt_in_mode=_HTTPStabilityMode.HTTP, + ) + + duration_attrs_old[SpanAttributes.HTTP_URL] = url + _record_histograms( - histograms, labels, request, result, elapsed_time + histograms, + duration_attrs_old, + duration_attrs_new, + request_size, + response_size, + duration_s, + sem_conv_opt_in_mode, ) if callable(response_hook): @@ -296,43 +363,108 @@ def _uninstrument_from(instr_root, restore_as_bound_func=False): setattr(instr_root, instr_func_name, original) -def _create_client_histograms(meter) -> Dict[str, Histogram]: - histograms = { - MetricInstruments.HTTP_CLIENT_DURATION: meter.create_histogram( - name=MetricInstruments.HTTP_CLIENT_DURATION, - unit="ms", - description="Measures the duration of outbound HTTP requests.", - ), - MetricInstruments.HTTP_CLIENT_REQUEST_SIZE: meter.create_histogram( - name=MetricInstruments.HTTP_CLIENT_REQUEST_SIZE, - unit="By", - description="Measures the size of HTTP request messages.", - ), - MetricInstruments.HTTP_CLIENT_RESPONSE_SIZE: meter.create_histogram( - name=MetricInstruments.HTTP_CLIENT_RESPONSE_SIZE, - unit="By", - description="Measures the size of HTTP response messages.", - ), - } +def _get_span_name(method: str) -> str: + method = sanitize_method(method.strip()) + if method == "_OTHER": + method = "HTTP" + return method + + +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_client_histograms( + meter, sem_conv_opt_in_mode=_HTTPStabilityMode.DEFAULT +) -> Dict[str, Histogram]: + histograms = {} + if _report_old(sem_conv_opt_in_mode): + histograms[MetricInstruments.HTTP_CLIENT_DURATION] = ( + meter.create_histogram( + name=MetricInstruments.HTTP_CLIENT_DURATION, + unit="ms", + description="Measures the duration of the outbound HTTP request", + ) + ) + histograms[MetricInstruments.HTTP_CLIENT_REQUEST_SIZE] = ( + meter.create_histogram( + name=MetricInstruments.HTTP_CLIENT_REQUEST_SIZE, + unit="By", + description="Measures the size of HTTP request messages.", + ) + ) + histograms[MetricInstruments.HTTP_CLIENT_RESPONSE_SIZE] = ( + meter.create_histogram( + name=MetricInstruments.HTTP_CLIENT_RESPONSE_SIZE, + unit="By", + description="Measures the size of HTTP response messages.", + ) + ) + if _report_new(sem_conv_opt_in_mode): + histograms[HTTP_CLIENT_REQUEST_DURATION] = meter.create_histogram( + name=HTTP_CLIENT_REQUEST_DURATION, + unit="s", + description="Duration of HTTP client requests.", + ) + histograms[HTTP_CLIENT_REQUEST_BODY_SIZE] = ( + create_http_client_request_body_size(meter) + ) + histograms[HTTP_CLIENT_RESPONSE_BODY_SIZE] = ( + create_http_client_response_body_size(meter) + ) return histograms def _record_histograms( - histograms, metric_attributes, request, response, elapsed_time + histograms: Dict[str, Histogram], + metric_attributes_old: dict, + metric_attributes_new: dict, + request_size: int, + response_size: int, + duration_s: float, + sem_conv_opt_in_mode: _HTTPStabilityMode = _HTTPStabilityMode.DEFAULT, ): - histograms[MetricInstruments.HTTP_CLIENT_DURATION].record( - elapsed_time, attributes=metric_attributes - ) - - data = getattr(request, "data", None) - request_size = 0 if data is None else len(data) - histograms[MetricInstruments.HTTP_CLIENT_REQUEST_SIZE].record( - request_size, attributes=metric_attributes - ) - - if response is not None: - response_size = int(response.headers.get("Content-Length", 0)) + if _report_old(sem_conv_opt_in_mode): + duration = max(round(duration_s * 1000), 0) + histograms[MetricInstruments.HTTP_CLIENT_DURATION].record( + duration, attributes=metric_attributes_old + ) + histograms[MetricInstruments.HTTP_CLIENT_REQUEST_SIZE].record( + request_size, attributes=metric_attributes_old + ) histograms[MetricInstruments.HTTP_CLIENT_RESPONSE_SIZE].record( - response_size, attributes=metric_attributes + response_size, attributes=metric_attributes_old + ) + if _report_new(sem_conv_opt_in_mode): + histograms[HTTP_CLIENT_REQUEST_DURATION].record( + duration_s, attributes=metric_attributes_new + ) + histograms[HTTP_CLIENT_REQUEST_BODY_SIZE].record( + request_size, attributes=metric_attributes_new + ) + histograms[HTTP_CLIENT_RESPONSE_BODY_SIZE].record( + response_size, attributes=metric_attributes_new ) diff --git a/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/package.py b/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/package.py index 942f175da1..1bb8350a06 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/package.py +++ b/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/package.py @@ -16,3 +16,5 @@ _instruments = tuple() _supports_metrics = True + +_semconv_status = "migration" diff --git a/instrumentation/opentelemetry-instrumentation-urllib/tests/test_metrics_instrumentation.py b/instrumentation/opentelemetry-instrumentation-urllib/tests/test_metrics_instrumentation.py index f79749dfd8..7a9bfd38f1 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/tests/test_metrics_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-urllib/tests/test_metrics_instrumentation.py @@ -15,16 +15,28 @@ from platform import python_implementation from timeit import default_timer +from unittest.mock import patch from urllib import request from urllib.parse import urlencode import httpretty from pytest import mark +from opentelemetry.instrumentation._semconv import ( + OTEL_SEMCONV_STABILITY_OPT_IN, + _OpenTelemetrySemanticConventionStability, +) from opentelemetry.instrumentation.urllib import ( # pylint: disable=no-name-in-module,import-error URLLibInstrumentor, ) +from opentelemetry.semconv._incubating.metrics.http_metrics import ( + HTTP_CLIENT_REQUEST_BODY_SIZE, + HTTP_CLIENT_RESPONSE_BODY_SIZE, +) from opentelemetry.semconv.metrics import MetricInstruments +from opentelemetry.semconv.metrics.http_metrics import ( + HTTP_CLIENT_REQUEST_DURATION, +) from opentelemetry.test.test_base import TestBase @@ -34,6 +46,23 @@ class TestUrllibMetricsInstrumentation(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 = patch.dict( + "os.environ", + { + OTEL_SEMCONV_STABILITY_OPT_IN: sem_conv_mode, + }, + ) + _OpenTelemetrySemanticConventionStability._initialized = False + self.env_patch.start() URLLibInstrumentor().instrument() httpretty.enable() httpretty.register_uri(httpretty.GET, self.URL, body=b"Hello!") @@ -71,18 +100,19 @@ def test_basic_metric(self): self.assertEqual( client_duration.name, MetricInstruments.HTTP_CLIENT_DURATION ) + self.assert_metric_expected( client_duration, self.create_histogram_data_points( client_duration_estimated, attributes={ - "http.status_code": str(result.code), + "http.status_code": int(result.code), "http.method": "GET", "http.url": str(result.url), "http.flavor": "1.1", }, ), - est_value_delta=200, + est_value_delta=40, ) self.assertEqual( @@ -94,7 +124,7 @@ def test_basic_metric(self): self.create_histogram_data_points( 0, attributes={ - "http.status_code": str(result.code), + "http.status_code": int(result.code), "http.method": "GET", "http.url": str(result.url), "http.flavor": "1.1", @@ -111,7 +141,7 @@ def test_basic_metric(self): self.create_histogram_data_points( result.length, attributes={ - "http.status_code": str(result.code), + "http.status_code": int(result.code), "http.method": "GET", "http.url": str(result.url), "http.flavor": "1.1", @@ -119,6 +149,188 @@ def test_basic_metric(self): ), ) + def test_basic_metric_new_semconv(self): + start_time = default_timer() + with request.urlopen(self.URL) as result: + duration_s = default_timer() - start_time + + metrics = self.get_sorted_metrics() + self.assertEqual(len(metrics), 3) + + ( + client_request_body_size, + client_request_duration, + client_response_body_size, + ) = metrics[:3] + + self.assertEqual( + client_request_duration.name, HTTP_CLIENT_REQUEST_DURATION + ) + + self.assert_metric_expected( + client_request_duration, + self.create_histogram_data_points( + duration_s, + attributes={ + "http.response.status_code": int(result.code), + "http.request.method": "GET", + "network.protocol.version": "1.1", + }, + ), + est_value_delta=40, + ) + + self.assertEqual( + client_request_body_size.name, + HTTP_CLIENT_REQUEST_BODY_SIZE, + ) + self.assert_metric_expected( + client_request_body_size, + self.create_histogram_data_points( + 0, + attributes={ + "http.response.status_code": int(result.code), + "http.request.method": "GET", + "network.protocol.version": "1.1", + }, + ), + ) + + self.assertEqual( + client_response_body_size.name, + HTTP_CLIENT_RESPONSE_BODY_SIZE, + ) + self.assert_metric_expected( + client_response_body_size, + self.create_histogram_data_points( + result.length, + attributes={ + "http.response.status_code": int(result.code), + "http.request.method": "GET", + "network.protocol.version": "1.1", + }, + ), + ) + + def test_basic_metric_both_semconv(self): + start_time = default_timer() + with request.urlopen(self.URL) as result: + duration_s = default_timer() - start_time + duration = max(round(duration_s * 1000), 0) + + metrics = self.get_sorted_metrics() + self.assertEqual(len(metrics), 6) + + ( + client_duration, + client_request_body_size, + client_request_duration, + client_request_size, + client_response_body_size, + client_response_size, + ) = metrics[:6] + + self.assertEqual( + client_duration.name, MetricInstruments.HTTP_CLIENT_DURATION + ) + + self.assert_metric_expected( + client_duration, + self.create_histogram_data_points( + duration, + attributes={ + "http.status_code": int(result.code), + "http.method": "GET", + "http.url": str(result.url), + "http.flavor": "1.1", + }, + ), + est_value_delta=40, + ) + + self.assertEqual( + client_request_size.name, + MetricInstruments.HTTP_CLIENT_REQUEST_SIZE, + ) + self.assert_metric_expected( + client_request_size, + self.create_histogram_data_points( + 0, + attributes={ + "http.status_code": int(result.code), + "http.method": "GET", + "http.url": str(result.url), + "http.flavor": "1.1", + }, + ), + ) + + self.assertEqual( + client_response_size.name, + MetricInstruments.HTTP_CLIENT_RESPONSE_SIZE, + ) + self.assert_metric_expected( + client_response_size, + self.create_histogram_data_points( + result.length, + attributes={ + "http.status_code": int(result.code), + "http.method": "GET", + "http.url": str(result.url), + "http.flavor": "1.1", + }, + ), + ) + + self.assertEqual( + client_request_duration.name, HTTP_CLIENT_REQUEST_DURATION + ) + + self.assert_metric_expected( + client_request_duration, + self.create_histogram_data_points( + duration_s, + attributes={ + "http.response.status_code": int(result.code), + "http.request.method": "GET", + "network.protocol.version": "1.1", + }, + ), + est_value_delta=40, + ) + + self.assertEqual( + client_request_body_size.name, + HTTP_CLIENT_REQUEST_BODY_SIZE, + ) + self.assert_metric_expected( + client_request_body_size, + self.create_histogram_data_points( + 0, + attributes={ + "http.response.status_code": int(result.code), + "http.request.method": "GET", + "network.protocol.version": "1.1", + }, + ), + ) + + self.assertEqual( + client_response_body_size.name, + HTTP_CLIENT_RESPONSE_BODY_SIZE, + ) + self.assert_metric_expected( + client_response_body_size, + self.create_histogram_data_points( + result.length, + attributes={ + "http.response.status_code": int(result.code), + "http.request.method": "GET", + "network.protocol.version": "1.1", + }, + ), + ) + def test_basic_metric_request_not_empty(self): data = {"header1": "value1", "header2": "value2"} data_encoded = urlencode(data).encode() @@ -144,13 +356,13 @@ def test_basic_metric_request_not_empty(self): self.create_histogram_data_points( client_duration_estimated, attributes={ - "http.status_code": str(result.code), + "http.status_code": int(result.code), "http.method": "POST", "http.url": str(result.url), "http.flavor": "1.1", }, ), - est_value_delta=200, + est_value_delta=40, ) self.assertEqual( @@ -162,7 +374,7 @@ def test_basic_metric_request_not_empty(self): self.create_histogram_data_points( len(data_encoded), attributes={ - "http.status_code": str(result.code), + "http.status_code": int(result.code), "http.method": "POST", "http.url": str(result.url), "http.flavor": "1.1", @@ -179,7 +391,7 @@ def test_basic_metric_request_not_empty(self): self.create_histogram_data_points( result.length, attributes={ - "http.status_code": str(result.code), + "http.status_code": int(result.code), "http.method": "POST", "http.url": str(result.url), "http.flavor": "1.1", diff --git a/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py b/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py index f73f0d1b97..8ac0284939 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py +++ b/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py @@ -25,6 +25,10 @@ import opentelemetry.instrumentation.urllib # pylint: disable=no-name-in-module,import-error from opentelemetry import trace +from opentelemetry.instrumentation._semconv import ( + OTEL_SEMCONV_STABILITY_OPT_IN, + _OpenTelemetrySemanticConventionStability, +) from opentelemetry.instrumentation.urllib import ( # pylint: disable=no-name-in-module,import-error URLLibInstrumentor, ) @@ -34,6 +38,12 @@ ) from opentelemetry.propagate import get_global_textmap, set_global_textmap from opentelemetry.sdk import resources +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.mock_textmap import MockTextMapPropagator from opentelemetry.test.test_base import TestBase @@ -43,7 +53,7 @@ # pylint: disable=too-many-public-methods -class RequestsIntegrationTestBase(abc.ABC): +class URLLibIntegrationTestBase(abc.ABC): # pylint: disable=no-member URL = "http://mock/status/200" @@ -54,12 +64,23 @@ class RequestsIntegrationTestBase(abc.ABC): 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_URLLIB_EXCLUDED_URLS": "http://localhost/env_excluded_arg/123,env_excluded_noarg" + "OTEL_PYTHON_URLLIB_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( @@ -141,6 +162,57 @@ def test_basic(self): span, opentelemetry.instrumentation.urllib ) + def test_basic_new_semconv(self): + result = self.perform_request(self.URL) + + self.assertEqual(result.read(), b"Hello!") + span = self.assert_span() + + self.assertIs(span.kind, trace.SpanKind.CLIENT) + self.assertEqual(span.name, "GET") + + self.assertEqual( + span.attributes, + { + HTTP_REQUEST_METHOD: "GET", + URL_FULL: self.URL, + HTTP_RESPONSE_STATUS_CODE: 200, + }, + ) + + self.assertIs(span.status.status_code, trace.StatusCode.UNSET) + + self.assertEqualSpanInstrumentationInfo( + span, opentelemetry.instrumentation.urllib + ) + + def test_basic_both_semconv(self): + result = self.perform_request(self.URL) + + self.assertEqual(result.read(), b"Hello!") + span = self.assert_span() + + self.assertIs(span.kind, trace.SpanKind.CLIENT) + self.assertEqual(span.name, "GET") + + self.assertEqual( + span.attributes, + { + SpanAttributes.HTTP_METHOD: "GET", + SpanAttributes.HTTP_URL: self.URL, + SpanAttributes.HTTP_STATUS_CODE: 200, + HTTP_REQUEST_METHOD: "GET", + URL_FULL: self.URL, + HTTP_RESPONSE_STATUS_CODE: 200, + }, + ) + + self.assertIs(span.status.status_code, trace.StatusCode.UNSET) + + self.assertEqualSpanInstrumentationInfo( + span, opentelemetry.instrumentation.urllib + ) + def test_excluded_urls_explicit(self): url_201 = "http://mock/status/201" httpretty.register_uri( @@ -197,6 +269,57 @@ def test_not_foundbasic(self): trace.StatusCode.ERROR, ) + def test_not_foundbasic_new_semconv(self): + url_404 = "http://mock/status/404/" + httpretty.register_uri( + httpretty.GET, + url_404, + status=404, + ) + exception = None + try: + self.perform_request(url_404) + except Exception as err: # pylint: disable=broad-except + exception = err + code = exception.code + self.assertEqual(code, 404) + + span = self.assert_span() + + self.assertEqual(span.attributes.get(HTTP_RESPONSE_STATUS_CODE), 404) + + self.assertIs( + span.status.status_code, + trace.StatusCode.ERROR, + ) + + def test_not_foundbasic_both_semconv(self): + url_404 = "http://mock/status/404/" + httpretty.register_uri( + httpretty.GET, + url_404, + status=404, + ) + exception = None + try: + self.perform_request(url_404) + except Exception as err: # pylint: disable=broad-except + exception = err + code = exception.code + self.assertEqual(code, 404) + + span = self.assert_span() + + self.assertEqual( + span.attributes.get(SpanAttributes.HTTP_STATUS_CODE), 404 + ) + self.assertEqual(span.attributes.get(HTTP_RESPONSE_STATUS_CODE), 404) + + self.assertIs( + span.status.status_code, + trace.StatusCode.ERROR, + ) + @staticmethod def mock_get_code(*args, **kwargs): return None @@ -339,6 +462,41 @@ def test_requests_exception_with_response(self, *_, **__): ) self.assertEqual(span.status.status_code, StatusCode.ERROR) + def test_requests_exception_with_response_new_semconv(self, *_, **__): + with self.assertRaises(HTTPError): + self.perform_request("http://mock/status/500") + + span = self.assert_span() + self.assertEqual( + dict(span.attributes), + { + HTTP_REQUEST_METHOD: "GET", + URL_FULL: "http://mock/status/500", + HTTP_RESPONSE_STATUS_CODE: 500, + ERROR_TYPE: "HTTPError", + }, + ) + self.assertEqual(span.status.status_code, StatusCode.ERROR) + + def test_requests_exception_with_response_both_semconv(self, *_, **__): + with self.assertRaises(HTTPError): + self.perform_request("http://mock/status/500") + + span = self.assert_span() + self.assertEqual( + dict(span.attributes), + { + SpanAttributes.HTTP_METHOD: "GET", + SpanAttributes.HTTP_URL: "http://mock/status/500", + SpanAttributes.HTTP_STATUS_CODE: 500, + HTTP_REQUEST_METHOD: "GET", + URL_FULL: "http://mock/status/500", + HTTP_RESPONSE_STATUS_CODE: 500, + ERROR_TYPE: "HTTPError", + }, + ) + self.assertEqual(span.status.status_code, StatusCode.ERROR) + def test_requests_basic_exception(self, *_, **__): with self.assertRaises(Exception): self.perform_request(self.URL_EXCEPTION) @@ -393,7 +551,7 @@ def test_no_op_tracer_provider(self): self.assert_span(num_spans=0) -class TestRequestsIntegration(RequestsIntegrationTestBase, TestBase): +class TestURLLibIntegration(URLLibIntegrationTestBase, TestBase): @staticmethod def perform_request(url: str, opener: OpenerDirector = None): if not opener: 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 a05084725d..4bcd0816fd 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/__init__.py @@ -47,14 +47,19 @@ def strip_query_params(url: str) -> str: .. code:: python - # `request` is an instance of urllib3.connectionpool.HTTPConnectionPool - def request_hook(span, request): - pass - - # `request` is an instance of urllib3.connectionpool.HTTPConnectionPool - # `response` is an instance of urllib3.response.HTTPResponse - def response_hook(span, request, response): - pass + def request_hook( + span: Span, + pool: urllib3.connectionpool.HTTPConnectionPool, + request_info: RequestInfo, + ) -> Any: + ... + + def response_hook( + span: Span, + pool: urllib3.connectionpool.HTTPConnectionPool, + response: urllib3.response.HTTPResponse, + ) -> Any: + ... URLLib3Instrumentor().instrument( request_hook=request_hook, response_hook=response_hook @@ -81,6 +86,7 @@ def response_hook(span, request, response): import collections.abc import io import typing +from dataclasses import dataclass from timeit import default_timer from typing import Collection @@ -135,14 +141,29 @@ def response_hook(span, request, response): _excluded_urls_from_env = get_excluded_urls("URLLIB3") + +@dataclass +class RequestInfo: + """Arguments that were passed to the ``urlopen()`` call.""" + + __slots__ = ("method", "url", "headers", "body") + + # The type annotations here come from ``HTTPConnectionPool.urlopen()``. + method: str + url: str + headers: typing.Optional[typing.Mapping[str, str]] + body: typing.Union[ + bytes, typing.IO[typing.Any], typing.Iterable[bytes], str, None + ] + + _UrlFilterT = typing.Optional[typing.Callable[[str], str]] _RequestHookT = typing.Optional[ typing.Callable[ [ Span, urllib3.connectionpool.HTTPConnectionPool, - typing.Dict, - typing.Optional[str], + RequestInfo, ], None, ] @@ -317,7 +338,16 @@ def instrumented_urlopen(wrapped, instance, args, kwargs): span_name, kind=SpanKind.CLIENT, attributes=span_attributes ) as span, set_ip_on_next_http_connection(span): if callable(request_hook): - request_hook(span, instance, headers, body) + request_hook( + span, + instance, + RequestInfo( + method=method, + url=url, + headers=headers, + body=body, + ), + ) inject(headers) # TODO: add error handling to also set exception `error.type` in new semconv with suppress_http_instrumentation(): diff --git a/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py b/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py index 5ad4e40ca3..69bed0eaee 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py +++ b/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py @@ -27,21 +27,18 @@ _HTTPStabilityMode, _OpenTelemetrySemanticConventionStability, ) -from opentelemetry.instrumentation.urllib3 import URLLib3Instrumentor +from opentelemetry.instrumentation.urllib3 import ( + RequestInfo, + 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 +from opentelemetry.trace import Span from opentelemetry.util.http import get_excluded_urls # pylint: disable=too-many-public-methods @@ -119,24 +116,29 @@ def assert_success_span( 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, + expected_attr_old = { + "http.method": "GET", + "http.url": url, + "http.status_code": 200, } - attr_new = { - HTTP_REQUEST_METHOD: "GET", - URL_FULL: url, - HTTP_RESPONSE_STATUS_CODE: 200, + expected_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}, + _HTTPStabilityMode.DEFAULT: expected_attr_old, + _HTTPStabilityMode.HTTP: expected_attr_new, + _HTTPStabilityMode.HTTP_DUP: { + **expected_attr_new, + **expected_attr_old, + }, } - self.assertEqual(span.attributes, attributes.get(sem_conv_opt_in_mode)) + self.assertDictEqual( + dict(span.attributes), attributes.get(sem_conv_opt_in_mode) + ) def assert_exception_span( self, @@ -145,24 +147,29 @@ def assert_exception_span( ): span = self.assert_span() - attr_old = { - SpanAttributes.HTTP_METHOD: "GET", - SpanAttributes.HTTP_URL: url, + expected_attr_old = { + "http.method": "GET", + "http.url": url, } - attr_new = { - HTTP_REQUEST_METHOD: "GET", - URL_FULL: url, + expected_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}, + _HTTPStabilityMode.DEFAULT: expected_attr_old, + _HTTPStabilityMode.HTTP: expected_attr_new, + _HTTPStabilityMode.HTTP_DUP: { + **expected_attr_new, + **expected_attr_old, + }, } - self.assertEqual(span.attributes, attributes.get(sem_conv_opt_in_mode)) + self.assertDictEqual( + dict(span.attributes), attributes.get(sem_conv_opt_in_mode) + ) self.assertEqual( trace.status.StatusCode.ERROR, span.status.status_code ) @@ -261,9 +268,7 @@ def test_basic_not_found(self): self.assertEqual(404, response.status) span = self.assert_span() - self.assertEqual( - 404, span.attributes.get(SpanAttributes.HTTP_STATUS_CODE) - ) + self.assertEqual(404, span.attributes.get("http.status_code")) self.assertIs(trace.status.StatusCode.ERROR, span.status.status_code) def test_basic_not_found_new_semconv(self): @@ -274,7 +279,7 @@ def test_basic_not_found_new_semconv(self): 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("http.response.status_code")) self.assertIs(trace.status.StatusCode.ERROR, span.status.status_code) def test_basic_not_found_both_semconv(self): @@ -285,10 +290,8 @@ def test_basic_not_found_both_semconv(self): 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.assertEqual(404, span.attributes.get("http.response.status_code")) + self.assertEqual(404, span.attributes.get("http.status_code")) self.assertIs(trace.status.StatusCode.ERROR, span.status.status_code) @mock.patch("httpretty.http.HttpBaseClass.METHODS", ("NONSTANDARD",)) @@ -299,12 +302,8 @@ def test_nonstandard_http_method(self): 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.method"), "_OTHER") + self.assertEqual(span.attributes.get("http.status_code"), 405) @mock.patch("httpretty.http.HttpBaseClass.METHODS", ("NONSTANDARD",)) def test_nonstandard_http_method_new_semconv(self): @@ -314,11 +313,11 @@ def test_nonstandard_http_method_new_semconv(self): 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"), "_OTHER") self.assertEqual( - span.attributes.get(HTTP_REQUEST_METHOD_ORIGINAL), "NONSTANDARD" + span.attributes.get("http.request.method_original"), "NONSTANDARD" ) - self.assertEqual(span.attributes.get(HTTP_RESPONSE_STATUS_CODE), 405) + 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): @@ -328,17 +327,13 @@ def test_nonstandard_http_method_both_semconv(self): self.perform_request(self.HTTP_URL, method="NONSTANDARD") span = self.assert_span() self.assertEqual("HTTP", span.name) + self.assertEqual(span.attributes.get("http.method"), "_OTHER") + self.assertEqual(span.attributes.get("http.status_code"), 405) + self.assertEqual(span.attributes.get("http.request.method"), "_OTHER") 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" + span.attributes.get("http.request.method_original"), "NONSTANDARD" ) - self.assertEqual(span.attributes.get(HTTP_RESPONSE_STATUS_CODE), 405) + self.assertEqual(span.attributes.get("http.response.status_code"), 405) def test_basic_http_non_default_port(self): url = "http://mock:666/status/200" @@ -521,10 +516,10 @@ def test_credential_removal(self): self.assert_success_span(response, self.HTTP_URL) def test_hooks(self): - def request_hook(span, request, body, headers): + def request_hook(span, pool, request_info): span.update_name("name set from hook") - def response_hook(span, request, response): + def response_hook(span, pool, response): span.set_attribute("response_hook_attr", "value") URLLib3Instrumentor().uninstrument() @@ -541,11 +536,17 @@ def response_hook(span, request, response): self.assertEqual(span.attributes["response_hook_attr"], "value") def test_request_hook_params(self): - def request_hook(span, request, headers, body): + def request_hook( + span: Span, + _pool: urllib3.connectionpool.ConnectionPool, + request_info: RequestInfo, + ) -> None: + span.set_attribute("request_hook_method", request_info.method) + span.set_attribute("request_hook_url", request_info.url) span.set_attribute( - "request_hook_headers", json.dumps(dict(headers)) + "request_hook_headers", json.dumps(dict(request_info.headers)) ) - span.set_attribute("request_hook_body", body) + span.set_attribute("request_hook_body", request_info.body) URLLib3Instrumentor().uninstrument() URLLib3Instrumentor().instrument( @@ -564,6 +565,10 @@ def request_hook(span, request, headers, body): span = self.assert_span() + self.assertEqual(span.attributes["request_hook_method"], "POST") + self.assertEqual( + span.attributes["request_hook_url"], "http://mock/status/200" + ) self.assertIn("request_hook_headers", span.attributes) self.assertEqual( span.attributes["request_hook_headers"], json.dumps(headers) @@ -572,8 +577,12 @@ def request_hook(span, request, headers, body): self.assertEqual(span.attributes["request_hook_body"], body) def test_request_positional_body(self): - def request_hook(span, request, headers, body): - span.set_attribute("request_hook_body", body) + def request_hook( + span: Span, + _pool: urllib3.connectionpool.ConnectionPool, + request_info: RequestInfo, + ) -> None: + span.set_attribute("request_hook_body", request_info.body) URLLib3Instrumentor().uninstrument() URLLib3Instrumentor().instrument( diff --git a/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_metrics.py b/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_metrics.py index c6e9011351..959f793398 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_metrics.py +++ b/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_metrics.py @@ -26,18 +26,6 @@ _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 @@ -85,6 +73,7 @@ def test_basic_metrics(self): response = self.pool.request("GET", self.HTTP_URL) duration_ms = max(round((default_timer() - start_time) * 1000), 0) metrics = self.get_sorted_metrics() + self.assertEqual(len(metrics), 3) ( client_duration, @@ -93,13 +82,13 @@ def test_basic_metrics(self): ) = 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", + "http.status_code": 200, + "http.host": "mock", + "net.peer.port": 80, + "net.peer.name": "mock", + "http.method": "GET", + "http.flavor": "1.1", + "http.scheme": "http", } self.assertEqual(client_duration.name, "http.client.duration") @@ -154,6 +143,7 @@ def test_basic_metrics_new_semconv(self): duration_s = max(default_timer() - start_time, 0) metrics = self.get_sorted_metrics() + self.assertEqual(len(metrics), 3) ( client_request_size, client_duration, @@ -161,11 +151,11 @@ def test_basic_metrics_new_semconv(self): ) = metrics attrs_new = { - NETWORK_PROTOCOL_VERSION: "1.1", - SERVER_ADDRESS: "mock", - SERVER_PORT: 80, - HTTP_REQUEST_METHOD: "GET", - HTTP_RESPONSE_STATUS_CODE: 200, + "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 } @@ -217,6 +207,139 @@ def test_basic_metrics_new_semconv(self): ], ) + def test_basic_metrics_both_semconv(self): + start_time = default_timer() + response = self.pool.request("GET", self.HTTP_URL) + duration_s = max(default_timer() - start_time, 0) + duration = max(round(duration_s * 1000), 0) + expected_size = len(response.data) + + metrics = self.get_sorted_metrics() + self.assertEqual(len(metrics), 6) + + ( + client_duration, + client_request_body_size, + client_request_duration, + client_request_size, + client_response_body_size, + client_response_size, + ) = metrics[:6] + + 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 + } + + attrs_old = { + "http.status_code": 200, + "http.host": "mock", + "net.peer.port": 80, + "net.peer.name": "mock", + "http.method": "GET", + "http.flavor": "1.1", + "http.scheme": "http", + } + + # assert new semconv metrics + self.assertEqual( + client_request_duration.name, "http.client.request.duration" + ) + self.assert_metric_expected( + client_request_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_body_size.name, "http.client.request.body.size" + ) + self.assert_metric_expected( + client_request_body_size, + [ + self.create_histogram_data_point( + count=1, + sum_data_point=0, + max_data_point=0, + min_data_point=0, + attributes=attrs_new, + ) + ], + ) + + self.assertEqual( + client_response_body_size.name, "http.client.response.body.size" + ) + self.assert_metric_expected( + client_response_body_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, + ) + ], + ) + # assert old semconv metrics + 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, + max_data_point=duration, + min_data_point=duration, + 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, + ) + ], + ) + + 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, + ) + ], + ) + @mock.patch("httpretty.http.HttpBaseClass.METHODS", ("NONSTANDARD",)) def test_basic_metrics_nonstandard_http_method(self): httpretty.register_uri( @@ -236,13 +359,13 @@ def test_basic_metrics_nonstandard_http_method(self): ) = 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", + "http.status_code": 405, + "http.host": "mock", + "net.peer.port": 80, + "net.peer.name": "mock", + "http.method": "_OTHER", + "http.flavor": "1.1", + "http.scheme": "http", } self.assertEqual(client_duration.name, "http.client.duration") @@ -309,11 +432,11 @@ def test_basic_metrics_nonstandard_http_method_new_semconv(self): ) = metrics attrs_new = { - NETWORK_PROTOCOL_VERSION: "1.1", - SERVER_ADDRESS: "mock", - SERVER_PORT: 80, - HTTP_REQUEST_METHOD: "_OTHER", - HTTP_RESPONSE_STATUS_CODE: 405, + "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 } diff --git a/processor/opentelemetry-processor-baggage/pyproject.toml b/processor/opentelemetry-processor-baggage/pyproject.toml index 3fa80e1517..29fc2e8681 100644 --- a/processor/opentelemetry-processor-baggage/pyproject.toml +++ b/processor/opentelemetry-processor-baggage/pyproject.toml @@ -26,6 +26,7 @@ classifiers = [ ] dependencies = [ "opentelemetry-api ~= 1.5", + "opentelemetry-sdk ~= 1.5", "wrapt >= 1.0.0, < 2.0.0", ] diff --git a/processor/opentelemetry-processor-baggage/test-requirements.txt b/processor/opentelemetry-processor-baggage/test-requirements.txt index fa7ad3d793..7e4fefd157 100644 --- a/processor/opentelemetry-processor-baggage/test-requirements.txt +++ b/processor/opentelemetry-processor-baggage/test-requirements.txt @@ -1,2 +1,7 @@ - --e processor/opentelemetry-processor-baggage \ No newline at end of file +importlib_metadata==8.0.0 +typing_extensions==4.12.2 +wrapt==1.16.0 +zipp==3.19.2 +pytest==7.4.4 +Deprecated==1.2.14 +-e processor/opentelemetry-processor-baggage diff --git a/tox.ini b/tox.ini index 4ba434b29b..028db9c19f 100644 --- a/tox.ini +++ b/tox.ini @@ -247,8 +247,12 @@ envlist = lint-instrumentation-wsgi ; opentelemetry-instrumentation-grpc - py3{8,9,10,11,12}-test-instrumentation-grpc - pypy3-test-instrumentation-grpc + ; The numbers at the end of the environment names + ; below mean these dependencies are being used: + ; 0: grpcio==1.62.0 + ; 1: grpcio==1.63.0 + py3{8,9,10,11,12}-test-instrumentation-grpc-{0,1} + pypy3-test-instrumentation-grpc-{0,1} lint-instrumentation-grpc ; opentelemetry-instrumentation-sqlalchemy @@ -446,7 +450,9 @@ commands_pre = grpc: pip install opentelemetry-semantic-conventions@{env:CORE_REPO}\#egg=opentelemetry-semantic-conventions&subdirectory=opentelemetry-semantic-conventions grpc: pip install opentelemetry-sdk@{env:CORE_REPO}\#egg=opentelemetry-sdk&subdirectory=opentelemetry-sdk grpc: pip install opentelemetry-test-utils@{env:CORE_REPO}\#egg=opentelemetry-test-utils&subdirectory=tests/opentelemetry-test-utils - grpc: pip install -r {toxinidir}/instrumentation/opentelemetry-instrumentation-grpc/test-requirements.txt + grpc-0: pip install -r {toxinidir}/instrumentation/opentelemetry-instrumentation-grpc/test-requirements-0.txt + grpc-1: pip install -r {toxinidir}/instrumentation/opentelemetry-instrumentation-grpc/test-requirements-1.txt + lint-instrumentation-grpc: pip install -r {toxinidir}/instrumentation/opentelemetry-instrumentation-grpc/test-requirements-1.txt wsgi: pip install opentelemetry-api@{env:CORE_REPO}\#egg=opentelemetry-api&subdirectory=opentelemetry-api wsgi: pip install opentelemetry-semantic-conventions@{env:CORE_REPO}\#egg=opentelemetry-semantic-conventions&subdirectory=opentelemetry-semantic-conventions