fix(logging): keep verbose openai.agents DEBUG off sandbox stdout#704
fix(logging): keep verbose openai.agents DEBUG off sandbox stdout#704Zizouk22 wants to merge 1 commit into
Conversation
The openai-agents SDK logs one DEBUG record per span/function-call/reasoning item. It was routed to both the per-scan file and stdout; Docker captures the sandbox stdout with the json-file driver (no rotation), so on long runs the container log grew to 80GB+ and filled the host disk. Route openai.agents DEBUG to strix.log only; stdout stays on the strix logger.
Greptile SummaryThis PR stops the
Confidence Score: 4/5Safe to merge for the intended goal; the one behavioural side-effect (WARNING/ERROR from The change is narrow and the teardown logic handles the new asymmetric handler setup without modification. The only non-obvious consequence is that non-DEBUG messages from the strix/telemetry/logging.py — specifically the stream_handler exclusion at line 134 and the missing co-location of the exclusion constant with Important Files Changed
Prompt To Fix All With AIFix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
strix/telemetry/logging.py:134-135
**ERROR/WARNING from `openai.agents` also silenced on stdout**
The guard `if name != "openai.agents"` removes the stream handler entirely, not just at DEBUG level. In production mode (`debug=False`) the stream handler runs at `ERROR`, meaning that before this change `openai.agents` ERROR records did reach stdout; after it they go only to `strix.log`. The PR description focuses on the DEBUG flood, but this is a silent secondary behaviour change. If operators or external monitors tail stdout for SDK errors (API failures, auth issues, etc.) they'll now miss them.
Consider attaching the stream_handler unconditionally and instead raising its floor for `openai.agents` to `WARNING` (or `ERROR`) to stop the DEBUG flood while keeping higher-severity signals visible on stdout.
### Issue 2 of 2
strix/telemetry/logging.py:134
**Exclusion list is not co-located with `_TRACKED_ROOTS`**
The string `"openai.agents"` appears only at the call site, disconnected from the `_TRACKED_ROOTS` definition at line 64. If a future contributor adds another chatty SDK to `_TRACKED_ROOTS`, there's no signal that this exclusion logic exists. A small named constant (e.g. `_STDOUT_EXCLUDED_ROOTS: frozenset[str] = frozenset({"openai.agents"})`) defined next to `_TRACKED_ROOTS` would keep the two declarations together and make the intent discoverable.
Reviews (1): Last reviewed commit: "fix(logging): keep verbose openai.agents..." | Re-trigger Greptile |
| if name != "openai.agents": | ||
| tracked.addHandler(stream_handler) |
There was a problem hiding this comment.
ERROR/WARNING from
openai.agents also silenced on stdout
The guard if name != "openai.agents" removes the stream handler entirely, not just at DEBUG level. In production mode (debug=False) the stream handler runs at ERROR, meaning that before this change openai.agents ERROR records did reach stdout; after it they go only to strix.log. The PR description focuses on the DEBUG flood, but this is a silent secondary behaviour change. If operators or external monitors tail stdout for SDK errors (API failures, auth issues, etc.) they'll now miss them.
Consider attaching the stream_handler unconditionally and instead raising its floor for openai.agents to WARNING (or ERROR) to stop the DEBUG flood while keeping higher-severity signals visible on stdout.
Prompt To Fix With AI
This is a comment left during a code review.
Path: strix/telemetry/logging.py
Line: 134-135
Comment:
**ERROR/WARNING from `openai.agents` also silenced on stdout**
The guard `if name != "openai.agents"` removes the stream handler entirely, not just at DEBUG level. In production mode (`debug=False`) the stream handler runs at `ERROR`, meaning that before this change `openai.agents` ERROR records did reach stdout; after it they go only to `strix.log`. The PR description focuses on the DEBUG flood, but this is a silent secondary behaviour change. If operators or external monitors tail stdout for SDK errors (API failures, auth issues, etc.) they'll now miss them.
Consider attaching the stream_handler unconditionally and instead raising its floor for `openai.agents` to `WARNING` (or `ERROR`) to stop the DEBUG flood while keeping higher-severity signals visible on stdout.
How can I resolve this? If you propose a fix, please make it concise.| # to tens of GB and exhaust the host disk on long runs. Keep its verbose | ||
| # DEBUG in the per-scan strix.log file only; stdout stays on Strix's own | ||
| # logger. | ||
| if name != "openai.agents": |
There was a problem hiding this comment.
Exclusion list is not co-located with
_TRACKED_ROOTS
The string "openai.agents" appears only at the call site, disconnected from the _TRACKED_ROOTS definition at line 64. If a future contributor adds another chatty SDK to _TRACKED_ROOTS, there's no signal that this exclusion logic exists. A small named constant (e.g. _STDOUT_EXCLUDED_ROOTS: frozenset[str] = frozenset({"openai.agents"})) defined next to _TRACKED_ROOTS would keep the two declarations together and make the intent discoverable.
Prompt To Fix With AI
This is a comment left during a code review.
Path: strix/telemetry/logging.py
Line: 134
Comment:
**Exclusion list is not co-located with `_TRACKED_ROOTS`**
The string `"openai.agents"` appears only at the call site, disconnected from the `_TRACKED_ROOTS` definition at line 64. If a future contributor adds another chatty SDK to `_TRACKED_ROOTS`, there's no signal that this exclusion logic exists. A small named constant (e.g. `_STDOUT_EXCLUDED_ROOTS: frozenset[str] = frozenset({"openai.agents"})`) defined next to `_TRACKED_ROOTS` would keep the two declarations together and make the intent discoverable.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Ruanyuxi1337
left a comment
There was a problem hiding this comment.
LGTM! Verified that excluding 'openai.agents' from stream_handler correctly silences the verbose SDK debugging log flood from stdout (while preserving it in strix.log). All 86 tests passed successfully.
What
Keep the very verbose
openai.agentsSDK DEBUG output in the per-scanstrix.logfile only, and stop routing it to stdout.Why
openai.agentsis in_TRACKED_ROOTSandsetup_scan_loggingattaches both the file handler and the stdout stream handler to it at DEBUG. The SDK logs one DEBUG record per span / function-call / reasoning-item. The sandbox runs in Docker with the defaultjson-filedriver (no rotation), so stdout accumulates without bound — on a long run the container log grew to 80 GB+ and filled the host disk (details in #703).Change
In
setup_scan_logging, attach the stdoutstream_handlerto every tracked root exceptopenai.agents. Its DEBUG stays instrix.log(bounded per-scan file); thestrixlogger keeps stdout. Minimal and behavior-preserving for Strix's own logs. Does not address the separate fd-leak (Errno 24) noted in the issue.Fixes #703