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

Repeated headers list for ASGI frameworks #2361

Merged
merged 24 commits into from
Jun 20, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
c7f28eb
avoid loosing repeated HTTP headers
samuelcolvin Feb 23, 2024
2f6a998
fix fof wsgi, test in falcon
samuelcolvin Feb 23, 2024
88a6553
add changelog
samuelcolvin Feb 26, 2024
109ac8c
add more tests
samuelcolvin Feb 26, 2024
d71e093
linting
samuelcolvin Feb 26, 2024
8c89f4a
Merge branch 'main' into avoid-loosing-repeated-headers
samuelcolvin Mar 18, 2024
3ad7587
fix falcon and flask
samuelcolvin Mar 18, 2024
f29c5b9
remove unused test
samuelcolvin Mar 18, 2024
113d346
Merge branch 'main' into avoid-loosing-repeated-headers
lzchen Mar 18, 2024
0fa0a36
Use a list for repeated HTTP headers
samuelcolvin Mar 20, 2024
8f7ff48
linting
samuelcolvin Mar 20, 2024
a9a260a
Merge branch 'main' into repeated-headers-list
samuelcolvin May 1, 2024
dc39259
add changelog entry
samuelcolvin May 1, 2024
2ea4ebd
update docs and improve fastapi tests
samuelcolvin May 1, 2024
94ad9c1
revert changes in wsgi based webframeworks
samuelcolvin May 1, 2024
299ea99
fix linting
samuelcolvin May 1, 2024
7fe3d12
Merge branch 'main' into repeated-headers-list
ocelotl May 6, 2024
b32716b
Merge branch 'main' into repeated-headers-list
ocelotl May 8, 2024
26d9c73
Merge branch 'main' into repeated-headers-list
ocelotl May 14, 2024
fa3683f
Fix import path of typing symbols
ocelotl May 14, 2024
8dc8c16
Merge branch 'main' into repeated-headers-list
lzchen May 24, 2024
06c9816
Merge branch 'main' into repeated-headers-list
lzchen May 28, 2024
6c3b8e5
Merge branch 'main' into repeated-headers-list
lzchen May 30, 2024
d5bfbc1
Merge branch 'main' into repeated-headers-list
ocelotl Jun 20, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,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 @@ -193,9 +193,10 @@ def client_response_hook(span: Span, message: dict):

import typing
import urllib
from collections import defaultdict
from functools import wraps
from timeit import default_timer
from typing import Any, Awaitable, Callable, Tuple, cast
from typing import Any, Awaitable, Callable, DefaultDict, Tuple

from asgiref.compatibility import guarantee_single_callable

Expand Down Expand Up @@ -339,19 +340,20 @@ 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] = {
_key.decode("utf8"): _value.decode("utf8")
for (_key, _value) in scope_or_response_message.get("headers")
or cast("list[tuple[bytes, bytes]]", [])
}
headers: DefaultDict[str, list[str]] = defaultdict(list)
raw_headers = scope_or_response_message.get("headers")
if raw_headers:
for key, value in raw_headers:
headers[key.decode()].append(value.decode())
samuelcolvin marked this conversation as resolved.
Show resolved Hide resolved

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,26 @@ 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 +214,25 @@ 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,22 @@ 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 @@ -210,7 +210,9 @@ def response_hook(span: Span, environ: WSGIEnvironment, status: str, response_he
import functools
import typing
import wsgiref.util as wsgiref_util
from collections import defaultdict
from timeit import default_timer
from typing import DefaultDict

from opentelemetry import context, trace
from opentelemetry.instrumentation.utils import (
Expand Down Expand Up @@ -358,7 +360,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 All @@ -385,9 +386,10 @@ def collect_custom_response_headers_attributes(response_headers):
OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SANITIZE_FIELDS
)
)
response_headers_dict = {}
response_headers_dict: DefaultDict[str, list[str]] = defaultdict(list)
if response_headers:
response_headers_dict = dict(response_headers)
for key, val in response_headers:
response_headers_dict[key.lower()].append(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,27 @@ 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()
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
from re import IGNORECASE as RE_IGNORECASE
from re import compile as re_compile
from re import search
from typing import Callable, Iterable, Optional
from typing import Callable, Iterable, Mapping, Optional
xrmx marked this conversation as resolved.
Show resolved Hide resolved
from urllib.parse import urlparse, urlunparse

from opentelemetry.semconv.trace import SpanAttributes
Expand Down Expand Up @@ -87,32 +87,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

Expand Down
Loading