-
Notifications
You must be signed in to change notification settings - Fork 6
Allow log throttles to be configured per component #181
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
| func (l *loggerProvider) Get(component LogComponentName) log.Logger { | ||
| logger, exists := l.loggers[component] | ||
| if !exists { | ||
| logger = l.root |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so if component doesn't exist, then the default behavior is to use root which means always log right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, though the default logger can be set with a throttle as well.
develop/old-config-with-TLS.yaml
Outdated
| adminservice: | ||
| throttleMaxRPS: 10 | ||
| testexample: | ||
| throttleMaxRPS: 11 | ||
| adminstreams: | ||
| throttleMaxRPS: 12 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we always config adminservice and adminstreams and treat logConfigs as overrides? this is to avoid adding these configs in all yaml files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, yeah some defaults based on the old values is probably a good idea. I'll add a block in logging
develop/old-config-with-TLS.yaml
Outdated
| profiling: {} | ||
| logConfigs: | ||
| adminservice: | ||
| throttleMaxRPS: 10 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can RPS be < 1?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, will update to demonstrate it
What changed?
Added a new package
loggingand config maplogConfigs["component"]=LoggingConfig{}to allow log throttles to be configured per-component. Logging configs are referenced directly by name and may set the rate of logs or disable logs for that config. A config not explicitly set in the yaml falls back to the root logger.Why?
There have been many times when we've wanted to specify different log throttles for different kinds of logs, and as we did so we ended up creating multiple
throttledLogForXYZmembers. This allows a single place to easily configure log throttles, and to easily add and remove custom log configs as needed.