-
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
wip: add count connector #6575
wip: add count connector #6575
Conversation
Signed-off-by: Robbie Lankford <[email protected]>
|
||
// SetToDefault implements river.Defaulter. | ||
func (args *Arguments) SetToDefault() { | ||
*args = DefaultArguments |
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.
If there is more than one such connector in the config file, I wonder if there'd be a bug due to reusing the same DefaultArguments
. E.g. if one connector overrides some defaults, but another one doesn't, then would the connector which only used defaults pick up the settings from the other one? I wonder if we should copy the slices with defaults every time we call SetToDefault
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 can test this out. I noticed several other components following this pattern though, I'd assume they have the same issue if this is the case.
It's something for later on, when we're ready to merge, but let's make sure we also include a converter in the otelconvert package? We can help with that. |
This PR has not had any activity in the past 30 days, so the |
PR Description
Which issue(s) this PR fixes
Notes to the Reviewer
PR Checklist