-
Notifications
You must be signed in to change notification settings - Fork 486
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
integrations: add missing setting to override eventhandler instance #2432
Conversation
@@ -38,7 +39,8 @@ type Config struct { | |||
// If you would like to limit events to a given namespace, use this parameter. | |||
Namespace string `yaml:"namespace,omitempty"` | |||
// Extra labels to append to log lines | |||
ExtraLabels labels.Labels `yaml:"extra_labels,omitempty"` | |||
ExtraLabels labels.Labels `yaml:"extra_labels,omitempty"` | |||
Common common.MetricsConfig `yaml:",inline"` |
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.
I don't think embedding all of MetricsConfig is appropriate here because eventhandler is a logging integration. Can we manually copy the instance key field instead of bringing in everything else?
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.
LGTM, thanks Marc!
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.
LGTM
PR Description
As part of the effort of completing #2017 we realized that eventhandler integration
didn't include
instance
into the config which allows also to override the instancekey for that integration.
Which issue(s) this PR fixes
#2366
PR Checklist