Skip to content

Commit

Permalink
avoid losing repeated HTTP headers (#2266)
Browse files Browse the repository at this point in the history
* 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 <lechen@microsoft.com>
Co-authored-by: Diego Hurtado <ocelotl@users.noreply.github.com>
  • Loading branch information
3 people authored Mar 21, 2024
1 parent b84d779 commit 5207a78
Show file tree
Hide file tree
Showing 7 changed files with 149 additions and 8 deletions.
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://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)

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://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,
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)
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",
),
}
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()

0 comments on commit 5207a78

Please sign in to comment.