From 4627b83002cbd979de3e75d68b20ff1022279eb3 Mon Sep 17 00:00:00 2001 From: avincent2 Date: Tue, 5 Apr 2022 15:50:59 +0200 Subject: [PATCH 1/2] remove special probes case in traces middleware. move probes_test --- app/middleware/traces_middleware.py | 5 ++--- tests/unit/probe_test.py | 21 --------------------- tests/unit/routers/probes_test.py | 11 +++++++++++ 3 files changed, 13 insertions(+), 24 deletions(-) delete mode 100644 tests/unit/probe_test.py create mode 100644 tests/unit/routers/probes_test.py diff --git a/app/middleware/traces_middleware.py b/app/middleware/traces_middleware.py index 32615c8b..ebdba206 100644 --- a/app/middleware/traces_middleware.py +++ b/app/middleware/traces_middleware.py @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -from typing import Any, Callable +from typing import Any from starlette.middleware.base import BaseHTTPMiddleware from starlette.requests import Request @@ -133,6 +133,5 @@ class TracingMiddleware(BaseHTTPMiddleware): raise finally: status = response.status_code if response else HTTP_500_INTERNAL_SERVER_ERROR - if not request.url.path.endswith('healthz'): - get_logger().info(utils.process_message(request, status)) + get_logger().info(utils.process_message(request, status)) self._after_request(request, response, tracer) diff --git a/tests/unit/probe_test.py b/tests/unit/probe_test.py deleted file mode 100644 index bc1d49b8..00000000 --- a/tests/unit/probe_test.py +++ /dev/null @@ -1,21 +0,0 @@ -import pytest -from fastapi.testclient import TestClient -from app.wdms_app import wdms_app -from tests.unit.test_utils import ctx_fixture -from app.helper import traces -wdms_app.trace_exporter = traces.CombinedExporter(service_name='tested-ddms') - -@pytest.fixture -def client(ctx_fixture, nope_logger_fixture): - yield TestClient(wdms_app) - wdms_app.dependency_overrides = {} - -@pytest.mark.parametrize("probe_url", [('/readiness'),('/healthz'),]) -def test_readiness_probe(client, probe_url): - response = client.get(probe_url) - response_json = response.json() - assert response.status_code == 200 - assert response_json == {'status': 'healthy'} - - - diff --git a/tests/unit/routers/probes_test.py b/tests/unit/routers/probes_test.py new file mode 100644 index 00000000..a944e4cc --- /dev/null +++ b/tests/unit/routers/probes_test.py @@ -0,0 +1,11 @@ +import pytest + + +@pytest.mark.parametrize("probe_path", ['/readiness', '/healthz', '/']) +def test_probe_request(app_configurable_with_testclient, probe_path, nope_logger_fixture): + app, client_after_startup = app_configurable_with_testclient() + + response = client_after_startup.get(probe_path) + response_json = response.json() + assert response.status_code == 200 + assert response_json == {'status': 'healthy'} -- GitLab From e60e4e5ef6c9dfe49f195f3af136e28406640bd2 Mon Sep 17 00:00:00 2001 From: avincent2 Date: Tue, 5 Apr 2022 16:16:29 +0200 Subject: [PATCH 2/2] add skip parameter to traces_middleware use it for probes routes --- app/middleware/traces_middleware.py | 8 ++++++-- app/wdms_app.py | 2 +- tests/unit/middleware/traces_middleware_test.py | 3 ++- tests/unit/routers/probes_test.py | 2 ++ 4 files changed, 11 insertions(+), 4 deletions(-) diff --git a/app/middleware/traces_middleware.py b/app/middleware/traces_middleware.py index ebdba206..4d5d353b 100644 --- a/app/middleware/traces_middleware.py +++ b/app/middleware/traces_middleware.py @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -from typing import Any +from typing import Any, List from starlette.middleware.base import BaseHTTPMiddleware from starlette.requests import Request @@ -30,9 +30,10 @@ from app.helper.logger import get_logger class TracingMiddleware(BaseHTTPMiddleware): - def __init__(self, **kwargs): + def __init__(self, *, skip_for_path_suffix: List[str], **kwargs): super().__init__(**kwargs) self._trace_propagator = traces.get_trace_propagator() + self._skip_for_path_suffix = skip_for_path_suffix @staticmethod def _before_request(request: Request, tracer: open_tracer.Tracer): @@ -107,6 +108,9 @@ class TracingMiddleware(BaseHTTPMiddleware): attribute_value=response_content_length) async def dispatch(self, request: Request, call_next: Any) -> Response: + if request.url.path.endswith(tuple(self._skip_for_path_suffix)): + # early call_next and return if we want to skip the middleware behaviour + return await call_next(request) # Create tracing context, from headers if exists, else create a new one span_context = self._trace_propagator.from_headers(request.headers) diff --git a/app/wdms_app.py b/app/wdms_app.py index 714668be..20fb17fb 100644 --- a/app/wdms_app.py +++ b/app/wdms_app.py @@ -312,7 +312,7 @@ update_operation_ids(wdms_app) # order is last executed first -wdms_app.add_middleware(TracingMiddleware) +wdms_app.add_middleware(TracingMiddleware, skip_for_path_suffix=[r.path for r in probes.router.routes]) # must be added last to be executed first, it's responsible to clean and create WDMS Context wdms_app.add_middleware(CreateBasicContextMiddleware, config=Config, injector=app_injector) diff --git a/tests/unit/middleware/traces_middleware_test.py b/tests/unit/middleware/traces_middleware_test.py index 73847e5a..4dc1c766 100644 --- a/tests/unit/middleware/traces_middleware_test.py +++ b/tests/unit/middleware/traces_middleware_test.py @@ -16,6 +16,7 @@ import re from opencensus.trace import base_exporter import pytest from app.wdms_app import DDMS_V2_PATH +from app.routers import probes from ..test_utils import gen_all_routes_request @@ -146,7 +147,7 @@ def test_call_trace_url(app_configurable_with_testclient, mock_storage_client_ho "/docs", "/docs/oauth2-redirect", "/redoc", - ]: + ] + [r.path for r in probes.router.routes]: # we also need to exclude probes routes as they are not traced continue path_with_id = path_sub(path) diff --git a/tests/unit/routers/probes_test.py b/tests/unit/routers/probes_test.py index a944e4cc..22ce5127 100644 --- a/tests/unit/routers/probes_test.py +++ b/tests/unit/routers/probes_test.py @@ -9,3 +9,5 @@ def test_probe_request(app_configurable_with_testclient, probe_path, nope_logger response_json = response.json() assert response.status_code == 200 assert response_json == {'status': 'healthy'} + + nope_logger_fixture.info.assert_not_called() -- GitLab