Persist auth-flow events to glean-server.log#31
Open
swarup-padhi-glean wants to merge 1 commit into
Open
Conversation
The OAuth sign-in lifecycle was stderr-only (`[auth] …` console.error in remote-client / auth-provider / token-store), and start.sh does not redirect stderr to glean-server.log — so the log could not show whether sign-in started, the token exchange succeeded/failed, or credentials were invalidated. A failed login looked identical to "never attempted." Extract logLine into a shared src/log.ts (lazy dir init — importing it has no filesystem side effects) and route the auth milestones through it, tagged with the chat session id where available: setup.sign-in-started, setup.complete, auth.code-exchange-start, auth.code-exchange-complete, auth.code-exchange-failed, auth.fresh-dcr, auth.invalidate, auth.persist-failed, auth.clear-failed. The verbose [fetch] request logging and the callback-listening line stay on stderr (high volume / benign). Adds tests/log.test.ts for the extracted module.
swarup-padhi-glean
requested review from
eshwar-sundar-glean,
garvit-scio and
mohit-gupta-glean
as code owners
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The OAuth sign-in lifecycle was stderr-only —
[auth] …console.errorcalls inremote-client.ts/auth-provider.ts/token-store.ts.start.shdoes not redirect stderr intoglean-server.log, so the persisted log could never show whether sign-in started, the token exchange succeeded or failed, or credentials were invalidated. A failed login was indistinguishable from one that was never attempted — exactly the blind spot that made a recent problematic session impossible to diagnose from logs.What
logLineinto a sharedsrc/log.tsso every module logs through one sink. The path is resolved per-call and the log dir is created lazily on first write, so importinglog.tshas no filesystem side effects (safe to pull into any unit under test).setup.sign-in-started,setup.complete,auth.code-exchange-start,auth.code-exchange-complete,auth.code-exchange-failed,auth.fresh-dcr,auth.invalidate,auth.persist-failed,auth.clear-failed.[fetch]request logging and the callback-listening line stay on stderr (high-volume / benign).tests/log.test.tsfor the extracted module.Verification
npm run typecheckclean;npm test163/163 pass (added 3 inlog.test.ts).server.startfrom Log plugin version at startup (server.start) #29) confirmed present in the builtdist/index.js.Notes
Per-line session-id tagging on all events (not just auth) is a broader observability change left for a follow-up — this PR scopes to the auth flow as requested.