fix: don't output overly verbose watermill trace logs by default#547
Conversation
WalkthroughThe PR introduces mock code generation for the Logger interface via mockgen and extends WatermillLoggerAdapter with a trace logging flag to conditionally include trace-level logs based on runtime configuration. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 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 |
d9b6e6c to
34a0899
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
publish/logging.go (1)
35-38:⚠️ Potential issue | 🟠 MajorTrace flag is lost when
With(...)is used.
With(...)returns a new adapter without copyingincludeTraceLogs, so trace gets silently disabled on derived loggers. Propagate the flag when creating the new adapter.🛠️ Proposed fix
func (w watermillLoggerAdapter) With(fields watermill.LogFields) watermill.LoggerAdapter { return watermillLoggerAdapter{ + includeTraceLogs: w.includeTraceLogs, Logger: w.Logger.WithFields(fields), } }
🤖 Fix all issues with AI agents
In `@publish/logging.go`:
- Around line 41-45: The public API was changed; restore the original
NewWatermillLoggerAdapter(logger logging.Logger) signature as a compatibility
wrapper and introduce a new constructor that accepts the trace flag. Rename the
existing NewWatermillLoggerAdapter(logger logging.Logger, trace bool)
implementation to NewWatermillLoggerAdapterWithTrace(logger logging.Logger,
trace bool) (or similar), then add a simple NewWatermillLoggerAdapter(logger
logging.Logger) that returns NewWatermillLoggerAdapterWithTrace(logger, false);
update references if needed so callers default to trace=false unless they call
the new WithTrace constructor.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #547 +/- ##
==========================================
- Coverage 29.13% 28.80% -0.33%
==========================================
Files 174 175 +1
Lines 6965 7062 +97
==========================================
+ Hits 2029 2034 +5
- Misses 4819 4911 +92
Partials 117 117 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Watermill consumers output extremely verbose trace logs. Our logging.Logger doesn't have a trace mode, but outputs trace logs as part of the Debug mode.
This gives us the option to still see debug logs and omit the more verbose trace logs.