From fa31c6f27a2cbb74c1821b90c1bedd37dbbd9cc4 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Fri, 10 Nov 2023 12:47:09 -0800 Subject: [PATCH 01/18] sanitize --- .../src/opentelemetry/instrumentation/requests/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py index 535b14285f..ccde0fc0d9 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py @@ -81,6 +81,7 @@ get_excluded_urls, parse_excluded_urls, remove_url_credentials, + sanitize_method, ) from opentelemetry.util.http.httplib import set_ip_on_next_http_connection @@ -254,7 +255,7 @@ def get_default_span_name(method): Returns: span name """ - return method.strip() + return sanitize_method(method.strip()) class RequestsInstrumentor(BaseInstrumentor): From d99e101978d8ef183aef003e48bc6889ada61138 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Mon, 20 Nov 2023 14:33:46 -0800 Subject: [PATCH 02/18] old --- .../instrumentation/requests/__init__.py | 116 ++++++--- .../opentelemetry/instrumentation/_semconv.py | 230 ++++++++++++++++++ .../instrumentation/instrumentor.py | 2 +- .../opentelemetry/instrumentation/utils.py | 57 ----- 4 files changed, 311 insertions(+), 94 deletions(-) create mode 100644 opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py diff --git a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py index ccde0fc0d9..29e2cae8bd 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py @@ -69,6 +69,21 @@ _SUPPRESS_INSTRUMENTATION_KEY, http_status_to_status_code, ) +from opentelemetry.instrumentation._semconv import ( + _report_old, + _report_new, + _set_http_hostname, + _set_http_method, + _set_http_net_peer_name, + _set_http_network_protocol_version, + _set_http_port, + _set_http_scheme, + _set_http_url, + _set_http_status_code, + _OpenTelemetrySemanticConventionStability, + _OpenTelemetryStabilityMode, + _OpenTelemetryStabilitySignalType, +) from opentelemetry.metrics import Histogram, get_meter from opentelemetry.propagate import inject from opentelemetry.semconv.metrics import MetricInstruments @@ -90,15 +105,22 @@ _RequestHookT = Optional[Callable[[Span, PreparedRequest], None]] _ResponseHookT = Optional[Callable[[Span, PreparedRequest], None]] +# TODO: will come through semconv package once updated +_SPAN_ATTRIBUTES_ERROR_TYPE = "error.type" +_SPAN_ATTRIBUTES_NETWORK_PEER_ADDRESS ="network.peer.address" +_SPAN_ATTRIBUTES_NETWORK_PEER_PORT = "network.peer.port" + # pylint: disable=unused-argument # pylint: disable=R0915 def _instrument( tracer: Tracer, - duration_histogram: Histogram, + duration_histogram_old: Histogram, + duration_histogram_new: Histogram, request_hook: _RequestHookT = None, response_hook: _ResponseHookT = None, excluded_urls: ExcludeList = None, + sem_conv_opt_in_mode: _OpenTelemetryStabilityMode = _OpenTelemetryStabilityMode.DEFAULT, ): """Enables tracing of all requests calls that go through :code:`requests.session.Session.request` (this includes @@ -133,31 +155,34 @@ def get_or_create_headers(): return wrapped_send(self, request, **kwargs) # See - # https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/http.md#http-client - method = request.method.upper() + # https://github.com/open-telemetry/semantic-conventions/blob/main/docs/http/http-spans.md#http-client + method = request.method span_name = get_default_span_name(method) url = remove_url_credentials(request.url) - span_attributes = { - SpanAttributes.HTTP_METHOD: method, - SpanAttributes.HTTP_URL: url, - } + span_attributes = {} + _set_http_method(span_attributes, method, span_name, sem_conv_opt_in_mode) + _set_http_url(span_attributes, url, sem_conv_opt_in_mode) - metric_labels = { - SpanAttributes.HTTP_METHOD: method, - } + metric_labels = {} + _set_http_method(metric_labels, method, span_name, sem_conv_opt_in_mode) try: parsed_url = urlparse(url) - metric_labels[SpanAttributes.HTTP_SCHEME] = parsed_url.scheme + if parsed_url.scheme: + _set_http_scheme(metric_labels, parsed_url.scheme, sem_conv_opt_in_mode) if parsed_url.hostname: - metric_labels[SpanAttributes.HTTP_HOST] = parsed_url.hostname - metric_labels[ - SpanAttributes.NET_PEER_NAME - ] = parsed_url.hostname + _set_http_hostname(metric_labels, parsed_url.hostname, sem_conv_opt_in_mode) + _set_http_net_peer_name(metric_labels, parsed_url.hostname, sem_conv_opt_in_mode) + if _report_new(sem_conv_opt_in_mode): + _set_http_hostname(span_attributes, parsed_url.hostname, sem_conv_opt_in_mode) + span_attributes[_SPAN_ATTRIBUTES_NETWORK_PEER_ADDRESS] = parsed_url.hostname if parsed_url.port: - metric_labels[SpanAttributes.NET_PEER_PORT] = parsed_url.port + _set_http_port(metric_labels, parsed_url.port, sem_conv_opt_in_mode) + if _report_new(sem_conv_opt_in_mode): + _set_http_port(span_attributes, parsed_url.port, sem_conv_opt_in_mode) + span_attributes[_SPAN_ATTRIBUTES_NETWORK_PEER_PORT] = parsed_url.port except ValueError: pass @@ -183,35 +208,40 @@ def get_or_create_headers(): exception = exc result = getattr(exc, "response", None) finally: - elapsed_time = max( - round((default_timer() - start_time) * 1000), 0 - ) + elapsed_time = max(default_timer() - start_time, 0) context.detach(token) if isinstance(result, Response): + span_attributes = {} if span.is_recording(): - span.set_attribute( - SpanAttributes.HTTP_STATUS_CODE, result.status_code - ) + _set_http_status_code(span_attributes, result.status_code, sem_conv_opt_in_mode) span.set_status( Status(http_status_to_status_code(result.status_code)) ) - metric_labels[ - SpanAttributes.HTTP_STATUS_CODE - ] = result.status_code + _set_http_status_code(metric_labels, result.status_code, sem_conv_opt_in_mode) if result.raw is not None: version = getattr(result.raw, "version", None) if version: - metric_labels[SpanAttributes.HTTP_FLAVOR] = ( - "1.1" if version == 11 else "1.0" - ) + # Only HTTP/1 is supported by requests + version_text = "1.1" if version == 11 else "1.0" + _set_http_network_protocol_version(metric_labels, version_text, sem_conv_opt_in_mode) + if _report_new(sem_conv_opt_in_mode): + _set_http_network_protocol_version(span_attributes, version_text, sem_conv_opt_in_mode) + if exception is not None: + if _report_new(sem_conv_opt_in_mode): + span_attributes[_SPAN_ATTRIBUTES_ERROR_TYPE] = type(exception).__name__ + metric_labels[_SPAN_ATTRIBUTES_ERROR_TYPE] = type(exception).__name__ + for k, v in span_attributes.items(): + span.set_attribute(k, v) if callable(response_hook): response_hook(span, request, result) - duration_histogram.record(elapsed_time, attributes=metric_labels) + if _report_old(sem_conv_opt_in_mode) and duration_histogram_old is not None: + duration_histogram_old.record(max(round(elapsed_time * 1000), 0), attributes=metric_labels) + # duration_histogram_new.record(elapsed_time, attributes=metric_labels) if exception is not None: raise exception.with_traceback(exception.__traceback__) @@ -255,7 +285,7 @@ def get_default_span_name(method): Returns: span name """ - return sanitize_method(method.strip()) + return sanitize_method(method.upper().strip()) class RequestsInstrumentor(BaseInstrumentor): @@ -292,19 +322,33 @@ def _instrument(self, **kwargs): meter_provider, schema_url="https://opentelemetry.io/schemas/1.11.0", ) - duration_histogram = meter.create_histogram( - name=MetricInstruments.HTTP_CLIENT_DURATION, - unit="ms", - description="measures the duration of the outbound HTTP request", - ) + semconv_opt_in_mode = _OpenTelemetrySemanticConventionStability._get_opentelemetry_stability_opt_in_mode( + _OpenTelemetryStabilitySignalType.HTTP, + ) + duration_histogram_old = None + if _report_old(semconv_opt_in_mode): + duration_histogram_old = meter.create_histogram( + name=MetricInstruments.HTTP_CLIENT_DURATION, + unit="ms", + description="measures the duration of the outbound HTTP request", + ) + duration_histogram_new = None + if _report_new(semconv_opt_in_mode): + duration_histogram_new = meter.create_histogram( + name=MetricInstruments.HTTP_CLIENT_DURATION, + unit="s", + description="Duration of HTTP client requests.", + ) _instrument( tracer, - duration_histogram, + duration_histogram_old, + duration_histogram_new, request_hook=kwargs.get("request_hook"), response_hook=kwargs.get("response_hook"), excluded_urls=_excluded_urls_from_env if excluded_urls is None else parse_excluded_urls(excluded_urls), + sem_conv_opt_in_mode=semconv_opt_in_mode, ) def _uninstrument(self, **kwargs): diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py new file mode 100644 index 0000000000..4773fb151b --- /dev/null +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py @@ -0,0 +1,230 @@ +# Copyright The OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import os +import threading +from enum import Enum +from re import escape, sub +from typing import Dict, Optional, Sequence + + + +from opentelemetry.semconv.trace import SpanAttributes + + +def set_string_attribute(dict, key, value): + if value: + dict[key] = value + + +def set_int_attribute(dict, key, value): + if value: + try: + dict[key] = int(value) + except ValueError: + return + + +def _set_http_method(result, original, normalized, sem_conv_opt_in_mode): + original = original.strip() + normalized = normalized.strip() + # See https://github.com/open-telemetry/semantic-conventions/blob/main/docs/http/http-spans.md#common-attributes + # Method is case sensitive. "http.request.method_original" should not be sanitized or automatically capitalized. + if original != normalized and sem_conv_opt_in_mode != _OpenTelemetryStabilityMode.DEFAULT: + set_string_attribute(result, SpanAttributes.HTTP_REQUEST_METHOD_ORIGINAL, original) + + if _report_old(sem_conv_opt_in_mode): + set_string_attribute(result, SpanAttributes.HTTP_METHOD, normalized) + if _report_new(sem_conv_opt_in_mode): + set_string_attribute(result, SpanAttributes.HTTP_REQUEST_METHOD, normalized) + + +def _set_http_url(result, url, sem_conv_opt_in_mode): + if _report_old(sem_conv_opt_in_mode): + set_string_attribute(result, SpanAttributes.HTTP_URL, url) + if _report_new(sem_conv_opt_in_mode): + set_string_attribute(result, SpanAttributes.URL_FULL, url) + + +def _set_http_scheme(result, scheme, sem_conv_opt_in_mode): + if _report_old(sem_conv_opt_in_mode): + set_string_attribute(result, SpanAttributes.HTTP_SCHEME, scheme) + # TODO: Support opt-in for scheme in new semconv + # if _report_new(sem_conv_opt_in_mode): + # set_string_attribute(result, SpanAttributes.URL_SCHEME, scheme) + + +def _set_http_hostname(result, hostname, sem_conv_opt_in_mode): + if _report_old(sem_conv_opt_in_mode): + set_string_attribute(result, SpanAttributes.HTTP_HOST, hostname) + if _report_new(sem_conv_opt_in_mode): + set_string_attribute(result, SpanAttributes.SERVER_ADDRESS, hostname) + + +def _set_http_net_peer_name(result, peer_name, sem_conv_opt_in_mode): + if _report_old(sem_conv_opt_in_mode): + set_string_attribute(result, SpanAttributes.NET_PEER_NAME, peer_name) + if _report_new(sem_conv_opt_in_mode): + set_string_attribute(result, SpanAttributes.SERVER_ADDRESS, peer_name) + + +def _set_http_port(result, port, sem_conv_opt_in_mode): + if _report_old(sem_conv_opt_in_mode): + set_string_attribute(result, SpanAttributes.NET_PEER_PORT, port) + if _report_new(sem_conv_opt_in_mode): + set_string_attribute(result, SpanAttributes.SERVER_PORT, port) + + +def _set_http_status_code(result, code, sem_conv_opt_in_mode): + if _report_old(sem_conv_opt_in_mode): + set_string_attribute(result, SpanAttributes.HTTP_STATUS_CODE, code) + if _report_new(sem_conv_opt_in_mode): + set_string_attribute(result, SpanAttributes.HTTP_RESPONSE_STATUS_CODE, code) + + +def _set_http_network_protocol_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, SpanAttributes.NET_PROTOCOL_VERSION, version) + + + +_OTEL_SEMCONV_STABILITY_OPT_IN_KEY = "OTEL_SEMCONV_STABILITY_OPT_IN" + +# Old to new http semantic convention mappings +_OTEL_HTTP_SEMCONV_MIGRATION_MAPPING = { + SpanAttributes.HTTP_METHOD: SpanAttributes.HTTP_REQUEST_METHOD, + SpanAttributes.HTTP_STATUS_CODE: SpanAttributes.HTTP_RESPONSE_STATUS_CODE, + SpanAttributes.HTTP_SCHEME: SpanAttributes.URL_SCHEME, + SpanAttributes.HTTP_URL: SpanAttributes.URL_FULL, + SpanAttributes.HTTP_REQUEST_CONTENT_LENGTH: SpanAttributes.HTTP_REQUEST_BODY_SIZE, + SpanAttributes.HTTP_RESPONSE_CONTENT_LENGTH: SpanAttributes.HTTP_RESPONSE_BODY_SIZE, + SpanAttributes.NET_HOST_NAME: SpanAttributes.SERVER_ADDRESS, + SpanAttributes.NET_HOST_PORT: SpanAttributes.SERVER_PORT, + SpanAttributes.NET_SOCK_HOST_ADDR: SpanAttributes.SERVER_SOCKET_ADDRESS, + SpanAttributes.NET_SOCK_HOST_PORT: SpanAttributes.SERVER_SOCKET_PORT, + SpanAttributes.NET_TRANSPORT: SpanAttributes.NETWORK_TRANSPORT, + SpanAttributes.NET_PROTOCOL_NAME: SpanAttributes.NETWORK_PROTOCOL_NAME, + SpanAttributes.NET_PROTOCOL_VERSION: SpanAttributes.NETWORK_PROTOCOL_VERSION, + SpanAttributes.NET_SOCK_FAMILY: SpanAttributes.NETWORK_TYPE, + SpanAttributes.NET_PEER_IP: SpanAttributes.CLIENT_SOCKET_ADDRESS, + SpanAttributes.NET_HOST_IP: SpanAttributes.SERVER_SOCKET_ADDRESS, + SpanAttributes.HTTP_SERVER_NAME: SpanAttributes.SERVER_ADDRESS, + SpanAttributes.HTTP_RETRY_COUNT: SpanAttributes.HTTP_RESEND_COUNT, + SpanAttributes.HTTP_REQUEST_CONTENT_LENGTH_UNCOMPRESSED: SpanAttributes.HTTP_REQUEST_BODY_SIZE, + SpanAttributes.HTTP_RESPONSE_CONTENT_LENGTH_UNCOMPRESSED: SpanAttributes.HTTP_RESPONSE_BODY_SIZE, + SpanAttributes.HTTP_USER_AGENT: SpanAttributes.USER_AGENT_ORIGINAL, + SpanAttributes.NET_APP_PROTOCOL_NAME: SpanAttributes.NETWORK_PROTOCOL_NAME, + SpanAttributes.NET_APP_PROTOCOL_VERSION: SpanAttributes.NETWORK_PROTOCOL_VERSION, + SpanAttributes.HTTP_CLIENT_IP: SpanAttributes.CLIENT_ADDRESS, + SpanAttributes.HTTP_FLAVOR: SpanAttributes.NETWORK_PROTOCOL_VERSION, + SpanAttributes.NET_HOST_CONNECTION_TYPE: SpanAttributes.NETWORK_CONNECTION_TYPE, + SpanAttributes.NET_HOST_CONNECTION_SUBTYPE: SpanAttributes.NETWORK_CONNECTION_SUBTYPE, + SpanAttributes.NET_HOST_CARRIER_NAME: SpanAttributes.NETWORK_CARRIER_NAME, + SpanAttributes.NET_HOST_CARRIER_MCC: SpanAttributes.NETWORK_CARRIER_MCC, + SpanAttributes.NET_HOST_CARRIER_MNC: SpanAttributes.NETWORK_CARRIER_MNC, +} + +# Special mappings handled case-by-case +_OTEL_HTTP_SEMCONV_MIGRATION_MAPPING_SPECIAL = { + SpanAttributes.HTTP_TARGET: (SpanAttributes.URL_PATH, SpanAttributes.URL_QUERY), + SpanAttributes.HTTP_HOST: (SpanAttributes.SERVER_ADDRESS, SpanAttributes.SERVER_PORT), +} + + +class _OpenTelemetryStabilitySignalType: + HTTP = "http" + + +class _OpenTelemetryStabilityMode(Enum): + # http - emit the new, stable HTTP and networking conventions ONLY + HTTP = "http" + # http/dup - emit both the old and the stable HTTP and networking conventions + HTTP_DUP = "http/dup" + # default - continue emitting old experimental HTTP and networking conventions + DEFAULT = "default" + + +def _report_new(mode): + return mode.name != _OpenTelemetryStabilityMode.DEFAULT.name + +def _report_old(mode): + return mode.name != _OpenTelemetryStabilityMode.HTTP.name + + +class _OpenTelemetrySemanticConventionStability: + _initialized = False + _lock = threading.Lock() + _OTEL_SEMCONV_STABILITY_SIGNAL_MAPPING = {} + + @classmethod + def _initialize(cls): + with _OpenTelemetrySemanticConventionStability._lock: + if not _OpenTelemetrySemanticConventionStability._initialized: + # Users can pass in comma delimited string for opt-in options + # Only values for http stability are supported for now + opt_in = os.environ.get(_OTEL_SEMCONV_STABILITY_OPT_IN_KEY, "") + opt_in_list = [] + if opt_in: + opt_in_list = [s.strip() for s in opt_in.split(",")] + http_opt_in = _OpenTelemetryStabilityMode.DEFAULT + if opt_in_list: + # Process http opt-in + # http/dup takes priority over http + if ( + _OpenTelemetryStabilityMode.HTTP_DUP.value + in opt_in_list + ): + http_opt_in = _OpenTelemetryStabilityMode.HTTP_DUP + elif _OpenTelemetryStabilityMode.HTTP.value in opt_in_list: + http_opt_in = _OpenTelemetryStabilityMode.HTTP + _OpenTelemetrySemanticConventionStability._OTEL_SEMCONV_STABILITY_SIGNAL_MAPPING[ + _OpenTelemetryStabilitySignalType.HTTP + ] = http_opt_in + _OpenTelemetrySemanticConventionStability._initialized = True + + + @classmethod + # Get OpenTelemetry opt-in mode based off of signal type (http, messaging, etc.) + def _get_opentelemetry_stability_opt_in_mode( + cls, + signal_type: _OpenTelemetryStabilitySignalType, + ) -> _OpenTelemetryStabilityMode: + return _OpenTelemetrySemanticConventionStability._OTEL_SEMCONV_STABILITY_SIGNAL_MAPPING.get( + signal_type, _OpenTelemetryStabilityMode.DEFAULT + ) + + +# Get new semantic convention attribute key based off old attribute key +# If no new attribute key exists for old attribute key, returns old attribute key +def _get_new_convention(old: str) -> str: + return _OTEL_HTTP_SEMCONV_MIGRATION_MAPPING.get(old, old) + + +# Get updated attributes based off of opt-in mode and client/server +def _get_attributes_based_on_stability_mode( + mode: _OpenTelemetryStabilityMode, + old_attributes: Dict[str, str], +) -> Dict[str, str]: + if mode is _OpenTelemetryStabilityMode.DEFAULT: + return old_attributes + attributes = {} + for old_key, value in old_attributes.items(): + new_key = _get_new_convention(old_key) + attributes[new_key] = value + if mode is _OpenTelemetryStabilityMode.HTTP_DUP: + attributes[old_key] = value + + return attributes diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/instrumentor.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/instrumentor.py index 6c6f86fd51..24f0d28cae 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/instrumentor.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/instrumentor.py @@ -25,7 +25,7 @@ DependencyConflict, get_dependency_conflicts, ) -from opentelemetry.instrumentation.utils import ( +from opentelemetry.instrumentation._semconv import ( _OpenTelemetrySemanticConventionStability, ) diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py index e4f9b37c37..2de2754bcc 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py @@ -155,60 +155,3 @@ def _python_path_without_directory(python_path, directory, path_separator): "", python_path, ) - - -_OTEL_SEMCONV_STABILITY_OPT_IN_KEY = "OTEL_SEMCONV_STABILITY_OPT_IN" - - -class _OpenTelemetryStabilitySignalType: - HTTP = "http" - - -class _OpenTelemetryStabilityMode(Enum): - # http - emit the new, stable HTTP and networking conventions ONLY - HTTP = "http" - # http/dup - emit both the old and the stable HTTP and networking conventions - HTTP_DUP = "http/dup" - # default - continue emitting old experimental HTTP and networking conventions - DEFAULT = "default" - - -class _OpenTelemetrySemanticConventionStability: - _initialized = False - _lock = threading.Lock() - _OTEL_SEMCONV_STABILITY_SIGNAL_MAPPING = {} - - @classmethod - def _initialize(cls): - with _OpenTelemetrySemanticConventionStability._lock: - if not _OpenTelemetrySemanticConventionStability._initialized: - # Users can pass in comma delimited string for opt-in options - # Only values for http stability are supported for now - opt_in = os.environ.get(_OTEL_SEMCONV_STABILITY_OPT_IN_KEY, "") - opt_in_list = [] - if opt_in: - opt_in_list = [s.strip() for s in opt_in.split(",")] - http_opt_in = _OpenTelemetryStabilityMode.DEFAULT - if opt_in_list: - # Process http opt-in - # http/dup takes priority over http - if ( - _OpenTelemetryStabilityMode.HTTP_DUP.value - in opt_in_list - ): - http_opt_in = _OpenTelemetryStabilityMode.HTTP_DUP - elif _OpenTelemetryStabilityMode.HTTP.value in opt_in_list: - http_opt_in = _OpenTelemetryStabilityMode.HTTP - _OpenTelemetrySemanticConventionStability._OTEL_SEMCONV_STABILITY_SIGNAL_MAPPING[ - _OpenTelemetryStabilitySignalType.HTTP - ] = http_opt_in - _OpenTelemetrySemanticConventionStability._initialized = True - - @classmethod - def _get_opentelemetry_stability_opt_in( - type: _OpenTelemetryStabilitySignalType, - ) -> _OpenTelemetryStabilityMode: - with _OpenTelemetrySemanticConventionStability._lock: - return _OpenTelemetrySemanticConventionStability._OTEL_SEMCONV_STABILITY_SIGNAL_MAPPING.get( - type, _OpenTelemetryStabilityMode.DEFAULT - ) From 919a2c7c6ba5383153940e6c3ee729db3b457b79 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Mon, 20 Nov 2023 14:35:11 -0800 Subject: [PATCH 03/18] sem --- .../opentelemetry/instrumentation/_semconv.py | 67 ------------------- 1 file changed, 67 deletions(-) diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py index 4773fb151b..526284a955 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py @@ -15,10 +15,6 @@ import os import threading from enum import Enum -from re import escape, sub -from typing import Dict, Optional, Sequence - - from opentelemetry.semconv.trace import SpanAttributes @@ -103,46 +99,6 @@ def _set_http_network_protocol_version(result, version, sem_conv_opt_in_mode): _OTEL_SEMCONV_STABILITY_OPT_IN_KEY = "OTEL_SEMCONV_STABILITY_OPT_IN" -# Old to new http semantic convention mappings -_OTEL_HTTP_SEMCONV_MIGRATION_MAPPING = { - SpanAttributes.HTTP_METHOD: SpanAttributes.HTTP_REQUEST_METHOD, - SpanAttributes.HTTP_STATUS_CODE: SpanAttributes.HTTP_RESPONSE_STATUS_CODE, - SpanAttributes.HTTP_SCHEME: SpanAttributes.URL_SCHEME, - SpanAttributes.HTTP_URL: SpanAttributes.URL_FULL, - SpanAttributes.HTTP_REQUEST_CONTENT_LENGTH: SpanAttributes.HTTP_REQUEST_BODY_SIZE, - SpanAttributes.HTTP_RESPONSE_CONTENT_LENGTH: SpanAttributes.HTTP_RESPONSE_BODY_SIZE, - SpanAttributes.NET_HOST_NAME: SpanAttributes.SERVER_ADDRESS, - SpanAttributes.NET_HOST_PORT: SpanAttributes.SERVER_PORT, - SpanAttributes.NET_SOCK_HOST_ADDR: SpanAttributes.SERVER_SOCKET_ADDRESS, - SpanAttributes.NET_SOCK_HOST_PORT: SpanAttributes.SERVER_SOCKET_PORT, - SpanAttributes.NET_TRANSPORT: SpanAttributes.NETWORK_TRANSPORT, - SpanAttributes.NET_PROTOCOL_NAME: SpanAttributes.NETWORK_PROTOCOL_NAME, - SpanAttributes.NET_PROTOCOL_VERSION: SpanAttributes.NETWORK_PROTOCOL_VERSION, - SpanAttributes.NET_SOCK_FAMILY: SpanAttributes.NETWORK_TYPE, - SpanAttributes.NET_PEER_IP: SpanAttributes.CLIENT_SOCKET_ADDRESS, - SpanAttributes.NET_HOST_IP: SpanAttributes.SERVER_SOCKET_ADDRESS, - SpanAttributes.HTTP_SERVER_NAME: SpanAttributes.SERVER_ADDRESS, - SpanAttributes.HTTP_RETRY_COUNT: SpanAttributes.HTTP_RESEND_COUNT, - SpanAttributes.HTTP_REQUEST_CONTENT_LENGTH_UNCOMPRESSED: SpanAttributes.HTTP_REQUEST_BODY_SIZE, - SpanAttributes.HTTP_RESPONSE_CONTENT_LENGTH_UNCOMPRESSED: SpanAttributes.HTTP_RESPONSE_BODY_SIZE, - SpanAttributes.HTTP_USER_AGENT: SpanAttributes.USER_AGENT_ORIGINAL, - SpanAttributes.NET_APP_PROTOCOL_NAME: SpanAttributes.NETWORK_PROTOCOL_NAME, - SpanAttributes.NET_APP_PROTOCOL_VERSION: SpanAttributes.NETWORK_PROTOCOL_VERSION, - SpanAttributes.HTTP_CLIENT_IP: SpanAttributes.CLIENT_ADDRESS, - SpanAttributes.HTTP_FLAVOR: SpanAttributes.NETWORK_PROTOCOL_VERSION, - SpanAttributes.NET_HOST_CONNECTION_TYPE: SpanAttributes.NETWORK_CONNECTION_TYPE, - SpanAttributes.NET_HOST_CONNECTION_SUBTYPE: SpanAttributes.NETWORK_CONNECTION_SUBTYPE, - SpanAttributes.NET_HOST_CARRIER_NAME: SpanAttributes.NETWORK_CARRIER_NAME, - SpanAttributes.NET_HOST_CARRIER_MCC: SpanAttributes.NETWORK_CARRIER_MCC, - SpanAttributes.NET_HOST_CARRIER_MNC: SpanAttributes.NETWORK_CARRIER_MNC, -} - -# Special mappings handled case-by-case -_OTEL_HTTP_SEMCONV_MIGRATION_MAPPING_SPECIAL = { - SpanAttributes.HTTP_TARGET: (SpanAttributes.URL_PATH, SpanAttributes.URL_QUERY), - SpanAttributes.HTTP_HOST: (SpanAttributes.SERVER_ADDRESS, SpanAttributes.SERVER_PORT), -} - class _OpenTelemetryStabilitySignalType: HTTP = "http" @@ -205,26 +161,3 @@ def _get_opentelemetry_stability_opt_in_mode( return _OpenTelemetrySemanticConventionStability._OTEL_SEMCONV_STABILITY_SIGNAL_MAPPING.get( signal_type, _OpenTelemetryStabilityMode.DEFAULT ) - - -# Get new semantic convention attribute key based off old attribute key -# If no new attribute key exists for old attribute key, returns old attribute key -def _get_new_convention(old: str) -> str: - return _OTEL_HTTP_SEMCONV_MIGRATION_MAPPING.get(old, old) - - -# Get updated attributes based off of opt-in mode and client/server -def _get_attributes_based_on_stability_mode( - mode: _OpenTelemetryStabilityMode, - old_attributes: Dict[str, str], -) -> Dict[str, str]: - if mode is _OpenTelemetryStabilityMode.DEFAULT: - return old_attributes - attributes = {} - for old_key, value in old_attributes.items(): - new_key = _get_new_convention(old_key) - attributes[new_key] = value - if mode is _OpenTelemetryStabilityMode.HTTP_DUP: - attributes[old_key] = value - - return attributes From f299243a3f269bd44b807f26bab5d28e59adb31d Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Tue, 21 Nov 2023 11:09:11 -0800 Subject: [PATCH 04/18] test --- .../tests/test_requests_integration.py | 47 ++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py index 3bd76a6995..12178bc987 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py +++ b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py @@ -27,6 +27,10 @@ from opentelemetry.context import _SUPPRESS_HTTP_INSTRUMENTATION_KEY from opentelemetry.instrumentation.requests import RequestsInstrumentor from opentelemetry.instrumentation.utils import _SUPPRESS_INSTRUMENTATION_KEY +from opentelemetry.instrumentation._semconv import ( + _OTEL_SEMCONV_STABILITY_OPT_IN_KEY, + _OpenTelemetrySemanticConventionStability, +) from opentelemetry.propagate import get_global_textmap, set_global_textmap from opentelemetry.sdk import resources from opentelemetry.semconv.trace import SpanAttributes @@ -64,17 +68,30 @@ class RequestsIntegrationTestBase(abc.ABC): # pylint: disable=too-many-public-methods URL = "http://mock/status/200" + HOST = "mock/status" # pylint: disable=invalid-name def setUp(self): super().setUp() + test_name = "" + if hasattr(self, "_testMethodName"): + test_name = self._testMethodName + sem_conv_mode = "default" + if "new_semconv" in test_name: + sem_conv_mode = "http" + elif "both_semconv" in test_name: + sem_conv_mode = "http/dup" self.env_patch = mock.patch.dict( "os.environ", { - "OTEL_PYTHON_REQUESTS_EXCLUDED_URLS": "http://localhost/env_excluded_arg/123,env_excluded_noarg" + "OTEL_PYTHON_REQUESTS_EXCLUDED_URLS": "http://localhost/env_excluded_arg/123,env_excluded_noarg", + _OTEL_SEMCONV_STABILITY_OPT_IN_KEY: sem_conv_mode, }, ) + + _OpenTelemetrySemanticConventionStability._initialized = False + self.env_patch.start() self.exclude_patch = mock.patch( @@ -133,6 +150,34 @@ def test_basic(self): span, opentelemetry.instrumentation.requests ) + + # def test_basic_new_semconv(self): + # print("here2") + # result = self.perform_request(self.URL) + # self.assertEqual(result.text, "Hello!") + # span = self.assert_span() + + # self.assertIs(span.kind, trace.SpanKind.CLIENT) + # self.assertEqual(span.name, "GET") + + # self.assertEqual( + # span.attributes, + # { + # SpanAttributes.HTTP_REQUEST_METHOD: "GET", + # SpanAttributes.HTTP_REQUEST_METHOD_ORIGINAL: "GET", + # SpanAttributes.URL_FULL: self.URL, + # SpanAttributes.HTTP_RESPONSE_STATUS_CODE: 200, + # SpanAttributes.SERVER_ADDRESS: self.HOST, + # }, + # ) + + # self.assertIs(span.status.status_code, trace.StatusCode.UNSET) + + # self.assertEqualSpanInstrumentationInfo( + # span, opentelemetry.instrumentation.requests + # ) + + def test_hooks(self): def request_hook(span, request_obj): span.update_name("name set from hook") From 5f00479f3d392e749d68eb01f419351cfaebfc18 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Tue, 21 Nov 2023 12:37:53 -0800 Subject: [PATCH 05/18] tests --- .../instrumentation/requests/__init__.py | 30 ++- .../tests/test_requests_integration.py | 240 +++++++++++++++--- .../opentelemetry/instrumentation/_semconv.py | 40 ++- 3 files changed, 265 insertions(+), 45 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py index 29e2cae8bd..0019cc310a 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py @@ -70,6 +70,7 @@ http_status_to_status_code, ) from opentelemetry.instrumentation._semconv import ( + _filter_duration_attrs, _report_old, _report_new, _set_http_hostname, @@ -80,6 +81,9 @@ _set_http_scheme, _set_http_url, _set_http_status_code, + _SPAN_ATTRIBUTES_ERROR_TYPE, + _SPAN_ATTRIBUTES_NETWORK_PEER_ADDRESS, + _SPAN_ATTRIBUTES_NETWORK_PEER_PORT, _OpenTelemetrySemanticConventionStability, _OpenTelemetryStabilityMode, _OpenTelemetryStabilitySignalType, @@ -105,11 +109,6 @@ _RequestHookT = Optional[Callable[[Span, PreparedRequest], None]] _ResponseHookT = Optional[Callable[[Span, PreparedRequest], None]] -# TODO: will come through semconv package once updated -_SPAN_ATTRIBUTES_ERROR_TYPE = "error.type" -_SPAN_ATTRIBUTES_NETWORK_PEER_ADDRESS ="network.peer.address" -_SPAN_ATTRIBUTES_NETWORK_PEER_PORT = "network.peer.port" - # pylint: disable=unused-argument # pylint: disable=R0915 @@ -177,11 +176,13 @@ def get_or_create_headers(): _set_http_net_peer_name(metric_labels, parsed_url.hostname, sem_conv_opt_in_mode) if _report_new(sem_conv_opt_in_mode): _set_http_hostname(span_attributes, parsed_url.hostname, sem_conv_opt_in_mode) + # Use semconv library when available span_attributes[_SPAN_ATTRIBUTES_NETWORK_PEER_ADDRESS] = parsed_url.hostname if parsed_url.port: _set_http_port(metric_labels, parsed_url.port, sem_conv_opt_in_mode) if _report_new(sem_conv_opt_in_mode): _set_http_port(span_attributes, parsed_url.port, sem_conv_opt_in_mode) + # Use semconv library when available span_attributes[_SPAN_ATTRIBUTES_NETWORK_PEER_PORT] = parsed_url.port except ValueError: pass @@ -229,20 +230,23 @@ def get_or_create_headers(): _set_http_network_protocol_version(metric_labels, version_text, sem_conv_opt_in_mode) if _report_new(sem_conv_opt_in_mode): _set_http_network_protocol_version(span_attributes, version_text, sem_conv_opt_in_mode) - if exception is not None: - if _report_new(sem_conv_opt_in_mode): - span_attributes[_SPAN_ATTRIBUTES_ERROR_TYPE] = type(exception).__name__ - metric_labels[_SPAN_ATTRIBUTES_ERROR_TYPE] = type(exception).__name__ for k, v in span_attributes.items(): span.set_attribute(k, v) if callable(response_hook): response_hook(span, request, result) - if _report_old(sem_conv_opt_in_mode) and duration_histogram_old is not None: - duration_histogram_old.record(max(round(elapsed_time * 1000), 0), attributes=metric_labels) - # duration_histogram_new.record(elapsed_time, attributes=metric_labels) - + if exception is not None and _report_new(sem_conv_opt_in_mode): + span.set_attribute(_SPAN_ATTRIBUTES_ERROR_TYPE, type(exception).__name__) + metric_labels[_SPAN_ATTRIBUTES_ERROR_TYPE] = type(exception).__name__ + + if duration_histogram_old is not None: + duration_attrs_old = _filter_duration_attrs(metric_labels, _OpenTelemetryStabilityMode.DEFAULT) + duration_histogram_old.record(max(round(elapsed_time * 1000), 0), attributes=duration_attrs_old) + if duration_histogram_new is not None: + duration_attrs_new = _filter_duration_attrs(metric_labels, _OpenTelemetryStabilityMode.HTTP) + duration_histogram_new.record(elapsed_time, attributes=duration_attrs_new) + if exception is not None: raise exception.with_traceback(exception.__traceback__) diff --git a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py index 12178bc987..c845f73d19 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py +++ b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py @@ -28,6 +28,8 @@ from opentelemetry.instrumentation.requests import RequestsInstrumentor from opentelemetry.instrumentation.utils import _SUPPRESS_INSTRUMENTATION_KEY from opentelemetry.instrumentation._semconv import ( + _SPAN_ATTRIBUTES_ERROR_TYPE, + _SPAN_ATTRIBUTES_NETWORK_PEER_ADDRESS, _OTEL_SEMCONV_STABILITY_OPT_IN_KEY, _OpenTelemetrySemanticConventionStability, ) @@ -68,7 +70,7 @@ class RequestsIntegrationTestBase(abc.ABC): # pylint: disable=too-many-public-methods URL = "http://mock/status/200" - HOST = "mock/status" + HOST = "mock" # pylint: disable=invalid-name def setUp(self): @@ -150,33 +152,62 @@ def test_basic(self): span, opentelemetry.instrumentation.requests ) + def test_basic_new_semconv(self): + result = self.perform_request(self.URL) + self.assertEqual(result.text, "Hello!") + span = self.assert_span() - # def test_basic_new_semconv(self): - # print("here2") - # result = self.perform_request(self.URL) - # self.assertEqual(result.text, "Hello!") - # span = self.assert_span() + self.assertIs(span.kind, trace.SpanKind.CLIENT) + self.assertEqual(span.name, "GET") - # self.assertIs(span.kind, trace.SpanKind.CLIENT) - # self.assertEqual(span.name, "GET") + self.assertEqual( + span.attributes, + { + SpanAttributes.HTTP_REQUEST_METHOD: "GET", + SpanAttributes.URL_FULL: self.URL, + SpanAttributes.SERVER_ADDRESS: self.HOST, + _SPAN_ATTRIBUTES_NETWORK_PEER_ADDRESS: "mock", + SpanAttributes.HTTP_RESPONSE_STATUS_CODE: 200, + SpanAttributes.NET_PROTOCOL_VERSION: "1.1", + }, + ) + + self.assertIs(span.status.status_code, trace.StatusCode.UNSET) + + self.assertEqualSpanInstrumentationScope( + span, opentelemetry.instrumentation.requests + ) + + def test_basic_both_semconv(self): + result = self.perform_request(self.URL) + self.assertEqual(result.text, "Hello!") + span = self.assert_span() - # self.assertEqual( - # span.attributes, - # { - # SpanAttributes.HTTP_REQUEST_METHOD: "GET", - # SpanAttributes.HTTP_REQUEST_METHOD_ORIGINAL: "GET", - # SpanAttributes.URL_FULL: self.URL, - # SpanAttributes.HTTP_RESPONSE_STATUS_CODE: 200, - # SpanAttributes.SERVER_ADDRESS: self.HOST, - # }, - # ) + self.assertIs(span.kind, trace.SpanKind.CLIENT) + self.assertEqual(span.name, "GET") - # self.assertIs(span.status.status_code, trace.StatusCode.UNSET) + self.assertEqual( + span.attributes, + { + SpanAttributes.HTTP_METHOD: "GET", + SpanAttributes.HTTP_REQUEST_METHOD: "GET", + SpanAttributes.HTTP_URL: self.URL, + SpanAttributes.URL_FULL: self.URL, + SpanAttributes.HTTP_HOST: self.HOST, + SpanAttributes.SERVER_ADDRESS: self.HOST, + _SPAN_ATTRIBUTES_NETWORK_PEER_ADDRESS: "mock", + SpanAttributes.HTTP_STATUS_CODE: 200, + SpanAttributes.HTTP_RESPONSE_STATUS_CODE: 200, + SpanAttributes.HTTP_FLAVOR: "1.1", + SpanAttributes.NET_PROTOCOL_VERSION: "1.1", + }, + ) - # self.assertEqualSpanInstrumentationInfo( - # span, opentelemetry.instrumentation.requests - # ) + self.assertIs(span.status.status_code, trace.StatusCode.UNSET) + self.assertEqualSpanInstrumentationScope( + span, opentelemetry.instrumentation.requests + ) def test_hooks(self): def request_hook(span, request_obj): @@ -259,6 +290,51 @@ def test_not_foundbasic(self): trace.StatusCode.ERROR, ) + def test_not_foundbasic_new_semconv(self): + url_404 = "http://mock/status/404" + httpretty.register_uri( + httpretty.GET, + url_404, + status=404, + ) + result = self.perform_request(url_404) + self.assertEqual(result.status_code, 404) + + span = self.assert_span() + + self.assertEqual( + span.attributes.get(SpanAttributes.HTTP_RESPONSE_STATUS_CODE), 404 + ) + + self.assertIs( + span.status.status_code, + trace.StatusCode.ERROR, + ) + + def test_not_foundbasic_both_semconv(self): + url_404 = "http://mock/status/404" + httpretty.register_uri( + httpretty.GET, + url_404, + status=404, + ) + result = self.perform_request(url_404) + self.assertEqual(result.status_code, 404) + + span = self.assert_span() + + self.assertEqual( + span.attributes.get(SpanAttributes.HTTP_STATUS_CODE), 404 + ) + self.assertEqual( + span.attributes.get(SpanAttributes.HTTP_RESPONSE_STATUS_CODE), 404 + ) + + self.assertIs( + span.status.status_code, + trace.StatusCode.ERROR, + ) + def test_uninstrument(self): RequestsInstrumentor().uninstrument() result = self.perform_request(self.URL) @@ -413,6 +489,27 @@ def test_requests_exception_without_response(self, *_, **__): ) self.assertEqual(span.status.status_code, StatusCode.ERROR) + @mock.patch( + "requests.adapters.HTTPAdapter.send", + side_effect=requests.RequestException, + ) + def test_requests_exception_new_semconv(self, *_, **__): + with self.assertRaises(requests.RequestException): + self.perform_request(self.URL) + + span = self.assert_span() + self.assertEqual( + span.attributes, + { + SpanAttributes.HTTP_REQUEST_METHOD: "GET", + SpanAttributes.URL_FULL: self.URL, + SpanAttributes.SERVER_ADDRESS: self.HOST, + _SPAN_ATTRIBUTES_NETWORK_PEER_ADDRESS: "mock", + _SPAN_ATTRIBUTES_ERROR_TYPE: "RequestException", + }, + ) + self.assertEqual(span.status.status_code, StatusCode.ERROR) + mocked_response = requests.Response() mocked_response.status_code = 500 mocked_response.reason = "Internal Server Error" @@ -534,6 +631,22 @@ class TestRequestsIntergrationMetric(TestBase): def setUp(self): super().setUp() + test_name = "" + if hasattr(self, "_testMethodName"): + test_name = self._testMethodName + sem_conv_mode = "default" + if "new_semconv" in test_name: + sem_conv_mode = "http" + elif "both_semconv" in test_name: + sem_conv_mode = "http/dup" + self.env_patch = mock.patch.dict( + "os.environ", + { + _OTEL_SEMCONV_STABILITY_OPT_IN_KEY: sem_conv_mode, + }, + ) + self.env_patch.start() + _OpenTelemetrySemanticConventionStability._initialized = False RequestsInstrumentor().instrument(meter_provider=self.meter_provider) httpretty.enable() @@ -541,6 +654,7 @@ def setUp(self): def tearDown(self): super().tearDown() + self.env_patch.stop() RequestsInstrumentor().uninstrument() httpretty.disable() @@ -552,22 +666,90 @@ def test_basic_metric_success(self): self.perform_request(self.URL) expected_attributes = { - "http.status_code": 200, - "http.host": "examplehost", - "net.peer.port": 8000, - "net.peer.name": "examplehost", - "http.method": "GET", - "http.flavor": "1.1", - "http.scheme": "http", + SpanAttributes.HTTP_STATUS_CODE: 200, + SpanAttributes.HTTP_HOST: "examplehost", + SpanAttributes.NET_PEER_PORT: 8000, + SpanAttributes.NET_PEER_NAME: "examplehost", + SpanAttributes.HTTP_METHOD: "GET", + SpanAttributes.HTTP_FLAVOR: "1.1", + SpanAttributes.HTTP_SCHEME: "http", } for ( resource_metrics ) in self.memory_metrics_reader.get_metrics_data().resource_metrics: for scope_metrics in resource_metrics.scope_metrics: + self.assertEqual(len(scope_metrics.metrics), 1) for metric in scope_metrics.metrics: + self.assertEqual(metric.unit, "ms") + self.assertEqual(metric.description, "measures the duration of the outbound HTTP request") for data_point in metric.data.data_points: self.assertDictEqual( expected_attributes, dict(data_point.attributes) ) self.assertEqual(data_point.count, 1) + + def test_basic_metric_new_semconv(self): + self.perform_request(self.URL) + + expected_attributes = { + SpanAttributes.HTTP_RESPONSE_STATUS_CODE: 200, + SpanAttributes.SERVER_ADDRESS: "examplehost", + SpanAttributes.SERVER_PORT: 8000, + SpanAttributes.SERVER_ADDRESS: "examplehost", + SpanAttributes.HTTP_REQUEST_METHOD: "GET", + SpanAttributes.NET_PROTOCOL_VERSION: "1.1", + } + + for ( + resource_metrics + ) in self.memory_metrics_reader.get_metrics_data().resource_metrics: + for scope_metrics in resource_metrics.scope_metrics: + self.assertEqual(len(scope_metrics.metrics), 1) + for metric in scope_metrics.metrics: + self.assertEqual(metric.unit, "s") + self.assertEqual(metric.description, "Duration of HTTP client requests.") + for data_point in metric.data.data_points: + self.assertDictEqual( + expected_attributes, dict(data_point.attributes) + ) + self.assertEqual(data_point.count, 1) + + def test_basic_metric_both_semconv(self): + self.perform_request(self.URL) + + expected_attributes_old = { + SpanAttributes.HTTP_STATUS_CODE: 200, + SpanAttributes.HTTP_HOST: "examplehost", + SpanAttributes.NET_PEER_PORT: 8000, + SpanAttributes.NET_PEER_NAME: "examplehost", + SpanAttributes.HTTP_METHOD: "GET", + SpanAttributes.HTTP_FLAVOR: "1.1", + SpanAttributes.HTTP_SCHEME: "http", + } + + expected_attributes_new = { + SpanAttributes.HTTP_RESPONSE_STATUS_CODE: 200, + SpanAttributes.SERVER_ADDRESS: "examplehost", + SpanAttributes.SERVER_PORT: 8000, + SpanAttributes.SERVER_ADDRESS: "examplehost", + SpanAttributes.HTTP_REQUEST_METHOD: "GET", + SpanAttributes.NET_PROTOCOL_VERSION: "1.1", + } + + for ( + resource_metrics + ) in self.memory_metrics_reader.get_metrics_data().resource_metrics: + for scope_metrics in resource_metrics.scope_metrics: + self.assertEqual(len(scope_metrics.metrics), 2) + for metric in scope_metrics.metrics: + for data_point in metric.data.data_points: + if metric.unit == "ms": + self.assertDictEqual( + expected_attributes_old, dict(data_point.attributes) + ) + else: + self.assertDictEqual( + expected_attributes_new, dict(data_point.attributes) + ) + self.assertEqual(data_point.count, 1) diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py index 526284a955..39dd1ec726 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py @@ -18,6 +18,40 @@ from opentelemetry.semconv.trace import SpanAttributes +# TODO: will come through semconv package once updated +_SPAN_ATTRIBUTES_ERROR_TYPE = "error.type" +_SPAN_ATTRIBUTES_NETWORK_PEER_ADDRESS ="network.peer.address" +_SPAN_ATTRIBUTES_NETWORK_PEER_PORT = "network.peer.port" + +_client_duration_attrs_old = [ + SpanAttributes.HTTP_STATUS_CODE, + SpanAttributes.HTTP_HOST, + SpanAttributes.NET_PEER_PORT, + SpanAttributes.NET_PEER_NAME, + SpanAttributes.HTTP_METHOD, + SpanAttributes.HTTP_FLAVOR, + SpanAttributes.HTTP_SCHEME, +] + +_client_duration_attrs_new = [ + _SPAN_ATTRIBUTES_ERROR_TYPE, + SpanAttributes.HTTP_REQUEST_METHOD_ORIGINAL, + SpanAttributes.HTTP_REQUEST_METHOD, + SpanAttributes.HTTP_RESPONSE_STATUS_CODE, + SpanAttributes.NET_PROTOCOL_VERSION, + SpanAttributes.SERVER_ADDRESS, + SpanAttributes.SERVER_PORT, + SpanAttributes.URL_SCHEME, +] + +def _filter_duration_attrs(attrs, sem_conv_opt_in_mode): + filtered_attrs = {} + allowed_attributes = _client_duration_attrs_new if sem_conv_opt_in_mode == _OpenTelemetryStabilityMode.HTTP else _client_duration_attrs_old + for key, val in attrs.items(): + if key in allowed_attributes: + filtered_attrs[key] = val + return filtered_attrs + def set_string_attribute(dict, key, value): if value: @@ -37,7 +71,7 @@ def _set_http_method(result, original, normalized, sem_conv_opt_in_mode): normalized = normalized.strip() # See https://github.com/open-telemetry/semantic-conventions/blob/main/docs/http/http-spans.md#common-attributes # Method is case sensitive. "http.request.method_original" should not be sanitized or automatically capitalized. - if original != normalized and sem_conv_opt_in_mode != _OpenTelemetryStabilityMode.DEFAULT: + if original != normalized and _report_new(): set_string_attribute(result, SpanAttributes.HTTP_REQUEST_METHOD_ORIGINAL, original) if _report_old(sem_conv_opt_in_mode): @@ -84,9 +118,9 @@ def _set_http_port(result, port, sem_conv_opt_in_mode): def _set_http_status_code(result, code, sem_conv_opt_in_mode): if _report_old(sem_conv_opt_in_mode): - set_string_attribute(result, SpanAttributes.HTTP_STATUS_CODE, code) + set_int_attribute(result, SpanAttributes.HTTP_STATUS_CODE, code) if _report_new(sem_conv_opt_in_mode): - set_string_attribute(result, SpanAttributes.HTTP_RESPONSE_STATUS_CODE, code) + set_int_attribute(result, SpanAttributes.HTTP_RESPONSE_STATUS_CODE, code) def _set_http_network_protocol_version(result, version, sem_conv_opt_in_mode): From aa891acadc713e4cceed15d3fdedbe24aae52ec1 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Tue, 21 Nov 2023 13:30:35 -0800 Subject: [PATCH 06/18] changelog --- .github/workflows/test.yml | 2 +- CHANGELOG.md | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 64c39745f2..4eba781aff 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -6,7 +6,7 @@ on: - 'release/*' pull_request: env: - CORE_REPO_SHA: 9831afaff5b4d371fd9a14266ab47884546bd971 + CORE_REPO_SHA: 35a021194787359324c46f5ca99d31802e4c92bd jobs: build: diff --git a/CHANGELOG.md b/CHANGELOG.md index fa52f4cde4..3a8ba0125c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#1987](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1987)) - `opentelemetry-instrumentation-httpx` Fix mixing async and non async hooks ([#1920](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1920)) +- `opentelemetry-instrumentation-requests` Implement new semantic convention opt-in with stable http semantic conventions + ([#2002](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2002)) ### Fixed From e0652cb710cc0203c460fc1c581a73c90cd98c98 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Wed, 22 Nov 2023 09:07:56 -0800 Subject: [PATCH 07/18] lint --- .../instrumentation/requests/__init__.py | 89 ++++++++++++++----- .../tests/test_requests_integration.py | 15 +++- .../opentelemetry/instrumentation/_semconv.py | 28 ++++-- 3 files changed, 98 insertions(+), 34 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py index 0019cc310a..3bc2f5be20 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py @@ -165,25 +165,45 @@ def get_or_create_headers(): _set_http_url(span_attributes, url, sem_conv_opt_in_mode) metric_labels = {} - _set_http_method(metric_labels, method, span_name, sem_conv_opt_in_mode) + _set_http_method( + metric_labels, method, span_name, sem_conv_opt_in_mode + ) try: parsed_url = urlparse(url) if parsed_url.scheme: - _set_http_scheme(metric_labels, parsed_url.scheme, sem_conv_opt_in_mode) + _set_http_scheme( + metric_labels, parsed_url.scheme, sem_conv_opt_in_mode + ) if parsed_url.hostname: - _set_http_hostname(metric_labels, parsed_url.hostname, sem_conv_opt_in_mode) - _set_http_net_peer_name(metric_labels, parsed_url.hostname, sem_conv_opt_in_mode) + _set_http_hostname( + metric_labels, parsed_url.hostname, sem_conv_opt_in_mode + ) + _set_http_net_peer_name( + metric_labels, parsed_url.hostname, sem_conv_opt_in_mode + ) if _report_new(sem_conv_opt_in_mode): - _set_http_hostname(span_attributes, parsed_url.hostname, sem_conv_opt_in_mode) + _set_http_hostname( + span_attributes, + parsed_url.hostname, + sem_conv_opt_in_mode, + ) # Use semconv library when available - span_attributes[_SPAN_ATTRIBUTES_NETWORK_PEER_ADDRESS] = parsed_url.hostname + span_attributes[ + _SPAN_ATTRIBUTES_NETWORK_PEER_ADDRESS + ] = parsed_url.hostname if parsed_url.port: - _set_http_port(metric_labels, parsed_url.port, sem_conv_opt_in_mode) + _set_http_port( + metric_labels, parsed_url.port, sem_conv_opt_in_mode + ) if _report_new(sem_conv_opt_in_mode): - _set_http_port(span_attributes, parsed_url.port, sem_conv_opt_in_mode) + _set_http_port( + span_attributes, parsed_url.port, sem_conv_opt_in_mode + ) # Use semconv library when available - span_attributes[_SPAN_ATTRIBUTES_NETWORK_PEER_PORT] = parsed_url.port + span_attributes[ + _SPAN_ATTRIBUTES_NETWORK_PEER_PORT + ] = parsed_url.port except ValueError: pass @@ -215,21 +235,33 @@ def get_or_create_headers(): if isinstance(result, Response): span_attributes = {} if span.is_recording(): - _set_http_status_code(span_attributes, result.status_code, sem_conv_opt_in_mode) + _set_http_status_code( + span_attributes, + result.status_code, + sem_conv_opt_in_mode, + ) span.set_status( Status(http_status_to_status_code(result.status_code)) ) - _set_http_status_code(metric_labels, result.status_code, sem_conv_opt_in_mode) + _set_http_status_code( + metric_labels, result.status_code, sem_conv_opt_in_mode + ) if result.raw is not None: version = getattr(result.raw, "version", None) if version: # Only HTTP/1 is supported by requests version_text = "1.1" if version == 11 else "1.0" - _set_http_network_protocol_version(metric_labels, version_text, sem_conv_opt_in_mode) + _set_http_network_protocol_version( + metric_labels, version_text, sem_conv_opt_in_mode + ) if _report_new(sem_conv_opt_in_mode): - _set_http_network_protocol_version(span_attributes, version_text, sem_conv_opt_in_mode) + _set_http_network_protocol_version( + span_attributes, + version_text, + sem_conv_opt_in_mode, + ) for k, v in span_attributes.items(): span.set_attribute(k, v) @@ -237,16 +269,29 @@ def get_or_create_headers(): response_hook(span, request, result) if exception is not None and _report_new(sem_conv_opt_in_mode): - span.set_attribute(_SPAN_ATTRIBUTES_ERROR_TYPE, type(exception).__name__) - metric_labels[_SPAN_ATTRIBUTES_ERROR_TYPE] = type(exception).__name__ + span.set_attribute( + _SPAN_ATTRIBUTES_ERROR_TYPE, type(exception).__name__ + ) + metric_labels[_SPAN_ATTRIBUTES_ERROR_TYPE] = type( + exception + ).__name__ if duration_histogram_old is not None: - duration_attrs_old = _filter_duration_attrs(metric_labels, _OpenTelemetryStabilityMode.DEFAULT) - duration_histogram_old.record(max(round(elapsed_time * 1000), 0), attributes=duration_attrs_old) + duration_attrs_old = _filter_duration_attrs( + metric_labels, _OpenTelemetryStabilityMode.DEFAULT + ) + duration_histogram_old.record( + max(round(elapsed_time * 1000), 0), + attributes=duration_attrs_old + ) if duration_histogram_new is not None: - duration_attrs_new = _filter_duration_attrs(metric_labels, _OpenTelemetryStabilityMode.HTTP) - duration_histogram_new.record(elapsed_time, attributes=duration_attrs_new) - + duration_attrs_new = _filter_duration_attrs( + metric_labels, _OpenTelemetryStabilityMode.HTTP + ) + duration_histogram_new.record( + elapsed_time, attributes=duration_attrs_new + ) + if exception is not None: raise exception.with_traceback(exception.__traceback__) @@ -327,8 +372,8 @@ def _instrument(self, **kwargs): schema_url="https://opentelemetry.io/schemas/1.11.0", ) semconv_opt_in_mode = _OpenTelemetrySemanticConventionStability._get_opentelemetry_stability_opt_in_mode( - _OpenTelemetryStabilitySignalType.HTTP, - ) + _OpenTelemetryStabilitySignalType.HTTP, + ) duration_histogram_old = None if _report_old(semconv_opt_in_mode): duration_histogram_old = meter.create_histogram( diff --git a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py index 66b18a3b07..756c18e456 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py +++ b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py @@ -682,7 +682,10 @@ def test_basic_metric_success(self): self.assertEqual(len(scope_metrics.metrics), 1) for metric in scope_metrics.metrics: self.assertEqual(metric.unit, "ms") - self.assertEqual(metric.description, "measures the duration of the outbound HTTP request") + self.assertEqual( + metric.description, + "measures the duration of the outbound HTTP request", + ) for data_point in metric.data.data_points: self.assertDictEqual( expected_attributes, dict(data_point.attributes) @@ -708,7 +711,9 @@ def test_basic_metric_new_semconv(self): self.assertEqual(len(scope_metrics.metrics), 1) for metric in scope_metrics.metrics: self.assertEqual(metric.unit, "s") - self.assertEqual(metric.description, "Duration of HTTP client requests.") + self.assertEqual( + metric.description, "Duration of HTTP client requests." + ) for data_point in metric.data.data_points: self.assertDictEqual( expected_attributes, dict(data_point.attributes) @@ -746,10 +751,12 @@ def test_basic_metric_both_semconv(self): for data_point in metric.data.data_points: if metric.unit == "ms": self.assertDictEqual( - expected_attributes_old, dict(data_point.attributes) + expected_attributes_old, + dict(data_point.attributes), ) else: self.assertDictEqual( - expected_attributes_new, dict(data_point.attributes) + expected_attributes_new, + dict(data_point.attributes), ) self.assertEqual(data_point.count, 1) diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py index 39dd1ec726..47d9f3bc73 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py @@ -20,7 +20,7 @@ # TODO: will come through semconv package once updated _SPAN_ATTRIBUTES_ERROR_TYPE = "error.type" -_SPAN_ATTRIBUTES_NETWORK_PEER_ADDRESS ="network.peer.address" +_SPAN_ATTRIBUTES_NETWORK_PEER_ADDRESS = "network.peer.address" _SPAN_ATTRIBUTES_NETWORK_PEER_PORT = "network.peer.port" _client_duration_attrs_old = [ @@ -30,7 +30,7 @@ SpanAttributes.NET_PEER_NAME, SpanAttributes.HTTP_METHOD, SpanAttributes.HTTP_FLAVOR, - SpanAttributes.HTTP_SCHEME, + SpanAttributes.HTTP_SCHEME, ] _client_duration_attrs_new = [ @@ -46,7 +46,11 @@ def _filter_duration_attrs(attrs, sem_conv_opt_in_mode): filtered_attrs = {} - allowed_attributes = _client_duration_attrs_new if sem_conv_opt_in_mode == _OpenTelemetryStabilityMode.HTTP else _client_duration_attrs_old + allowed_attributes = ( + _client_duration_attrs_new + if sem_conv_opt_in_mode == _OpenTelemetryStabilityMode.HTTP + else _client_duration_attrs_old + ) for key, val in attrs.items(): if key in allowed_attributes: filtered_attrs[key] = val @@ -72,12 +76,16 @@ def _set_http_method(result, original, normalized, sem_conv_opt_in_mode): # See https://github.com/open-telemetry/semantic-conventions/blob/main/docs/http/http-spans.md#common-attributes # Method is case sensitive. "http.request.method_original" should not be sanitized or automatically capitalized. if original != normalized and _report_new(): - set_string_attribute(result, SpanAttributes.HTTP_REQUEST_METHOD_ORIGINAL, original) + set_string_attribute( + result, SpanAttributes.HTTP_REQUEST_METHOD_ORIGINAL, original + ) if _report_old(sem_conv_opt_in_mode): set_string_attribute(result, SpanAttributes.HTTP_METHOD, normalized) if _report_new(sem_conv_opt_in_mode): - set_string_attribute(result, SpanAttributes.HTTP_REQUEST_METHOD, normalized) + set_string_attribute( + result, SpanAttributes.HTTP_REQUEST_METHOD, normalized + ) def _set_http_url(result, url, sem_conv_opt_in_mode): @@ -120,14 +128,18 @@ def _set_http_status_code(result, code, sem_conv_opt_in_mode): if _report_old(sem_conv_opt_in_mode): set_int_attribute(result, SpanAttributes.HTTP_STATUS_CODE, code) if _report_new(sem_conv_opt_in_mode): - set_int_attribute(result, SpanAttributes.HTTP_RESPONSE_STATUS_CODE, code) + set_int_attribute( + result, SpanAttributes.HTTP_RESPONSE_STATUS_CODE, code + ) def _set_http_network_protocol_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, SpanAttributes.NET_PROTOCOL_VERSION, version) + set_string_attribute( + result, SpanAttributes.NET_PROTOCOL_VERSION, version + ) @@ -150,6 +162,7 @@ class _OpenTelemetryStabilityMode(Enum): def _report_new(mode): return mode.name != _OpenTelemetryStabilityMode.DEFAULT.name + def _report_old(mode): return mode.name != _OpenTelemetryStabilityMode.HTTP.name @@ -185,7 +198,6 @@ def _initialize(cls): ] = http_opt_in _OpenTelemetrySemanticConventionStability._initialized = True - @classmethod # Get OpenTelemetry opt-in mode based off of signal type (http, messaging, etc.) def _get_opentelemetry_stability_opt_in_mode( From 9eb7f82d47ff73139d277f8fdae85b05471f7ad5 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Wed, 22 Nov 2023 09:16:38 -0800 Subject: [PATCH 08/18] lint --- .../opentelemetry/instrumentation/requests/__init__.py | 8 +++++--- .../src/opentelemetry/instrumentation/_semconv.py | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py index 3bc2f5be20..3e258667fc 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py @@ -161,7 +161,9 @@ def get_or_create_headers(): url = remove_url_credentials(request.url) span_attributes = {} - _set_http_method(span_attributes, method, span_name, sem_conv_opt_in_mode) + _set_http_method( + span_attributes, method, span_name, sem_conv_opt_in_mode + ) _set_http_url(span_attributes, url, sem_conv_opt_in_mode) metric_labels = {} @@ -275,14 +277,14 @@ def get_or_create_headers(): metric_labels[_SPAN_ATTRIBUTES_ERROR_TYPE] = type( exception ).__name__ - + if duration_histogram_old is not None: duration_attrs_old = _filter_duration_attrs( metric_labels, _OpenTelemetryStabilityMode.DEFAULT ) duration_histogram_old.record( max(round(elapsed_time * 1000), 0), - attributes=duration_attrs_old + attributes=duration_attrs_old, ) if duration_histogram_new is not None: duration_attrs_new = _filter_duration_attrs( diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py index 47d9f3bc73..f573b689b5 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py @@ -44,6 +44,7 @@ SpanAttributes.URL_SCHEME, ] + def _filter_duration_attrs(attrs, sem_conv_opt_in_mode): filtered_attrs = {} allowed_attributes = ( @@ -142,7 +143,6 @@ def _set_http_network_protocol_version(result, version, sem_conv_opt_in_mode): ) - _OTEL_SEMCONV_STABILITY_OPT_IN_KEY = "OTEL_SEMCONV_STABILITY_OPT_IN" From 168a7529e1e85c303b9623465c2005aa5640d241 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Wed, 22 Nov 2023 10:09:27 -0800 Subject: [PATCH 09/18] lint --- .../instrumentation/requests/__init__.py | 14 +++++++------- .../tests/test_requests_integration.py | 4 ++-- .../opentelemetry/instrumentation/instrumentor.py | 6 +++--- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py index 3e258667fc..e86ae8c1ae 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py @@ -62,13 +62,6 @@ # FIXME: fix the importing of this private attribute when the location of the _SUPPRESS_HTTP_INSTRUMENTATION_KEY is defined. from opentelemetry.context import _SUPPRESS_HTTP_INSTRUMENTATION_KEY -from opentelemetry.instrumentation.instrumentor import BaseInstrumentor -from opentelemetry.instrumentation.requests.package import _instruments -from opentelemetry.instrumentation.requests.version import __version__ -from opentelemetry.instrumentation.utils import ( - _SUPPRESS_INSTRUMENTATION_KEY, - http_status_to_status_code, -) from opentelemetry.instrumentation._semconv import ( _filter_duration_attrs, _report_old, @@ -88,6 +81,13 @@ _OpenTelemetryStabilityMode, _OpenTelemetryStabilitySignalType, ) +from opentelemetry.instrumentation.instrumentor import BaseInstrumentor +from opentelemetry.instrumentation.requests.package import _instruments +from opentelemetry.instrumentation.requests.version import __version__ +from opentelemetry.instrumentation.utils import ( + _SUPPRESS_INSTRUMENTATION_KEY, + http_status_to_status_code, +) from opentelemetry.metrics import Histogram, get_meter from opentelemetry.propagate import inject from opentelemetry.semconv.metrics import MetricInstruments diff --git a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py index 756c18e456..19fd96b5b2 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py +++ b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py @@ -25,14 +25,14 @@ # FIXME: fix the importing of this private attribute when the location of the _SUPPRESS_HTTP_INSTRUMENTATION_KEY is defined. from opentelemetry.context import _SUPPRESS_HTTP_INSTRUMENTATION_KEY -from opentelemetry.instrumentation.requests import RequestsInstrumentor -from opentelemetry.instrumentation.utils import _SUPPRESS_INSTRUMENTATION_KEY from opentelemetry.instrumentation._semconv import ( _SPAN_ATTRIBUTES_ERROR_TYPE, _SPAN_ATTRIBUTES_NETWORK_PEER_ADDRESS, _OTEL_SEMCONV_STABILITY_OPT_IN_KEY, _OpenTelemetrySemanticConventionStability, ) +from opentelemetry.instrumentation.requests import RequestsInstrumentor +from opentelemetry.instrumentation.utils import _SUPPRESS_INSTRUMENTATION_KEY from opentelemetry.propagate import get_global_textmap, set_global_textmap from opentelemetry.sdk import resources from opentelemetry.semconv.trace import SpanAttributes diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/instrumentor.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/instrumentor.py index 24f0d28cae..c612bfeceb 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/instrumentor.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/instrumentor.py @@ -21,13 +21,13 @@ from logging import getLogger from typing import Collection, Optional +from opentelemetry.instrumentation._semconv import ( + _OpenTelemetrySemanticConventionStability, +) from opentelemetry.instrumentation.dependencies import ( DependencyConflict, get_dependency_conflicts, ) -from opentelemetry.instrumentation._semconv import ( - _OpenTelemetrySemanticConventionStability, -) _LOG = getLogger(__name__) From c4de574a8b2144658bc01861da93bdcd3e393695 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Wed, 22 Nov 2023 10:43:30 -0800 Subject: [PATCH 10/18] lint --- .../instrumentation/requests/__init__.py | 16 ++++++++-------- .../tests/test_requests_integration.py | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py index e86ae8c1ae..ff053cb2aa 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py @@ -63,23 +63,23 @@ # FIXME: fix the importing of this private attribute when the location of the _SUPPRESS_HTTP_INSTRUMENTATION_KEY is defined. from opentelemetry.context import _SUPPRESS_HTTP_INSTRUMENTATION_KEY from opentelemetry.instrumentation._semconv import ( + _SPAN_ATTRIBUTES_ERROR_TYPE, + _SPAN_ATTRIBUTES_NETWORK_PEER_ADDRESS, + _SPAN_ATTRIBUTES_NETWORK_PEER_PORT, _filter_duration_attrs, - _report_old, + _OpenTelemetrySemanticConventionStability, + _OpenTelemetryStabilityMode, + _OpenTelemetryStabilitySignalType, _report_new, + _report_old, _set_http_hostname, _set_http_method, _set_http_net_peer_name, _set_http_network_protocol_version, _set_http_port, _set_http_scheme, - _set_http_url, _set_http_status_code, - _SPAN_ATTRIBUTES_ERROR_TYPE, - _SPAN_ATTRIBUTES_NETWORK_PEER_ADDRESS, - _SPAN_ATTRIBUTES_NETWORK_PEER_PORT, - _OpenTelemetrySemanticConventionStability, - _OpenTelemetryStabilityMode, - _OpenTelemetryStabilitySignalType, + _set_http_url, ) from opentelemetry.instrumentation.instrumentor import BaseInstrumentor from opentelemetry.instrumentation.requests.package import _instruments diff --git a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py index 19fd96b5b2..2edea9cda4 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py +++ b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py @@ -26,9 +26,9 @@ # FIXME: fix the importing of this private attribute when the location of the _SUPPRESS_HTTP_INSTRUMENTATION_KEY is defined. from opentelemetry.context import _SUPPRESS_HTTP_INSTRUMENTATION_KEY from opentelemetry.instrumentation._semconv import ( + _OTEL_SEMCONV_STABILITY_OPT_IN_KEY, _SPAN_ATTRIBUTES_ERROR_TYPE, _SPAN_ATTRIBUTES_NETWORK_PEER_ADDRESS, - _OTEL_SEMCONV_STABILITY_OPT_IN_KEY, _OpenTelemetrySemanticConventionStability, ) from opentelemetry.instrumentation.requests import RequestsInstrumentor From 8c798fce69a82219a13e3fa3d818fc459a0af958 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Wed, 22 Nov 2023 11:16:14 -0800 Subject: [PATCH 11/18] Update utils.py --- .../src/opentelemetry/instrumentation/utils.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py index 2de2754bcc..35a55a1279 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py @@ -12,10 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -import os -import threading import urllib.parse -from enum import Enum from re import escape, sub from typing import Dict, Sequence From 807643310b83f4e27d0871b4e0dd5f9422387baf Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Wed, 22 Nov 2023 11:53:15 -0800 Subject: [PATCH 12/18] Update __init__.py --- .../src/opentelemetry/instrumentation/requests/__init__.py | 1 - 1 file changed, 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py index ff053cb2aa..5dd478ec50 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py @@ -91,7 +91,6 @@ from opentelemetry.metrics import Histogram, get_meter from opentelemetry.propagate import inject from opentelemetry.semconv.metrics import MetricInstruments -from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.trace import SpanKind, Tracer, get_tracer from opentelemetry.trace.span import Span from opentelemetry.trace.status import Status From a5137d053c10eda43316ae51f9cd7dd3aa3f9bc7 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Wed, 22 Nov 2023 12:45:55 -0800 Subject: [PATCH 13/18] Update _semconv.py --- .../src/opentelemetry/instrumentation/_semconv.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py index f573b689b5..a39c85a689 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py @@ -58,15 +58,15 @@ def _filter_duration_attrs(attrs, sem_conv_opt_in_mode): return filtered_attrs -def set_string_attribute(dict, key, value): +def set_string_attribute(result, key, value): if value: - dict[key] = value + result[key] = value -def set_int_attribute(dict, key, value): +def set_int_attribute(result, key, value): if value: try: - dict[key] = int(value) + result[key] = int(value) except ValueError: return @@ -76,7 +76,7 @@ def _set_http_method(result, original, normalized, sem_conv_opt_in_mode): normalized = normalized.strip() # See https://github.com/open-telemetry/semantic-conventions/blob/main/docs/http/http-spans.md#common-attributes # Method is case sensitive. "http.request.method_original" should not be sanitized or automatically capitalized. - if original != normalized and _report_new(): + if original != normalized and _report_new(sem_conv_opt_in_mode): set_string_attribute( result, SpanAttributes.HTTP_REQUEST_METHOD_ORIGINAL, original ) From 9d2d1aaf1679ddf834b9a1c2fc1f1eb1184d719c Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Wed, 22 Nov 2023 13:38:35 -0800 Subject: [PATCH 14/18] lint --- .../src/opentelemetry/instrumentation/requests/__init__.py | 4 ++-- .../tests/test_requests_integration.py | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py index 5dd478ec50..fad7a401dd 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py @@ -263,8 +263,8 @@ def get_or_create_headers(): version_text, sem_conv_opt_in_mode, ) - for k, v in span_attributes.items(): - span.set_attribute(k, v) + for key, val in span_attributes.items(): + span.set_attribute(key, val) if callable(response_hook): response_hook(span, request, result) diff --git a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py index 2edea9cda4..6a405f2b41 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py +++ b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py @@ -699,7 +699,6 @@ def test_basic_metric_new_semconv(self): SpanAttributes.HTTP_RESPONSE_STATUS_CODE: 200, SpanAttributes.SERVER_ADDRESS: "examplehost", SpanAttributes.SERVER_PORT: 8000, - SpanAttributes.SERVER_ADDRESS: "examplehost", SpanAttributes.HTTP_REQUEST_METHOD: "GET", SpanAttributes.NET_PROTOCOL_VERSION: "1.1", } @@ -737,7 +736,6 @@ def test_basic_metric_both_semconv(self): SpanAttributes.HTTP_RESPONSE_STATUS_CODE: 200, SpanAttributes.SERVER_ADDRESS: "examplehost", SpanAttributes.SERVER_PORT: 8000, - SpanAttributes.SERVER_ADDRESS: "examplehost", SpanAttributes.HTTP_REQUEST_METHOD: "GET", SpanAttributes.NET_PROTOCOL_VERSION: "1.1", } From a2f37000bcc174e40e53346793bd5edff83ab316 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Mon, 27 Nov 2023 14:23:30 -0800 Subject: [PATCH 15/18] feedback --- .../instrumentation/requests/__init__.py | 34 ++++---- .../tests/test_requests_integration.py | 81 +++++++++++++++---- .../opentelemetry/instrumentation/_semconv.py | 16 ++-- 3 files changed, 95 insertions(+), 36 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py index fad7a401dd..7bd91093b9 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py @@ -63,10 +63,12 @@ # FIXME: fix the importing of this private attribute when the location of the _SUPPRESS_HTTP_INSTRUMENTATION_KEY is defined. from opentelemetry.context import _SUPPRESS_HTTP_INSTRUMENTATION_KEY from opentelemetry.instrumentation._semconv import ( + _METRIC_ATTRIBUTES_CLIENT_DURATION_NAME, _SPAN_ATTRIBUTES_ERROR_TYPE, _SPAN_ATTRIBUTES_NETWORK_PEER_ADDRESS, _SPAN_ATTRIBUTES_NETWORK_PEER_PORT, _filter_duration_attrs, + _get_schema_url, _OpenTelemetrySemanticConventionStability, _OpenTelemetryStabilityMode, _OpenTelemetryStabilitySignalType, @@ -93,7 +95,7 @@ from opentelemetry.semconv.metrics import MetricInstruments from opentelemetry.trace import SpanKind, Tracer, get_tracer from opentelemetry.trace.span import Span -from opentelemetry.trace.status import Status +from opentelemetry.trace.status import StatusCode from opentelemetry.util.http import ( ExcludeList, get_excluded_urls, @@ -241,13 +243,14 @@ def get_or_create_headers(): result.status_code, sem_conv_opt_in_mode, ) - span.set_status( - Status(http_status_to_status_code(result.status_code)) + _set_http_status_code( + metric_labels, result.status_code, sem_conv_opt_in_mode ) - - _set_http_status_code( - metric_labels, result.status_code, sem_conv_opt_in_mode - ) + status_code = http_status_to_status_code(result.status_code) + span.set_status(status_code) + if _report_new(sem_conv_opt_in_mode) and status_code is StatusCode.ERROR: + span_attributes[_SPAN_ATTRIBUTES_ERROR_TYPE] = str(result.status_code) + metric_labels[_SPAN_ATTRIBUTES_ERROR_TYPE] = str(result.status_code) if result.raw is not None: version = getattr(result.raw, "version", None) @@ -271,11 +274,11 @@ def get_or_create_headers(): if exception is not None and _report_new(sem_conv_opt_in_mode): span.set_attribute( - _SPAN_ATTRIBUTES_ERROR_TYPE, type(exception).__name__ + _SPAN_ATTRIBUTES_ERROR_TYPE, type(exception).__qualname__ ) metric_labels[_SPAN_ATTRIBUTES_ERROR_TYPE] = type( exception - ).__name__ + ).__qualname__ if duration_histogram_old is not None: duration_attrs_old = _filter_duration_attrs( @@ -357,12 +360,16 @@ def _instrument(self, **kwargs): ``excluded_urls``: A string containing a comma-delimited list of regexes used to exclude URLs from tracking """ + semconv_opt_in_mode = _OpenTelemetrySemanticConventionStability._get_opentelemetry_stability_opt_in_mode( + _OpenTelemetryStabilitySignalType.HTTP, + ) + schema_url = _get_schema_url(semconv_opt_in_mode) tracer_provider = kwargs.get("tracer_provider") tracer = get_tracer( __name__, __version__, tracer_provider, - schema_url="https://opentelemetry.io/schemas/1.11.0", + schema_url=schema_url, ) excluded_urls = kwargs.get("excluded_urls") meter_provider = kwargs.get("meter_provider") @@ -370,10 +377,7 @@ def _instrument(self, **kwargs): __name__, __version__, meter_provider, - schema_url="https://opentelemetry.io/schemas/1.11.0", - ) - semconv_opt_in_mode = _OpenTelemetrySemanticConventionStability._get_opentelemetry_stability_opt_in_mode( - _OpenTelemetryStabilitySignalType.HTTP, + schema_url=schema_url, ) duration_histogram_old = None if _report_old(semconv_opt_in_mode): @@ -385,7 +389,7 @@ def _instrument(self, **kwargs): duration_histogram_new = None if _report_new(semconv_opt_in_mode): duration_histogram_new = meter.create_histogram( - name=MetricInstruments.HTTP_CLIENT_DURATION, + name=_METRIC_ATTRIBUTES_CLIENT_DURATION_NAME, unit="s", description="Duration of HTTP client requests.", ) diff --git a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py index 6a405f2b41..8d24d948c8 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py +++ b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py @@ -29,6 +29,7 @@ _OTEL_SEMCONV_STABILITY_OPT_IN_KEY, _SPAN_ATTRIBUTES_ERROR_TYPE, _SPAN_ATTRIBUTES_NETWORK_PEER_ADDRESS, + _SPAN_ATTRIBUTES_NETWORK_PEER_PORT, _OpenTelemetrySemanticConventionStability, ) from opentelemetry.instrumentation.requests import RequestsInstrumentor @@ -70,7 +71,6 @@ class RequestsIntegrationTestBase(abc.ABC): # pylint: disable=too-many-public-methods URL = "http://mock/status/200" - HOST = "mock" # pylint: disable=invalid-name def setUp(self): @@ -137,6 +137,11 @@ def test_basic(self): self.assertIs(span.kind, trace.SpanKind.CLIENT) self.assertEqual(span.name, "GET") + self.assertEqual( + span.instrumentation_scope.schema_url, + "https://opentelemetry.io/schemas/1.11.0", + ) + self.assertEqual( span.attributes, { @@ -153,22 +158,35 @@ def test_basic(self): ) def test_basic_new_semconv(self): - result = self.perform_request(self.URL) + url_with_port = "http://mock:80/status/200" + httpretty.register_uri( + httpretty.GET, + url_with_port, + status=200, + body="Hello!" + ) + result = self.perform_request(url_with_port) self.assertEqual(result.text, "Hello!") span = self.assert_span() self.assertIs(span.kind, trace.SpanKind.CLIENT) self.assertEqual(span.name, "GET") + self.assertEqual( + span.instrumentation_scope.schema_url, + SpanAttributes.SCHEMA_URL, + ) self.assertEqual( span.attributes, { SpanAttributes.HTTP_REQUEST_METHOD: "GET", - SpanAttributes.URL_FULL: self.URL, - SpanAttributes.SERVER_ADDRESS: self.HOST, + SpanAttributes.URL_FULL: url_with_port, + SpanAttributes.SERVER_ADDRESS: "mock", _SPAN_ATTRIBUTES_NETWORK_PEER_ADDRESS: "mock", SpanAttributes.HTTP_RESPONSE_STATUS_CODE: 200, - SpanAttributes.NET_PROTOCOL_VERSION: "1.1", + SpanAttributes.NETWORK_PROTOCOL_VERSION: "1.1", + SpanAttributes.SERVER_PORT: 80, + _SPAN_ATTRIBUTES_NETWORK_PEER_PORT: 80, }, ) @@ -179,27 +197,41 @@ def test_basic_new_semconv(self): ) def test_basic_both_semconv(self): - result = self.perform_request(self.URL) + url_with_port = "http://mock:80/status/200" + httpretty.register_uri( + httpretty.GET, + url_with_port, + status=200, + body="Hello!" + ) + result = self.perform_request(url_with_port) self.assertEqual(result.text, "Hello!") span = self.assert_span() self.assertIs(span.kind, trace.SpanKind.CLIENT) self.assertEqual(span.name, "GET") + self.assertEqual( + span.instrumentation_scope.schema_url, + SpanAttributes.SCHEMA_URL, + ) self.assertEqual( span.attributes, { SpanAttributes.HTTP_METHOD: "GET", SpanAttributes.HTTP_REQUEST_METHOD: "GET", - SpanAttributes.HTTP_URL: self.URL, - SpanAttributes.URL_FULL: self.URL, - SpanAttributes.HTTP_HOST: self.HOST, - SpanAttributes.SERVER_ADDRESS: self.HOST, + SpanAttributes.HTTP_URL: url_with_port, + SpanAttributes.URL_FULL: url_with_port, + SpanAttributes.HTTP_HOST: "mock", + SpanAttributes.SERVER_ADDRESS: "mock", _SPAN_ATTRIBUTES_NETWORK_PEER_ADDRESS: "mock", + SpanAttributes.NET_PEER_PORT: 80, SpanAttributes.HTTP_STATUS_CODE: 200, SpanAttributes.HTTP_RESPONSE_STATUS_CODE: 200, SpanAttributes.HTTP_FLAVOR: "1.1", - SpanAttributes.NET_PROTOCOL_VERSION: "1.1", + SpanAttributes.NETWORK_PROTOCOL_VERSION: "1.1", + SpanAttributes.SERVER_PORT: 80, + _SPAN_ATTRIBUTES_NETWORK_PEER_PORT: 80, }, ) @@ -305,6 +337,9 @@ def test_not_foundbasic_new_semconv(self): self.assertEqual( span.attributes.get(SpanAttributes.HTTP_RESPONSE_STATUS_CODE), 404 ) + self.assertEqual( + span.attributes.get(_SPAN_ATTRIBUTES_ERROR_TYPE), "404" + ) self.assertIs( span.status.status_code, @@ -329,6 +364,9 @@ def test_not_foundbasic_both_semconv(self): self.assertEqual( span.attributes.get(SpanAttributes.HTTP_RESPONSE_STATUS_CODE), 404 ) + self.assertEqual( + span.attributes.get(_SPAN_ATTRIBUTES_ERROR_TYPE), "404" + ) self.assertIs( span.status.status_code, @@ -494,16 +532,26 @@ def test_requests_exception_without_response(self, *_, **__): side_effect=requests.RequestException, ) def test_requests_exception_new_semconv(self, *_, **__): + url_with_port = "http://mock:80/status/200" + httpretty.register_uri( + httpretty.GET, + url_with_port, + status=200, + body="Hello!" + ) with self.assertRaises(requests.RequestException): - self.perform_request(self.URL) + self.perform_request(url_with_port) span = self.assert_span() + print(span.attributes) self.assertEqual( span.attributes, { SpanAttributes.HTTP_REQUEST_METHOD: "GET", - SpanAttributes.URL_FULL: self.URL, - SpanAttributes.SERVER_ADDRESS: self.HOST, + SpanAttributes.URL_FULL: url_with_port, + SpanAttributes.SERVER_ADDRESS: "mock", + SpanAttributes.SERVER_PORT: 80, + _SPAN_ATTRIBUTES_NETWORK_PEER_PORT: 80, _SPAN_ATTRIBUTES_NETWORK_PEER_ADDRESS: "mock", _SPAN_ATTRIBUTES_ERROR_TYPE: "RequestException", }, @@ -700,7 +748,7 @@ def test_basic_metric_new_semconv(self): SpanAttributes.SERVER_ADDRESS: "examplehost", SpanAttributes.SERVER_PORT: 8000, SpanAttributes.HTTP_REQUEST_METHOD: "GET", - SpanAttributes.NET_PROTOCOL_VERSION: "1.1", + SpanAttributes.NETWORK_PROTOCOL_VERSION: "1.1", } for ( @@ -737,9 +785,10 @@ def test_basic_metric_both_semconv(self): SpanAttributes.SERVER_ADDRESS: "examplehost", SpanAttributes.SERVER_PORT: 8000, SpanAttributes.HTTP_REQUEST_METHOD: "GET", - SpanAttributes.NET_PROTOCOL_VERSION: "1.1", + SpanAttributes.NETWORK_PROTOCOL_VERSION: "1.1", } + for ( resource_metrics ) in self.memory_metrics_reader.get_metrics_data().resource_metrics: diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py index a39c85a689..8c4e9452c6 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py @@ -22,6 +22,7 @@ _SPAN_ATTRIBUTES_ERROR_TYPE = "error.type" _SPAN_ATTRIBUTES_NETWORK_PEER_ADDRESS = "network.peer.address" _SPAN_ATTRIBUTES_NETWORK_PEER_PORT = "network.peer.port" +_METRIC_ATTRIBUTES_CLIENT_DURATION_NAME = "http.client.request.duration" _client_duration_attrs_old = [ SpanAttributes.HTTP_STATUS_CODE, @@ -35,10 +36,9 @@ _client_duration_attrs_new = [ _SPAN_ATTRIBUTES_ERROR_TYPE, - SpanAttributes.HTTP_REQUEST_METHOD_ORIGINAL, SpanAttributes.HTTP_REQUEST_METHOD, SpanAttributes.HTTP_RESPONSE_STATUS_CODE, - SpanAttributes.NET_PROTOCOL_VERSION, + SpanAttributes.NETWORK_PROTOCOL_VERSION, SpanAttributes.SERVER_ADDRESS, SpanAttributes.SERVER_PORT, SpanAttributes.URL_SCHEME, @@ -120,9 +120,9 @@ def _set_http_net_peer_name(result, peer_name, sem_conv_opt_in_mode): def _set_http_port(result, port, sem_conv_opt_in_mode): if _report_old(sem_conv_opt_in_mode): - set_string_attribute(result, SpanAttributes.NET_PEER_PORT, port) + set_int_attribute(result, SpanAttributes.NET_PEER_PORT, port) if _report_new(sem_conv_opt_in_mode): - set_string_attribute(result, SpanAttributes.SERVER_PORT, port) + set_int_attribute(result, SpanAttributes.SERVER_PORT, port) def _set_http_status_code(result, code, sem_conv_opt_in_mode): @@ -139,7 +139,7 @@ def _set_http_network_protocol_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.NET_PROTOCOL_VERSION, version + result, SpanAttributes.NETWORK_PROTOCOL_VERSION, version ) @@ -207,3 +207,9 @@ def _get_opentelemetry_stability_opt_in_mode( return _OpenTelemetrySemanticConventionStability._OTEL_SEMCONV_STABILITY_SIGNAL_MAPPING.get( signal_type, _OpenTelemetryStabilityMode.DEFAULT ) + +# Get schema version based off of opt-in mode +def _get_schema_url(mode: _OpenTelemetryStabilityMode) -> str: + if mode is _OpenTelemetryStabilityMode.DEFAULT: + return "https://opentelemetry.io/schemas/1.11.0" + return SpanAttributes.SCHEMA_URL From 4881884eb52ae323a1d56622084450db5d58e2d2 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Mon, 27 Nov 2023 14:25:25 -0800 Subject: [PATCH 16/18] Update _semconv.py --- .../src/opentelemetry/instrumentation/_semconv.py | 1 + 1 file changed, 1 insertion(+) diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py index 8c4e9452c6..cda2d5f9b5 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py @@ -208,6 +208,7 @@ def _get_opentelemetry_stability_opt_in_mode( signal_type, _OpenTelemetryStabilityMode.DEFAULT ) + # Get schema version based off of opt-in mode def _get_schema_url(mode: _OpenTelemetryStabilityMode) -> str: if mode is _OpenTelemetryStabilityMode.DEFAULT: From 459406317699f7d35ce225d422d5618a703af148 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Mon, 27 Nov 2023 15:10:07 -0800 Subject: [PATCH 17/18] lint --- .../instrumentation/requests/__init__.py | 17 ++++++++++---- .../tests/test_requests_integration.py | 22 +++---------------- .../opentelemetry/instrumentation/_semconv.py | 3 ++- 3 files changed, 18 insertions(+), 24 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py index 7bd91093b9..0a8e4ee729 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py @@ -246,11 +246,20 @@ def get_or_create_headers(): _set_http_status_code( metric_labels, result.status_code, sem_conv_opt_in_mode ) - status_code = http_status_to_status_code(result.status_code) + status_code = http_status_to_status_code( + result.status_code + ) span.set_status(status_code) - if _report_new(sem_conv_opt_in_mode) and status_code is StatusCode.ERROR: - span_attributes[_SPAN_ATTRIBUTES_ERROR_TYPE] = str(result.status_code) - metric_labels[_SPAN_ATTRIBUTES_ERROR_TYPE] = str(result.status_code) + if ( + _report_new(sem_conv_opt_in_mode) + and status_code is StatusCode.ERROR + ): + span_attributes[_SPAN_ATTRIBUTES_ERROR_TYPE] = str( + result.status_code + ) + metric_labels[_SPAN_ATTRIBUTES_ERROR_TYPE] = str( + result.status_code + ) if result.raw is not None: version = getattr(result.raw, "version", None) diff --git a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py index 8d24d948c8..09400914ce 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py +++ b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py @@ -159,12 +159,7 @@ def test_basic(self): def test_basic_new_semconv(self): url_with_port = "http://mock:80/status/200" - httpretty.register_uri( - httpretty.GET, - url_with_port, - status=200, - body="Hello!" - ) + httpretty.register_uri(httpretty.GET, url_with_port, status=200, body="Hello!") result = self.perform_request(url_with_port) self.assertEqual(result.text, "Hello!") span = self.assert_span() @@ -198,12 +193,7 @@ def test_basic_new_semconv(self): def test_basic_both_semconv(self): url_with_port = "http://mock:80/status/200" - httpretty.register_uri( - httpretty.GET, - url_with_port, - status=200, - body="Hello!" - ) + httpretty.register_uri(httpretty.GET, url_with_port, status=200, body="Hello!") result = self.perform_request(url_with_port) self.assertEqual(result.text, "Hello!") span = self.assert_span() @@ -533,12 +523,7 @@ def test_requests_exception_without_response(self, *_, **__): ) def test_requests_exception_new_semconv(self, *_, **__): url_with_port = "http://mock:80/status/200" - httpretty.register_uri( - httpretty.GET, - url_with_port, - status=200, - body="Hello!" - ) + httpretty.register_uri(httpretty.GET, url_with_port, status=200, body="Hello!") with self.assertRaises(requests.RequestException): self.perform_request(url_with_port) @@ -750,7 +735,6 @@ def test_basic_metric_new_semconv(self): SpanAttributes.HTTP_REQUEST_METHOD: "GET", SpanAttributes.NETWORK_PROTOCOL_VERSION: "1.1", } - for ( resource_metrics ) in self.memory_metrics_reader.get_metrics_data().resource_metrics: diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py index cda2d5f9b5..fbfc92cf21 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py @@ -41,7 +41,8 @@ SpanAttributes.NETWORK_PROTOCOL_VERSION, SpanAttributes.SERVER_ADDRESS, SpanAttributes.SERVER_PORT, - SpanAttributes.URL_SCHEME, + # TODO: Support opt-in for scheme in new semconv + # SpanAttributes.URL_SCHEME, ] From a0fd4bc8d9e03cd133e772cabd063e205b829cb7 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Mon, 27 Nov 2023 15:19:08 -0800 Subject: [PATCH 18/18] Update test_requests_integration.py --- .../tests/test_requests_integration.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py index 09400914ce..11eada2589 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py +++ b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py @@ -159,7 +159,9 @@ def test_basic(self): def test_basic_new_semconv(self): url_with_port = "http://mock:80/status/200" - httpretty.register_uri(httpretty.GET, url_with_port, status=200, body="Hello!") + httpretty.register_uri( + httpretty.GET, url_with_port, status=200, body="Hello!" + ) result = self.perform_request(url_with_port) self.assertEqual(result.text, "Hello!") span = self.assert_span() @@ -193,7 +195,9 @@ def test_basic_new_semconv(self): def test_basic_both_semconv(self): url_with_port = "http://mock:80/status/200" - httpretty.register_uri(httpretty.GET, url_with_port, status=200, body="Hello!") + httpretty.register_uri( + httpretty.GET, url_with_port, status=200, body="Hello!" + ) result = self.perform_request(url_with_port) self.assertEqual(result.text, "Hello!") span = self.assert_span() @@ -523,7 +527,9 @@ def test_requests_exception_without_response(self, *_, **__): ) def test_requests_exception_new_semconv(self, *_, **__): url_with_port = "http://mock:80/status/200" - httpretty.register_uri(httpretty.GET, url_with_port, status=200, body="Hello!") + httpretty.register_uri( + httpretty.GET, url_with_port, status=200, body="Hello!" + ) with self.assertRaises(requests.RequestException): self.perform_request(url_with_port) @@ -772,7 +778,6 @@ def test_basic_metric_both_semconv(self): SpanAttributes.NETWORK_PROTOCOL_VERSION: "1.1", } - for ( resource_metrics ) in self.memory_metrics_reader.get_metrics_data().resource_metrics: