feat(middleware): migrate lib-commons from v3 to v4#78
Conversation
Replace lib-commons/v3 with v4.0.0 stable release. Updates transitive dependencies including OTel from 1.40.0 to 1.42.0, grpc from 1.79.1 to 1.79.2, and adds new transitive deps required by v4 (go-playground/validator, shopspring/decimal, gabriel-vasile/mimetype). Removes Masterminds/squirrel and lann/* which are no longer indirect requirements. X-Lerian-Ref: 0x1
Adapt all middleware call sites to the breaking changes introduced in lib-commons v4: - opentelemetry: span args changed from pointer to value (HandleSpanError, SetSpanAttributesFromValue) - opentelemetry: ExtractHTTPContext and InjectHTTPContext signatures reversed (ctx first) - SetSpanAttributesFromStruct replaced by SetSpanAttributesFromValue - NoneLogger replaced by log.NewNop() - Add logErrorf/logInfof helpers for nil-safe logger calls - Add initializeDefaultLogger to replace zap.InitializeLoggerWithError() - Update testLogger to implement the new v4 log.Logger interface (Log/With/WithGroup/Enabled/Sync) X-Lerian-Ref: 0x1
WalkthroughThis pull request updates the middleware package to use lib-commons v4 and upgrades several module dependencies (Go 1.25.7, OpenTelemetry v1.42.0, gRPC v1.79.2, and others). It adds logging helpers ( 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
gandalf-at-lerian
left a comment
There was a problem hiding this comment.
LGTM ✅
Reviewed the migration — clean and well-structured:
Import paths: v2 → v4 correctly updated across all files.
Breaking API adaptations:
HandleSpanError/SetSpanAttributesFromValue: pointer → value — ✅NewTrackingFromContextreplacing separateNewTracerFromContext+NewHeaderIDFromContext— ✅- Logger:
zap.InitializeLogger()→zap.New(zap.Config{})with proper error handling + fallback tolog.NewNop()— ✅ logErrorf/logInfofnil-safe helpers — good defensive pattern
New additions:
NewGRPCAuthStreamPolicy+wrappedServerStream— mirrors the unary interceptor, MT tenant claim propagation via metadata. Clean.extractTenantClaimsusingParseUnverified— makes sense for internal service-to-service propagation where the token was already validated upstream.
Tests: 436 lines of new tests covering subject construction, missing claims, server failures, invalid JSON, invalid tokens. Good coverage.
One minor note (non-blocking): the initializeDefaultLogger reads ENV_NAME and OTEL_LIBRARY_NAME from env vars directly. Works fine for now, but if lib-auth ever needs to support multiple instances with different configs in the same process, this would need to become configurable. Not a concern today.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@auth/middleware/middleware.go`:
- Around line 110-115: When initializeDefaultLogger() returns an error, the code
assigns l = log.NewNop() then calls logErrorf with that Nop logger so the
failure is never emitted; instead, emit the error to a real sink (e.g., standard
library log to stderr or a temporary logger) before replacing l with
log.NewNop(). Concretely: capture err from initializeDefaultLogger(), call the
standard library log.Printf or another concrete logger to report "failed to
initialize logger, using NopLogger: %v" including err, and only after that
assign l = log.NewNop() and continue using l and logErrorf as needed. Ensure you
reference initializeDefaultLogger, l, log.NewNop, and logErrorf when making the
change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9402ab0e-3059-483a-bfee-54fa76e5b085
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (4)
auth/middleware/middleware.goauth/middleware/middlewareGRPC.goauth/middleware/middleware_test.gogo.mod
If the default logger initialization fails, the previous code attempts to log this error using the uninitialized logger instance itself. This causes a nil pointer dereference and panics the application. This commit replaces the faulty call with `stdlog.Printf`. Using the standard library logger ensures the initialization error is reported safely without crashing. The application then gracefully falls back to using a no-op logger.
|
🎉 This PR is included in version 2.5.0-beta.7 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 2.5.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Summary
lib-commonsfromv3.0.0-beta.8to the stablev4.0.0release1.40.0 → 1.42.0, gRPC1.79.1 → 1.79.2Changes
chore(deps)— go.mod / go.sumlib-commons/v3withlib-commons/v4 v4.0.0Masterminds/squirrelandlann/*(no longer indirect deps)go-playground/validator,shopspring/decimal,gabriel-vasile/mimetype,golang.org/x/cryptofeat(middleware)— middleware.go / middlewareGRPC.go / middleware_test.goHandleSpanError,SetSpanAttributesFromValue)ExtractHTTPContextandInjectHTTPContextsignatures reversed (ctx first)SetSpanAttributesFromStruct→SetSpanAttributesFromValueNoneLogger→log.NewNop()logErrorf/logInfofhelpers for nil-safe logger callsinitializeDefaultLoggerto replacezap.InitializeLoggerWithError()testLoggerstub to implement the new v4log.LoggerinterfaceTest plan
go build ./...passes with lib-commons v4go test ./auth/middleware/...passes