From 27e4c2ce34156bd0dd967cafb79c91aa1e85f498 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Wed, 3 Apr 2024 15:20:57 -0700 Subject: [PATCH 01/36] Update .pylintrc --- .pylintrc | 1 + 1 file changed, 1 insertion(+) diff --git a/.pylintrc b/.pylintrc index 5ea4385ea0..114dadef75 100644 --- a/.pylintrc +++ b/.pylintrc @@ -81,6 +81,7 @@ disable=missing-docstring, missing-module-docstring, # temp-pylint-upgrade import-error, # needed as a workaround as reported here: https://github.com/open-telemetry/opentelemetry-python-contrib/issues/290 cyclic-import, + not-context-manager # Enable the message, report, category or checker with the given id(s). You can # either give multiple identifier separated by comma (,) or put this option From 9d4d8aca40134c50671bdf77c3488e89a094109c Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Fri, 14 Jun 2024 10:11:00 -0700 Subject: [PATCH 02/36] initial --- CHANGELOG.md | 5 ++ .../instrumentation/asgi/__init__.py | 77 +++++++++++++------ 2 files changed, 59 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e17542b104..d8ca06fd89 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,11 @@ 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)) +### Added + +- `opentelemetry-instrumentation-asgi` Implement new semantic convention opt-in with stable http semantic conventions + ([#2425](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2425)) + ### Fixed - `opentelemetry-instrumentation-httpx` Ensure httpx.get or httpx.request like methods are instrumented 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 e416e8dec2..fe774bfae5 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -205,6 +205,13 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A ClientResponseHook, ServerRequestHook, ) +from opentelemetry.instrumentation._semconv import ( + _get_schema_url, + _OpenTelemetrySemanticConventionStability, + _OpenTelemetryStabilitySignalType, + _report_new, + _report_old, +) from opentelemetry.instrumentation.asgi.version import __version__ # noqa from opentelemetry.instrumentation.propagators import ( get_global_response_propagator, @@ -215,6 +222,12 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A ) from opentelemetry.metrics import get_meter from opentelemetry.propagators.textmap import Getter, Setter +from opentelemetry.semconv._incubating.metrics.http_metrics import ( + create_http_server_active_requests, + create_http_server_request_body_size, + create_http_server_request_duration, + create_http_server_response_body_size, +) from opentelemetry.semconv.metrics import MetricInstruments from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.trace import set_span_in_context @@ -408,7 +421,9 @@ def get_default_span_details(scope: dict) -> Tuple[str, dict]: a tuple of the span name, and any attributes to attach to the span. """ path = scope.get("path", "").strip() - method = scope.get("method", "").strip() + method = sanitize_method(scope.get("method", "").strip()) + if method == "_OTHER": + method = "HTTP" if method and path: # http return f"{method} {path}", {} if path: # websocket @@ -482,13 +497,18 @@ def __init__( http_capture_headers_server_response: list[str] | None = None, http_capture_headers_sanitize_fields: list[str] | None = None, ): + # initialize semantic conventions opt-in if needed + _OpenTelemetrySemanticConventionStability._initialize() + sem_conv_opt_in_mode = _OpenTelemetrySemanticConventionStability._get_opentelemetry_stability_opt_in_mode( + _OpenTelemetryStabilitySignalType.HTTP, + ) self.app = guarantee_single_callable(app) self.tracer = ( trace.get_tracer( __name__, __version__, tracer_provider, - schema_url="https://opentelemetry.io/schemas/1.11.0", + schema_url=_get_schema_url(sem_conv_opt_in_mode), ) if tracer is None else tracer @@ -498,31 +518,42 @@ def __init__( __name__, __version__, meter_provider, - schema_url="https://opentelemetry.io/schemas/1.11.0", + schema_url=_get_schema_url(sem_conv_opt_in_mode), ) if meter is None else meter ) - self.duration_histogram = self.meter.create_histogram( - name=MetricInstruments.HTTP_SERVER_DURATION, - unit="ms", - description="Duration of HTTP client requests.", - ) - self.server_response_size_histogram = self.meter.create_histogram( - name=MetricInstruments.HTTP_SERVER_RESPONSE_SIZE, - unit="By", - description="measures the size of HTTP response messages (compressed).", - ) - self.server_request_size_histogram = self.meter.create_histogram( - name=MetricInstruments.HTTP_SERVER_REQUEST_SIZE, - unit="By", - description="Measures the size of HTTP request messages (compressed).", - ) - self.active_requests_counter = self.meter.create_up_down_counter( - name=MetricInstruments.HTTP_SERVER_ACTIVE_REQUESTS, - unit="requests", - description="measures the number of concurrent HTTP requests that are currently in-flight", - ) + self.duration_histogram = None + if _report_old(sem_conv_opt_in_mode): + self.duration_histogram = self.meter.create_histogram( + name=MetricInstruments.HTTP_SERVER_DURATION, + unit="ms", + description="Duration of HTTP client requests.", + ) + self.duration_histogram_new = None + if _report_new(sem_conv_opt_in_mode): + self.duration_histogram_new = create_http_server_request_duration(self.meter) + self.server_response_size_histogram = None + if _report_old(sem_conv_opt_in_mode): + self.server_response_size_histogram = self.meter.create_histogram( + name=MetricInstruments.HTTP_SERVER_RESPONSE_SIZE, + unit="By", + description="measures the size of HTTP response messages (compressed).", + ) + self.server_response_body_size_histogram = None + if _report_new(sem_conv_opt_in_mode): + self.server_response_body_size_histogram = create_http_server_response_body_size(self.meter) + self.server_request_size_histogram = None + if _report_old(sem_conv_opt_in_mode): + self.server_request_size_histogram = self.meter.create_histogram( + name=MetricInstruments.HTTP_SERVER_REQUEST_SIZE, + unit="By", + description="Measures the size of HTTP request messages (compressed).", + ) + self.server_request_body_size_histogram = None + if _report_new(sem_conv_opt_in_mode): + self.server_request_body_size_histogram = create_http_server_request_body_size(self.meter) + self.active_requests_counter = create_http_server_active_requests(self.meter) self.excluded_urls = excluded_urls self.default_span_details = ( default_span_details or get_default_span_details From 245a42348789da48fe232cbb1f3caa27ed6032d3 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Fri, 14 Jun 2024 10:48:01 -0700 Subject: [PATCH 03/36] method --- .../src/opentelemetry/instrumentation/asgi/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 fe774bfae5..74f038e5b8 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -243,6 +243,7 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A normalise_request_header_name, normalise_response_header_name, remove_url_credentials, + sanitize_method, ) @@ -528,7 +529,7 @@ def __init__( self.duration_histogram = self.meter.create_histogram( name=MetricInstruments.HTTP_SERVER_DURATION, unit="ms", - description="Duration of HTTP client requests.", + description="Duration of HTTP server requests.", ) self.duration_histogram_new = None if _report_new(sem_conv_opt_in_mode): From e4655e9e7d77571c4c5b5296e2604e4f6199d73c Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Mon, 17 Jun 2024 12:05:04 -0700 Subject: [PATCH 04/36] method --- .../tests/test_asgi_middleware.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py index d2fe6bc52b..9d37cece7a 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py @@ -565,6 +565,7 @@ def test_traceresponse_header(self): def test_websocket(self): self.scope = { + "method": "GET", "type": "websocket", "http_version": "1.1", "scheme": "ws", @@ -584,17 +585,17 @@ def test_websocket(self): self.assertEqual(len(span_list), 6) expected = [ { - "name": "/ websocket receive", + "name": "GET / websocket receive", "kind": trace_api.SpanKind.INTERNAL, "attributes": {"asgi.event.type": "websocket.connect"}, }, { - "name": "/ websocket send", + "name": "GET / websocket send", "kind": trace_api.SpanKind.INTERNAL, "attributes": {"asgi.event.type": "websocket.accept"}, }, { - "name": "/ websocket receive", + "name": "GET / websocket receive", "kind": trace_api.SpanKind.INTERNAL, "attributes": { "asgi.event.type": "websocket.receive", @@ -602,7 +603,7 @@ def test_websocket(self): }, }, { - "name": "/ websocket send", + "name": "GET / websocket send", "kind": trace_api.SpanKind.INTERNAL, "attributes": { "asgi.event.type": "websocket.send", @@ -610,12 +611,12 @@ def test_websocket(self): }, }, { - "name": "/ websocket receive", + "name": "GET / websocket receive", "kind": trace_api.SpanKind.INTERNAL, "attributes": {"asgi.event.type": "websocket.disconnect"}, }, { - "name": "/", + "name": "GET /", "kind": trace_api.SpanKind.SERVER, "attributes": { SpanAttributes.HTTP_SCHEME: self.scope["scheme"], From 06f1286f36e698b400fdf0b72a031f3d234050ad Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Mon, 17 Jun 2024 12:06:55 -0700 Subject: [PATCH 05/36] Update CHANGELOG.md --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 648f7954ea..a541333d53 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,8 @@ 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 + ([#2425](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2425)) ### Added From 283555e9ded852e4863b0f9327cd6aa18d4075d6 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Mon, 17 Jun 2024 12:16:28 -0700 Subject: [PATCH 06/36] test --- .../tests/test_asgi_middleware.py | 1 + 1 file changed, 1 insertion(+) diff --git a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py index 9d37cece7a..a7c23fad5b 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py @@ -628,6 +628,7 @@ def test_websocket(self): SpanAttributes.NET_PEER_IP: self.scope["client"][0], SpanAttributes.NET_PEER_PORT: self.scope["client"][1], SpanAttributes.HTTP_STATUS_CODE: 200, + SpanAttributes.HTTP_METHOD: self.scope["method"], }, }, ] From 6df07da032b64f9c789d28a2315185f09afe7706 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Thu, 20 Jun 2024 10:51:42 -0700 Subject: [PATCH 07/36] request_attrs --- .../instrumentation/asgi/__init__.py | 64 +++++++++++++++---- .../opentelemetry/instrumentation/_semconv.py | 27 +++++--- 2 files changed, 68 insertions(+), 23 deletions(-) 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 74f038e5b8..7b822ea214 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -225,10 +225,21 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A from opentelemetry.semconv._incubating.metrics.http_metrics import ( create_http_server_active_requests, create_http_server_request_body_size, - create_http_server_request_duration, create_http_server_response_body_size, ) from opentelemetry.semconv.metrics import MetricInstruments +from opentelemetry.semconv.attributes.client_attributes import CLIENT_PORT +from opentelemetry.semconv.metrics.http_metrics import ( + HTTP_SERVER_REQUEST_DURATION, +) +from opentelemetry.semconv.attributes.server_attributes import ( + SERVER_ADDRESS, + SERVER_PORT, +) +from opentelemetry.semconv.attributes.url_attributes import ( + URL_FULL, + URL_SCHEME, +) from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.trace import set_span_in_context from opentelemetry.trace.status import Status, StatusCode @@ -308,7 +319,7 @@ def set( asgi_setter = ASGISetter() -def collect_request_attributes(scope): +def collect_request_attributes(scope, sem_conv_opt_in_mode): """Collects HTTP request attributes from the ASGI scope and returns a dictionary to be used as span creation attributes.""" server_host, port, http_url = get_host_port_url_tuple(scope) @@ -317,18 +328,29 @@ def collect_request_attributes(scope): if isinstance(query_string, bytes): query_string = query_string.decode("utf8") http_url += "?" + urllib.parse.unquote(query_string) + result = {} + + if _report_old(sem_conv_opt_in_mode): + result[SpanAttributes.HTTP_SCHEME] = scope.get("scheme") + result[SpanAttributes.HTTP_HOST] = server_host + result[SpanAttributes.NET_HOST_PORT] = port + result[SpanAttributes.HTTP_FLAVOR] = scope.get("http_version") + result[SpanAttributes.HTTP_TARGET] = scope.get("path") + result[SpanAttributes.HTTP_URL] = remove_url_credentials(http_url) + + if _report_new(sem_conv_opt_in_mode): + result[URL_SCHEME] = scope.get("scheme") + result[SERVER_ADDRESS] = server_host + result[SERVER_PORT] = port + result[NETWORK_PROTOCOL_VERSION] = scope.get("http_version") + result[URL_FULL] = scope.get("path") - result = { - SpanAttributes.HTTP_SCHEME: scope.get("scheme"), - SpanAttributes.HTTP_HOST: server_host, - SpanAttributes.NET_HOST_PORT: port, - SpanAttributes.HTTP_FLAVOR: scope.get("http_version"), - SpanAttributes.HTTP_TARGET: scope.get("path"), - SpanAttributes.HTTP_URL: remove_url_credentials(http_url), - } http_method = scope.get("method") if http_method: - result[SpanAttributes.HTTP_METHOD] = http_method + if _report_old(sem_conv_opt_in_mode): + result[SpanAttributes.HTTP_METHOD] = http_method + if _report_new(sem_conv_opt_in_mode): + result[HTTP_REQUEST_METHOD] = http_method http_host_value_list = asgi_getter.get(scope, "host") if http_host_value_list: @@ -337,11 +359,20 @@ def collect_request_attributes(scope): ) http_user_agent = asgi_getter.get(scope, "user-agent") if http_user_agent: - result[SpanAttributes.HTTP_USER_AGENT] = http_user_agent[0] + if _report_old(sem_conv_opt_in_mode): + result[SpanAttributes.HTTP_USER_AGENT] = http_user_agent[0] + if _report_new(sem_conv_opt_in_mode): + result[USER_AGENT_ORIGINAL] = http_user_agent[0] if "client" in scope and scope["client"] is not None: result[SpanAttributes.NET_PEER_IP] = scope.get("client")[0] result[SpanAttributes.NET_PEER_PORT] = scope.get("client")[1] + if _report_old(sem_conv_opt_in_mode): + result[SpanAttributes.NET_PEER_IP] = scope.get("client")[0] + result[SpanAttributes.NET_PEER_PORT] = scope.get("client")[1] + if _report_new(sem_conv_opt_in_mode): + result[CLIENT_SOCKET_ADDRESS] = scope.get("client")[0] + result[CLIENT_PORT] = scope.get("client")[1] # remove None values result = {k: v for k, v in result.items() if v is not None} @@ -533,7 +564,11 @@ def __init__( ) self.duration_histogram_new = None if _report_new(sem_conv_opt_in_mode): - self.duration_histogram_new = create_http_server_request_duration(self.meter) + self.duration_histogram_new = self.meter.create_histogram( + name=HTTP_SERVER_REQUEST_DURATION, + description="Duration of HTTP server requests.", + unit="s", + ) self.server_response_size_histogram = None if _report_old(sem_conv_opt_in_mode): self.server_response_size_histogram = self.meter.create_histogram( @@ -563,6 +598,7 @@ def __init__( self.client_request_hook = client_request_hook self.client_response_hook = client_response_hook self.content_length_header = None + self._sem_conv_opt_in_mode = sem_conv_opt_in_mode # Environment variables as constructor parameters self.http_capture_headers_server_request = ( @@ -616,7 +652,7 @@ async def __call__( span_name, additional_attributes = self.default_span_details(scope) - attributes = collect_request_attributes(scope) + attributes = collect_request_attributes(scope, self._sem_conv_opt_in_mode) attributes.update(additional_attributes) span, token = _start_internal_or_server_span( tracer=self.tracer, diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py index baa06ff99b..ff2dcf2282 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py @@ -17,6 +17,10 @@ from enum import Enum from opentelemetry.instrumentation.utils import http_status_to_status_code +from opentelemetry.semconv.attributes.client_attributes import ( + CLIENT_ADDRESS, + CLIENT_PORT, +) from opentelemetry.semconv.attributes.error_attributes import ERROR_TYPE from opentelemetry.semconv.attributes.http_attributes import ( HTTP_REQUEST_METHOD, @@ -33,8 +37,13 @@ ) from opentelemetry.semconv.attributes.url_attributes import ( URL_FULL, + URL_PATH, + URL_QUERY, URL_SCHEME, ) +from opentelemetry.semconv.attributes.user_agent_attributes import ( + USER_AGENT_ORIGINAL, +) from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.trace.status import Status, StatusCode @@ -280,14 +289,14 @@ def _set_http_net_host(result, host, sem_conv_opt_in_mode): if _report_old(sem_conv_opt_in_mode): set_string_attribute(result, SpanAttributes.NET_HOST_NAME, host) if _report_new(sem_conv_opt_in_mode): - set_string_attribute(result, SpanAttributes.SERVER_ADDRESS, host) + set_string_attribute(result, SERVER_ADDRESS, host) def _set_http_net_host_port(result, port, sem_conv_opt_in_mode): if _report_old(sem_conv_opt_in_mode): set_int_attribute(result, SpanAttributes.NET_HOST_PORT, port) if _report_new(sem_conv_opt_in_mode): - set_int_attribute(result, SpanAttributes.SERVER_PORT, port) + set_int_attribute(result, SERVER_PORT, port) def _set_http_target(result, target, path, query, sem_conv_opt_in_mode): @@ -295,23 +304,23 @@ def _set_http_target(result, target, path, query, sem_conv_opt_in_mode): set_string_attribute(result, SpanAttributes.HTTP_TARGET, target) if _report_new(sem_conv_opt_in_mode): if path: - set_string_attribute(result, SpanAttributes.URL_PATH, path) + set_string_attribute(result, URL_PATH, path) if query: - set_string_attribute(result, SpanAttributes.URL_QUERY, query) + set_string_attribute(result, URL_QUERY, query) def _set_http_peer_ip(result, ip, sem_conv_opt_in_mode): if _report_old(sem_conv_opt_in_mode): set_string_attribute(result, SpanAttributes.NET_PEER_IP, ip) if _report_new(sem_conv_opt_in_mode): - set_string_attribute(result, SpanAttributes.CLIENT_ADDRESS, ip) + set_string_attribute(result, CLIENT_ADDRESS, ip) def _set_http_peer_port_server(result, port, sem_conv_opt_in_mode): if _report_old(sem_conv_opt_in_mode): set_int_attribute(result, SpanAttributes.NET_PEER_PORT, port) if _report_new(sem_conv_opt_in_mode): - set_int_attribute(result, SpanAttributes.CLIENT_PORT, port) + set_int_attribute(result, CLIENT_PORT, port) def _set_http_user_agent(result, user_agent, sem_conv_opt_in_mode): @@ -321,7 +330,7 @@ def _set_http_user_agent(result, user_agent, sem_conv_opt_in_mode): ) if _report_new(sem_conv_opt_in_mode): set_string_attribute( - result, SpanAttributes.USER_AGENT_ORIGINAL, user_agent + result, USER_AGENT_ORIGINAL, user_agent ) @@ -329,7 +338,7 @@ def _set_http_net_peer_name_server(result, name, sem_conv_opt_in_mode): if _report_old(sem_conv_opt_in_mode): set_string_attribute(result, SpanAttributes.NET_PEER_NAME, name) if _report_new(sem_conv_opt_in_mode): - set_string_attribute(result, SpanAttributes.CLIENT_ADDRESS, name) + set_string_attribute(result, CLIENT_ADDRESS, name) def _set_http_flavor_version(result, version, sem_conv_opt_in_mode): @@ -337,7 +346,7 @@ def _set_http_flavor_version(result, version, sem_conv_opt_in_mode): set_string_attribute(result, SpanAttributes.HTTP_FLAVOR, version) if _report_new(sem_conv_opt_in_mode): set_string_attribute( - result, SpanAttributes.NETWORK_PROTOCOL_VERSION, version + result, NETWORK_PROTOCOL_VERSION, version ) From a66696838484e34d109b0e6971747d1c3fc7826c Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Thu, 20 Jun 2024 13:56:27 -0700 Subject: [PATCH 08/36] imports --- .../instrumentation/asgi/__init__.py | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) 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 7b822ea214..3e7430251b 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -207,6 +207,7 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A ) from opentelemetry.instrumentation._semconv import ( _get_schema_url, + _HTTPStabilityMode, _OpenTelemetrySemanticConventionStability, _OpenTelemetryStabilitySignalType, _report_new, @@ -228,10 +229,19 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A create_http_server_response_body_size, ) from opentelemetry.semconv.metrics import MetricInstruments -from opentelemetry.semconv.attributes.client_attributes import CLIENT_PORT from opentelemetry.semconv.metrics.http_metrics import ( HTTP_SERVER_REQUEST_DURATION, ) +from opentelemetry.semconv.attributes.client_attributes import ( + CLIENT_ADDRESS, + CLIENT_PORT, +) +from opentelemetry.semconv.attributes.http_attributes import ( + HTTP_REQUEST_METHOD, +) +from opentelemetry.semconv.attributes.network_attributes import ( + NETWORK_PROTOCOL_VERSION +) from opentelemetry.semconv.attributes.server_attributes import ( SERVER_ADDRESS, SERVER_PORT, @@ -240,6 +250,9 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A URL_FULL, URL_SCHEME, ) +from opentelemetry.semconv.attributes.user_agent_attributes import ( + USER_AGENT_ORIGINAL, +) from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.trace import set_span_in_context from opentelemetry.trace.status import Status, StatusCode @@ -319,7 +332,7 @@ def set( asgi_setter = ASGISetter() -def collect_request_attributes(scope, sem_conv_opt_in_mode): +def collect_request_attributes(scope, sem_conv_opt_in_mode=_HTTPStabilityMode.DEFAULT): """Collects HTTP request attributes from the ASGI scope and returns a dictionary to be used as span creation attributes.""" server_host, port, http_url = get_host_port_url_tuple(scope) @@ -371,7 +384,7 @@ def collect_request_attributes(scope, sem_conv_opt_in_mode): result[SpanAttributes.NET_PEER_IP] = scope.get("client")[0] result[SpanAttributes.NET_PEER_PORT] = scope.get("client")[1] if _report_new(sem_conv_opt_in_mode): - result[CLIENT_SOCKET_ADDRESS] = scope.get("client")[0] + result[CLIENT_ADDRESS] = scope.get("client")[0] result[CLIENT_PORT] = scope.get("client")[1] # remove None values From 69453d820dd1ed928f56ba296dd3201acd50d001 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Thu, 20 Jun 2024 15:25:52 -0700 Subject: [PATCH 09/36] duration --- .../instrumentation/asgi/__init__.py | 166 ++++++++++-------- .../opentelemetry/instrumentation/_semconv.py | 3 + 2 files changed, 99 insertions(+), 70 deletions(-) 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 3e7430251b..8f100c7df7 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -206,12 +206,28 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A ServerRequestHook, ) from opentelemetry.instrumentation._semconv import ( + _filter_semconv_active_request_count_attr, + _filter_semconv_duration_attrs, _get_schema_url, _HTTPStabilityMode, _OpenTelemetrySemanticConventionStability, _OpenTelemetryStabilitySignalType, _report_new, _report_old, + _server_active_requests_count_attrs_new, + _server_active_requests_count_attrs_old, + _server_duration_attrs_new, + _server_duration_attrs_old, + _set_http_host, + _set_http_flavor_version, + _set_http_method, + _set_http_net_host_port, + _set_http_peer_ip, + _set_http_peer_port_server, + _set_http_scheme, + _set_http_target, + _set_http_url, + _set_http_user_agent, ) from opentelemetry.instrumentation.asgi.version import __version__ # noqa from opentelemetry.instrumentation.propagators import ( @@ -232,37 +248,15 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A from opentelemetry.semconv.metrics.http_metrics import ( HTTP_SERVER_REQUEST_DURATION, ) -from opentelemetry.semconv.attributes.client_attributes import ( - CLIENT_ADDRESS, - CLIENT_PORT, -) -from opentelemetry.semconv.attributes.http_attributes import ( - HTTP_REQUEST_METHOD, -) -from opentelemetry.semconv.attributes.network_attributes import ( - NETWORK_PROTOCOL_VERSION -) -from opentelemetry.semconv.attributes.server_attributes import ( - SERVER_ADDRESS, - SERVER_PORT, -) -from opentelemetry.semconv.attributes.url_attributes import ( - URL_FULL, - URL_SCHEME, -) -from opentelemetry.semconv.attributes.user_agent_attributes import ( - USER_AGENT_ORIGINAL, -) from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.trace import set_span_in_context from opentelemetry.trace.status import Status, StatusCode from opentelemetry.util.http import ( + _parse_url_query, OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SANITIZE_FIELDS, 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, normalise_request_header_name, normalise_response_header_name, @@ -343,49 +337,41 @@ def collect_request_attributes(scope, sem_conv_opt_in_mode=_HTTPStabilityMode.DE http_url += "?" + urllib.parse.unquote(query_string) result = {} - if _report_old(sem_conv_opt_in_mode): - result[SpanAttributes.HTTP_SCHEME] = scope.get("scheme") - result[SpanAttributes.HTTP_HOST] = server_host - result[SpanAttributes.NET_HOST_PORT] = port - result[SpanAttributes.HTTP_FLAVOR] = scope.get("http_version") - result[SpanAttributes.HTTP_TARGET] = scope.get("path") - result[SpanAttributes.HTTP_URL] = remove_url_credentials(http_url) - - if _report_new(sem_conv_opt_in_mode): - result[URL_SCHEME] = scope.get("scheme") - result[SERVER_ADDRESS] = server_host - result[SERVER_PORT] = port - result[NETWORK_PROTOCOL_VERSION] = scope.get("http_version") - result[URL_FULL] = scope.get("path") + scheme = scope.get("scheme") + if scheme: + _set_http_scheme(result, scheme, sem_conv_opt_in_mode) + if server_host: + _set_http_host(result, server_host, sem_conv_opt_in_mode) + if port: + _set_http_net_host_port(result, port, sem_conv_opt_in_mode) + flavor = scope.get("http_version") + if flavor: + _set_http_flavor_version(result, flavor, sem_conv_opt_in_mode) + path = scope.get("path") + if path: + if query_string: + target = path + query_string + _set_http_target(result, target, path, query_string, sem_conv_opt_in_mode) + if http_url: + _set_http_url(result, remove_url_credentials(http_url), sem_conv_opt_in_mode) http_method = scope.get("method") if http_method: - if _report_old(sem_conv_opt_in_mode): - result[SpanAttributes.HTTP_METHOD] = http_method - if _report_new(sem_conv_opt_in_mode): - result[HTTP_REQUEST_METHOD] = http_method + _set_http_method(result, http_method, sem_conv_opt_in_mode) http_host_value_list = asgi_getter.get(scope, "host") if http_host_value_list: - result[SpanAttributes.HTTP_SERVER_NAME] = ",".join( - http_host_value_list - ) + if _report_old(sem_conv_opt_in_mode): + result[SpanAttributes.HTTP_SERVER_NAME] = ",".join( + http_host_value_list + ) http_user_agent = asgi_getter.get(scope, "user-agent") if http_user_agent: - if _report_old(sem_conv_opt_in_mode): - result[SpanAttributes.HTTP_USER_AGENT] = http_user_agent[0] - if _report_new(sem_conv_opt_in_mode): - result[USER_AGENT_ORIGINAL] = http_user_agent[0] + _set_http_user_agent(result, http_user_agent[0], sem_conv_opt_in_mode) if "client" in scope and scope["client"] is not None: - result[SpanAttributes.NET_PEER_IP] = scope.get("client")[0] - result[SpanAttributes.NET_PEER_PORT] = scope.get("client")[1] - if _report_old(sem_conv_opt_in_mode): - result[SpanAttributes.NET_PEER_IP] = scope.get("client")[0] - result[SpanAttributes.NET_PEER_PORT] = scope.get("client")[1] - if _report_new(sem_conv_opt_in_mode): - result[CLIENT_ADDRESS] = scope.get("client")[0] - result[CLIENT_PORT] = scope.get("client")[1] + _set_http_peer_ip(result, scope.get("client")[0], sem_conv_opt_in_mode) + _set_http_peer_port_server(result, scope.get("client")[1], sem_conv_opt_in_mode) # remove None values result = {k: v for k, v in result.items() if v is not None} @@ -568,9 +554,9 @@ def __init__( if meter is None else meter ) - self.duration_histogram = None + self.duration_histogram_old = None if _report_old(sem_conv_opt_in_mode): - self.duration_histogram = self.meter.create_histogram( + self.duration_histogram_old = self.meter.create_histogram( name=MetricInstruments.HTTP_SERVER_DURATION, unit="ms", description="Duration of HTTP server requests.", @@ -678,7 +664,6 @@ async def __call__( active_requests_count_attrs = _parse_active_request_count_attrs( attributes ) - duration_attrs = _parse_duration_attrs(attributes) if scope["type"] == "http": self.active_requests_counter.add(1, active_requests_count_attrs) @@ -714,7 +699,7 @@ async def __call__( span_name, scope, send, - duration_attrs, + attributes, ) await self.app(scope, otel_receive, otel_send) @@ -722,16 +707,28 @@ async def __call__( if scope["type"] == "http": target = _collect_target_attribute(scope) if target: - duration_attrs[SpanAttributes.HTTP_TARGET] = target + path, query = _parse_url_query(target) + _set_http_target(attributes, target, path, query, self._sem_conv_opt_in_mode) duration = max(round((default_timer() - start) * 1000), 0) - self.duration_histogram.record(duration, duration_attrs) + duration_attrs_old = _parse_duration_attrs(attributes, _HTTPStabilityMode.DEFAULT) + duration_attrs_new = _parse_duration_attrs(attributes, _HTTPStabilityMode.HTTP) + if self.duration_histogram_old: + self.duration_histogram_old.record(duration, duration_attrs_old) + if self.duration_histogram_new: + self.duration_histogram_new.record(duration, duration_attrs_new) self.active_requests_counter.add( -1, active_requests_count_attrs ) if self.content_length_header: - self.server_response_size_histogram.record( - self.content_length_header, duration_attrs - ) + if self.server_response_size_histogram: + self.server_response_size_histogram.record( + self.content_length_header, duration_attrs_old + ) + if self.server_response_body_size_histogram: + self.server_response_body_size_histogram.record( + self.content_length_header, duration_attrs_new + ) + request_size = asgi_getter.get(scope, "content-length") if request_size: try: @@ -739,9 +736,14 @@ async def __call__( except ValueError: pass else: - self.server_request_size_histogram.record( - request_size_amount, duration_attrs - ) + if self.server_request_size_histogram: + self.server_request_size_histogram.record( + request_size_amount, duration_attrs_old + ) + if self.server_request_body_size_histogram: + self.server_request_size_histogram.record( + request_size_amount, duration_attrs_new + ) if token: context.detach(token) if span.is_recording(): @@ -769,7 +771,7 @@ async def otel_receive(): return otel_receive def _get_otel_send( - self, server_span, server_span_name, scope, send, duration_attrs + self, server_span, server_span_name, scope, send, duration_attrs, ): expecting_trailers = False @@ -787,9 +789,11 @@ async def otel_send(message: dict[str, Any]): duration_attrs[SpanAttributes.HTTP_STATUS_CODE] = ( status_code ) + duration_attrs[SpanAttributes.HTTP_RESPONSE_STATUS_CODE] = ( + status_code + ) set_status_code(server_span, status_code) set_status_code(send_span, status_code) - expecting_trailers = message.get("trailers", False) elif message["type"] == "websocket.send": set_status_code(server_span, 200) @@ -846,3 +850,25 @@ async def otel_send(message: dict[str, Any]): server_span.end() return otel_send + + +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/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py index ff2dcf2282..a81073b2d6 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py @@ -73,6 +73,7 @@ SpanAttributes.HTTP_HOST, SpanAttributes.HTTP_SCHEME, SpanAttributes.HTTP_STATUS_CODE, + SpanAttributes.HTTP_TARGET, SpanAttributes.HTTP_FLAVOR, SpanAttributes.HTTP_SERVER_NAME, SpanAttributes.NET_HOST_NAME, @@ -85,6 +86,8 @@ HTTP_RESPONSE_STATUS_CODE, HTTP_ROUTE, NETWORK_PROTOCOL_VERSION, + URL_PATH, + URL_QUERY, URL_SCHEME, ] From 9c6404198f581d5683a936d3d09072bd7baf0372 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Thu, 20 Jun 2024 15:32:08 -0700 Subject: [PATCH 10/36] fix --- .../src/opentelemetry/instrumentation/asgi/__init__.py | 1 + 1 file changed, 1 insertion(+) 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 8f100c7df7..45a5e3eb86 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -349,6 +349,7 @@ def collect_request_attributes(scope, sem_conv_opt_in_mode=_HTTPStabilityMode.DE _set_http_flavor_version(result, flavor, sem_conv_opt_in_mode) path = scope.get("path") if path: + target = path if query_string: target = path + query_string _set_http_target(result, target, path, query_string, sem_conv_opt_in_mode) From 6bc08156233916ad2fe9dbdba794182cbbec1814 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Fri, 21 Jun 2024 09:47:10 -0700 Subject: [PATCH 11/36] method --- .../src/opentelemetry/instrumentation/asgi/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 45a5e3eb86..0dc2ca655c 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -356,9 +356,9 @@ def collect_request_attributes(scope, sem_conv_opt_in_mode=_HTTPStabilityMode.DE if http_url: _set_http_url(result, remove_url_credentials(http_url), sem_conv_opt_in_mode) - http_method = scope.get("method") + http_method = scope.get("method", "") if http_method: - _set_http_method(result, http_method, sem_conv_opt_in_mode) + _set_http_method(result, http_method, sanitize_method(http_method), sem_conv_opt_in_mode) http_host_value_list = asgi_getter.get(scope, "host") if http_host_value_list: From 481581ee87fe8677b0d0ca748f84e3bdd64c3e5a Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Fri, 21 Jun 2024 10:07:17 -0700 Subject: [PATCH 12/36] test --- .../tests/test_asgi_middleware.py | 40 +++++++++++++------ 1 file changed, 28 insertions(+), 12 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py index a7c23fad5b..94e2da7e23 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py @@ -23,6 +23,12 @@ import opentelemetry.instrumentation.asgi as otel_asgi from opentelemetry import trace as trace_api +from opentelemetry.instrumentation._semconv import ( + _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 ( TraceResponsePropagator, get_global_response_propagator, @@ -40,22 +46,32 @@ ) from opentelemetry.test.test_base import TestBase from opentelemetry.trace import SpanKind, format_span_id, format_trace_id -from opentelemetry.util.http import ( - _active_requests_count_attrs, - _duration_attrs, -) -_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, - "http.server.response.size": _duration_attrs, - "http.server.request.size": _duration_attrs, +_expected_metric_names_new = [ + "http.server.active_requests", + "http.server.request.duration", + "http.server.response.body.size", + "http.server.request.body.size", +] + +_recommended_attrs_old = { + "http.server.active_requests": _server_active_requests_count_attrs_old, + "http.server.duration": _server_duration_attrs_old, + "http.server.response.size": _server_duration_attrs_old, + "http.server.request.size": _server_duration_attrs_old, +} + +_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, } _SIMULATED_BACKGROUND_TASK_EXECUTION_TIME_S = 0.01 @@ -739,7 +755,7 @@ def test_asgi_metrics(self): "opentelemetry.instrumentation.asgi", ) 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: @@ -750,7 +766,7 @@ def test_asgi_metrics(self): number_data_point_seen = True for attr in point.attributes: self.assertIn( - attr, _recommended_attrs[metric.name] + attr, _recommended_attrs_old[metric.name] ) self.assertTrue(number_data_point_seen and histogram_data_point_seen) From dba3963af71bc96a60e9e4321e8853a1b7c0730b Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Fri, 21 Jun 2024 10:28:44 -0700 Subject: [PATCH 13/36] test --- .../src/opentelemetry/instrumentation/asgi/__init__.py | 2 -- .../tests/test_asgi_middleware.py | 2 ++ 2 files changed, 2 insertions(+), 2 deletions(-) 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 0dc2ca655c..42234b6d53 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -350,8 +350,6 @@ def collect_request_attributes(scope, sem_conv_opt_in_mode=_HTTPStabilityMode.DE path = scope.get("path") if path: target = path - if query_string: - target = path + query_string _set_http_target(result, target, path, query_string, sem_conv_opt_in_mode) if http_url: _set_http_url(result, remove_url_credentials(http_url), sem_conv_opt_in_mode) diff --git a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py index 94e2da7e23..82b4c8b64a 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py @@ -789,6 +789,8 @@ def test_basic_metric_success(self): "http.host": "127.0.0.1", "http.scheme": "http", "http.flavor": "1.0", + "http.server.name" + "net.host.port": 80, } metrics_list = self.memory_metrics_reader.get_metrics_data() # pylint: disable=too-many-nested-blocks From 63af4ab1a75d737b146f15ece46fc5038e6675a5 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Fri, 21 Jun 2024 10:34:59 -0700 Subject: [PATCH 14/36] test --- .../tests/test_asgi_middleware.py | 1 - 1 file changed, 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py index 82b4c8b64a..86934cc94c 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py @@ -789,7 +789,6 @@ def test_basic_metric_success(self): "http.host": "127.0.0.1", "http.scheme": "http", "http.flavor": "1.0", - "http.server.name" "net.host.port": 80, } metrics_list = self.memory_metrics_reader.get_metrics_data() From 531a78bfc5b630d6754e53b4dd17fddb114bb179 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Fri, 21 Jun 2024 10:48:40 -0700 Subject: [PATCH 15/36] target --- .../src/opentelemetry/instrumentation/_semconv.py | 1 - 1 file changed, 1 deletion(-) diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py index a81073b2d6..78e632e9bb 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py @@ -73,7 +73,6 @@ SpanAttributes.HTTP_HOST, SpanAttributes.HTTP_SCHEME, SpanAttributes.HTTP_STATUS_CODE, - SpanAttributes.HTTP_TARGET, SpanAttributes.HTTP_FLAVOR, SpanAttributes.HTTP_SERVER_NAME, SpanAttributes.NET_HOST_NAME, From ba94680636cd538869334ddeab5e214c9ab8861e Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Fri, 21 Jun 2024 11:08:08 -0700 Subject: [PATCH 16/36] tests --- .../tests/test_asgi_middleware.py | 1 + .../src/opentelemetry/instrumentation/_semconv.py | 1 + 2 files changed, 2 insertions(+) diff --git a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py index 86934cc94c..baa5818223 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py @@ -783,6 +783,7 @@ def test_basic_metric_success(self): "http.flavor": "1.0", "net.host.port": 80, "http.status_code": 200, + "http.target": "/", } expected_requests_count_attributes = { "http.method": "GET", diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py index 78e632e9bb..a81073b2d6 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py @@ -73,6 +73,7 @@ SpanAttributes.HTTP_HOST, SpanAttributes.HTTP_SCHEME, SpanAttributes.HTTP_STATUS_CODE, + SpanAttributes.HTTP_TARGET, SpanAttributes.HTTP_FLAVOR, SpanAttributes.HTTP_SERVER_NAME, SpanAttributes.NET_HOST_NAME, From c3a5263233d20da73fe7205002edcc6c9f4b80d2 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Fri, 21 Jun 2024 11:24:30 -0700 Subject: [PATCH 17/36] test --- .../src/opentelemetry/instrumentation/asgi/__init__.py | 2 ++ .../tests/test_asgi_middleware.py | 1 - .../src/opentelemetry/instrumentation/_semconv.py | 1 - 3 files changed, 2 insertions(+), 2 deletions(-) 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 42234b6d53..f3830e563a 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -710,6 +710,8 @@ async def __call__( _set_http_target(attributes, target, path, query, self._sem_conv_opt_in_mode) duration = max(round((default_timer() - start) * 1000), 0) duration_attrs_old = _parse_duration_attrs(attributes, _HTTPStabilityMode.DEFAULT) + if target: + duration_attrs_old[SpanAttributes.HTTP_TARGET] = target duration_attrs_new = _parse_duration_attrs(attributes, _HTTPStabilityMode.HTTP) if self.duration_histogram_old: self.duration_histogram_old.record(duration, duration_attrs_old) diff --git a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py index baa5818223..86934cc94c 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py @@ -783,7 +783,6 @@ def test_basic_metric_success(self): "http.flavor": "1.0", "net.host.port": 80, "http.status_code": 200, - "http.target": "/", } expected_requests_count_attributes = { "http.method": "GET", diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py index a81073b2d6..78e632e9bb 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py @@ -73,7 +73,6 @@ SpanAttributes.HTTP_HOST, SpanAttributes.HTTP_SCHEME, SpanAttributes.HTTP_STATUS_CODE, - SpanAttributes.HTTP_TARGET, SpanAttributes.HTTP_FLAVOR, SpanAttributes.HTTP_SERVER_NAME, SpanAttributes.NET_HOST_NAME, From 4ef007ad8fc519ca703acc928097c62ee6200aa3 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Fri, 21 Jun 2024 11:39:11 -0700 Subject: [PATCH 18/36] net.host.port remove --- .../tests/test_asgi_middleware.py | 1 - .../src/opentelemetry/instrumentation/_semconv.py | 5 +++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py index 86934cc94c..94e2da7e23 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py @@ -789,7 +789,6 @@ def test_basic_metric_success(self): "http.host": "127.0.0.1", "http.scheme": "http", "http.flavor": "1.0", - "net.host.port": 80, } metrics_list = self.memory_metrics_reader.get_metrics_data() # pylint: disable=too-many-nested-blocks diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py index 78e632e9bb..7c79933335 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py @@ -47,6 +47,8 @@ from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.trace.status import Status, StatusCode +# These lists represent attributes for metrics that are currently supported + _client_duration_attrs_old = [ SpanAttributes.HTTP_STATUS_CODE, SpanAttributes.HTTP_HOST, @@ -96,13 +98,12 @@ SpanAttributes.HTTP_SCHEME, SpanAttributes.HTTP_FLAVOR, SpanAttributes.HTTP_SERVER_NAME, - SpanAttributes.NET_HOST_NAME, - SpanAttributes.NET_HOST_PORT, ] _server_active_requests_count_attrs_new = [ HTTP_REQUEST_METHOD, URL_SCHEME, + # TODO: Support SERVER_ADDRESS AND SERVER_PORT ] OTEL_SEMCONV_STABILITY_OPT_IN = "OTEL_SEMCONV_STABILITY_OPT_IN" From e9c4da3452ab257f86a355b3bbaac02baa1cf4be Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Fri, 21 Jun 2024 11:54:34 -0700 Subject: [PATCH 19/36] remove net.host.port from tests --- .../opentelemetry-instrumentation-falcon/tests/test_falcon.py | 2 -- .../tests/test_programmatic.py | 4 ---- .../tests/test_automatic.py | 2 -- 3 files changed, 8 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py b/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py index bf7f1d4f49..dbc2512ca0 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py @@ -345,8 +345,6 @@ def test_falcon_metric_values(self): "http.scheme": "http", "http.flavor": "1.1", "http.server_name": "falconframework.org", - "net.host.name": "falconframework.org", - "net.host.port": 80, } start = default_timer() self.client().simulate_get("/hello/756") diff --git a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py index f50d3245a0..da2cfcde21 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py +++ b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py @@ -584,8 +584,6 @@ def test_basic_metric_success(self): "http.scheme": "http", "http.flavor": "1.1", "http.server_name": "localhost", - "net.host.name": "localhost", - "net.host.port": 80, } self._assert_basic_metric( expected_duration_attributes, @@ -627,8 +625,6 @@ def test_basic_metric_nonstandard_http_method_success(self): "http.scheme": "http", "http.flavor": "1.1", "http.server_name": "localhost", - "net.host.name": "localhost", - "net.host.port": 80, } self._assert_basic_metric( expected_duration_attributes, diff --git a/instrumentation/opentelemetry-instrumentation-pyramid/tests/test_automatic.py b/instrumentation/opentelemetry-instrumentation-pyramid/tests/test_automatic.py index 7b48e16e17..cba99a587b 100644 --- a/instrumentation/opentelemetry-instrumentation-pyramid/tests/test_automatic.py +++ b/instrumentation/opentelemetry-instrumentation-pyramid/tests/test_automatic.py @@ -223,8 +223,6 @@ def test_basic_metric_success(self): "http.scheme": "http", "http.flavor": "1.1", "http.server_name": "localhost", - "net.host.name": "localhost", - "net.host.port": 80, } metrics_list = self.memory_metrics_reader.get_metrics_data() for metric in ( From 3239277b91affc4835eaf7acc34789126f4b20d0 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Fri, 21 Jun 2024 12:22:19 -0700 Subject: [PATCH 20/36] remove url.path and url.query from duration --- .../src/opentelemetry/instrumentation/_semconv.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py index 7c79933335..17cbdfff7d 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py @@ -87,8 +87,6 @@ HTTP_RESPONSE_STATUS_CODE, HTTP_ROUTE, NETWORK_PROTOCOL_VERSION, - URL_PATH, - URL_QUERY, URL_SCHEME, ] From 67f928282ed6984b6731d36fbfab7f7a1d1d9f65 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Mon, 24 Jun 2024 09:53:28 -0700 Subject: [PATCH 21/36] set status --- .../instrumentation/asgi/__init__.py | 64 +++++++++++++------ .../instrumentation/wsgi/__init__.py | 2 +- .../opentelemetry/instrumentation/_semconv.py | 2 +- 3 files changed, 45 insertions(+), 23 deletions(-) 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 f3830e563a..25621f1f14 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -228,6 +228,7 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A _set_http_target, _set_http_url, _set_http_user_agent, + _set_status, ) from opentelemetry.instrumentation.asgi.version import __version__ # noqa from opentelemetry.instrumentation.propagators import ( @@ -419,25 +420,31 @@ def get_host_port_url_tuple(scope): return server_host, port, http_url -def set_status_code(span, status_code): +def set_status_code( + span, + status_code, + metric_attributes=None, + 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 = repr(status_code) + + status_code = 0 try: status_code = int(status_code) except ValueError: - span.set_status( - Status( - StatusCode.ERROR, - "Non-integer HTTP status: " + repr(status_code), - ) - ) - else: - span.set_attribute(SpanAttributes.HTTP_STATUS_CODE, status_code) - span.set_status( - Status(http_status_to_status_code(status_code, server_span=True)) - ) - + status_code = -1 + if metric_attributes is None: + metric_attributes = {} + _set_status( + span, + metric_attributes, + status_code, + status_code_str, + sem_conv_opt_in_mode, + ) def get_default_span_details(scope: dict) -> Tuple[str, dict]: """ @@ -787,18 +794,33 @@ async def otel_send(message: dict[str, Any]): if send_span.is_recording(): if message["type"] == "http.response.start": status_code = message["status"] - duration_attrs[SpanAttributes.HTTP_STATUS_CODE] = ( - status_code + # We record metrics only once + set_status_code( + server_span, + status_code, + duration_attrs, + self._sem_conv_opt_in_mode, ) - duration_attrs[SpanAttributes.HTTP_RESPONSE_STATUS_CODE] = ( - status_code + set_status_code( + send_span, + status_code, + None, + self._sem_conv_opt_in_mode, ) - set_status_code(server_span, status_code) - set_status_code(send_span, status_code) expecting_trailers = message.get("trailers", False) elif message["type"] == "websocket.send": - set_status_code(server_span, 200) - set_status_code(send_span, 200) + 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() 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 6a1883fa7e..d75147d6aa 100644 --- a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py @@ -494,8 +494,8 @@ def add_response_attributes( _set_status( span, duration_attrs, - status_code_str, status_code, + status_code_str, sem_conv_opt_in_mode, ) diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py index 17cbdfff7d..c5909f4260 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py @@ -354,8 +354,8 @@ def _set_http_flavor_version(result, version, sem_conv_opt_in_mode): def _set_status( span, metrics_attributes, - status_code_str, status_code, + status_code_str, sem_conv_opt_in_mode, ): if status_code < 0: From 8456fbde28d590e9b5f88b15e81513336e1a60e3 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Mon, 24 Jun 2024 10:03:21 -0700 Subject: [PATCH 22/36] Update __init__.py --- .../src/opentelemetry/instrumentation/asgi/__init__.py | 1 - 1 file changed, 1 deletion(-) 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 25621f1f14..39eda79fdd 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -431,7 +431,6 @@ def set_status_code( return status_code_str = repr(status_code) - status_code = 0 try: status_code = int(status_code) except ValueError: From f898374a79e2bb6282622d7fa4716fb7a19ddc5e Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Wed, 26 Jun 2024 09:36:44 -0700 Subject: [PATCH 23/36] tests --- .../instrumentation/asgi/__init__.py | 87 +- .../tests/test_asgi_middleware.py | 839 +++++++++++++++++- .../opentelemetry/instrumentation/_semconv.py | 16 +- 3 files changed, 876 insertions(+), 66 deletions(-) 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 39eda79fdd..68900991e5 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -327,7 +327,9 @@ def set( asgi_setter = ASGISetter() -def collect_request_attributes(scope, sem_conv_opt_in_mode=_HTTPStabilityMode.DEFAULT): +def collect_request_attributes( + scope, sem_conv_opt_in_mode=_HTTPStabilityMode.DEFAULT +): """Collects HTTP request attributes from the ASGI scope and returns a dictionary to be used as span creation attributes.""" server_host, port, http_url = get_host_port_url_tuple(scope) @@ -351,13 +353,22 @@ def collect_request_attributes(scope, sem_conv_opt_in_mode=_HTTPStabilityMode.DE path = scope.get("path") if path: target = path - _set_http_target(result, target, path, query_string, sem_conv_opt_in_mode) + _set_http_target( + result, target, path, query_string, sem_conv_opt_in_mode + ) if http_url: - _set_http_url(result, remove_url_credentials(http_url), sem_conv_opt_in_mode) + _set_http_url( + result, remove_url_credentials(http_url), sem_conv_opt_in_mode + ) http_method = scope.get("method", "") if http_method: - _set_http_method(result, http_method, sanitize_method(http_method), sem_conv_opt_in_mode) + _set_http_method( + result, + http_method, + sanitize_method(http_method), + sem_conv_opt_in_mode, + ) http_host_value_list = asgi_getter.get(scope, "host") if http_host_value_list: @@ -371,7 +382,9 @@ def collect_request_attributes(scope, sem_conv_opt_in_mode=_HTTPStabilityMode.DE if "client" in scope and scope["client"] is not None: _set_http_peer_ip(result, scope.get("client")[0], sem_conv_opt_in_mode) - _set_http_peer_port_server(result, scope.get("client")[1], sem_conv_opt_in_mode) + _set_http_peer_port_server( + result, scope.get("client")[1], sem_conv_opt_in_mode + ) # remove None values result = {k: v for k, v in result.items() if v is not None} @@ -430,7 +443,7 @@ def set_status_code( if not span.is_recording(): return status_code_str = repr(status_code) - + try: status_code = int(status_code) except ValueError: @@ -445,6 +458,7 @@ def set_status_code( sem_conv_opt_in_mode, ) + def get_default_span_details(scope: dict) -> Tuple[str, dict]: """ Default span name is the HTTP method and URL path, or just the method. @@ -582,7 +596,9 @@ def __init__( ) self.server_response_body_size_histogram = None if _report_new(sem_conv_opt_in_mode): - self.server_response_body_size_histogram = create_http_server_response_body_size(self.meter) + self.server_response_body_size_histogram = ( + create_http_server_response_body_size(self.meter) + ) self.server_request_size_histogram = None if _report_old(sem_conv_opt_in_mode): self.server_request_size_histogram = self.meter.create_histogram( @@ -592,8 +608,12 @@ def __init__( ) self.server_request_body_size_histogram = None if _report_new(sem_conv_opt_in_mode): - self.server_request_body_size_histogram = create_http_server_request_body_size(self.meter) - self.active_requests_counter = create_http_server_active_requests(self.meter) + self.server_request_body_size_histogram = ( + create_http_server_request_body_size(self.meter) + ) + self.active_requests_counter = create_http_server_active_requests( + self.meter + ) self.excluded_urls = excluded_urls self.default_span_details = ( default_span_details or get_default_span_details @@ -656,7 +676,9 @@ async def __call__( span_name, additional_attributes = self.default_span_details(scope) - attributes = collect_request_attributes(scope, self._sem_conv_opt_in_mode) + attributes = collect_request_attributes( + scope, self._sem_conv_opt_in_mode + ) attributes.update(additional_attributes) span, token = _start_internal_or_server_span( tracer=self.tracer, @@ -667,7 +689,8 @@ async def __call__( attributes=attributes, ) active_requests_count_attrs = _parse_active_request_count_attrs( - attributes + attributes, + self._sem_conv_opt_in_mode, ) if scope["type"] == "http": @@ -713,19 +736,35 @@ async def __call__( target = _collect_target_attribute(scope) if target: path, query = _parse_url_query(target) - _set_http_target(attributes, target, path, query, self._sem_conv_opt_in_mode) + _set_http_target( + attributes, + target, + path, + query, + self._sem_conv_opt_in_mode, + ) duration = max(round((default_timer() - start) * 1000), 0) - duration_attrs_old = _parse_duration_attrs(attributes, _HTTPStabilityMode.DEFAULT) + duration_attrs_old = _parse_duration_attrs( + attributes, _HTTPStabilityMode.DEFAULT + ) if target: duration_attrs_old[SpanAttributes.HTTP_TARGET] = target - duration_attrs_new = _parse_duration_attrs(attributes, _HTTPStabilityMode.HTTP) + duration_attrs_new = _parse_duration_attrs( + attributes, _HTTPStabilityMode.HTTP + ) if self.duration_histogram_old: - self.duration_histogram_old.record(duration, duration_attrs_old) + self.duration_histogram_old.record( + duration, duration_attrs_old + ) if self.duration_histogram_new: - self.duration_histogram_new.record(duration, duration_attrs_new) + self.duration_histogram_new.record( + duration, duration_attrs_new + ) self.active_requests_counter.add( -1, active_requests_count_attrs ) + print(attributes) + print(active_requests_count_attrs) if self.content_length_header: if self.server_response_size_histogram: self.server_response_size_histogram.record( @@ -748,7 +787,7 @@ async def __call__( request_size_amount, duration_attrs_old ) if self.server_request_body_size_histogram: - self.server_request_size_histogram.record( + self.server_request_body_size_histogram.record( request_size_amount, duration_attrs_new ) if token: @@ -769,7 +808,12 @@ async def otel_receive(): self.client_request_hook(receive_span, scope, message) if receive_span.is_recording(): if message["type"] == "websocket.receive": - set_status_code(receive_span, 200) + set_status_code( + receive_span, + 200, + None, + self._sem_conv_opt_in_mode, + ) receive_span.set_attribute( "asgi.event.type", message["type"] ) @@ -778,7 +822,12 @@ async def otel_receive(): return otel_receive def _get_otel_send( - self, server_span, server_span_name, scope, send, duration_attrs, + self, + server_span, + server_span_name, + scope, + send, + duration_attrs, ): expecting_trailers = False diff --git a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py index 94e2da7e23..144e35af27 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py @@ -34,11 +34,40 @@ get_global_response_propagator, set_global_response_propagator, ) +from opentelemetry.instrumentation._semconv import ( + _HTTPStabilityMode, + _OpenTelemetrySemanticConventionStability, + OTEL_SEMCONV_STABILITY_OPT_IN, +) from opentelemetry.sdk import resources from opentelemetry.sdk.metrics.export import ( HistogramDataPoint, NumberDataPoint, ) +from opentelemetry.semconv.attributes.client_attributes import ( + CLIENT_ADDRESS, + CLIENT_PORT, +) +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.attributes.url_attributes import ( + URL_FULL, + URL_PATH, + URL_QUERY, + URL_SCHEME, +) +from opentelemetry.semconv.attributes.user_agent_attributes import ( + USER_AGENT_ORIGINAL, +) from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.test.asgitestutil import ( AsgiTestBase, @@ -59,6 +88,8 @@ "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, @@ -74,6 +105,12 @@ "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 +) + _SIMULATED_BACKGROUND_TASK_EXECUTION_TIME_S = 0.01 @@ -245,7 +282,36 @@ async def error_asgi(scope, receive, send): # pylint: disable=too-many-public-methods class TestAsgiApplication(AsgiTestBase): - def validate_outputs(self, outputs, error=None, modifiers=None): + 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() + + def validate_outputs( + self, + outputs, + error=None, + modifiers=None, + old_sem_conv=True, + new_sem_conv=False, + ): # Ensure modifiers is a list modifiers = modifiers or [] # Check for expected outputs @@ -280,7 +346,79 @@ def validate_outputs(self, outputs, error=None, modifiers=None): # Check spans span_list = self.memory_exporter.get_finished_spans() - expected = [ + expected_old = [ + { + "name": "GET / http receive", + "kind": trace_api.SpanKind.INTERNAL, + "attributes": {"asgi.event.type": "http.request"}, + }, + { + "name": "GET / http send", + "kind": trace_api.SpanKind.INTERNAL, + "attributes": { + SpanAttributes.HTTP_STATUS_CODE: 200, + "asgi.event.type": "http.response.start", + }, + }, + { + "name": "GET / http send", + "kind": trace_api.SpanKind.INTERNAL, + "attributes": {"asgi.event.type": "http.response.body"}, + }, + { + "name": "GET /", + "kind": trace_api.SpanKind.SERVER, + "attributes": { + SpanAttributes.HTTP_METHOD: "GET", + SpanAttributes.HTTP_SCHEME: "http", + SpanAttributes.NET_HOST_PORT: 80, + SpanAttributes.HTTP_HOST: "127.0.0.1", + SpanAttributes.HTTP_FLAVOR: "1.0", + SpanAttributes.HTTP_TARGET: "/", + SpanAttributes.HTTP_URL: "http://127.0.0.1/", + SpanAttributes.NET_PEER_IP: "127.0.0.1", + SpanAttributes.NET_PEER_PORT: 32767, + SpanAttributes.HTTP_STATUS_CODE: 200, + }, + }, + ] + expected_new = [ + { + "name": "GET / http receive", + "kind": trace_api.SpanKind.INTERNAL, + "attributes": {"asgi.event.type": "http.request"}, + }, + { + "name": "GET / http send", + "kind": trace_api.SpanKind.INTERNAL, + "attributes": { + HTTP_RESPONSE_STATUS_CODE: 200, + "asgi.event.type": "http.response.start", + }, + }, + { + "name": "GET / http send", + "kind": trace_api.SpanKind.INTERNAL, + "attributes": {"asgi.event.type": "http.response.body"}, + }, + { + "name": "GET /", + "kind": trace_api.SpanKind.SERVER, + "attributes": { + HTTP_REQUEST_METHOD: "GET", + URL_SCHEME: "http", + SERVER_PORT: 80, + SERVER_ADDRESS: "127.0.0.1", + NETWORK_PROTOCOL_VERSION: "1.0", + URL_PATH: "/", + URL_FULL: "http://127.0.0.1/", + CLIENT_ADDRESS: "127.0.0.1", + CLIENT_PORT: 32767, + HTTP_RESPONSE_STATUS_CODE: 200, + }, + }, + ] + expected_both = [ { "name": "GET / http receive", "kind": trace_api.SpanKind.INTERNAL, @@ -291,6 +429,7 @@ def validate_outputs(self, outputs, error=None, modifiers=None): "kind": trace_api.SpanKind.INTERNAL, "attributes": { SpanAttributes.HTTP_STATUS_CODE: 200, + HTTP_RESPONSE_STATUS_CODE: 200, "asgi.event.type": "http.response.start", }, }, @@ -303,6 +442,16 @@ def validate_outputs(self, outputs, error=None, modifiers=None): "name": "GET /", "kind": trace_api.SpanKind.SERVER, "attributes": { + HTTP_REQUEST_METHOD: "GET", + URL_SCHEME: "http", + SERVER_PORT: 80, + SERVER_ADDRESS: "127.0.0.1", + NETWORK_PROTOCOL_VERSION: "1.0", + URL_PATH: "/", + URL_FULL: "http://127.0.0.1/", + CLIENT_ADDRESS: "127.0.0.1", + CLIENT_PORT: 32767, + HTTP_RESPONSE_STATUS_CODE: 200, SpanAttributes.HTTP_METHOD: "GET", SpanAttributes.HTTP_SCHEME: "http", SpanAttributes.NET_HOST_PORT: 80, @@ -316,6 +465,13 @@ def validate_outputs(self, outputs, error=None, modifiers=None): }, }, ] + expected = expected_old + if new_sem_conv: + if old_sem_conv: + expected = expected_both + else: + expected = expected_new + # Run our expected modifiers for modifier in modifiers: expected = modifier(expected) @@ -338,6 +494,22 @@ def test_basic_asgi_call(self): outputs = self.get_all_output() self.validate_outputs(outputs) + def test_basic_asgi_call_new_semconv(self): + """Test that spans are emitted as expected.""" + app = otel_asgi.OpenTelemetryMiddleware(simple_asgi) + self.seed_app(app) + self.send_default_request() + outputs = self.get_all_output() + self.validate_outputs(outputs, old_sem_conv=False, new_sem_conv=True) + + def test_basic_asgi_call_both_semconv(self): + """Test that spans are emitted as expected.""" + app = otel_asgi.OpenTelemetryMiddleware(simple_asgi) + self.seed_app(app) + self.send_default_request() + outputs = self.get_all_output() + self.validate_outputs(outputs, old_sem_conv=True, new_sem_conv=True) + def test_asgi_not_recording(self): mock_tracer = mock.Mock() mock_span = mock.Mock() @@ -512,6 +684,59 @@ def update_expected_server(expected): outputs = self.get_all_output() self.validate_outputs(outputs, modifiers=[update_expected_server]) + def test_behavior_with_scope_server_as_none_new_semconv(self): + """Test that middleware is ok when server is none in scope.""" + + def update_expected_server(expected): + expected[3]["attributes"].update( + { + SERVER_ADDRESS: "0.0.0.0", + SERVER_PORT: 80, + URL_FULL: "http://0.0.0.0/", + } + ) + return expected + + self.scope["server"] = None + app = otel_asgi.OpenTelemetryMiddleware(simple_asgi) + self.seed_app(app) + self.send_default_request() + outputs = self.get_all_output() + self.validate_outputs( + outputs, + modifiers=[update_expected_server], + old_sem_conv=False, + new_sem_conv=True, + ) + + def test_behavior_with_scope_server_as_none_both_semconv(self): + """Test that middleware is ok when server is none in scope.""" + + def update_expected_server(expected): + expected[3]["attributes"].update( + { + SpanAttributes.HTTP_HOST: "0.0.0.0", + SpanAttributes.NET_HOST_PORT: 80, + SpanAttributes.HTTP_URL: "http://0.0.0.0/", + SERVER_ADDRESS: "0.0.0.0", + SERVER_PORT: 80, + URL_FULL: "http://0.0.0.0/", + } + ) + return expected + + self.scope["server"] = None + app = otel_asgi.OpenTelemetryMiddleware(simple_asgi) + self.seed_app(app) + self.send_default_request() + outputs = self.get_all_output() + self.validate_outputs( + outputs, + modifiers=[update_expected_server], + old_sem_conv=True, + new_sem_conv=True, + ) + def test_host_header(self): """Test that host header is converted to http.server_name.""" hostname = b"server_name_1" @@ -529,6 +754,28 @@ def update_expected_server(expected): outputs = self.get_all_output() self.validate_outputs(outputs, modifiers=[update_expected_server]) + def test_host_header_both_semconv(self): + """Test that host header is converted to http.server_name.""" + hostname = b"server_name_1" + + def update_expected_server(expected): + expected[3]["attributes"].update( + {SpanAttributes.HTTP_SERVER_NAME: hostname.decode("utf8")} + ) + return expected + + self.scope["headers"].append([b"host", hostname]) + app = otel_asgi.OpenTelemetryMiddleware(simple_asgi) + self.seed_app(app) + self.send_default_request() + outputs = self.get_all_output() + self.validate_outputs( + outputs, + modifiers=[update_expected_server], + old_sem_conv=True, + new_sem_conv=True, + ) + def test_user_agent(self): """Test that host header is converted to http.server_name.""" user_agent = b"test-agent" @@ -546,6 +793,53 @@ def update_expected_user_agent(expected): outputs = self.get_all_output() self.validate_outputs(outputs, modifiers=[update_expected_user_agent]) + def test_user_agent_new_semconv(self): + """Test that host header is converted to http.server_name.""" + user_agent = b"test-agent" + + def update_expected_user_agent(expected): + expected[3]["attributes"].update( + {USER_AGENT_ORIGINAL: user_agent.decode("utf8")} + ) + return expected + + self.scope["headers"].append([b"user-agent", user_agent]) + app = otel_asgi.OpenTelemetryMiddleware(simple_asgi) + self.seed_app(app) + self.send_default_request() + outputs = self.get_all_output() + self.validate_outputs( + outputs, + modifiers=[update_expected_user_agent], + old_sem_conv=False, + new_sem_conv=True, + ) + + def test_user_agent_both_semconv(self): + """Test that host header is converted to http.server_name.""" + user_agent = b"test-agent" + + def update_expected_user_agent(expected): + expected[3]["attributes"].update( + { + SpanAttributes.HTTP_USER_AGENT: user_agent.decode("utf8"), + USER_AGENT_ORIGINAL: user_agent.decode("utf8"), + } + ) + return expected + + self.scope["headers"].append([b"user-agent", user_agent]) + app = otel_asgi.OpenTelemetryMiddleware(simple_asgi) + self.seed_app(app) + self.send_default_request() + outputs = self.get_all_output() + self.validate_outputs( + outputs, + modifiers=[update_expected_user_agent], + old_sem_conv=True, + new_sem_conv=True, + ) + def test_traceresponse_header(self): """Test a traceresponse header is sent when a global propagator is set.""" @@ -653,13 +947,9 @@ def test_websocket(self): self.assertEqual(span.kind, expected["kind"]) self.assertDictEqual(dict(span.attributes), expected["attributes"]) - def test_websocket_traceresponse_header(self): - """Test a traceresponse header is set for websocket messages""" - - orig = get_global_response_propagator() - set_global_response_propagator(TraceResponsePropagator()) - + def test_websocket_new_semconv(self): self.scope = { + "method": "GET", "type": "websocket", "http_version": "1.1", "scheme": "ws", @@ -674,35 +964,199 @@ def test_websocket_traceresponse_header(self): self.send_input({"type": "websocket.connect"}) self.send_input({"type": "websocket.receive", "text": "ping"}) self.send_input({"type": "websocket.disconnect"}) - _, socket_send, *_ = self.get_all_output() - - span = self.memory_exporter.get_finished_spans()[-1] - self.assertEqual(trace_api.SpanKind.SERVER, span.kind) - - trace_id = format_trace_id(span.get_span_context().trace_id) - span_id = format_span_id(span.get_span_context().span_id) - traceresponse = f"00-{trace_id}-{span_id}-01" - - self.assertListEqual( - socket_send["headers"], - [ - [b"traceresponse", f"{traceresponse}".encode()], - [b"access-control-expose-headers", b"traceresponse"], - ], - ) - - set_global_response_propagator(orig) - - def test_lifespan(self): - self.scope["type"] = "lifespan" - app = otel_asgi.OpenTelemetryMiddleware(simple_asgi) - self.seed_app(app) - self.send_default_request() + self.get_all_output() span_list = self.memory_exporter.get_finished_spans() - self.assertEqual(len(span_list), 0) + self.assertEqual(len(span_list), 6) + expected = [ + { + "name": "GET / websocket receive", + "kind": trace_api.SpanKind.INTERNAL, + "attributes": {"asgi.event.type": "websocket.connect"}, + }, + { + "name": "GET / websocket send", + "kind": trace_api.SpanKind.INTERNAL, + "attributes": {"asgi.event.type": "websocket.accept"}, + }, + { + "name": "GET / websocket receive", + "kind": trace_api.SpanKind.INTERNAL, + "attributes": { + "asgi.event.type": "websocket.receive", + HTTP_RESPONSE_STATUS_CODE: 200, + }, + }, + { + "name": "GET / websocket send", + "kind": trace_api.SpanKind.INTERNAL, + "attributes": { + "asgi.event.type": "websocket.send", + HTTP_RESPONSE_STATUS_CODE: 200, + }, + }, + { + "name": "GET / websocket receive", + "kind": trace_api.SpanKind.INTERNAL, + "attributes": {"asgi.event.type": "websocket.disconnect"}, + }, + { + "name": "GET /", + "kind": trace_api.SpanKind.SERVER, + "attributes": { + URL_SCHEME: self.scope["scheme"], + SERVER_PORT: self.scope["server"][1], + SERVER_ADDRESS: self.scope["server"][0], + NETWORK_PROTOCOL_VERSION: self.scope["http_version"], + URL_PATH: self.scope["path"], + URL_FULL: f'{self.scope["scheme"]}://{self.scope["server"][0]}{self.scope["path"]}', + CLIENT_ADDRESS: self.scope["client"][0], + CLIENT_PORT: self.scope["client"][1], + HTTP_RESPONSE_STATUS_CODE: 200, + HTTP_REQUEST_METHOD: self.scope["method"], + }, + }, + ] + for span, expected in zip(span_list, expected): + self.assertEqual(span.name, expected["name"]) + self.assertEqual(span.kind, expected["kind"]) + self.assertDictEqual(dict(span.attributes), expected["attributes"]) - def test_hooks(self): - def server_request_hook(span, scope): + def test_websocket_both_semconv(self): + self.scope = { + "method": "GET", + "type": "websocket", + "http_version": "1.1", + "scheme": "ws", + "path": "/", + "query_string": b"", + "headers": [], + "client": ("127.0.0.1", 32767), + "server": ("127.0.0.1", 80), + } + app = otel_asgi.OpenTelemetryMiddleware(simple_asgi) + self.seed_app(app) + self.send_input({"type": "websocket.connect"}) + self.send_input({"type": "websocket.receive", "text": "ping"}) + self.send_input({"type": "websocket.disconnect"}) + self.get_all_output() + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 6) + expected = [ + { + "name": "GET / websocket receive", + "kind": trace_api.SpanKind.INTERNAL, + "attributes": {"asgi.event.type": "websocket.connect"}, + }, + { + "name": "GET / websocket send", + "kind": trace_api.SpanKind.INTERNAL, + "attributes": {"asgi.event.type": "websocket.accept"}, + }, + { + "name": "GET / websocket receive", + "kind": trace_api.SpanKind.INTERNAL, + "attributes": { + "asgi.event.type": "websocket.receive", + HTTP_RESPONSE_STATUS_CODE: 200, + SpanAttributes.HTTP_STATUS_CODE: 200, + }, + }, + { + "name": "GET / websocket send", + "kind": trace_api.SpanKind.INTERNAL, + "attributes": { + "asgi.event.type": "websocket.send", + HTTP_RESPONSE_STATUS_CODE: 200, + SpanAttributes.HTTP_STATUS_CODE: 200, + }, + }, + { + "name": "GET / websocket receive", + "kind": trace_api.SpanKind.INTERNAL, + "attributes": {"asgi.event.type": "websocket.disconnect"}, + }, + { + "name": "GET /", + "kind": trace_api.SpanKind.SERVER, + "attributes": { + SpanAttributes.HTTP_SCHEME: self.scope["scheme"], + SpanAttributes.NET_HOST_PORT: self.scope["server"][1], + SpanAttributes.HTTP_HOST: self.scope["server"][0], + SpanAttributes.HTTP_FLAVOR: self.scope["http_version"], + SpanAttributes.HTTP_TARGET: self.scope["path"], + SpanAttributes.HTTP_URL: f'{self.scope["scheme"]}://{self.scope["server"][0]}{self.scope["path"]}', + SpanAttributes.NET_PEER_IP: self.scope["client"][0], + SpanAttributes.NET_PEER_PORT: self.scope["client"][1], + SpanAttributes.HTTP_STATUS_CODE: 200, + SpanAttributes.HTTP_METHOD: self.scope["method"], + URL_SCHEME: self.scope["scheme"], + SERVER_PORT: self.scope["server"][1], + SERVER_ADDRESS: self.scope["server"][0], + NETWORK_PROTOCOL_VERSION: self.scope["http_version"], + URL_PATH: self.scope["path"], + URL_FULL: f'{self.scope["scheme"]}://{self.scope["server"][0]}{self.scope["path"]}', + CLIENT_ADDRESS: self.scope["client"][0], + CLIENT_PORT: self.scope["client"][1], + HTTP_RESPONSE_STATUS_CODE: 200, + HTTP_REQUEST_METHOD: self.scope["method"], + }, + }, + ] + for span, expected in zip(span_list, expected): + self.assertEqual(span.name, expected["name"]) + self.assertEqual(span.kind, expected["kind"]) + self.assertDictEqual(dict(span.attributes), expected["attributes"]) + + def test_websocket_traceresponse_header(self): + """Test a traceresponse header is set for websocket messages""" + + orig = get_global_response_propagator() + set_global_response_propagator(TraceResponsePropagator()) + + self.scope = { + "type": "websocket", + "http_version": "1.1", + "scheme": "ws", + "path": "/", + "query_string": b"", + "headers": [], + "client": ("127.0.0.1", 32767), + "server": ("127.0.0.1", 80), + } + app = otel_asgi.OpenTelemetryMiddleware(simple_asgi) + self.seed_app(app) + self.send_input({"type": "websocket.connect"}) + self.send_input({"type": "websocket.receive", "text": "ping"}) + self.send_input({"type": "websocket.disconnect"}) + _, socket_send, *_ = self.get_all_output() + + span = self.memory_exporter.get_finished_spans()[-1] + self.assertEqual(trace_api.SpanKind.SERVER, span.kind) + + trace_id = format_trace_id(span.get_span_context().trace_id) + span_id = format_span_id(span.get_span_context().span_id) + traceresponse = f"00-{trace_id}-{span_id}-01" + + self.assertListEqual( + socket_send["headers"], + [ + [b"traceresponse", f"{traceresponse}".encode()], + [b"access-control-expose-headers", b"traceresponse"], + ], + ) + + set_global_response_propagator(orig) + + def test_lifespan(self): + self.scope["type"] = "lifespan" + app = otel_asgi.OpenTelemetryMiddleware(simple_asgi) + self.seed_app(app) + self.send_default_request() + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 0) + + def test_hooks(self): + def server_request_hook(span, scope): span.update_name("name from server hook") def client_request_hook(receive_span, scope, message): @@ -770,6 +1224,78 @@ def test_asgi_metrics(self): ) self.assertTrue(number_data_point_seen and histogram_data_point_seen) + def test_asgi_metrics_new_semconv(self): + app = otel_asgi.OpenTelemetryMiddleware(simple_asgi) + self.seed_app(app) + self.send_default_request() + self.seed_app(app) + self.send_default_request() + self.seed_app(app) + self.send_default_request() + 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) != 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) + self.assertEqual( + scope_metric.scope.name, + "opentelemetry.instrumentation.asgi", + ) + 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_asgi_metrics_both_semconv(self): + app = otel_asgi.OpenTelemetryMiddleware(simple_asgi) + self.seed_app(app) + self.send_default_request() + self.seed_app(app) + self.send_default_request() + self.seed_app(app) + self.send_default_request() + 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) != 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) + self.assertEqual( + scope_metric.scope.name, + "opentelemetry.instrumentation.asgi", + ) + 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: + 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_both[metric.name] + ) + self.assertTrue(number_data_point_seen and histogram_data_point_seen) + def test_basic_metric_success(self): app = otel_asgi.OpenTelemetryMiddleware(simple_asgi) self.seed_app(app) @@ -817,6 +1343,140 @@ def test_basic_metric_success(self): ) self.assertEqual(point.value, 0) + def test_basic_metric_success_new_semconv(self): + 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.request.method": "GET", + "url.scheme": "http", + "network.protocol.version": "1.0", + "http.response.status_code": 200, + } + expected_requests_count_attributes = { + "http.request.method": "GET", + "url.scheme": "http", + } + 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.request.duration": + self.assertAlmostEqual( + duration, point.sum, delta=5 + ) + elif ( + metric.name == "http.server.response.body.size" + ): + self.assertEqual(1024, point.sum) + elif ( + metric.name == "http.server.request.body.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_both_semconv(self): + 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_old = { + "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", + "http.request.method": "GET", + "url.scheme": "http", + } + expected_duration_attributes_new = { + "http.request.method": "GET", + "url.scheme": "http", + "network.protocol.version": "1.0", + "http.response.status_code": 200, + } + 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.assertEqual(point.count, 1) + if metric.name == "http.server.request.duration": + self.assertAlmostEqual( + duration, point.sum, delta=5 + ) + self.assertDictEqual( + expected_duration_attributes_new, + dict(point.attributes), + ) + elif ( + metric.name == "http.server.response.body.size" + ): + self.assertEqual(1024, point.sum) + self.assertDictEqual( + expected_duration_attributes_new, + dict(point.attributes), + ) + elif ( + metric.name == "http.server.request.body.size" + ): + self.assertEqual(128, point.sum) + self.assertDictEqual( + expected_duration_attributes_new, + dict(point.attributes), + ) + elif metric.name == "http.server.duration": + self.assertAlmostEqual( + duration, point.sum, delta=5 + ) + self.assertDictEqual( + expected_duration_attributes_old, + dict(point.attributes), + ) + elif metric.name == "http.server.response.size": + self.assertEqual(1024, point.sum) + self.assertDictEqual( + expected_duration_attributes_old, + dict(point.attributes), + ) + elif metric.name == "http.server.request.size": + self.assertEqual(128, point.sum) + self.assertDictEqual( + expected_duration_attributes_old, + dict(point.attributes), + ) + elif isinstance(point, NumberDataPoint): + self.assertDictEqual( + expected_requests_count_attributes, + dict(point.attributes), + ) + self.assertEqual(point.value, 0) + def test_metric_target_attribute(self): expected_target = "/api/user/{id}" @@ -900,6 +1560,70 @@ def test_request_attributes(self): }, ) + def test_request_attributes_new_semconv(self): + self.scope["query_string"] = b"foo=bar" + headers = [] + headers.append((b"host", b"test")) + self.scope["headers"] = headers + + attrs = otel_asgi.collect_request_attributes( + self.scope, + _HTTPStabilityMode.HTTP, + ) + + self.assertDictEqual( + attrs, + { + HTTP_REQUEST_METHOD: "GET", + SERVER_ADDRESS: "127.0.0.1", + URL_PATH: "/", + URL_QUERY: "foo=bar", + URL_FULL: "http://127.0.0.1/?foo=bar", + SERVER_PORT: 80, + URL_SCHEME: "http", + NETWORK_PROTOCOL_VERSION: "1.0", + CLIENT_ADDRESS: "127.0.0.1", + CLIENT_PORT: 32767, + }, + ) + + def test_request_attributes_both_semconv(self): + self.scope["query_string"] = b"foo=bar" + headers = [] + headers.append((b"host", b"test")) + self.scope["headers"] = headers + + attrs = otel_asgi.collect_request_attributes( + self.scope, + _HTTPStabilityMode.HTTP_DUP, + ) + + self.assertDictEqual( + attrs, + { + SpanAttributes.HTTP_METHOD: "GET", + SpanAttributes.HTTP_HOST: "127.0.0.1", + SpanAttributes.HTTP_TARGET: "/", + SpanAttributes.HTTP_URL: "http://127.0.0.1/?foo=bar", + SpanAttributes.NET_HOST_PORT: 80, + SpanAttributes.HTTP_SCHEME: "http", + SpanAttributes.HTTP_SERVER_NAME: "test", + SpanAttributes.HTTP_FLAVOR: "1.0", + SpanAttributes.NET_PEER_IP: "127.0.0.1", + SpanAttributes.NET_PEER_PORT: 32767, + HTTP_REQUEST_METHOD: "GET", + SERVER_ADDRESS: "127.0.0.1", + URL_PATH: "/", + URL_QUERY: "foo=bar", + URL_FULL: "http://127.0.0.1/?foo=bar", + SERVER_PORT: 80, + URL_SCHEME: "http", + NETWORK_PROTOCOL_VERSION: "1.0", + CLIENT_ADDRESS: "127.0.0.1", + CLIENT_PORT: 32767, + }, + ) + def test_query_string(self): self.scope["query_string"] = b"foo=bar" attrs = otel_asgi.collect_request_attributes(self.scope) @@ -907,6 +1631,25 @@ def test_query_string(self): attrs[SpanAttributes.HTTP_URL], "http://127.0.0.1/?foo=bar" ) + def test_query_string_new_semconv(self): + self.scope["query_string"] = b"foo=bar" + attrs = otel_asgi.collect_request_attributes( + self.scope, + _HTTPStabilityMode.HTTP, + ) + self.assertEqual(attrs[URL_FULL], "http://127.0.0.1/?foo=bar") + + def test_query_string_both_semconv(self): + self.scope["query_string"] = b"foo=bar" + attrs = otel_asgi.collect_request_attributes( + self.scope, + _HTTPStabilityMode.HTTP_DUP, + ) + self.assertEqual(attrs[URL_FULL], "http://127.0.0.1/?foo=bar") + self.assertEqual( + attrs[SpanAttributes.HTTP_URL], "http://127.0.0.1/?foo=bar" + ) + def test_query_string_percent_bytes(self): self.scope["query_string"] = b"foo%3Dbar" attrs = otel_asgi.collect_request_attributes(self.scope) @@ -928,6 +1671,32 @@ def test_response_attributes(self): self.assertEqual(self.span.set_attribute.call_count, 1) self.span.set_attribute.assert_has_calls(expected, any_order=True) + def test_response_attributes_new_semconv(self): + otel_asgi.set_status_code( + self.span, + 404, + None, + _HTTPStabilityMode.HTTP, + ) + expected = (mock.call(HTTP_RESPONSE_STATUS_CODE, 404),) + self.assertEqual(self.span.set_attribute.call_count, 1) + self.assertEqual(self.span.set_attribute.call_count, 1) + self.span.set_attribute.assert_has_calls(expected, any_order=True) + + def test_response_attributes_both_semconv(self): + otel_asgi.set_status_code( + self.span, + 404, + None, + _HTTPStabilityMode.HTTP_DUP, + ) + expected = (mock.call(SpanAttributes.HTTP_STATUS_CODE, 404),) + expected2 = (mock.call(HTTP_RESPONSE_STATUS_CODE, 404),) + self.assertEqual(self.span.set_attribute.call_count, 2) + self.assertEqual(self.span.set_attribute.call_count, 2) + self.span.set_attribute.assert_has_calls(expected, any_order=True) + self.span.set_attribute.assert_has_calls(expected2, any_order=True) + def test_response_attributes_invalid_status_code(self): otel_asgi.set_status_code(self.span, "Invalid Status Code") self.assertEqual(self.span.set_status.call_count, 1) diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py index c5909f4260..85b8e2e3ec 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py @@ -330,9 +330,7 @@ def _set_http_user_agent(result, user_agent, sem_conv_opt_in_mode): result, SpanAttributes.HTTP_USER_AGENT, user_agent ) if _report_new(sem_conv_opt_in_mode): - set_string_attribute( - result, USER_AGENT_ORIGINAL, user_agent - ) + set_string_attribute(result, USER_AGENT_ORIGINAL, user_agent) def _set_http_net_peer_name_server(result, name, sem_conv_opt_in_mode): @@ -346,9 +344,7 @@ def _set_http_flavor_version(result, version, sem_conv_opt_in_mode): if _report_old(sem_conv_opt_in_mode): set_string_attribute(result, SpanAttributes.HTTP_FLAVOR, version) if _report_new(sem_conv_opt_in_mode): - set_string_attribute( - result, NETWORK_PROTOCOL_VERSION, version - ) + set_string_attribute(result, NETWORK_PROTOCOL_VERSION, version) def _set_status( @@ -376,12 +372,8 @@ def _set_status( 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( - SpanAttributes.HTTP_RESPONSE_STATUS_CODE, status_code - ) - metrics_attributes[SpanAttributes.HTTP_RESPONSE_STATUS_CODE] = ( - status_code - ) + 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) metrics_attributes[ERROR_TYPE] = status_code_str From 2976612447527e6b8f50dc1fd4ba380129fd65f9 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Wed, 26 Jun 2024 09:46:27 -0700 Subject: [PATCH 24/36] isort --- .../opentelemetry/instrumentation/asgi/__init__.py | 14 +++++++------- .../tests/test_asgi_middleware.py | 8 +++----- 2 files changed, 10 insertions(+), 12 deletions(-) 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 68900991e5..a98b545016 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -200,11 +200,6 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A from asgiref.compatibility import guarantee_single_callable from opentelemetry import context, trace -from opentelemetry.instrumentation.asgi.types import ( - ClientRequestHook, - ClientResponseHook, - ServerRequestHook, -) from opentelemetry.instrumentation._semconv import ( _filter_semconv_active_request_count_attr, _filter_semconv_duration_attrs, @@ -218,8 +213,8 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A _server_active_requests_count_attrs_old, _server_duration_attrs_new, _server_duration_attrs_old, - _set_http_host, _set_http_flavor_version, + _set_http_host, _set_http_method, _set_http_net_host_port, _set_http_peer_ip, @@ -230,6 +225,11 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A _set_http_user_agent, _set_status, ) +from opentelemetry.instrumentation.asgi.types import ( + ClientRequestHook, + ClientResponseHook, + ServerRequestHook, +) from opentelemetry.instrumentation.asgi.version import __version__ # noqa from opentelemetry.instrumentation.propagators import ( get_global_response_propagator, @@ -253,11 +253,11 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A from opentelemetry.trace import set_span_in_context from opentelemetry.trace.status import Status, StatusCode from opentelemetry.util.http import ( - _parse_url_query, OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SANITIZE_FIELDS, OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SERVER_REQUEST, OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SERVER_RESPONSE, SanitizeValue, + _parse_url_query, get_custom_headers, normalise_request_header_name, normalise_response_header_name, diff --git a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py index 144e35af27..c2b3bbdc4e 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py @@ -24,6 +24,9 @@ import opentelemetry.instrumentation.asgi as otel_asgi from opentelemetry import trace as trace_api from opentelemetry.instrumentation._semconv import ( + OTEL_SEMCONV_STABILITY_OPT_IN, + _HTTPStabilityMode, + _OpenTelemetrySemanticConventionStability, _server_active_requests_count_attrs_new, _server_active_requests_count_attrs_old, _server_duration_attrs_new, @@ -34,11 +37,6 @@ get_global_response_propagator, set_global_response_propagator, ) -from opentelemetry.instrumentation._semconv import ( - _HTTPStabilityMode, - _OpenTelemetrySemanticConventionStability, - OTEL_SEMCONV_STABILITY_OPT_IN, -) from opentelemetry.sdk import resources from opentelemetry.sdk.metrics.export import ( HistogramDataPoint, From 05e03e0e3489c4536a7f97d68c028a3c250d618e Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Wed, 26 Jun 2024 09:52:25 -0700 Subject: [PATCH 25/36] lint --- .../src/opentelemetry/instrumentation/asgi/__init__.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) 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 54b288ea31..bc65b21169 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -235,10 +235,7 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A from opentelemetry.instrumentation.propagators import ( get_global_response_propagator, ) -from opentelemetry.instrumentation.utils import ( - _start_internal_or_server_span, - http_status_to_status_code, -) +from opentelemetry.instrumentation.utils import _start_internal_or_server_span from opentelemetry.metrics import get_meter from opentelemetry.propagators.textmap import Getter, Setter from opentelemetry.semconv._incubating.metrics.http_metrics import ( @@ -252,7 +249,6 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A ) from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.trace import set_span_in_context -from opentelemetry.trace.status import Status, StatusCode from opentelemetry.util.http import ( OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SANITIZE_FIELDS, OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SERVER_REQUEST, From 50d8623080dcf7854b26c35c442fe55a0daf91ec Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Wed, 26 Jun 2024 09:59:28 -0700 Subject: [PATCH 26/36] pylint --- .../src/opentelemetry/instrumentation/asgi/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 bc65b21169..2c445f9fc5 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -323,7 +323,7 @@ def set( asgi_setter = ASGISetter() - +# pylint: disable=too-many-branches def collect_request_attributes( scope, sem_conv_opt_in_mode=_HTTPStabilityMode.DEFAULT ): @@ -645,6 +645,7 @@ def __init__( or [] ) + # pylint: disable=too-many-branches async def __call__( self, scope: dict[str, Any], From d486136b5982fa4294b48a185f784e7fad7b156a Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Wed, 26 Jun 2024 10:08:38 -0700 Subject: [PATCH 27/36] Update __init__.py --- .../src/opentelemetry/instrumentation/asgi/__init__.py | 1 + 1 file changed, 1 insertion(+) 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 2c445f9fc5..ffe7ae6389 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -323,6 +323,7 @@ def set( asgi_setter = ASGISetter() + # pylint: disable=too-many-branches def collect_request_attributes( scope, sem_conv_opt_in_mode=_HTTPStabilityMode.DEFAULT From ea20437751635b8acec00207f306faeaa232e70f Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Wed, 26 Jun 2024 10:12:23 -0700 Subject: [PATCH 28/36] Update __init__.py --- .../src/opentelemetry/instrumentation/asgi/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 ffe7ae6389..80d312295f 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -646,7 +646,7 @@ def __init__( or [] ) - # pylint: disable=too-many-branches + # pylint: disable=too-many-statements async def __call__( self, scope: dict[str, Any], From 8189c5aa9ff2563d1a5385d74df3372cf2a0ed3b Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Wed, 26 Jun 2024 10:20:51 -0700 Subject: [PATCH 29/36] Update test_asgi_middleware.py --- .../tests/test_asgi_middleware.py | 1 + 1 file changed, 1 insertion(+) diff --git a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py index c2b3bbdc4e..d46b080b23 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py @@ -302,6 +302,7 @@ def setUp(self): self.env_patch.start() + # pylint: disable=too-many-locals def validate_outputs( self, outputs, From 46c7dac7645eaf1d63be687958d5736f9497d47f Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Thu, 27 Jun 2024 09:55:35 -0700 Subject: [PATCH 30/36] comments --- CHANGELOG.md | 4 ++-- .../src/opentelemetry/instrumentation/asgi/__init__.py | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 31468c2ba6..b1aa1e90a5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,12 +26,12 @@ 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 - ([#2425](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2425)) + ([#2610](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2610)) ### Added - `opentelemetry-instrumentation-asgi` Implement new semantic convention opt-in with stable http semantic conventions - ([#2425](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2425)) + ([#2610](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2610)) ### Fixed 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 80d312295f..f609eeda52 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -757,8 +757,6 @@ async def __call__( self.active_requests_counter.add( -1, active_requests_count_attrs ) - print(attributes) - print(active_requests_count_attrs) if self.content_length_header: if self.server_response_size_histogram: self.server_response_size_histogram.record( From b1eba7a16e5a79fbe9a13ba7a82f7d3367d82e4c Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Mon, 1 Jul 2024 09:45:47 -0700 Subject: [PATCH 31/36] duration --- .../src/opentelemetry/instrumentation/asgi/__init__.py | 6 +++--- .../tests/test_asgi_middleware.py | 7 ++++--- 2 files changed, 7 insertions(+), 6 deletions(-) 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 f609eeda52..94ef8e80e7 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -737,7 +737,7 @@ async def __call__( query, self._sem_conv_opt_in_mode, ) - duration = max(round((default_timer() - start) * 1000), 0) + duration_s = default_timer() - start duration_attrs_old = _parse_duration_attrs( attributes, _HTTPStabilityMode.DEFAULT ) @@ -748,11 +748,11 @@ async def __call__( ) if self.duration_histogram_old: self.duration_histogram_old.record( - duration, duration_attrs_old + max(round(duration_s * 1000), 0), duration_attrs_old ) if self.duration_histogram_new: self.duration_histogram_new.record( - duration, duration_attrs_new + max(duration_s, 0), duration_attrs_new ) self.active_requests_counter.add( -1, active_requests_count_attrs diff --git a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py index d46b080b23..ff266cb5bf 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py @@ -1347,7 +1347,7 @@ def test_basic_metric_success_new_semconv(self): self.seed_app(app) start = default_timer() self.send_default_request() - duration = max(round((default_timer() - start) * 1000), 0) + duration_s = max(default_timer() - start, 0) expected_duration_attributes = { "http.request.method": "GET", "url.scheme": "http", @@ -1372,7 +1372,7 @@ def test_basic_metric_success_new_semconv(self): self.assertEqual(point.count, 1) if metric.name == "http.server.request.duration": self.assertAlmostEqual( - duration, point.sum, delta=5 + duration_s, point.sum, places=2 ) elif ( metric.name == "http.server.response.body.size" @@ -1395,6 +1395,7 @@ def test_basic_metric_success_both_semconv(self): start = default_timer() self.send_default_request() duration = max(round((default_timer() - start) * 1000), 0) + duration_s = max(default_timer() - start, 0) expected_duration_attributes_old = { "http.method": "GET", "http.host": "127.0.0.1", @@ -1427,7 +1428,7 @@ def test_basic_metric_success_both_semconv(self): self.assertEqual(point.count, 1) if metric.name == "http.server.request.duration": self.assertAlmostEqual( - duration, point.sum, delta=5 + duration_s, point.sum, places=2 ) self.assertDictEqual( expected_duration_attributes_new, From fb78ff8a68b3c85e37976c99c7a60d1ad5d8ee65 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Mon, 1 Jul 2024 09:47:32 -0700 Subject: [PATCH 32/36] Update README.md --- instrumentation/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/README.md b/instrumentation/README.md index 682334db1a..a6b6075976 100644 --- a/instrumentation/README.md +++ b/instrumentation/README.md @@ -5,7 +5,7 @@ | [opentelemetry-instrumentation-aiohttp-client](./opentelemetry-instrumentation-aiohttp-client) | aiohttp ~= 3.0 | No | experimental | [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 | No | experimental +| [opentelemetry-instrumentation-asgi](./opentelemetry-instrumentation-asgi) | asgiref ~= 3.0 | No | migration | [opentelemetry-instrumentation-asyncio](./opentelemetry-instrumentation-asyncio) | asyncio | No | experimental | [opentelemetry-instrumentation-asyncpg](./opentelemetry-instrumentation-asyncpg) | asyncpg >= 0.12.0 | No | experimental | [opentelemetry-instrumentation-aws-lambda](./opentelemetry-instrumentation-aws-lambda) | aws_lambda | No | experimental From 19a3af799f1f91343683d4ac31d5c6d223e008d3 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Mon, 1 Jul 2024 09:59:08 -0700 Subject: [PATCH 33/36] Update package.py --- .../src/opentelemetry/instrumentation/asgi/package.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/package.py b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/package.py index c219ec5499..cd35b1f73a 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/package.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/package.py @@ -14,3 +14,7 @@ _instruments = ("asgiref ~= 3.0",) + +_supports_metrics = True + +_semconv_status = "migration" From 07b897622e0ff0f8748f4bb2f4aa98751f61cb15 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Mon, 1 Jul 2024 10:08:56 -0700 Subject: [PATCH 34/36] Update README.md --- instrumentation/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/README.md b/instrumentation/README.md index a6b6075976..26c7d24daf 100644 --- a/instrumentation/README.md +++ b/instrumentation/README.md @@ -5,7 +5,7 @@ | [opentelemetry-instrumentation-aiohttp-client](./opentelemetry-instrumentation-aiohttp-client) | aiohttp ~= 3.0 | No | experimental | [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 | No | migration +| [opentelemetry-instrumentation-asgi](./opentelemetry-instrumentation-asgi) | asgiref ~= 3.0 | Yes | migration | [opentelemetry-instrumentation-asyncio](./opentelemetry-instrumentation-asyncio) | asyncio | No | experimental | [opentelemetry-instrumentation-asyncpg](./opentelemetry-instrumentation-asyncpg) | asyncpg >= 0.12.0 | No | experimental | [opentelemetry-instrumentation-aws-lambda](./opentelemetry-instrumentation-aws-lambda) | aws_lambda | No | experimental From fa1127083604f7305f18f4ceb5a865d23d62a288 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Wed, 3 Jul 2024 10:27:41 -0700 Subject: [PATCH 35/36] feedback --- .../src/opentelemetry/instrumentation/asgi/__init__.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) 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 94ef8e80e7..b414cdbfc8 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -350,9 +350,8 @@ def collect_request_attributes( _set_http_flavor_version(result, flavor, sem_conv_opt_in_mode) path = scope.get("path") if path: - target = path _set_http_target( - result, target, path, query_string, sem_conv_opt_in_mode + result, path, path, query_string, sem_conv_opt_in_mode ) if http_url: _set_http_url( @@ -435,7 +434,7 @@ def set_status_code( """Adds HTTP response attributes to span using the status_code argument.""" if not span.is_recording(): return - status_code_str = repr(status_code) + status_code_str = str(status_code) try: status_code = int(status_code) From 26aa9f7bf8b1e24478c41a3ab9529c940d484312 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Wed, 3 Jul 2024 11:00:09 -0700 Subject: [PATCH 36/36] Update CHANGELOG.md --- CHANGELOG.md | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d93e0d56d..8e12ff8586 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,19 +7,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased -- `opentelemetry-instrumentation-django` Handle exceptions from request/response hooks - ([#2153](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2153)) -- `opentelemetry-instrumentation-asyncio` instrumented `asyncio.wait_for` properly raises `asyncio.TimeoutError` as expected - ([#2637](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2637)) -- `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-system-metrics` Permit to use psutil 6.0+. - ([#2630](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2630)) -- `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)) - - ### Added - `opentelemetry-instrumentation-pyramid` Record exceptions raised when serving a request @@ -30,6 +17,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#2616](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2616)) - `opentelemetry-instrumentation-confluent-kafka` Add support for produce purge ([#2638](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2638)) +- `opentelemetry-instrumentation-system-metrics` Permit to use psutil 6.0+. + ([#2630](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2630)) ### Breaking changes @@ -57,6 +46,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#2644](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2644)) - `opentelemetry-instrumentation-confluent-kafka` Confluent Kafka: Ensure consume span is ended when consumer is closed ([#2640](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2640)) +- `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. + ([#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)) +- `opentelemetry-instrumentation-django` Handle exceptions from request/response hooks + ([#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)) + ## Version 1.25.0/0.46b0 (2024-05-31)