From dda95d3ed63926c29c3046cb1dacdeb72cb3f21c Mon Sep 17 00:00:00 2001 From: Matthew Elwell Date: Wed, 4 Sep 2024 11:21:17 +0100 Subject: [PATCH 1/5] Make health check configurable --- src/edge_proxy/health_check/__init__.py | 0 src/edge_proxy/health_check/responses.py | 25 ++++++++++++++++++++ src/edge_proxy/server.py | 30 +++++++++++++++++------- src/edge_proxy/settings.py | 8 ++++++- 4 files changed, 54 insertions(+), 9 deletions(-) create mode 100644 src/edge_proxy/health_check/__init__.py create mode 100644 src/edge_proxy/health_check/responses.py diff --git a/src/edge_proxy/health_check/__init__.py b/src/edge_proxy/health_check/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/src/edge_proxy/health_check/responses.py b/src/edge_proxy/health_check/responses.py new file mode 100644 index 0000000..7a4181f --- /dev/null +++ b/src/edge_proxy/health_check/responses.py @@ -0,0 +1,25 @@ +import typing +from datetime import datetime +from typing import Optional + +from fastapi.responses import ORJSONResponse +from starlette.background import BackgroundTask + + +class HealthCheckResponse(ORJSONResponse): + def __init__( + self, + status_code: int = 200, + status: str = "ok", + reason: Optional[str] = None, + last_successful_update: Optional[datetime] = None, + headers: typing.Mapping[str, str] | None = None, + media_type: str | None = None, + background: BackgroundTask | None = None, + ): + content = { + "status": status, + "reason": reason, + "last_successful_update": last_successful_update, + } + super().__init__(status_code=status_code, content=content, headers=headers, media_type=media_type, background=background) diff --git a/src/edge_proxy/server.py b/src/edge_proxy/server.py index 3c26d4a..44d3d1e 100644 --- a/src/edge_proxy/server.py +++ b/src/edge_proxy/server.py @@ -1,5 +1,4 @@ -from contextlib import suppress -from datetime import datetime +from datetime import datetime, timedelta import httpx import structlog @@ -8,6 +7,7 @@ from fastapi.middleware.gzip import GZipMiddleware from fastapi.responses import ORJSONResponse +from edge_proxy.health_check.responses import HealthCheckResponse from fastapi_utils.tasks import repeat_every from edge_proxy.cache import LocalMemEnvironmentsCache @@ -41,13 +41,27 @@ async def unknown_key_error(request, exc): @app.get("/health", response_class=ORJSONResponse, deprecated=True) @app.get("/proxy/health", response_class=ORJSONResponse) async def health_check(): - with suppress(TypeError): - last_updated = datetime.now() - environment_service.last_updated_at - buffer = 30 * len(settings.environment_key_pairs) # 30s per environment - if last_updated.total_seconds() <= settings.api_poll_frequency_seconds + buffer: - return ORJSONResponse(status_code=200, content={"status": "ok"}) + last_updated_at = environment_service.last_updated_at + if not last_updated_at: + return HealthCheckResponse( + status_code=500, + status="error", + reason="environment document(s) not updated.", + last_successful_update=None, + ) - return ORJSONResponse(status_code=500, content={"status": "error"}) + if settings.health_check.count_stale_documents_as_failing: + buffer = (settings.health_check.grace_period_seconds or 30) * len(settings.environment_key_pairs) + threshold = datetime.now() - timedelta(seconds=buffer) + if last_updated_at < threshold: + return HealthCheckResponse( + status_code=500, + status="error", + reason="environment document(s) stale.", + last_successful_update=last_updated_at, + ) + + return HealthCheckResponse(last_successful_update=last_updated_at) @app.get("/api/v1/flags/", response_class=ORJSONResponse) diff --git a/src/edge_proxy/settings.py b/src/edge_proxy/settings.py index c6c565d..7a582e5 100644 --- a/src/edge_proxy/settings.py +++ b/src/edge_proxy/settings.py @@ -4,7 +4,7 @@ import sys from enum import Enum from pathlib import Path -from typing import Any +from typing import Any, Optional import structlog @@ -100,6 +100,11 @@ class ServerSettings(BaseModel): reload: bool = False +class HealthCheckSettings(BaseModel): + count_stale_documents_as_failing: bool = True + grace_period_seconds: Optional[int] = None + + class AppSettings(BaseModel): environment_key_pairs: list[EnvironmentKeyPair] = Field( default_factory=lambda: [ @@ -128,6 +133,7 @@ class AppSettings(BaseModel): allow_origins: list[str] = Field(default_factory=lambda: ["*"]) logging: LoggingSettings = LoggingSettings() server: ServerSettings = ServerSettings() + health_check: HealthCheckSettings = HealthCheckSettings() class AppConfig(AppSettings, BaseSettings): From b0bdcbe88e9084513c358a8459564b897e3e5225 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 4 Sep 2024 10:22:48 +0000 Subject: [PATCH 2/5] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- src/edge_proxy/health_check/responses.py | 8 +++++++- src/edge_proxy/server.py | 4 +++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/edge_proxy/health_check/responses.py b/src/edge_proxy/health_check/responses.py index 7a4181f..dd52e59 100644 --- a/src/edge_proxy/health_check/responses.py +++ b/src/edge_proxy/health_check/responses.py @@ -22,4 +22,10 @@ def __init__( "reason": reason, "last_successful_update": last_successful_update, } - super().__init__(status_code=status_code, content=content, headers=headers, media_type=media_type, background=background) + super().__init__( + status_code=status_code, + content=content, + headers=headers, + media_type=media_type, + background=background, + ) diff --git a/src/edge_proxy/server.py b/src/edge_proxy/server.py index 44d3d1e..ec5c79e 100644 --- a/src/edge_proxy/server.py +++ b/src/edge_proxy/server.py @@ -51,7 +51,9 @@ async def health_check(): ) if settings.health_check.count_stale_documents_as_failing: - buffer = (settings.health_check.grace_period_seconds or 30) * len(settings.environment_key_pairs) + buffer = (settings.health_check.grace_period_seconds or 30) * len( + settings.environment_key_pairs + ) threshold = datetime.now() - timedelta(seconds=buffer) if last_updated_at < threshold: return HealthCheckResponse( From c970bd7bcbd70cf61eb2f1d02b83797047a3818b Mon Sep 17 00:00:00 2001 From: Matthew Elwell Date: Wed, 4 Sep 2024 11:37:27 +0100 Subject: [PATCH 3/5] Add test for ignoring staleness check --- tests/test_server.py | 42 +++++++++++++++++++++++++++++++++++++++--- 1 file changed, 39 insertions(+), 3 deletions(-) diff --git a/tests/test_server.py b/tests/test_server.py index 410427c..48c06fa 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -6,6 +6,7 @@ from fastapi.testclient import TestClient from pytest_mock import MockerFixture +from edge_proxy.settings import AppSettings, HealthCheckSettings from tests.fixtures.response_data import environment_1 if typing.TYPE_CHECKING: @@ -30,18 +31,53 @@ def test_health_check_returns_500_if_cache_was_not_updated( ) -> None: response = client.get("/proxy/health") assert response.status_code == 500 - assert response.json() == {"status": "error"} + assert response.json() == { + "status": "error", + "reason": "environment document(s) not updated.", + "last_successful_update": None, + } def test_health_check_returns_500_if_cache_is_stale( mocker: MockerFixture, client: TestClient, ) -> None: + last_updated_at = datetime.now() - timedelta(days=10) mocked_environment_service = mocker.patch("edge_proxy.server.environment_service") - mocked_environment_service.last_updated_at = datetime.now() - timedelta(days=10) + mocked_environment_service.last_updated_at = last_updated_at response = client.get("/proxy/health") assert response.status_code == 500 - assert response.json() == {"status": "error"} + assert response.json() == { + "status": "error", + "reason": "environment document(s) stale.", + "last_successful_update": last_updated_at.isoformat(), + } + + +def test_health_check_returns_200_if_cache_is_stale_and_health_check_configured_correctly( + mocker: MockerFixture, + client: TestClient, +) -> None: + # Given + settings = AppSettings( + health_check=HealthCheckSettings(count_stale_documents_as_failing=False) + ) + mocker.patch("edge_proxy.server.settings", settings) + + last_updated_at = datetime.now() - timedelta(days=10) + mocked_environment_service = mocker.patch("edge_proxy.server.environment_service") + mocked_environment_service.last_updated_at = last_updated_at + + # When + response = client.get("/proxy/health") + + # Then + assert response.status_code == 200 + assert response.json() == { + "status": "ok", + "reason": None, + "last_successful_update": last_updated_at.isoformat(), + } def test_get_flags( From 7b52c4c4249b829793d53bb51fa1686824d70321 Mon Sep 17 00:00:00 2001 From: Matthew Elwell Date: Wed, 4 Sep 2024 11:44:06 +0100 Subject: [PATCH 4/5] Add default to settings --- src/edge_proxy/server.py | 2 +- src/edge_proxy/settings.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/edge_proxy/server.py b/src/edge_proxy/server.py index ec5c79e..1b5be8b 100644 --- a/src/edge_proxy/server.py +++ b/src/edge_proxy/server.py @@ -51,7 +51,7 @@ async def health_check(): ) if settings.health_check.count_stale_documents_as_failing: - buffer = (settings.health_check.grace_period_seconds or 30) * len( + buffer = settings.health_check.grace_period_seconds * len( settings.environment_key_pairs ) threshold = datetime.now() - timedelta(seconds=buffer) diff --git a/src/edge_proxy/settings.py b/src/edge_proxy/settings.py index 7a582e5..d8e2c1f 100644 --- a/src/edge_proxy/settings.py +++ b/src/edge_proxy/settings.py @@ -4,7 +4,7 @@ import sys from enum import Enum from pathlib import Path -from typing import Any, Optional +from typing import Any import structlog @@ -102,7 +102,7 @@ class ServerSettings(BaseModel): class HealthCheckSettings(BaseModel): count_stale_documents_as_failing: bool = True - grace_period_seconds: Optional[int] = None + grace_period_seconds: int = 30 class AppSettings(BaseModel): From 4a6b81199b24ff236692330e756a8438ad71f21c Mon Sep 17 00:00:00 2001 From: Matthew Elwell Date: Wed, 4 Sep 2024 12:25:20 +0100 Subject: [PATCH 5/5] Re-add poll frequency --- src/edge_proxy/server.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/edge_proxy/server.py b/src/edge_proxy/server.py index 1b5be8b..5dbd029 100644 --- a/src/edge_proxy/server.py +++ b/src/edge_proxy/server.py @@ -54,7 +54,9 @@ async def health_check(): buffer = settings.health_check.grace_period_seconds * len( settings.environment_key_pairs ) - threshold = datetime.now() - timedelta(seconds=buffer) + threshold = datetime.now() - timedelta( + seconds=settings.api_poll_frequency_seconds + buffer + ) if last_updated_at < threshold: return HealthCheckResponse( status_code=500,