diff --git a/src/eligibility_signposting_api/app.py b/src/eligibility_signposting_api/app.py index 70375fafe..a8772c855 100644 --- a/src/eligibility_signposting_api/app.py +++ b/src/eligibility_signposting_api/app.py @@ -11,6 +11,7 @@ from eligibility_signposting_api.common.error_handler import handle_exception from eligibility_signposting_api.common.request_validator import validate_request_params from eligibility_signposting_api.config.config import config +from eligibility_signposting_api.logging.logs_helper import log_request_ids from eligibility_signposting_api.logging.logs_manager import add_request_id_to_logs, init_logging from eligibility_signposting_api.views import eligibility_blueprint @@ -25,6 +26,7 @@ def main() -> None: # pragma: no cover @add_request_id_to_logs() +@log_request_ids() @validate_request_params() def lambda_handler(event: LambdaEvent, context: LambdaContext) -> dict[str, Any]: # pragma: no cover """Run the Flask app as an AWS Lambda.""" diff --git a/src/eligibility_signposting_api/logging/logs_helper.py b/src/eligibility_signposting_api/logging/logs_helper.py new file mode 100644 index 000000000..cb71bc911 --- /dev/null +++ b/src/eligibility_signposting_api/logging/logs_helper.py @@ -0,0 +1,29 @@ +import logging +from collections.abc import Callable +from functools import wraps +from typing import Any + +from mangum.types import LambdaContext, LambdaEvent + +logger = logging.getLogger(__name__) + + +def log_request_ids() -> Callable: + def decorator(func: Callable) -> Callable: + @wraps(func) + def wrapper(event: LambdaEvent, context: LambdaContext) -> dict[str, Any] | None: + gateway_request_id = event.get("requestContext", {}).get("requestId") + headers = event.get("headers", {}) + logger.info( + "request trace metadata", + extra={ + "x_request_id": headers.get("X-Request-ID"), + "x_correlation_id": headers.get("X-Correlation-ID"), + "gateway_request_id": gateway_request_id, + }, + ) + return func(event, context) + + return wrapper + + return decorator diff --git a/src/eligibility_signposting_api/views/eligibility.py b/src/eligibility_signposting_api/views/eligibility.py index 6f279653a..802515347 100644 --- a/src/eligibility_signposting_api/views/eligibility.py +++ b/src/eligibility_signposting_api/views/eligibility.py @@ -29,13 +29,6 @@ @eligibility_blueprint.before_request def before_request() -> None: - logger.info( - "request details", - extra={ - "X-Request-ID": request.headers.get("X-Request-ID"), - "X-Correlation-ID": request.headers.get("X-Correlation-ID"), - }, - ) AuditContext.add_request_details(request) diff --git a/tests/unit/logging/test_logs_helper.py b/tests/unit/logging/test_logs_helper.py new file mode 100644 index 000000000..5594cbe36 --- /dev/null +++ b/tests/unit/logging/test_logs_helper.py @@ -0,0 +1,70 @@ +import logging +from http import HTTPStatus +from unittest.mock import Mock + +import pytest +from mangum.types import LambdaContext + + +@pytest.fixture +def lambda_context(): + context = Mock(spec=LambdaContext) + context.aws_request_id = "test-request-id" + return context + + +@pytest.mark.parametrize( + ("headers", "gateway_request_id", "expected_extra"), + [ + ( + {"X-Request-ID": "req-123", "X-Correlation-ID": "corr-abc"}, + "gw-id-999", + { + "x_request_id": "req-123", + "x_correlation_id": "corr-abc", + "gateway_request_id": "gw-id-999", + }, + ), + ( + {}, # No headers + "gw-id-000", + { + "x_request_id": None, + "x_correlation_id": None, + "gateway_request_id": "gw-id-000", + }, + ), + ( + {"X-Request-ID": "req-local"}, + None, # No requestContext (non-Gateway trigger) + { + "x_request_id": "req-local", + "x_correlation_id": None, + "gateway_request_id": None, + }, + ), + ], +) +def test_log_request_ids_decorator_logs_metadata(headers, gateway_request_id, expected_extra, lambda_context, caplog): + from eligibility_signposting_api.app import log_request_ids + + event = {"headers": headers} + if gateway_request_id is not None: + event["requestContext"] = {"requestId": gateway_request_id} + + @log_request_ids() + def test_handler(event, context): # noqa : ARG001 + logger = logging.getLogger("test_logger") + logger.info("Inside test handler") + return HTTPStatus.OK + + with caplog.at_level(logging.INFO): + test_handler(event, lambda_context) + + for record in caplog.records: + if record.message == "request trace metadata": + for key, val in expected_extra.items(): + assert getattr(record, key) == val + break + else: + pytest.fail("'request trace metadata' log not found") diff --git a/tests/unit/views/test_eligibility.py b/tests/unit/views/test_eligibility.py index ad39c01e7..963b1a9ab 100644 --- a/tests/unit/views/test_eligibility.py +++ b/tests/unit/views/test_eligibility.py @@ -586,46 +586,6 @@ def test_build_response_include_values_that_are_not_null(client: FlaskClient): assert action["urlLabel"] == "GP contact" -@pytest.mark.parametrize( - ("headers", "expected_request_id"), - [ - ({"X-Request-ID": "test-request-id-123"}, "test-request-id-123"), - ( - {"X-Request-ID": ""}, - "", - ), - ( - {}, # No headers provided - None, - ), - ], -) -def test_request_id_from_header_logging_variants( - app: Flask, client: FlaskClient, caplog, headers: dict[str, str], expected_request_id: str -): - """ - This test checks that the x-request-ID is logged so that it can be used to correlate logs - with that of the logs from api-gateway - """ - with ( - get_app_container(app).override.service(EligibilityService, new=FakeEligibilityService()), - get_app_container(app).override.service(AuditService, new=FakeAuditService()), - ): - with caplog.at_level(logging.INFO): - response = client.get("/patient-check/12345", headers=headers) - - request_id_logged = False - for record in caplog.records: - request_id = getattr(record, "X-Request-ID", None) - - if request_id == expected_request_id: - request_id_logged = True - break - - assert request_id_logged - assert response.status_code == HTTPStatus.OK - - def test_get_or_default_query_params_with_no_args(app: Flask): with app.test_request_context("/patient-check"): result = get_or_default_query_params()