From 94b28881b38c78688eeeedb654300cff5f92d798 Mon Sep 17 00:00:00 2001 From: Gutts-n <57202549+Gutts-n@users.noreply.github.com> Date: Wed, 26 Aug 2026 15:58:54 -0300 Subject: [PATCH 1/2] Add request_source field (#152) --- datastore/analytics.py | 6 ++++++ tests/test_analytics.py | 21 +++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/datastore/analytics.py b/datastore/analytics.py index b1a7538..4f52722 100644 --- a/datastore/analytics.py +++ b/datastore/analytics.py @@ -99,6 +99,11 @@ class AnalyticsMiddleware: REAL_IP_HEADER = "x-real-ip" FORWARDED_FOR_HEADER = "x-forwarded-for" + #: Set by known internal callers (e.g. the data explorer sends + #: ``data-explorer``) so usage reporting can tell UI-driven traffic apart + #: from genuine external API use. Absent on ordinary calls. + REQUEST_SOURCE_HEADER = "request-source" + def __init__(self, app: ASGIApp, service: str = "datastore-api") -> None: self.app = app self.service = service @@ -169,6 +174,7 @@ def _event( "status_code": status, "user_agent": headers.get("user-agent") or None, "request_ip": self._request_ip(scope, headers), + "request_source": headers.get(self.REQUEST_SOURCE_HEADER) or None, "user": resolved.get("user"), "dataset": resolved.get("dataset"), "resource": resolved.get("resource") or self._resource_ref(scope, body), diff --git a/tests/test_analytics.py b/tests/test_analytics.py index 6c3a398..f5bba8d 100644 --- a/tests/test_analytics.py +++ b/tests/test_analytics.py @@ -39,6 +39,7 @@ "status_code", "user_agent", "request_ip", + "request_source", "user", "dataset", "resource", @@ -220,6 +221,26 @@ def test_the_ip_falls_back_to_the_last_forwarded_for_entry( assert recorded[0]["request_ip"] == "203.0.113.7" +def test_request_source_is_recorded_when_the_header_is_sent( + client: TestClient, recorded: list[dict] +) -> None: + client.get( + SEARCH_URL, + params={"resource_id": RESOURCE}, + headers={"Request-Source": "data-explorer"}, + ) + + assert recorded[0]["request_source"] == "data-explorer" + + +def test_request_source_is_none_when_the_header_is_absent( + client: TestClient, recorded: list[dict] +) -> None: + client.get(SEARCH_URL, params={"resource_id": RESOURCE}) + + assert recorded[0]["request_source"] is None + + # --- what does not get recorded, and what cannot break ------------------------ From f9d1bd7f4a3d274a050d47b01eb6e0543d09739c Mon Sep 17 00:00:00 2001 From: Gutts-n <57202549+Gutts-n@users.noreply.github.com> Date: Thu, 27 Aug 2026 08:07:54 -0300 Subject: [PATCH 2/2] Ignore analytics for configured header (#152) --- .env.example | 5 ++ datastore/analytics.py | 35 ++++++++++--- datastore/core/config.py | 23 ++++++++ datastore/main.py | 7 ++- tests/test_analytics.py | 110 +++++++++++++++++++++++++++++++++++---- 5 files changed, 160 insertions(+), 20 deletions(-) diff --git a/.env.example b/.env.example index 5dff45d..81a76b1 100644 --- a/.env.example +++ b/.env.example @@ -7,6 +7,11 @@ LOG_LEVEL=INFO # One JSON analytics event per datastore action / dump request. # false leaves the analytics middleware unmounted. ANALYTICS_ENABLED=false +# A request whose ANALYTICS_IGNORE_HEADER value is in ANALYTICS_IGNORE_VALUES +# (comma-separated, case-insensitive) skips analytics entirely - not +# logged, just never recorded. Both empty disables the check. +ANALYTICS_IGNORE_HEADER= +ANALYTICS_IGNORE_VALUES= # Cross-origin requests: `*` allows every origin, a comma-separated list # allows only those domains (e.g. https://data.example.org,https://app.example.org), # empty disables CORS entirely. diff --git a/datastore/analytics.py b/datastore/analytics.py index 4f52722..d579bff 100644 --- a/datastore/analytics.py +++ b/datastore/analytics.py @@ -14,7 +14,10 @@ Pure ASGI, so handled error responses are recorded with their status and an unhandled crash is recorded as a 500 before it propagates. Tracks the versioned action namespace and ``/dump/*``; probes, docs and the - welcome page are excluded by definition. + welcome page are excluded by definition. A request carrying the + configured ignore header/value (``ANALYTICS_IGNORE_HEADER`` / + ``ANALYTICS_IGNORE_VALUES``) is skipped entirely - not logged with a + distinguishing field, just never recorded. ``authorization_dict`` Called by ``RequestContext.authorize`` with the authorized data_dict. @@ -99,14 +102,29 @@ class AnalyticsMiddleware: REAL_IP_HEADER = "x-real-ip" FORWARDED_FOR_HEADER = "x-forwarded-for" - #: Set by known internal callers (e.g. the data explorer sends - #: ``data-explorer``) so usage reporting can tell UI-driven traffic apart - #: from genuine external API use. Absent on ordinary calls. - REQUEST_SOURCE_HEADER = "request-source" - - def __init__(self, app: ASGIApp, service: str = "datastore-api") -> None: + def __init__( + self, + app: ASGIApp, + service: str = "datastore-api", + ignore_header: str = "", + ignore_values: frozenset[str] = frozenset(), + ) -> None: self.app = app self.service = service + #: A request whose ``ignore_header`` value is in ``ignore_values`` + #: skips analytics entirely - not recorded with a distinguishing + #: field, just never logged. Known internal callers (e.g. the data + #: explorer) set this so their UI-driven traffic never counts as API + #: usage. Both configured via ``ANALYTICS_IGNORE_HEADER`` / + #: ``ANALYTICS_IGNORE_VALUES``; either empty disables the check. + self.ignore_header = ignore_header.lower() + self.ignore_values = ignore_values + + def _is_ignored(self, scope: Scope) -> bool: + if not self.ignore_header or not self.ignore_values: + return False + value = Headers(scope=scope).get(self.ignore_header) + return value is not None and value.strip().lower() in self.ignore_values async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: if scope["type"] != "http" or scope["method"] not in self.METHODS: @@ -114,6 +132,8 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: action = action_name(scope["path"]) if action is None: return await self.app(scope, receive, send) + if self._is_ignored(scope): + return await self.app(scope, receive, send) # Created here if authorize has not run yet, so both sides mutate the # one dict Starlette's `Request.state` also uses. @@ -174,7 +194,6 @@ def _event( "status_code": status, "user_agent": headers.get("user-agent") or None, "request_ip": self._request_ip(scope, headers), - "request_source": headers.get(self.REQUEST_SOURCE_HEADER) or None, "user": resolved.get("user"), "dataset": resolved.get("dataset"), "resource": resolved.get("resource") or self._resource_ref(scope, body), diff --git a/datastore/core/config.py b/datastore/core/config.py index 95b13e7..827edaa 100644 --- a/datastore/core/config.py +++ b/datastore/core/config.py @@ -68,6 +68,29 @@ class Config(BaseSettings): "request. `false` leaves the analytics middleware unmounted." ), ) + ANALYTICS_IGNORE_HEADER: str = Field( + default="", + description=( + "Request header name checked to decide whether to skip " + "analytics for a request entirely (e.g. `Request-Source`). " + "Empty disables the check - every tracked request is logged." + ), + ) + ANALYTICS_IGNORE_VALUES: str = Field( + default="", + description=( + "Comma-separated header values that skip analytics logging " + "when ANALYTICS_IGNORE_HEADER matches (e.g. `data-explorer`). " + "Matched case-insensitively. Empty disables the check." + ), + ) + + @property + def analytics_ignore_values_set(self) -> frozenset[str]: + """`ANALYTICS_IGNORE_VALUES` split on commas, lowercased, blanks dropped.""" + return frozenset( + v.strip().lower() for v in self.ANALYTICS_IGNORE_VALUES.split(",") if v.strip() + ) # CORS # Public base URL of this service. Used only to render absolute URLs in diff --git a/datastore/main.py b/datastore/main.py index d0e0745..41449ce 100644 --- a/datastore/main.py +++ b/datastore/main.py @@ -106,7 +106,12 @@ def create_app() -> FastAPI: # Outside the body-size guard, so a rejected oversize upload is an # event too; inside CORS, which only decorates headers. if config.ANALYTICS_ENABLED: - app.add_middleware(AnalyticsMiddleware, service="Datastore") + app.add_middleware( + AnalyticsMiddleware, + service="Datastore", + ignore_header=config.ANALYTICS_IGNORE_HEADER, + ignore_values=config.analytics_ignore_values_set, + ) # Added last = outermost, so 4xx/5xx envelopes carry CORS headers too. # `CORS_ORIGINS=*` allows every origin, a comma-separated list allows # only those domains, empty skips the middleware entirely. diff --git a/tests/test_analytics.py b/tests/test_analytics.py index f5bba8d..b86809d 100644 --- a/tests/test_analytics.py +++ b/tests/test_analytics.py @@ -39,7 +39,6 @@ "status_code", "user_agent", "request_ip", - "request_source", "user", "dataset", "resource", @@ -221,24 +220,113 @@ def test_the_ip_falls_back_to_the_last_forwarded_for_entry( assert recorded[0]["request_ip"] == "203.0.113.7" -def test_request_source_is_recorded_when_the_header_is_sent( - client: TestClient, recorded: list[dict] +def _client_with_ignore_config( + monkeypatch: pytest.MonkeyPatch, + fake_ckan: FakeCKAN, + cache: InMemoryCache, + *, + header: str, + values: str, +) -> TestClient: + monkeypatch.setenv("ANALYTICS_IGNORE_HEADER", header) + monkeypatch.setenv("ANALYTICS_IGNORE_VALUES", values) + get_config.cache_clear() + + app = create_app() + app.dependency_overrides[get_ckan_client] = lambda: fake_ckan + app.dependency_overrides[get_auth_provider] = lambda: CKANAuthProvider( + ckan=fake_ckan, cache=cache, cache_ttl=60, + ) + c = TestClient(app) + c.headers["Authorization"] = "test-token" + return c + + +def test_a_request_with_the_ignore_header_is_not_recorded( + fake_ckan: FakeCKAN, + cache: InMemoryCache, + recorded: list[dict], + monkeypatch: pytest.MonkeyPatch, ) -> None: - client.get( - SEARCH_URL, - params={"resource_id": RESOURCE}, - headers={"Request-Source": "data-explorer"}, + c = _client_with_ignore_config( + monkeypatch, fake_ckan, cache, header="Request-Source", values="data-explorer" + ) + with c: + response = c.get( + SEARCH_URL, + params={"resource_id": RESOURCE}, + headers={"Request-Source": "data-explorer"}, + ) + + assert response.status_code == 200 + assert recorded == [] + + +def test_the_ignore_header_match_is_case_insensitive( + fake_ckan: FakeCKAN, + cache: InMemoryCache, + recorded: list[dict], + monkeypatch: pytest.MonkeyPatch, +) -> None: + c = _client_with_ignore_config( + monkeypatch, fake_ckan, cache, header="Request-Source", values="data-explorer" + ) + with c: + c.get( + SEARCH_URL, + params={"resource_id": RESOURCE}, + headers={"request-source": "Data-Explorer"}, + ) + + assert recorded == [] + + +def test_a_request_without_the_ignore_header_is_still_recorded( + fake_ckan: FakeCKAN, + cache: InMemoryCache, + recorded: list[dict], + monkeypatch: pytest.MonkeyPatch, +) -> None: + c = _client_with_ignore_config( + monkeypatch, fake_ckan, cache, header="Request-Source", values="data-explorer" + ) + with c: + c.get(SEARCH_URL, params={"resource_id": RESOURCE}) + + assert len(recorded) == 1 + + +def test_a_request_with_a_different_header_value_is_still_recorded( + fake_ckan: FakeCKAN, + cache: InMemoryCache, + recorded: list[dict], + monkeypatch: pytest.MonkeyPatch, +) -> None: + c = _client_with_ignore_config( + monkeypatch, fake_ckan, cache, header="Request-Source", values="data-explorer" ) + with c: + c.get( + SEARCH_URL, + params={"resource_id": RESOURCE}, + headers={"Request-Source": "some-other-tool"}, + ) - assert recorded[0]["request_source"] == "data-explorer" + assert len(recorded) == 1 -def test_request_source_is_none_when_the_header_is_absent( +def test_the_ignore_check_is_disabled_when_unconfigured( client: TestClient, recorded: list[dict] ) -> None: - client.get(SEARCH_URL, params={"resource_id": RESOURCE}) + """Default env (empty header/values, set by conftest indirectly through + Config defaults) never skips - the shared `client` fixture proves it.""" + client.get( + SEARCH_URL, + params={"resource_id": RESOURCE}, + headers={"Request-Source": "data-explorer"}, + ) - assert recorded[0]["request_source"] is None + assert len(recorded) == 1 # --- what does not get recorded, and what cannot break ------------------------