refactor(logging): consolidate logging setup into shared helper in goose crate - #8817
Conversation
…ose crate Move the duplicated file-appender / env-filter / layer-composition logic from goose-cli and goose-server into a new build_logging_subscriber() helper in crates/goose/src/logging.rs. Both callers now delegate to the shared LoggingConfig struct, eliminating ~80 lines of duplication. Signed-off-by: Rodolfo Olivieri <rolivier@redhat.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b9b0e362a3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| for directive in extra_directives { | ||
| if let Ok(d) = directive.parse() { | ||
| filter = filter.add_directive(d); |
There was a problem hiding this comment.
build_env_filter now appends extra_directives even when RUST_LOG is explicitly set, which changes prior CLI/server behavior where those target directives were only defaults when env parsing failed. In practice, settings like RUST_LOG=error can no longer suppress goose_cli/goose_server info logs because the appended target directives re-enable them, so explicit operator overrides are no longer respected. Restrict these extra directives to the fallback path (or only when RUST_LOG is absent/invalid) to keep RUST_LOG authoritative.
Useful? React with 👍 / 👎.
Signed-off-by: Douwe Osinga <douwe@squareup.com>
…fig for file layer format - Address Codex review: extra_directives are now only appended to the fallback filter, not when RUST_LOG is explicitly set - Preserve server's original plain-text file log format by adding a 'json' field to LoggingConfig (CLI uses json=true, server uses json=false) Signed-off-by: Douwe Osinga <douwe@squareup.com>
DOsinga
left a comment
DOsinga
left a comment
There was a problem hiding this comment.
Pushed a fix commit that addresses two issues:
-
Codex review feedback:
extra_directivesare now only applied in the fallback path (whenRUST_LOGis not set). Previously they were unconditionally appended, which meantRUST_LOG=errorcouldn't fully suppress component-specific targets. -
Server log format preservation: Added a
jsonfield toLoggingConfig. The server's file layer originally used plain text (.with_file(true)), not JSON. The shared code now respects this — CLI getsjson: true, server getsjson: false.
Also merged origin/main and resolved the Cargo.toml conflict.
Now that logging setup lives in the goose crate, these crates no longer depend on tracing-appender directly. Signed-off-by: Douwe Osinga <douwe@squareup.com>
* main: (66 commits) Switch GH pages deploy to actions/artifact workflow (#9025) fix(summon): re-apply canonical limits when delegate overrides model (#9183) Split code signing from build (#8587) refactor(logging): consolidate logging setup into shared helper in goose crate (#8817) fix(cli): report cumulative total_tokens in stream-json/json output (#8910) plugins: add open plugins (just skills for now) (#9063) fix(providers): refresh GCP metadata server token on expiration (#8929) chore(deps): bump the cargo-minor-and-patch group across 1 directory with 14 updates (#9178) chore(deps): bump bzip2 from 0.5.2 to 0.6.1 (#8964) chore(deps): bump tauri from 2.10.3 to 2.11.1 in /ui/goose2/src-tauri (#9066) chore(deps): bump hono from 4.12.14 to 4.12.18 in /evals/open-model-gym/mcp-harness (#9073) localize hardcoded strings in provider settings UI (#8931) chore(deps): bump @babel/plugin-transform-modules-systemjs from 7.28.5 to 7.29.4 in /documentation (#9122) move settings into app shell (#9047) Add Location column to CLI skills table (#8785) (feat): add routstr as a declarative provider (#9175) Add FuturMix provider (#8840) fix: convert quoted numeric config values to numbers if needed (#8844) fix(ui): keep SSE reconnect loop alive on long disconnects (#8717) (#8846) fix(openai): apply request_params to outgoing API payload (#9151) ...
…ose crate (aaif-goose#8817) Signed-off-by: Rodolfo Olivieri <rolivier@redhat.com> Signed-off-by: Douwe Osinga <douwe@squareup.com> Co-authored-by: Douwe Osinga <douwe@squareup.com>
2 participants
Summary
goose-cliandgoose-serverinto a newbuild_logging_subscriber()function incrates/goose/src/logging.rs.LoggingConfigstruct with fields:component,name,extra_directives, andconsole(the only behavioural difference between CLI and server).tracing-appenderandtracing-subscriber(withenv-filter,fmt,json,timefeatures) togoose's dependencies so the shared code compiles.