Summary
The logging configuration options (log.format, log.level, and log.color) do not respect the intended configuration precedence when merging multiple configuration files. User-level configuration settings are incorrectly overridden by system defaults and system-level configuration files.
Issue Origin
This bug was identified in PR #108 - Comment while adding the --log-color configuration option.
Expected Behavior
The configuration precedence should follow this order (highest to lowest priority):
- CLI flags
- Config file passed with
--config flag
- User config file (
~/.config/ochami/config.yaml)
- System config file (
/etc/ochami/config.yaml)
- Compiled defaults
Any logging option explicitly set at a higher priority level should override the same option at a lower priority level.
Actual Behavior
When loading and merging configuration files, logging options set in the user config file are being overridden by system config file values and compiled defaults. This reverses the intended precedence, making it impossible to override system-level logging settings with user-level settings.
Root Cause
The issue occurs in internal/config/config.go in the LoadGlobalConfigMerged() function (lines 497-579). The problem is:
- The
ConfigLog struct (lines 159-162) lacks a custom UnmarshalYAML() method to intelligently handle partial configuration merges
- When a user config file specifies only some logging options (e.g., only
log.level), the ConfigLog struct gets unmarshalled with unset fields
- During the merge operation, these unset fields in the user config override previously-merged values from the system config
- The behavior differs from other config fields like
timeout, which has its own UnmarshalYAML() method (lines 86-146) that properly handles default value precedence
The Config struct's UnmarshalYAML() method demonstrates the correct approach by detecting which keys were explicitly set and only applying defaults when a key was not present in the YAML.
Reproduction Steps
Setup
-
Create /etc/ochami/config.yaml:
log:
format: json
level: debug
color: "on"
-
Create ~/.config/ochami/config.yaml:
log:
level: info
color: "off"
-
Run ochami without any config file flag or logging CLI flags
Expected Result
The merged configuration should be:
log.format: json (from system config, not overridden by user)
log.level: info (from user config, higher priority than system)
log.color: off (from user config, higher priority than system)
Actual Result
The actual merged configuration is:
log.format: rfc3339 (compiled default)
log.level: warning (compiled default)
log.color: auto (compiled default or system value)
The user config file settings are ignored, and the system config settings are also overridden by defaults.
Impact
This bug affects:
Users cannot reliably configure logging behavior using config files, making it difficult to maintain consistent logging settings across different environments (development, staging, production).
Solution Approach
The ConfigLog struct needs a custom UnmarshalYAML() method similar to the one implemented for the Config struct. This method should:
- Detect which logging configuration keys were explicitly set in the YAML
- Only apply default values for keys that were not present in the YAML
- Preserve values that were merged from higher-priority sources before this unmarshalling step
This approach is already proven to work effectively for the timeout configuration in the Config.UnmarshalYAML() method.
Summary
The logging configuration options (
log.format,log.level, andlog.color) do not respect the intended configuration precedence when merging multiple configuration files. User-level configuration settings are incorrectly overridden by system defaults and system-level configuration files.Issue Origin
This bug was identified in PR #108 - Comment while adding the
--log-colorconfiguration option.Expected Behavior
The configuration precedence should follow this order (highest to lowest priority):
--configflag~/.config/ochami/config.yaml)/etc/ochami/config.yaml)Any logging option explicitly set at a higher priority level should override the same option at a lower priority level.
Actual Behavior
When loading and merging configuration files, logging options set in the user config file are being overridden by system config file values and compiled defaults. This reverses the intended precedence, making it impossible to override system-level logging settings with user-level settings.
Root Cause
The issue occurs in
internal/config/config.goin theLoadGlobalConfigMerged()function (lines 497-579). The problem is:ConfigLogstruct (lines 159-162) lacks a customUnmarshalYAML()method to intelligently handle partial configuration mergeslog.level), theConfigLogstruct gets unmarshalled with unset fieldstimeout, which has its ownUnmarshalYAML()method (lines 86-146) that properly handles default value precedenceThe
Configstruct'sUnmarshalYAML()method demonstrates the correct approach by detecting which keys were explicitly set and only applying defaults when a key was not present in the YAML.Reproduction Steps
Setup
Create
/etc/ochami/config.yaml:Create
~/.config/ochami/config.yaml:Run ochami without any config file flag or logging CLI flags
Expected Result
The merged configuration should be:
log.format:json(from system config, not overridden by user)log.level:info(from user config, higher priority than system)log.color:off(from user config, higher priority than system)Actual Result
The actual merged configuration is:
log.format:rfc3339(compiled default)log.level:warning(compiled default)log.color:auto(compiled default or system value)The user config file settings are ignored, and the system config settings are also overridden by defaults.
Impact
This bug affects:
log.formatconfigurationlog.levelconfigurationlog.colorconfiguration (newly added in PR feat: add log message color configuration #108)Users cannot reliably configure logging behavior using config files, making it difficult to maintain consistent logging settings across different environments (development, staging, production).
Solution Approach
The
ConfigLogstruct needs a customUnmarshalYAML()method similar to the one implemented for theConfigstruct. This method should:This approach is already proven to work effectively for the
timeoutconfiguration in theConfig.UnmarshalYAML()method.