RSDK-13826: Do not deduplicate for debug-level loggers (or when debug is on)#5964
RSDK-13826: Do not deduplicate for debug-level loggers (or when debug is on)#5964benjirewis wants to merge 2 commits intoviamrobotics:mainfrom
Conversation
viambot
added
the
safe to test
benjirewis
changed the title
| logger := &impl{ | ||
| name: "impl", | ||
| level: NewAtomicLevelAt(DEBUG), | ||
| level: NewAtomicLevelAt(INFO), |
There was a problem hiding this comment.
Needs to be INFO to deduplicate now.
| } | ||
|
|
||
| for range 4 { | ||
| logger.Info("identical message") |
There was a problem hiding this comment.
Note that an Info, Warn, or Error (in addition to Debug) log from a logger with a level set to "debug" will not be deduplicated. E.g., if I set the builtin datamanager's log level to "debug", logs of all levels from that logger will not be deduplicated.
| // If the logger is at DEBUG level, or viam-server is run with "debug": true or | ||
| // `-debug`, never deduplicate. If a user asks for debug logs, they likely want to see | ||
| // all logs. |
There was a problem hiding this comment.
or viam-server is run with "debug": true or
-debug
Setting "debug": true or running with -debug is now equivalent to putting "disable_log_deduplication": true in your JSON config.
benjirewis
changed the title
There was a problem hiding this comment.
Pull request overview
This PR updates the logging deduplication behavior so that when debug logging is enabled (either via a DEBUG-level logger or global debug mode), repeated log messages are no longer squashed.
Changes:
- Skip log deduplication in
impl.Writewhen the logger is set toDEBUGor whenGlobalLogLeveliszapcore.DebugLevel. - Update
TestLoggingDeduplicationto run with an INFO-level logger (since DEBUG no longer deduplicates). - Add
TestDebugLevelNeverDeduplicatesto validate that DEBUG-level loggers do not deduplicate and that INFO re-enables it.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| logging/impl.go | Changes deduplication gating logic to disable dedup when debug logging is enabled (per-logger or global). |
| logging/impl_test.go | Adjusts existing dedup test to INFO and adds a new test asserting DEBUG-level loggers never deduplicate. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if imp.registry.DeduplicateLogs.Load() && !imp.neverDeduplicate && | ||
| // If the logger is at DEBUG level, or viam-server is run with "debug": true or | ||
| // `-debug`, never deduplicate. If a user asks for debug logs, they likely want to see | ||
| // all logs. | ||
| imp.level.Get() != DEBUG && GlobalLogLevel.Level() != zapcore.DebugLevel { |
viambot
added
safe to test
RSDK-13286
[aided by Claude 🤖]
What
Skips log deduplication when a logger's level is DEBUG or when
viam-serveris run with-debugor "debug": true.Why
Users who enable debug logging want to see every log line. Deduplicating (squashing repeated messages) defeats the purpose of asking for verbose output.
Testing
Added
TestDebugLevelNeverDeduplicateswhich verifies that a logger at DEBUG level emits all repeated messages, and that bumping it back to INFO re-enables deduplication. Also manually tested that various debug settings do or do not deduplicate messages as expected.