Repository navigation
fix(core): log tracebacks without per-frame values on the long-lived sinks - #1679
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 70b1cd196f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…sinks The file and stdout sinks used loguru's diagnose=True, which calls repr() on every value named in every frame of a logged traceback. Each ASGI middleware frame holds `scope`, and under FastAPI 0.139 repr(scope) is about 190 MB because its "fastapi" entry carries the route's dependency tree. The API's catch-all exception handler logs with logger.exception, so one unhandled request exception repeated that repr once per frame: about 15-20 s of CPU on the event loop on a laptop, during which no other request was served. Use diagnose=False on those two sinks. The traceback is still logged (backtrace=True); only the per-frame value dump goes. Test mode keeps diagnose=True. Signed-off-by: sammywachtel <subp@wachtel.us>
logfire.loguru_handler() sets no diagnose, so loguru's default of True still repr()'d every frame local on processes that ship logs to Logfire. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01APFUk2bjEwMptMQqpRhjea Signed-off-by: phernandez <paul@basicmachines.co>
70b1cd1 to
2a0937f
Compare
…ion request A 2 s wall-clock ceiling flaked under coverage and host load. The resolver's frame now holds a repr-counting value, which fails deterministically when a sink still runs loguru's diagnose mode. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01APFUk2bjEwMptMQqpRhjea Signed-off-by: phernandez <paul@basicmachines.co>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 745fd82865
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01APFUk2bjEwMptMQqpRhjea Signed-off-by: phernandez <paul@basicmachines.co>
|
Thanks for carrying this through and for covering the Logfire sink too. |
Salvages #1663 by @sammywachtel onto an in-repo branch so the full CI runs. Their commit is unchanged; one follow-up commit is added.
Why
With loguru's
diagnose=True, logging a traceback callsrepr()on every frame local. Each ASGI middleware frame holdsscope, and under FastAPI 0.139repr(scope)is huge, so one unhandled API exception blocked the event loop for 15-20 s. It also wrote request bodies, headers and note content into the logs.What changed
diagnose=False.backtrace=Truestays, so the full traceback is still logged. Test mode keepsdiagnose=True.diagnose=False.logfire.loguru_handler()sets nodiagnose, so loguru's default ofTruekept the cost and the leak on every process with telemetry on (cloud).Testing
test_the_logfire_sink_does_not_repr_frame_valuesfails without the follow-up and passes with it.pytest tests/api/v2/test_unhandled_exception_logging.py tests/utils: 185 passed.just typecheck,ruff check,ruff format --check: clean.Closes #1663.
🤖 Generated with Claude Code
https://claude.ai/code/session_01APFUk2bjEwMptMQqpRhjea