From e5eb524e8985a853518507320913ea1defba99aa Mon Sep 17 00:00:00 2001 From: Marcelo Trylesinski Date: Thu, 2 Jan 2025 20:44:58 +0100 Subject: [PATCH 1/2] Improve type hints in `opentelemetry.instrumentation.utils` (#3128) --- .../opentelemetry/instrumentation/utils.py | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py index a0d9ae18f9..d5bf5db7fd 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py @@ -12,11 +12,13 @@ # See the License for the specific language governing permissions and # limitations under the License. +from __future__ import annotations + import urllib.parse from contextlib import contextmanager from importlib import import_module from re import escape, sub -from typing import Dict, Iterable, Sequence, Union +from typing import Any, Dict, Generator, Sequence from wrapt import ObjectProxy @@ -44,9 +46,9 @@ def extract_attributes_from_object( - obj: any, attributes: Sequence[str], existing: Dict[str, str] = None + obj: Any, attributes: Sequence[str], existing: Dict[str, str] | None = None ) -> Dict[str, str]: - extracted = {} + extracted: dict[str, str] = {} if existing: extracted.update(existing) for attr in attributes: @@ -81,7 +83,7 @@ def http_status_to_status_code( return StatusCode.ERROR -def unwrap(obj: Union[object, str], attr: str): +def unwrap(obj: object, attr: str): """Given a function that was wrapped by wrapt.wrap_function_wrapper, unwrap it The object containing the function to unwrap may be passed as dotted module path string. @@ -152,7 +154,7 @@ def _start_internal_or_server_span( return span, token -def _url_quote(s) -> str: # pylint: disable=invalid-name +def _url_quote(s: Any) -> str: # pylint: disable=invalid-name if not isinstance(s, (str, bytes)): return s quoted = urllib.parse.quote(s) @@ -163,13 +165,13 @@ def _url_quote(s) -> str: # pylint: disable=invalid-name return quoted.replace("%", "%%") -def _get_opentelemetry_values() -> dict: +def _get_opentelemetry_values() -> dict[str, Any]: """ Return the OpenTelemetry Trace and Span IDs if Span ID is set in the OpenTelemetry execution context. """ # Insert the W3C TraceContext generated - _headers = {} + _headers: dict[str, Any] = {} propagator.inject(_headers) return _headers @@ -196,7 +198,7 @@ def is_http_instrumentation_enabled() -> bool: @contextmanager -def _suppress_instrumentation(*keys: str) -> Iterable[None]: +def _suppress_instrumentation(*keys: str) -> Generator[None]: """Suppress instrumentation within the context.""" ctx = context.get_current() for key in keys: @@ -209,7 +211,7 @@ def _suppress_instrumentation(*keys: str) -> Iterable[None]: @contextmanager -def suppress_instrumentation() -> Iterable[None]: +def suppress_instrumentation() -> Generator[None]: """Suppress instrumentation within the context.""" with _suppress_instrumentation( _SUPPRESS_INSTRUMENTATION_KEY, _SUPPRESS_INSTRUMENTATION_KEY_PLAIN @@ -218,7 +220,7 @@ def suppress_instrumentation() -> Iterable[None]: @contextmanager -def suppress_http_instrumentation() -> Iterable[None]: +def suppress_http_instrumentation() -> Generator[None]: """Suppress instrumentation within the context.""" with _suppress_instrumentation(_SUPPRESS_HTTP_INSTRUMENTATION_KEY): yield From 95f14cd8df05b87b564024d47d14f29211f3b98e Mon Sep 17 00:00:00 2001 From: GonzaloGuasch Date: Thu, 2 Jan 2025 16:58:17 -0300 Subject: [PATCH 2/2] wsgi: always record span status code to have it available in metrics (#3148) --- CHANGELOG.md | 2 ++ .../opentelemetry/instrumentation/wsgi/__init__.py | 4 ---- .../tests/test_wsgi_middleware.py | 13 +++++++++++++ 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dfe1997d70..8a4de30776 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#3133](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3133)) - `opentelemetry-instrumentation-falcon` add support version to v4 ([#3086](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3086)) +- `opentelemetry-instrumentation-wsgi` always record span status code to have it available in metrics + ([#3148](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3148)) - add support to Python 3.13 ([#3134](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3134)) diff --git a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py index eb7cbced9c..bd3b2d18db 100644 --- a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py @@ -480,11 +480,7 @@ def add_response_attributes( """Adds HTTP response attributes to span using the arguments passed to a PEP3333-conforming start_response callable. """ - if not span.is_recording(): - return status_code_str, _ = start_response_status.split(" ", 1) - - status_code = 0 try: status_code = int(status_code_str) except ValueError: diff --git a/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py b/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py index da1a3c2696..9d7d1240a7 100644 --- a/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py @@ -779,6 +779,19 @@ def test_response_attributes(self): self.span.set_attribute.assert_has_calls(expected, any_order=True) self.span.set_attribute.assert_has_calls(expected_new, any_order=True) + def test_response_attributes_noop(self): + mock_span = mock.Mock() + mock_span.is_recording.return_value = False + + attrs = {} + otel_wsgi.add_response_attributes( + mock_span, "404 Not Found", {}, duration_attrs=attrs + ) + + self.assertEqual(mock_span.set_attribute.call_count, 0) + self.assertEqual(mock_span.is_recording.call_count, 2) + self.assertEqual(attrs[SpanAttributes.HTTP_STATUS_CODE], 404) + def test_credential_removal(self): self.environ["HTTP_HOST"] = "username:password@mock" self.environ["PATH_INFO"] = "/status/200"