feat: add DebugLogInterceptor for per-request debug logging#38
Conversation
New interceptor that enables per-request debug logging via: - Proto field: bool debug or bool enable_debug on request message - gRPC metadata/HTTP header: x-debug-log-level (configurable) Combined with trace ID propagation, this allows targeted production debugging with zero blast radius. Inserted after TraceIdInterceptor in the default chain. Also adds goleak.VerifyTestMain for goroutine leak detection.
📝 WalkthroughWalkthroughUpdated three go-coldbrew package dependencies to newer patch/minor versions and added go.uber.org/goleak for goroutine leak detection. Implemented a new DebugLogInterceptor that dynamically adjusts request log levels based on proto debug fields or gRPC metadata headers, with comprehensive test coverage. Changes
Sequence DiagramsequenceDiagram
participant Client
participant gRPCServer as gRPC Server
participant DebugLogInterceptor
participant ProtoMsg as Proto Message
participant MetadataHeader as Metadata Header
participant Logger
Client->>gRPCServer: gRPC Request (with metadata)
gRPCServer->>DebugLogInterceptor: Intercept Request
DebugLogInterceptor->>ProtoMsg: Check GetDebug()/GetEnableDebug()
alt Proto Has Debug Flag
ProtoMsg-->>DebugLogInterceptor: Debug bool value
DebugLogInterceptor->>Logger: Override log level based on proto
else Proto Lacks Debug Flag
ProtoMsg-->>DebugLogInterceptor: Field not found
DebugLogInterceptor->>MetadataHeader: Check x-debug-log-level header
MetadataHeader-->>DebugLogInterceptor: Header value (if present)
DebugLogInterceptor->>Logger: Parse header and override log level
end
DebugLogInterceptor-->>gRPCServer: Continue with handler
gRPCServer-->>Client: Response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds a new unary server interceptor that can elevate per-request logging verbosity (intended for targeted production debugging) and wires it into the default interceptor chain, along with tests and goroutine-leak detection.
Changes:
- Introduces
DebugLogInterceptorwith opt-in via request proto fields (GetDebug()/GetEnableDebug()) or gRPC metadata header (configurable name). - Updates
DefaultInterceptors()to includeDebugLogInterceptor(configurable disable switch). - Adds
goleak.VerifyTestMainplus new unit tests covering the debug-log behavior and configuration.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| interceptors.go | Adds debug-log interceptor, config setters/getter, and inserts it into the default interceptor chain. |
| interceptors_test.go | Adds unit tests for proto-field and metadata-triggered log-level overrides plus config behaviors. |
| goleak_test.go | Adds package TestMain to run goleak and ignore known unavoidable goroutines. |
| go.mod | Adds go.uber.org/goleak dependency for test leak detection. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address review: trim and reject empty header names in SetDebugLogHeaderName. Clarify that proto fields always set DebugLevel while the metadata header accepts any valid log level.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
interceptors.go (1)
431-438:⚠️ Potential issue | 🟠 MajorMove
DebugLogInterceptorbefore the response-time logger.Line 433 keeps
ResponseTimeLoggingInterceptoroutside the new interceptor, and its deferred log closes over the originalctx. That means requests withdebug=trueorx-debug-log-levelstill emit the default response-time log at the normal level, so the feature only affects downstream handlers/interceptors.Proposed fix
ints = append(ints, DefaultTimeoutInterceptor(), - ResponseTimeLoggingInterceptor(defaultFilterFunc), TraceIdInterceptor(), ) if !disableDebugLogInterceptor { ints = append(ints, DebugLogInterceptor()) } + ints = append(ints, + ResponseTimeLoggingInterceptor(defaultFilterFunc), + ) if !disableProtoValidate { ints = append(ints, ProtoValidateInterceptor()) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@interceptors.go` around lines 431 - 438, The ResponseTimeLoggingInterceptor is appended before DebugLogInterceptor so its deferred log closes over the original ctx and ignores debug log level changes; when disableDebugLogInterceptor is false, move DebugLogInterceptor() into the ints slice before ResponseTimeLoggingInterceptor(defaultFilterFunc) (i.e., ensure ints is appended with DebugLogInterceptor() prior to ResponseTimeLoggingInterceptor(defaultFilterFunc)) so the debug interceptor wraps and influences the response-time logger; update the append order around DefaultTimeoutInterceptor(), TraceIdInterceptor(), ResponseTimeLoggingInterceptor(...), and DebugLogInterceptor() accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@interceptors.go`:
- Around line 431-438: The ResponseTimeLoggingInterceptor is appended before
DebugLogInterceptor so its deferred log closes over the original ctx and ignores
debug log level changes; when disableDebugLogInterceptor is false, move
DebugLogInterceptor() into the ints slice before
ResponseTimeLoggingInterceptor(defaultFilterFunc) (i.e., ensure ints is appended
with DebugLogInterceptor() prior to
ResponseTimeLoggingInterceptor(defaultFilterFunc)) so the debug interceptor
wraps and influences the response-time logger; update the append order around
DefaultTimeoutInterceptor(), TraceIdInterceptor(),
ResponseTimeLoggingInterceptor(...), and DebugLogInterceptor() accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0ca7fb61-444a-4c8e-acc4-31c1ded87889
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (4)
go.modgoleak_test.gointerceptors.gointerceptors_test.go
Summary
DebugLogInterceptorenables per-request debug logging via:bool debugorbool enable_debugon request message (auto-detected via type assertion, same pattern asTraceIdInterceptor)x-debug-log-level(configurable viaSetDebugLogHeaderName)TraceIdInterceptorin the default chainSetDisableDebugLogInterceptor(bool),SetDebugLogHeaderName(string)goleak.VerifyTestMainfor goroutine leak detectionTest plan
TestDebugLogInterceptor_ProtoFieldDebug— GetDebug() true triggers debug levelTestDebugLogInterceptor_ProtoFieldEnableDebug— GetEnableDebug() true triggers debug levelTestDebugLogInterceptor_NoField— plain request, no overrideTestDebugLogInterceptor_DebugFalse— GetDebug() false, no overrideTestDebugLogInterceptor_Metadata— header triggers debug levelTestDebugLogInterceptor_CustomHeaderName— custom header name worksTestDebugLogInterceptor_Disabled— disabled via config, no overridemake testandmake lintcleanSummary by CodeRabbit
New Features
x-debug-log-level) or protocol buffer message fields.Tests
Chores