From 51b139192bb18d275e2882cff2a1362f6b841670 Mon Sep 17 00:00:00 2001 From: Jarkko Jaakola Date: Tue, 7 Jan 2025 13:04:40 +0200 Subject: [PATCH] fix: use server host and port instead of host header values --- src/karapace/config.py | 2 +- src/schema_registry/telemetry/middleware.py | 5 ++++- src/schema_registry/telemetry/tracer.py | 10 +++++++--- .../unit/schema_registry/telemetry/test_middleware.py | 7 +++++-- tests/unit/schema_registry/telemetry/test_tracer.py | 2 +- 5 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/karapace/config.py b/src/karapace/config.py index ccc8a9ad3..f47a43d05 100644 --- a/src/karapace/config.py +++ b/src/karapace/config.py @@ -30,7 +30,7 @@ from opentelemetry import version as otel_version OTEL_VERSION = otel_version.__version__ -except: # pylint: disable=bare-except +except Exception: pass diff --git a/src/schema_registry/telemetry/middleware.py b/src/schema_registry/telemetry/middleware.py index c6d14bbe9..65e60bf5b 100644 --- a/src/schema_registry/telemetry/middleware.py +++ b/src/schema_registry/telemetry/middleware.py @@ -7,6 +7,8 @@ from dependency_injector.wiring import inject, Provide from fastapi import FastAPI, Request, Response from opentelemetry.trace import SpanKind +from karapace.config import Config +from karapace.container import KarapaceContainer from schema_registry.telemetry.container import TelemetryContainer from schema_registry.telemetry.tracer import Tracer @@ -20,10 +22,11 @@ async def telemetry_middleware( request: Request, call_next: Callable[[Request], Awaitable[Response]], tracer: Tracer = Provide[TelemetryContainer.tracer], + config: Config = Provide[KarapaceContainer.config], ) -> Response: resource = request.url.path.split("/")[1] with tracer.get_tracer().start_as_current_span(name=f"{request.method}: /{resource}", kind=SpanKind.SERVER) as span: - tracer.update_span_with_request(request=request, span=span) + tracer.update_span_with_request(request=request, span=span, config=config) response: Response = await call_next(request) tracer.update_span_with_response(response=response, span=span) return response diff --git a/src/schema_registry/telemetry/tracer.py b/src/schema_registry/telemetry/tracer.py index b2c19e583..dd4f19988 100644 --- a/src/schema_registry/telemetry/tracer.py +++ b/src/schema_registry/telemetry/tracer.py @@ -80,12 +80,16 @@ def add_span_attribute(span: Span, key: str, value: str | int) -> None: span.set_attribute(key, value) @staticmethod - def update_span_with_request(request: Request, span: Span) -> None: + def update_span_with_request( + request: Request, + span: Span, + config: Config = Provide[KarapaceContainer.config], + ) -> None: if span.is_recording(): span.set_attribute(C.CLIENT_ADDRESS, request.client.host or "" if request.client else "") span.set_attribute(C.CLIENT_PORT, request.client.port or "" if request.client else "") - span.set_attribute(S.SERVER_ADDRESS, request.url.hostname or "") - span.set_attribute(S.SERVER_PORT, request.url.port or "") + span.set_attribute(S.SERVER_ADDRESS, config.host) + span.set_attribute(S.SERVER_PORT, config.port) span.set_attribute(U.URL_SCHEME, request.url.scheme) span.set_attribute(U.URL_PATH, request.url.path) span.set_attribute(H.HTTP_REQUEST_METHOD, request.method) diff --git a/tests/unit/schema_registry/telemetry/test_middleware.py b/tests/unit/schema_registry/telemetry/test_middleware.py index ecbe79307..441fe4c55 100644 --- a/tests/unit/schema_registry/telemetry/test_middleware.py +++ b/tests/unit/schema_registry/telemetry/test_middleware.py @@ -8,6 +8,7 @@ from _pytest.logging import LogCaptureFixture from fastapi import FastAPI, Request, Response from opentelemetry.trace import SpanKind +from karapace.config import Config from schema_registry.telemetry.middleware import setup_telemetry_middleware, telemetry_middleware from schema_registry.telemetry.tracer import Tracer from unittest.mock import AsyncMock, MagicMock @@ -39,15 +40,17 @@ async def test_telemetry_middleware() -> None: response_mock = AsyncMock(spec=Response) response_mock.status_code = 200 + config_mock = MagicMock(spec=Config) + call_next = AsyncMock() call_next.return_value = response_mock - response = await telemetry_middleware(request=request_mock, call_next=call_next, tracer=tracer) + response = await telemetry_middleware(request=request_mock, call_next=call_next, tracer=tracer, config=config_mock) span = tracer.get_tracer.return_value.start_as_current_span.return_value.__enter__.return_value tracer.get_tracer.assert_called_once() tracer.get_tracer.return_value.start_as_current_span.assert_called_once_with(name="GET: /test", kind=SpanKind.SERVER) - tracer.update_span_with_request.assert_called_once_with(request=request_mock, span=span) + tracer.update_span_with_request.assert_called_once_with(request=request_mock, span=span, config=config_mock) tracer.update_span_with_response.assert_called_once_with(response=response_mock, span=span) # Check that the request handler is called diff --git a/tests/unit/schema_registry/telemetry/test_tracer.py b/tests/unit/schema_registry/telemetry/test_tracer.py index 6e307f8d2..4610e213a 100644 --- a/tests/unit/schema_registry/telemetry/test_tracer.py +++ b/tests/unit/schema_registry/telemetry/test_tracer.py @@ -137,7 +137,7 @@ def test_update_span_with_request(): [ call("client.address", "client"), call("client.port", 8080), - call("server.address", "server"), + call("server.address", "127.0.0.1"), call("server.port", 8081), call("url.scheme", "http"), call("url.path", "/test"),