diff --git a/CHANGELOG.md b/CHANGELOG.md index 642bf90d0f..b9b5478b65 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,7 +48,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#2425](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2425)) - `opentelemetry-instrumentation-flask` Add `http.method` to `span.name` ([#2454](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2454)) -- ASGI, FastAPI, Starlette: provide both send and receive hooks with `scope` and `message` for internal spans ([#2546](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2546)) +- Record repeated HTTP headers in lists, rather than a comma separate strings for ASGI based web frameworks + ([#2361](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2361)) +- ASGI, FastAPI, Starlette: provide both send and receive hooks with `scope` and `message` for internal spans +- ([#2546](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2546)) ### Added diff --git a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py index e416e8dec2..f24160fcb6 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -129,10 +129,10 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A The name of the added span attribute will follow the format ``http.request.header.`` where ```` is the normalized HTTP header name (lowercase, with ``-`` replaced by ``_``). The value of the attribute will be a -single item list containing all the header values. +list containing the header values. For example: -``http.request.header.custom_request_header = [","]`` +``http.request.header.custom_request_header = ["", ""]`` Response headers **************** @@ -163,10 +163,10 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A The name of the added span attribute will follow the format ``http.response.header.`` where ```` is the normalized HTTP header name (lowercase, with ``-`` replaced by ``_``). The value of the attribute will be a -single item list containing all the header values. +list containing the header values. For example: -``http.response.header.custom_response_header = [","]`` +``http.response.header.custom_response_header = ["", ""]`` Sanitizing headers ****************** @@ -193,9 +193,10 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A import typing import urllib +from collections import defaultdict from functools import wraps from timeit import default_timer -from typing import Any, Awaitable, Callable, Tuple +from typing import Any, Awaitable, Callable, DefaultDict, Tuple from asgiref.compatibility import guarantee_single_callable @@ -340,24 +341,19 @@ def collect_custom_headers_attributes( sanitize: SanitizeValue, header_regexes: list[str], normalize_names: Callable[[str], str], -) -> dict[str, str]: +) -> dict[str, list[str]]: """ Returns custom HTTP request or response headers to be added into SERVER span as span attributes. Refer specifications: - https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/http.md#http-request-and-response-headers """ - # Decode headers before processing. - headers: dict[str, str] = {} + headers: DefaultDict[str, list[str]] = defaultdict(list) raw_headers = scope_or_response_message.get("headers") if raw_headers: - for _key, _value in raw_headers: - key = _key.decode().lower() - value = _value.decode() - if key in headers: - headers[key] += f",{value}" - else: - headers[key] = value + for key, value in raw_headers: + # Decode headers before processing. + headers[key.decode()].append(value.decode()) return sanitize.sanitize_header_values( headers, diff --git a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_custom_headers.py b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_custom_headers.py index 7a9c91a3e7..5394d62ff0 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_custom_headers.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_custom_headers.py @@ -152,7 +152,8 @@ def test_http_repeat_request_headers_in_span_attributes(self): span_list = self.exporter.get_finished_spans() expected = { "http.request.header.custom_test_header_1": ( - "test-header-value-1,test-header-value-2", + "test-header-value-1", + "test-header-value-2", ), } span = next(span for span in span_list if span.kind == SpanKind.SERVER) @@ -225,7 +226,8 @@ def test_http_repeat_response_headers_in_span_attributes(self): span_list = self.exporter.get_finished_spans() expected = { "http.response.header.custom_test_header_1": ( - "test-header-value-1,test-header-value-2", + "test-header-value-1", + "test-header-value-2", ), } span = next(span for span in span_list if span.kind == SpanKind.SERVER) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py index 263cc0fb78..4c673d214a 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py @@ -115,7 +115,7 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A single item list containing all the header values. For example: -``http.request.header.custom_request_header = [","]`` +``http.request.header.custom_request_header = ["", ""]`` Response headers **************** @@ -146,10 +146,10 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A The name of the added span attribute will follow the format ``http.response.header.`` where ```` is the normalized HTTP header name (lowercase, with ``-`` replaced by ``_``). The value of the attribute will be a -single item list containing all the header values. +list containing the header values. For example: -``http.response.header.custom_response_header = [","]`` +``http.response.header.custom_response_header = ["", ""]`` Sanitizing headers ****************** diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index a7cd5045ee..26d9e743a8 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -11,9 +11,10 @@ # 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 unittest +from collections.abc import Mapping from timeit import default_timer +from typing import Tuple from unittest.mock import patch import fastapi @@ -557,6 +558,24 @@ def test_mark_span_internal_in_presence_of_span_from_other_framework(self): ) +class MultiMapping(Mapping): + + def __init__(self, *items: Tuple[str, str]): + self._items = items + + def __len__(self): + return len(self._items) + + def __getitem__(self, __key): + raise NotImplementedError("use .items() instead") + + def __iter__(self): + raise NotImplementedError("use .items() instead") + + def items(self): + return self._items + + @patch.dict( "os.environ", { @@ -583,13 +602,15 @@ def _create_app(): @app.get("/foobar") async def _(): - headers = { - "custom-test-header-1": "test-header-value-1", - "custom-test-header-2": "test-header-value-2", - "my-custom-regex-header-1": "my-custom-regex-value-1,my-custom-regex-value-2", - "My-Custom-Regex-Header-2": "my-custom-regex-value-3,my-custom-regex-value-4", - "My-Secret-Header": "My Secret Value", - } + headers = MultiMapping( + ("custom-test-header-1", "test-header-value-1"), + ("custom-test-header-2", "test-header-value-2"), + ("my-custom-regex-header-1", "my-custom-regex-value-1"), + ("my-custom-regex-header-1", "my-custom-regex-value-2"), + ("My-Custom-Regex-Header-2", "my-custom-regex-value-3"), + ("My-Custom-Regex-Header-2", "my-custom-regex-value-4"), + ("My-Secret-Header", "My Secret Value"), + ) content = {"message": "hello world"} return JSONResponse(content=content, headers=headers) @@ -665,10 +686,12 @@ def test_http_custom_response_headers_in_span_attributes(self): "test-header-value-2", ), "http.response.header.my_custom_regex_header_1": ( - "my-custom-regex-value-1,my-custom-regex-value-2", + "my-custom-regex-value-1", + "my-custom-regex-value-2", ), "http.response.header.my_custom_regex_header_2": ( - "my-custom-regex-value-3,my-custom-regex-value-4", + "my-custom-regex-value-3", + "my-custom-regex-value-4", ), "http.response.header.my_secret_header": ("[REDACTED]",), } diff --git a/instrumentation/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py b/instrumentation/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py index 4bb3608935..474a942a98 100644 --- a/instrumentation/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py @@ -110,10 +110,10 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A The name of the added span attribute will follow the format ``http.request.header.`` where ```` is the normalized HTTP header name (lowercase, with ``-`` replaced by ``_``). The value of the attribute will be a -single item list containing all the header values. +list containing the header values. For example: -``http.request.header.custom_request_header = [","]`` +``http.request.header.custom_request_header = ["", ""]`` Response headers **************** @@ -144,10 +144,10 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A The name of the added span attribute will follow the format ``http.response.header.`` where ```` is the normalized HTTP header name (lowercase, with ``-`` replaced by ``_``). The value of the attribute will be a -single item list containing all the header values. +list containing the header values. For example: -``http.response.header.custom_response_header = [","]`` +``http.response.header.custom_response_header = ["", ""]`` Sanitizing headers ****************** diff --git a/util/opentelemetry-util-http/src/opentelemetry/util/http/__init__.py b/util/opentelemetry-util-http/src/opentelemetry/util/http/__init__.py index e8a2cf2034..f5dacf0fff 100644 --- a/util/opentelemetry-util-http/src/opentelemetry/util/http/__init__.py +++ b/util/opentelemetry-util-http/src/opentelemetry/util/http/__init__.py @@ -14,6 +14,7 @@ from __future__ import annotations +from collections.abc import Mapping from os import environ from re import IGNORECASE as RE_IGNORECASE from re import compile as re_compile @@ -87,32 +88,32 @@ def sanitize_header_value(self, header: str, value: str) -> str: def sanitize_header_values( self, - headers: dict[str, str], + headers: Mapping[str, str | list[str]], header_regexes: list[str], normalize_function: Callable[[str], str], - ) -> dict[str, str]: - values: dict[str, str] = {} + ) -> dict[str, list[str]]: + values: dict[str, list[str]] = {} if header_regexes: header_regexes_compiled = re_compile( - "|".join("^" + i + "$" for i in header_regexes), + "|".join(header_regexes), RE_IGNORECASE, ) - for header_name in list( - filter( - header_regexes_compiled.match, - headers.keys(), - ) - ): - header_values = headers.get(header_name) - if header_values: + for header_name, header_value in headers.items(): + if header_regexes_compiled.fullmatch(header_name): key = normalize_function(header_name.lower()) - values[key] = [ - self.sanitize_header_value( - header=header_name, value=header_values - ) - ] + if isinstance(header_value, str): + values[key] = [ + self.sanitize_header_value( + header_name, header_value + ) + ] + else: + values[key] = [ + self.sanitize_header_value(header_name, value) + for value in header_value + ] return values