fix: demote non-gRPC auth error log from Error to Debug - #18
Merged
Conversation
A client failing to authenticate is a client-side condition, not a server fault. Both the Connect and gRPC auth middlewares logged every non-gRPC error from the authenticator at Error level, which floods logs and lights up error-rate alerting on ordinary anonymous traffic (no token, malformed Authorization header, expired JWT, unknown API key). Demote that branch to Debug in middleware/connect and middleware/grpc. Genuine auth-service failures (Internal, Unavailable, Unknown) still log at Error. Returned errors are unchanged, so consumers mapping the returned error to their own status code are unaffected.
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
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.
Problem
Both auth middlewares log every non-gRPC error returned by the
dauth.Authenticatorat Error level.status.FromErrorreturnsok=falsefor any error that is not agoogle.golang.org/grpc/statuserror and does not implementGRPCStatus(). Authenticator implementations that return a plain application error on rejection — no token, malformedAuthorizationheader, expired/invalid JWT, unknown API key — therefore land in theelsebranch and get logged at Error.That is by far the most common outcome for anonymous traffic and for a browser tab that has not logged in yet. Real-world result in
sf-saas-priv's portal-api, on a single page load:A client failing to authenticate is a client-side condition, not a server fault. At Error level it buries genuine errors and lights up error-rate alerting.
Change
Demote that branch to
Debugin bothmiddleware/connectandmiddleware/grpc. Same message text, samezap.Error(err)field.Genuine auth-service failures —
codes.Internal,codes.Unavailable,codes.Unknown— are untouched and still log at Error.Note on the Connect middleware
middleware/connectalready had a partial gate from 1ec7a43:logger.Check(level, ...), wherelevelis Debug only whenctx.Err() != nil || dauth.IsErrInvalidAuthentication(err). Authenticators returning a plain error rather than a*dauth.ErrInvalidAuthenticationstill evaluated to Error, so the flood was real. Theelsebranch is now unconditionally Debug; thelevelvariable continues to govern the Internal/Unavailable/Unknown branch.Compatibility
No behavior change beyond log level —
obfuscateErrorMessagereturns exactly the errors it returned before in every branch. Consumers that map the returned error to their own status code (e.g. sf-saas-priv's outerErrorMappingInterceptor, which maps the plain error toconnect.CodeUnauthenticatedwith error details attached) are unaffected.Verification
go build ./...,go vet ./middleware/...andgo test ./...all clean.