-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Add validate interface in configmodels to force each component do configuration validation #2802
Conversation
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.
The way how I see this, based on the discussion in the other comment, I think we should have (in current configmodels):
package configmodels
// Processor is the configuration of a processor. Specific processors must implement this
// interface and will typically embed ProcessorSettings struct or a struct that extends it.
type Processor interface {
NamedEntity
Validate() errror // and for all interfaces Exporter, etc.
}
// In the unmarshaler PR, optional interface for configs
type Unmarshaler interface {
Unmarshal(componentParser *config.Parser, intoCfg interface{}) error
}
type UnmarshalFunc func(componentParser *config.Parser, intoCfg interface{}) error
@mx-psi you may need #2808 before in order to avoid circular deps.
189190d
to
b55e01e
Compare
Codecov Report
@@ Coverage Diff @@
## main #2802 +/- ##
=======================================
Coverage 91.76% 91.77%
=======================================
Files 281 286 +5
Lines 15149 15163 +14
=======================================
+ Hits 13902 13916 +14
Misses 853 853
Partials 394 394
Continue to review full report at Codecov.
|
Yep, made the changes for it. Thanks. |
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.
The design makes sense to me and I would say we can apply it to all component types (if Bogdan gives his 👍).
…figuration validation
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.
Let's merge as first draft, I will followup with some cleanups.
"testing" | ||
|
||
"github.com/stretchr/testify/assert" | ||
) | ||
|
||
type NopConfig struct { |
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.
No need to be public.
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'll move
Let's merge as first draft, I will followup with some cleanups.
@bogdandrutu , I am going to expand the same checks to all the other type of components in the next PR. Sounds good to you?
…figuration validation (open-telemetry#2802)
Description:
The goal is to add
Validate()
interface inconfig.Receiver|Exporter|Processor|Extension
so we can force each component to add validation logic on component level configuration. Only have the changes forconfig.Receiver
in this PR.Added
CustomConfigOptions
interface inconfig.Receiver
. AndCustomConfigOptions
interface has embeddedCustomConfigValidator
interface so it can force each receiver component to implementValidate()
for Config validation check. Also,CustomConfigOptions
interface can be extended to add more CustomConfig options.The reason why we should have
Validate()
for each component.Currently, since we don't have a unified fashion for components to
validate
the configuration. The component contributors write the config validate logic in different places in the component code.For example. in
prometheusreceiver
the cfg validate logic is coded in createMetricsReceiver. and inawsemfexporter
it was written in here.By doing the current implementation, all the contributors will follow the same fashion for the configuration validation. And we can fail the collector in the start time with friendly error messages.
Custom Validation Example for
prometheusreceiver
,Need help to review this approach before moving to the others (exporter, processor, extension)
Link to tracking Issue:
#2541
#2597
Testing:
WIP
Documentation:
N/A