Skip to content

Commit e87f3ff

Browse files
erosselliclaude
andauthored
ENG-3270 Reduce per-request overhead for API throughput (#8284)
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent f076aaf commit e87f3ff

8 files changed

Lines changed: 144 additions & 70 deletions

File tree

CHANGELOG.md

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,12 @@ Changes can also be flagged with a GitHub label for tracking purposes. The URL o
1919
- https://github.com/ethyca/fides/labels/high-risk: to indicate that a change is a "high-risk" change that could potentially lead to unanticipated regressions or degradations
2020
- https://github.com/ethyca/fides/labels/db-migration: to indicate that a given change includes a DB migration
2121

22-
## [Unreleased](https://github.com/ethyca/fides/compare/2.86.1..main)
22+
## [Unreleased](https://github.com/ethyca/fides/compare/2.86.2..main)
23+
24+
## [2.86.2](https://github.com/ethyca/fides/compare/2.86.1..2.86.2)
25+
26+
### Changed
27+
- Reduced per-request overhead in logging and JWE token decryption to improve API performance [#8284](https://github.com/ethyca/fides/pull/8284)
2328

2429
## [2.86.1](https://github.com/ethyca/fides/compare/2.86.0..2.86.1)
2530

src/fides/api/asgi_middleware.py

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -199,23 +199,26 @@ async def handle_http(self, scope: Scope, receive: Receive, send: Send) -> None:
199199

200200
status_code = 500
201201

202-
try:
203-
await self.app(scope, receive, wrapped_send)
204-
status_code = get_status()
205-
except Exception as e:
206-
logger.exception(f"Unhandled exception processing request: '{e}'")
207-
await self.send_response(send_with_headers, 500, b"Internal Server Error")
208-
status_code = 500
209-
210-
handler_time = round((perf_counter() - start_time) * 1000, 3)
211-
212-
logger.bind(
213-
method=method,
214-
status_code=status_code,
215-
handler_time=f"{handler_time}ms",
216-
path=path,
217-
fides_client=fides_client,
218-
).info("Request received")
202+
with logger.contextualize(request_id=request_id):
203+
try:
204+
await self.app(scope, receive, wrapped_send)
205+
status_code = get_status()
206+
except Exception as e:
207+
logger.exception(f"Unhandled exception processing request: '{e}'")
208+
await self.send_response(
209+
send_with_headers, 500, b"Internal Server Error"
210+
)
211+
status_code = 500
212+
213+
handler_time = round((perf_counter() - start_time) * 1000, 3)
214+
215+
logger.bind(
216+
method=method,
217+
status_code=status_code,
218+
handler_time=f"{handler_time}ms",
219+
path=path,
220+
fides_client=fides_client,
221+
).info("Request received")
219222

220223

221224
class AuditLogMiddleware(BaseASGIMiddleware):

src/fides/api/oauth/jwt.py

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,41 @@
1+
from functools import lru_cache
2+
13
from joserfc import jwe
24
from joserfc.jwk import OctKey
35

46
JWT_ENCRYPTION_ALGORITHM = "A256GCM"
57

68

7-
def generate_jwe(payload: str, encryption_key: str, encoding: str = "UTF-8") -> str:
8-
"""Generates a JWE with the provided payload.
9+
# Cache size is set to 3 somewhat arbitrarily. We currently don't allow
10+
# encryption key rotation so we should effectively only have a single
11+
# value in this cache. When we implement key rotation, there should only
12+
# ever be 2 keys in use at a time, so 3 should be enough.
13+
# This will need to be re-thought if we ever support multiple different
14+
# encryption keys for encrypting tokens.
15+
@lru_cache(maxsize=3)
16+
def _get_oct_key(key_bytes: bytes) -> OctKey:
17+
"""Cache the OctKey so it's built once per distinct key value."""
18+
return OctKey.import_key(key_bytes)
919

10-
Returns a string representation.
11-
"""
20+
21+
def _to_oct_key(encryption_key: str) -> OctKey:
1222
key_bytes = (
1323
encryption_key.encode("utf-8")
1424
if isinstance(encryption_key, str)
1525
else encryption_key
1626
)
17-
key = OctKey.import_key(key_bytes)
27+
return _get_oct_key(key_bytes)
28+
29+
30+
def generate_jwe(payload: str, encryption_key: str, encoding: str = "UTF-8") -> str:
31+
"""Generates a JWE with the provided payload.
32+
33+
Returns a string representation.
34+
"""
1835
return jwe.encrypt_compact(
1936
{"alg": "dir", "enc": JWT_ENCRYPTION_ALGORITHM},
2037
payload.encode(encoding),
21-
key,
38+
_to_oct_key(encryption_key),
2239
)
2340

2441

@@ -33,13 +50,7 @@ def decrypt_jwe(token: str, encryption_key: str, encoding: str = "UTF-8") -> str
3350
Returns:
3451
The decrypted payload as a string.
3552
"""
36-
key_bytes = (
37-
encryption_key.encode("utf-8")
38-
if isinstance(encryption_key, str)
39-
else encryption_key
40-
)
41-
key = OctKey.import_key(key_bytes)
42-
result = jwe.decrypt_compact(token, key)
53+
result = jwe.decrypt_compact(token, _to_oct_key(encryption_key))
4354
if result.plaintext is None:
4455
raise ValueError("JWE decryption produced no plaintext")
4556
return result.plaintext.decode(encoding)

src/fides/api/oauth/utils.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from __future__ import annotations
22

3+
import asyncio
34
import json
45
from datetime import datetime, timezone
56
from functools import update_wrapper
@@ -702,7 +703,11 @@ async def extract_token_and_load_client_async(
702703
raise AuthenticationError(detail="Authentication Failure")
703704

704705
try:
705-
token_data = json.loads(extract_payload(authorization, get_encryption_key()))
706+
loop = asyncio.get_running_loop()
707+
payload = await loop.run_in_executor(
708+
None, extract_payload, authorization, get_encryption_key()
709+
)
710+
token_data = await loop.run_in_executor(None, json.loads, payload)
706711
except (JoseError, ValueError) as exc:
707712
logger.debug("Unable to parse auth token.")
708713
raise AuthorizationError(detail="Not Authorized for this action") from exc

src/fides/api/tasks/__init__.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import contextvars
12
from typing import Any, ContextManager, Dict, List, Optional
23

34
import celery_redis_cluster_backend # type: ignore[import-untyped] # noqa: F401 - registers redis+cluster/rediss+cluster backends
@@ -210,6 +211,11 @@ def _propagate_request_id(headers: Dict[str, Any], **kwargs: Any) -> None:
210211
headers["request_id"] = request_id
211212

212213

214+
_task_log_context: contextvars.ContextVar = contextvars.ContextVar(
215+
"_task_log_context", default=None
216+
)
217+
218+
213219
@task_prerun.connect
214220
def _restore_request_id(task: Task, **kwargs: Any) -> None:
215221
"""Restore request_id from the task headers into the worker's ContextVar.
@@ -220,6 +226,9 @@ def _restore_request_id(task: Task, **kwargs: Any) -> None:
220226
request_id = getattr(task.request, "request_id", None)
221227
if request_id is not None:
222228
set_request_id(request_id)
229+
ctx = logger.contextualize(request_id=request_id)
230+
ctx.__enter__()
231+
_task_log_context.set(ctx)
223232

224233

225234
@task_postrun.connect
@@ -230,6 +239,10 @@ def _clear_request_id(**kwargs: Any) -> None:
230239
a request_id from Task A would leak into Task B if Task B was dispatched
231240
without a request_id header.
232241
"""
242+
ctx = _task_log_context.get()
243+
if ctx is not None:
244+
ctx.__exit__(None, None, None)
245+
_task_log_context.set(None)
233246
set_request_id(None)
234247

235248

src/fides/api/util/logger.py

Lines changed: 7 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
from loguru import logger
1717
from loguru._handler import Message
1818

19-
from fides.api.request_context import get_request_context
2019
from fides.api.schemas.privacy_request import LogEntry, PrivacyRequestSource
2120
from fides.api.util.sqlalchemy_filter import SQLAlchemyGeneratedFilter
2221
from fides.config import CONFIG, FidesConfig
@@ -26,11 +25,11 @@
2625

2726
MASKED = "MASKED"
2827

29-
# Keys injected by the Loguru patcher into every log record's extra dict.
30-
# These are excluded when deciding whether a record has "custom" extra context
31-
# (e.g. from logger.bind()) so that the log format only appends the extra
32-
# block when the caller explicitly added context beyond the automatic fields.
33-
_PATCHER_INJECTED_KEYS = {"request_id"}
28+
# Keys injected into log records by logger.contextualize() in the ASGI
29+
# middleware. Excluded when deciding whether a record has "custom" extra
30+
# context so that the log format only appends the extra block when the
31+
# caller explicitly added context via logger.bind().
32+
_CONTEXTUALIZED_KEYS = {"request_id"}
3433

3534

3635
def _safe_stdout_sink(message: str) -> None:
@@ -194,7 +193,7 @@ def create_handler_dicts(
194193
def has_custom_extra(log_record: Dict) -> bool:
195194
"""Check if log record has custom extra context beyond Loguru's defaults."""
196195
extra = log_record.get("extra", {})
197-
return bool(extra.keys() - _PATCHER_INJECTED_KEYS)
196+
return bool(extra.keys() - _CONTEXTUALIZED_KEYS)
198197

199198
# Helper to filter logs without custom extra
200199
def filter_standard(log_record: Dict) -> bool:
@@ -218,20 +217,6 @@ def filter_standard(log_record: Dict) -> bool:
218217
return [standard_dict, extra_dict]
219218

220219

221-
def _inject_request_context(record: Dict[str, Any]) -> None:
222-
"""Loguru patcher that injects request-scoped context into every log record.
223-
224-
Reads the current ``request_id`` from the ``RequestContext`` ContextVar and
225-
adds it to ``record["extra"]``. Because the patcher runs on *every* log
226-
record (including those from stdlib loggers intercepted by
227-
``InterceptHandler``), all logs emitted during a request are automatically
228-
tagged without any changes to individual call-sites.
229-
"""
230-
ctx = get_request_context()
231-
if ctx.request_id is not None:
232-
record["extra"]["request_id"] = ctx.request_id
233-
234-
235220
def setup(config: FidesConfig) -> None:
236221
"""
237222
Configures logging with the appropriate sink based on configuration.
@@ -278,7 +263,7 @@ def setup(config: FidesConfig) -> None:
278263
)
279264
)
280265

281-
logger.configure(handlers=handlers, patcher=_inject_request_context) # type: ignore[arg-type]
266+
logger.configure(handlers=handlers) # type: ignore[arg-type]
282267

283268
# Add InterceptHandler to root logger to capture standard library logs
284269
# This intercepts logs from SQLAlchemy, Alembic, Celery, etc.

src/fides/api/v1/endpoints/health.py

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -154,20 +154,26 @@ async def database_health(db: Session = Depends(get_db)) -> Dict:
154154
pools: Dict[str, PoolStatus] = {}
155155
async_readonly_pool_prewarmed: Optional[bool] = None
156156

157-
migration_health, current_revision = get_db_health(
158-
db_cred_provider.get_database_url(), db=db
157+
loop = asyncio.get_running_loop()
158+
159+
migration_health, current_revision = await loop.run_in_executor(
160+
None, get_db_health, db_cred_provider.get_database_url(), db
159161
)
160162

161163
# Primary sync pool (already checked out by dependency-injected session).
162-
pools["api_sync_primary"] = PoolStatus(health=_check_sync_session(db))
164+
pools["api_sync_primary"] = PoolStatus(
165+
health=await loop.run_in_executor(None, _check_sync_session, db)
166+
)
163167

164168
# Optional sync readonly pool.
165169
if CONFIG.database.sqlalchemy_readonly_database_uri:
166170
readonly_db: Optional[Session] = None
167171
try:
168-
readonly_db = get_readonly_api_session()
172+
readonly_db = await loop.run_in_executor(None, get_readonly_api_session)
169173
pools["api_sync_readonly"] = PoolStatus(
170-
health=_check_sync_session(readonly_db)
174+
health=await loop.run_in_executor(
175+
None, _check_sync_session, readonly_db
176+
)
171177
)
172178
except Exception as error: # pylint: disable=broad-except
173179
logger.error(

tests/fides/api/middleware/test_request_id.py

Lines changed: 57 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
_clear_request_id,
1919
_propagate_request_id,
2020
_restore_request_id,
21+
_task_log_context,
2122
)
2223

2324
from .conftest import (
@@ -29,9 +30,14 @@
2930

3031
@pytest.fixture(autouse=True)
3132
def _clean_request_context():
32-
"""Reset request context before and after each test."""
33+
"""Reset request context and Loguru log context before and after each test."""
3334
reset_request_context()
3435
yield
36+
# Clean up any leaked Loguru contextualize from Celery signal tests
37+
ctx = _task_log_context.get()
38+
if ctx is not None:
39+
ctx.__exit__(None, None, None)
40+
_task_log_context.set(None)
3541
reset_request_context()
3642

3743

@@ -193,7 +199,7 @@ async def test_rejects_oversized_request_id(self, mock_asgi_app):
193199
assert UUID_PATTERN.match(request_id_headers[0])
194200

195201
async def test_request_id_in_log_output(self, mock_asgi_app, loguru_caplog):
196-
"""Request ID appears in log records via the patcher."""
202+
"""Request ID appears in log records via logger.contextualize()."""
197203
app, _ = mock_asgi_app()
198204
middleware = LogRequestMiddleware(app)
199205

@@ -268,24 +274,32 @@ async def test_concurrent_requests_get_different_ids(self, mock_asgi_app):
268274
assert id1 != id2
269275

270276

271-
class TestLoggerPatcher:
272-
"""Tests for the Loguru patcher that injects request_id."""
277+
class TestLoggerContextualize:
278+
"""Tests for request_id injection via logger.contextualize()."""
273279

274-
def test_patcher_injects_request_id(self, loguru_caplog):
275-
"""When request_id is set, it appears in log records."""
276-
set_request_id("patcher-test-abc")
277-
logger.info("test message")
280+
def test_contextualize_injects_request_id(self, loguru_caplog):
281+
"""When logger.contextualize is active, request_id appears in log records."""
282+
with logger.contextualize(request_id="ctx-test-abc"):
283+
logger.info("test message")
278284

279285
record = loguru_caplog.records[-1]
280-
assert record.extra.get("request_id") == "patcher-test-abc"
286+
assert record.extra.get("request_id") == "ctx-test-abc"
281287

282-
def test_patcher_omits_request_id_when_none(self, loguru_caplog):
283-
"""When no request_id is set, it's not added to log records."""
288+
def test_no_request_id_outside_context(self, loguru_caplog):
289+
"""When no contextualize is active, request_id is not in log records."""
284290
logger.info("test message without context")
285291

286292
record = loguru_caplog.records[-1]
287293
assert "request_id" not in record.extra
288294

295+
def test_set_request_id_alone_does_not_inject_into_logs(self, loguru_caplog):
296+
"""set_request_id without contextualize does not inject into log records."""
297+
set_request_id("only-context-var")
298+
logger.info("test message")
299+
300+
record = loguru_caplog.records[-1]
301+
assert "request_id" not in record.extra
302+
289303

290304
class TestCelerySignals:
291305
"""Tests for Celery request_id propagation signals."""
@@ -325,10 +339,42 @@ def test_restore_request_id_skips_when_absent(self):
325339

326340
assert get_request_id() is None
327341

342+
def test_restore_injects_request_id_into_logs(self, loguru_caplog):
343+
"""task_prerun sets up logger.contextualize so logs get request_id."""
344+
mock_task = type(
345+
"MockTask",
346+
(),
347+
{"request": type("MockRequest", (), {"request_id": "celery-log-789"})()},
348+
)()
349+
350+
_restore_request_id(task=mock_task)
351+
logger.info("task log message")
352+
353+
record = loguru_caplog.records[-1]
354+
assert record.extra.get("request_id") == "celery-log-789"
355+
356+
# Cleanup
357+
_clear_request_id()
358+
328359
def test_clear_request_id_after_task(self):
329360
"""task_postrun clears only the request_id, not other context."""
330361
set_request_context(request_id="should-be-cleared", user_id="keep-me")
331362
_clear_request_id()
332363

333364
assert get_request_id() is None
334365
assert get_user_id() == "keep-me"
366+
367+
def test_clear_removes_log_context(self, loguru_caplog):
368+
"""task_postrun cleans up logger.contextualize so logs no longer get request_id."""
369+
mock_task = type(
370+
"MockTask",
371+
(),
372+
{"request": type("MockRequest", (), {"request_id": "celery-clear-test"})()},
373+
)()
374+
375+
_restore_request_id(task=mock_task)
376+
_clear_request_id()
377+
378+
logger.info("after task")
379+
record = loguru_caplog.records[-1]
380+
assert "request_id" not in record.extra

0 commit comments

Comments
 (0)