Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions json_logging/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -187,9 +187,12 @@ def get_request_from_call_stack(self, within_formatter=False):
if isinstance(f_locals['req'], class_type):
return f_locals['req']

for key in f_locals:
if key not in {'request', 'req'} and isinstance(f_locals[key], class_type):
return f_locals[key]
for key, value in tuple(f_locals.items()):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

1. Extra locals snapshot allocations 🐞 Bug ➹ Performance

get_request_from_call_stack() now allocates tuple(f_locals.items()) for every inspected frame,
creating an O(n) tuple of 2-tuples per frame walked. This increases CPU/memory overhead for
correlation-id lookup during log formatting in FastAPI, where the library falls back to stack
scanning (no global request object).
Agent Prompt
## Issue description
`RequestUtil.get_request_from_call_stack()` snapshots `tuple(f_locals.items())`, which allocates a tuple plus a 2-tuple per local entry for every frame visited.

## Issue Context
This codepath is exercised from `JSONLogWebFormatter` via `request_util.get_correlation_id(within_formatter=True)`. For FastAPI, `support_global_request_object()` is `False`, so correlation-id lookup may frequently fall back to scanning the call stack.

## Fix Focus Areas
- json_logging/util.py[180-200]

### Suggested approach
Preserve mutation-safety but reduce allocations by snapshotting only values:
- Keep the existing fast-path checks for `request` and `req`.
- Replace `for key, value in tuple(f_locals.items()): ...` with `for value in tuple(f_locals.values()): ...` (and drop the key filter), since you only need to detect any local whose value is an instance of `class_type`.
- Ensure the regression test still passes.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

if (
key not in {'request', 'req'}
and isinstance(value, class_type)
):
return value
if f.f_back is not None:
f = f.f_back
else:
Expand All @@ -207,4 +210,4 @@ def _get_correlation_id_in_request_header(request_adapter, request):


def is_not_match_any_pattern(path, patterns):
return all(map(lambda pattern: re.search(pattern, path) is None, patterns))
return all(map(lambda pattern: re.search(pattern, path) is None, patterns))
32 changes: 32 additions & 0 deletions tests/test_fastapi.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import logging
import pathlib
import re
import sys

import fastapi
import fastapi.testclient
Expand Down Expand Up @@ -147,6 +148,37 @@ def test_get_correlation_id(client_and_log_handler):
assert response.json()["correlation_id"] == "abc-def"


def test_get_request_from_call_stack_handles_frame_locals_mutation(
client_and_log_handler,
):
"""Test stack inspection while a tracer changes the frame locals."""
import json_logging
from json_logging.util import RequestUtil

mutation_happened = False

def tracer(frame, event, arg):
nonlocal mutation_happened
if (
frame.f_code is RequestUtil.get_request_from_call_stack.__code__
and event == "line"
and "key" in frame.f_locals
and not mutation_happened
):
frame.f_locals["added_by_tracer"] = object()
mutation_happened = True
return tracer

previous_trace = sys.gettrace()
sys.settrace(tracer)
try:
assert json_logging._request_util.get_request_from_call_stack() is None
finally:
sys.settrace(previous_trace)

assert mutation_happened


def test_extra_property(client_and_log_handler):
"""Test adding an extra property to a log message"""
api_client, handler = client_and_log_handler
Expand Down