Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

avoid losing repeated HTTP headers #2266

Merged
Merged
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
([#2297](https:/open-telemetry/opentelemetry-python-contrib/issues/2297))
- Ensure all http.server.duration metrics have the same description
([#2151](https:/open-telemetry/opentelemetry-python-contrib/issues/2298))
- Avoid losing repeated HTTP headers
([#2266](https:/open-telemetry/opentelemetry-python-contrib/pull/2266))

## Version 1.23.0/0.44b0 (2024-02-23)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -347,11 +347,17 @@ def collect_custom_headers_attributes(
- https:/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] = {}
samuelcolvin marked this conversation as resolved.
Show resolved Hide resolved
aabmass marked this conversation as resolved.
Show resolved Hide resolved
raw_headers = scope_or_response_message.get("headers")
if raw_headers:
for _key, _value in raw_headers:
key = _key.decode().lower()
samuelcolvin marked this conversation as resolved.
Show resolved Hide resolved
value = _value.decode()
if key in headers:
headers[key] += f",{value}"
else:
headers[key] = value

return sanitize.sanitize_header_values(
headers,
header_regexes,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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)
samuelcolvin marked this conversation as resolved.
Show resolved Hide resolved
self.assertSpanHasAttributes(span, expected)

def test_http_custom_request_headers_not_in_span_attributes(self):
self.scope["headers"].extend(
[
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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)
Original file line number Diff line number Diff line change
Expand Up @@ -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",
aabmass marked this conversation as resolved.
Show resolved Hide resolved
),
}
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):
Expand Down Expand Up @@ -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):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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()
Loading