feat(logging): keep this session's logs in memory and on disk - #5566
aodhanroche wants to merge 1 commit into
Conversation
Reporting a problem currently means reproducing it: whatever the engine logged on the way to the failure is gone as soon as the console scrolls, so the first answer to any bug report is "can you run it again with more logging on". Attaches two sinks to the shared `griptape_nodes` logger: - A ring buffer of the most recent records, on by default at 5000 lines. It holds what just happened, so a report can carry the run that actually failed rather than a second one staged for the occasion. - A rotating log file, on by default, so logs outlive the process. One file per engine process, rolling at 10 MB and keeping 5 rollovers, so a long-running engine cannot fill a disk. Files unwritten for `log_retention_days` are aged out when the engine starts and whenever a logging setting changes; the file being written is never deleted, however old it is. Both sit at DEBUG and add no filtering, so `log_level` still decides what reaches them. Both are process-global, because the logger they attach to is shared by every `Engine` in the process, which makes `configure_diagnostic_logging` idempotent and last-call-wins -- the same shape `ConfigManager` already uses for the shared log level. `ConfigManager` applies them at the end of every config load, so no caller has to remember to, and skips the work when nothing relevant changed rather than re-scanning the log directory on every config write. The settings are read with secret expansion off: expansion resolves through `secrets_manager`, and this runs inside `ConfigManager.__init__`, before `Engine` has one. They are also coerced to their declared types, because `load_configs` keeps values as written -- so `"log_to_file": "false"` from a config file is a truthy string that would otherwise leave file logging on for someone who turned it off. The settings reference now expands nested settings one level, so `logging.log_to_file` gets its own row carrying the `GTN_CONFIG_LOGGING__LOG_TO_FILE` spelling instead of only appearing as "(nested object)". Test isolation is set up in `pytest_configure` rather than a fixture: the `ConfigManager` built at import time by `agent_manager` and `servers.mcp` applies logging settings, so merely collecting those modules wrote into the developer's real log directory and pruned week-old files out of it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
collindutter
left a comment
collindutter
left a comment
There was a problem hiding this comment.
I've got mixed feelings about the engine writing logs to a file. If anything, this seems like it should live in the app, not the engine. But even then, is there a precedent for CLI applications like this writing their logs to a file instead of stdout/stderr?
|
@collindutter @alex-rumsey-foundry AFAIK npm, pip, gcloud and Homebrew all keep a rolling file On app vs engine, We could split also it. The ring buffer stays in the engine, since the Given we're steering people to the desktop app, is that the trade you want? |
|
Hey @collindutter! Thanks for sharing your concern. AFAIR Python's native logging module lets one logger fan out to multiple handlers at once, e.g. StreamHandler for stdout/stderr + RotatingFileHandler for the file, each with its own level/formatter. Engine can default to a stream and optionally support file handler, app can point file handler to the setting from the app. So it's composable IMO rather than "engine vs app." And yeah, plenty of precedent: pip logs verbosely to a file while keeping stdout clean, lots of |
3 participants
Issue
Engine logs are gone by the time someone asks for them, so every bug report starts with "can you run it again with logging on".
Fix
Two sinks on the
griptape_nodeslogger, both on by default: a 5,000-line ring buffer in memory, and a rotating file (10 MB × 5, aged out at 7 days). Console output unchanged,log_levelstill gates both.Two gotchas: secret expansion is off, because
secrets_managerdoesn't exist yet insideConfigManager.__init__. And settings get type-coerced, because"log_to_file": "false"would otherwise be a truthy string.🤖 Generated with Claude Code
📚 Documentation preview 📚: https://griptape-nodes--5566.org.readthedocs.build/en/5566/