fix(auth): route "Using keyring backend" log through tracing#767
fix(auth): route "Using keyring backend" log through tracing#767accounts-hash wants to merge 2 commits intogoogleworkspace:mainfrom
Conversation
Replace raw eprintln! with tracing::info! so the audit-log line respects GOOGLE_WORKSPACE_CLI_LOG and reaches the JSON log pipeline configured by GOOGLE_WORKSPACE_CLI_LOG_FILE. Before, the line bypassed the tracing infrastructure the crate already builds, with two consequences: 1. GOOGLE_WORKSPACE_CLI_LOG cannot suppress it. Setting `gws=off`, `gws=warn`, or `gws=error` all leave the banner on stderr. 2. Pipelines that merge stdout + stderr (agent tool harnesses, `tee 2>&1`, CI log captures) end up with a non-JSON line prepended to otherwise pure-JSON stdout, breaking downstream parsers. The replacement matches the structured-fields style already used in executor.rs (e.g. tracing::warn! / tracing::debug! with `field = value, "Message"`). Default tracing level is INFO so the line remains visible by default; users who want clean output can now set GOOGLE_WORKSPACE_CLI_LOG=gws=warn as the help text advertises. Agent: cursor-agent Model: claude-opus-4-7 Co-Authored-By: Claude <noreply@anthropic.com> Made-with: Cursor
🦋 Changeset detectedLatest commit: eec74d6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request improves the logging behavior of the credential store by migrating a hardcoded stderr message to the application's structured tracing system. This change ensures that audit telemetry is properly configurable via environment variables and prevents unexpected output from breaking automated pipelines that rely on clean stream processing. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Code Review
This pull request replaces a standard error print statement with structured logging using tracing::info! in the get_or_create_key function. This change allows for better log management and audit trailing, controlled by the GOOGLE_WORKSPACE_CLI_LOG environment variable. I have no feedback to provide.
Agent: cursor-agent Model: claude-opus-4-7 Co-Authored-By: Claude <noreply@anthropic.com> Made-with: Cursor
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request migrates the "Using keyring backend" log message from a raw eprintln! to tracing::info!. This change allows the log output to be managed via the GOOGLE_WORKSPACE_CLI_LOG environment variable and ensures it is captured by the configured JSON log pipeline. I have no feedback to provide.
Problem
credential_store.rs::get_or_create_keywrites the keyring-backend selectionline via raw
eprintln!, bypassing thetracinginfrastructure the cratealready builds:
Two visible consequences:
GOOGLE_WORKSPACE_CLI_LOGenv var cannot suppress it.gws --helpadvertisesGOOGLE_WORKSPACE_CLI_LOG Log level for stderr (e.g., gws=debug),but
gws=off,gws=warn, andgws=errorall leave the banner on stderr.both streams (
tee 2>&1, CI log captures, AI-agent tool harnesses that storemerged output to a file) ends up with a non-JSON line prepended to otherwise
pure-JSON stdout, breaking downstream parsers like
| jqandpython3 -c 'json.load(sys.stdin)'. This was the immediate trigger for thePR — an agent harness merged streams into a tool-result file, and the next
step's
cat … | jqchoked on the leading banner.Reproducer
$ GOOGLE_WORKSPACE_CLI_LOG=gws=off gws calendar events list \ --params '{"calendarId":"primary","maxResults":1}' 2>&1 1>/dev/null Using keyring backend: file # expected: nothing on stderrFix
Replace the
eprintln!withtracing::info!using the structured-fields stylealready used in
executor.rs:468-477(tracing::warn!/tracing::debug!withfield = value, "Message"):Why this is the right shape
selected") indicates the line is intentional audit telemetry. Routing it
through
tracingkeeps it audit-grade and adds it to the JSON log filepipeline that
GOOGLE_WORKSPACE_CLI_LOG_FILEalready builds.tracingsubscriber level isINFO, so themessage remains visible by default. Only users who actively opt out
(
GOOGLE_WORKSPACE_CLI_LOG=gws=warn) see a behavior change — and that changeis exactly what
gws --helpalready promises.fully-qualified
tracing::*calls used elsewhere). Structuredbackend = ...field is queryable in JSON logs, mirroring
executor.rs.Verification
tracing = "0.1"is already a workspace dep(
crates/google-workspace-cli/Cargo.toml:64).executor.rs:468-477. CI (.github/workflows/ci.yml) will compile-check theRust paths filter on this PR.
tracingis invoked fully-qualified, matching thefile's existing convention.
Happy to amend with additional tests or a release-notes entry if useful.
Made with Cursor