Skip to content

RSDK-13456: Fix debug log level toggling in viam-server#5833

Open
EvanDorsky wants to merge 2 commits intoviamrobotics:mainfrom
EvanDorsky:RSDK-13456-fix-debug-log-toggle
Open

RSDK-13456: Fix debug log level toggling in viam-server#5833
EvanDorsky wants to merge 2 commits intoviamrobotics:mainfrom
EvanDorsky:RSDK-13456-fix-debug-log-toggle

Conversation

@EvanDorsky
Copy link
Member

Fixed a bug in the logic in alignModuleLogLevels that would essentially latch all modules at the debug logging level until viam-server was restarted.

This is still WIP since I need to do some more testing to make sure this change doesn't cause excessive module restarts or any other undesired behavior. And I need to update the comment that I've marked TODO before merging.

@viambot viambot added the safe to test This pull request is marked safe to test from a trusted zone label Mar 9, 2026
@cheukt
Copy link
Member

cheukt commented Mar 9, 2026

can you add a test for this?

@EvanDorsky EvanDorsky requested a review from dgottlieb March 10, 2026 21:05
Copy link
Member

@benjirewis benjirewis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

// they match the new global log level.
// TODO: confirm that this causes modules to be restarted with the new log level
func alignModuleLogLevels(config *Config) {
if globalLogger.actualGlobalLogger != nil && globalLogger.actualGlobalLogger.GetLevel() != logging.DEBUG && config.Debug {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[q] Was the issue here that because of && config.Debug, we were only "aligning module log levels" when debug: true was added to the JSON config, but not when debug: true was removed or debug: false was added?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

safe to test This pull request is marked safe to test from a trusted zone

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants