avoid losing repeated HTTP headers (open-telemetry#2266)

* avoid loosing repeated HTTP headers

* fix fof wsgi, test in falcon

* add changelog

* add more tests

* linting

* fix falcon and flask

* remove unused test

---------

Co-authored-by: Leighton Chen <[email protected]>
Co-authored-by: Diego Hurtado <[email protected]>

Author: 3 people

CHANGELOG.md

@@ -19,6 +19,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   ([#2297](https://github.com/open-telemetry/opentelemetry-python-contrib/issues/2297))
 - Ensure all http.server.duration metrics have the same description
   ([#2151](https://github.com/open-telemetry/opentelemetry-python-contrib/issues/2298))
+- Avoid losing repeated HTTP headers
+  ([#2266](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2266))
 
 ## Version 1.23.0/0.44b0 (2024-02-23)
 

instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py

@@ -195,7 +195,7 @@ def client_response_hook(span: Span, message: dict):
 import urllib
 from functools import wraps
 from timeit import default_timer
-from typing import Any, Awaitable, Callable, Tuple, cast
+from typing import Any, Awaitable, Callable, Tuple
 
 from asgiref.compatibility import guarantee_single_callable
 
@@ -347,11 +347,17 @@ def collect_custom_headers_attributes(
      - 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] = {
-        _key.decode("utf8"): _value.decode("utf8")
-        for (_key, _value) in scope_or_response_message.get("headers")
-        or cast("list[tuple[bytes, bytes]]", [])
-    }
+    headers: dict[str, str] = {}
+    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
+
     return sanitize.sanitize_header_values(
         headers,
         header_regexes,

instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_custom_headers.py

@@ -40,6 +40,24 @@ async def http_app_with_custom_headers(scope, receive, send):
     await send({"type": "http.response.body", "body": b"*"})
 
 
+async def http_app_with_repeat_headers(scope, receive, send):
+    message = await receive()
+    assert scope["type"] == "http"
+    if message.get("type") == "http.request":
+        await send(
+            {
+                "type": "http.response.start",
+                "status": 200,
+                "headers": [
+                    (b"Content-Type", b"text/plain"),
+                    (b"custom-test-header-1", b"test-header-value-1"),
+                    (b"custom-test-header-1", b"test-header-value-2"),
+                ],
+            }
+        )
+    await send({"type": "http.response.body", "body": b"*"})
+
+
 async def websocket_app_with_custom_headers(scope, receive, send):
     assert scope["type"] == "websocket"
     while True:
@@ -121,6 +139,25 @@ def test_http_custom_request_headers_in_span_attributes(self):
             if span.kind == SpanKind.SERVER:
                 self.assertSpanHasAttributes(span, expected)
 
+    def test_http_repeat_request_headers_in_span_attributes(self):
+        self.scope["headers"].extend(
+            [
+                (b"custom-test-header-1", b"test-header-value-1"),
+                (b"custom-test-header-1", b"test-header-value-2"),
+            ]
+        )
+        self.seed_app(self.app)
+        self.send_default_request()
+        self.get_all_output()
+        span_list = self.exporter.get_finished_spans()
+        expected = {
+            "http.request.header.custom_test_header_1": (
+                "test-header-value-1,test-header-value-2",
+            ),
+        }
+        span = next(span for span in span_list if span.kind == SpanKind.SERVER)
+        self.assertSpanHasAttributes(span, expected)
+
     def test_http_custom_request_headers_not_in_span_attributes(self):
         self.scope["headers"].extend(
             [
@@ -176,6 +213,24 @@ def test_http_custom_response_headers_in_span_attributes(self):
             if span.kind == SpanKind.SERVER:
                 self.assertSpanHasAttributes(span, expected)
 
+    def test_http_repeat_response_headers_in_span_attributes(self):
+        self.app = otel_asgi.OpenTelemetryMiddleware(
+            http_app_with_repeat_headers,
+            tracer_provider=self.tracer_provider,
+            **self.constructor_params,
+        )
+        self.seed_app(self.app)
+        self.send_default_request()
+        self.get_all_output()
+        span_list = self.exporter.get_finished_spans()
+        expected = {
+            "http.response.header.custom_test_header_1": (
+                "test-header-value-1,test-header-value-2",
+            ),
+        }
+        span = next(span for span in span_list if span.kind == SpanKind.SERVER)
+        self.assertSpanHasAttributes(span, expected)
+
     def test_http_custom_response_headers_not_in_span_attributes(self):
         self.app = otel_asgi.OpenTelemetryMiddleware(
             http_app_with_custom_headers,

instrumentation/opentelemetry-instrumentation-flask/tests/base_test.py

@@ -88,6 +88,14 @@ def _custom_response_headers():
         resp.headers["my-secret-header"] = "my-secret-value"
         return resp
 
+    @staticmethod
+    def _repeat_custom_response_headers():
+        headers = {
+            "content-type": "text/plain; charset=utf-8",
+            "my-custom-header": ["my-custom-value-1", "my-custom-header-2"],
+        }
+        return flask.Response("test response", headers=headers)
+
     def _common_initialization(self):
         def excluded_endpoint():
             return "excluded"
@@ -106,6 +114,9 @@ def excluded2_endpoint():
         self.app.route("/test_custom_response_headers")(
             self._custom_response_headers
         )
+        self.app.route("/test_repeat_custom_response_headers")(
+            self._repeat_custom_response_headers
+        )
 
         # pylint: disable=attribute-defined-outside-init
         self.client = Client(self.app, Response)

instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py

@@ -671,6 +671,22 @@ def test_custom_request_header_added_in_server_span(self):
         self.assertEqual(span.kind, trace.SpanKind.SERVER)
         self.assertSpanHasAttributes(span, expected)
 
+    def test_repeat_custom_request_header_added_in_server_span(self):
+        headers = [
+            ("Custom-Test-Header-1", "Test Value 1"),
+            ("Custom-Test-Header-1", "Test Value 2"),
+        ]
+        resp = self.client.get("/hello/123", headers=headers)
+        self.assertEqual(200, resp.status_code)
+        span = self.memory_exporter.get_finished_spans()[0]
+        expected = {
+            "http.request.header.custom_test_header_1": (
+                "Test Value 1, Test Value 2",
+            ),
+        }
+        self.assertEqual(span.kind, trace.SpanKind.SERVER)
+        self.assertSpanHasAttributes(span, expected)
+
     def test_custom_request_header_not_added_in_internal_span(self):
         tracer = trace.get_tracer(__name__)
         with tracer.start_as_current_span("test", kind=trace.SpanKind.SERVER):
@@ -724,6 +740,21 @@ def test_custom_response_header_added_in_server_span(self):
         self.assertEqual(span.kind, trace.SpanKind.SERVER)
         self.assertSpanHasAttributes(span, expected)
 
+    def test_repeat_custom_response_header_added_in_server_span(self):
+        resp = self.client.get("/test_repeat_custom_response_headers")
+        self.assertEqual(resp.status_code, 200)
+        span = self.memory_exporter.get_finished_spans()[0]
+        expected = {
+            "http.response.header.content_type": (
+                "text/plain; charset=utf-8",
+            ),
+            "http.response.header.my_custom_header": (
+                "my-custom-value-1,my-custom-header-2",
+            ),
+        }
+        self.assertEqual(span.kind, trace.SpanKind.SERVER)
+        self.assertSpanHasAttributes(span, expected)
+
     def test_custom_response_header_not_added_in_internal_span(self):
         tracer = trace.get_tracer(__name__)
         with tracer.start_as_current_span("test", kind=trace.SpanKind.SERVER):

instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py

@@ -358,7 +358,6 @@ def collect_custom_request_headers_attributes(environ):
             OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SANITIZE_FIELDS
         )
     )
-
     headers = {
         key[_CARRIER_KEY_PREFIX_LEN:].replace("_", "-"): val
         for key, val in environ.items()
@@ -387,7 +386,12 @@ def collect_custom_response_headers_attributes(response_headers):
     )
     response_headers_dict = {}
     if response_headers:
-        response_headers_dict = dict(response_headers)
+        for key, val in response_headers:
+            key = key.lower()
+            if key in response_headers_dict:
+                response_headers_dict[key] += "," + val
+            else:
+                response_headers_dict[key] = val
 
     return sanitize.sanitize_header_values(
         response_headers_dict,

instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py

@@ -115,6 +115,18 @@ def wsgi_with_custom_response_headers(environ, start_response):
     return [b"*"]
 
 
+def wsgi_with_repeat_custom_response_headers(environ, start_response):
+    assert isinstance(environ, dict)
+    start_response(
+        "200 OK",
+        [
+            ("my-custom-header", "my-custom-value-1"),
+            ("my-custom-header", "my-custom-value-2"),
+        ],
+    )
+    return [b"*"]
+
+
 _expected_metric_names = [
     "http.server.active_requests",
     "http.server.duration",
@@ -711,6 +723,26 @@ def test_custom_response_headers_not_added_in_internal_span(self):
             for key, _ in not_expected.items():
                 self.assertNotIn(key, span.attributes)
 
+    @mock.patch.dict(
+        "os.environ",
+        {
+            OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SERVER_RESPONSE: "my-custom-header",
+        },
+    )
+    def test_repeat_custom_response_headers_added_in_server_span(self):
+        app = otel_wsgi.OpenTelemetryMiddleware(
+            wsgi_with_repeat_custom_response_headers
+        )
+        response = app(self.environ, self.start_response)
+        self.iterate_response(response)
+        span = self.memory_exporter.get_finished_spans()[0]
+        expected = {
+            "http.response.header.my_custom_header": (
+                "my-custom-value-1,my-custom-value-2",
+            ),
+        }
+        self.assertSpanHasAttributes(span, expected)
+
 
 if __name__ == "__main__":
     unittest.main()