-
Notifications
You must be signed in to change notification settings - Fork 153
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
[chore] Adopt to changes in confmap.ResolverSettings API #4848
Conversation
465cf12
to
ecae16c
Compare
ecae16c
to
fb0cc7a
Compare
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.
Looks like some integration tests are failing, potentially related to these changes. The following are failing, there are potentially more that I missed:
TestCoreValidateDetectsInvalidYamlProvider
TestCoreValidateYamlProvider
TestCoreValidateDefaultConfig
@@ -114,7 +109,6 @@ func newDiscoverer(logger *zap.Logger) (*discoverer, error) { | |||
configs: map[string]*Config{}, | |||
duration: duration, | |||
mu: sync.Mutex{}, | |||
expandConverter: expandconverter.NewFactory().Create(confmap.ConverterSettings{}), |
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.
Lint is failing as expandConverter
is no longer used, looks like we can remove it from the discoverer
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.
isn't discoverer used by the discovery mechanism somewhere?
Need open-telemetry/opentelemetry-collector#10203 to resolve integration tests and fix |
Adopt the codebase to confmap.ResolverSettings changes upstream: - `Providers` field replaced with `ProviderFactories` - `Converters` field replaced with `ConverterFactories`
fb0cc7a
to
abd9776
Compare
I’m skipping the integration tests for now, so we can merge this separately from the core upgrade to 0.101.0 |
Adopt the codebase to confmap.ResolverSettings changes upstream:
Providers
field replaced withProviderFactories
Converters
field replaced withConverterFactories