Skip to content
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

[otelcol] confmap ResolverSettings should retain all features custom config provider has #10005

Closed
splunkericl opened this issue Apr 19, 2024 · 7 comments
Labels
area:confmap bug Something isn't working

Comments

@splunkericl
Copy link
Contributor

Describe the bug
As part of #9228, custom configProvider is being deprecated from the collector.

The main proposal was to move all features from configProvider to derive from confmap.ResolverSettings. However, one feature that was missing was that custom configProvider allows users to provide a a custom Watch method, which allows notifying the collector framework of settings update. This is a crucial use case of our product as we update out settings dynamically.

Steps to reproduce

  • Attempts to change from configProvider to confmap.ResolverSettings

What did you expect to see?

  • equivalent features that can be derived from confmap.ResolverSettings

What did you see instead?

  • features are missing while custom configProvider is deprecated

What version did you use?
starting v0.95

What config did you use?
N/A

Environment
mac sonoma 14.2.1

Additional context

@splunkericl splunkericl added the bug Something isn't working label Apr 19, 2024
@splunkericl
Copy link
Contributor Author

Hey @evan-bradley and @mx-psi , I filed a github issue here to follow up with the discussion from #9228

@evan-bradley
Copy link
Contributor

evan-bradley commented Apr 19, 2024

Thanks for opening this @splunkericl. Could you provide more details on why you need a custom Watch method? I would expect that updating the config at runtime would be triggered by confmap.Provider implementations that support live updates. All Providers are given a confmap.WatcherFunc that they can trigger whenever there is a configuration change and the Collector's configuration should be updated. This will then bubble up to the Watch method on otelcol.ConfigProvider and reload the Collector service.

@splunkericl
Copy link
Contributor Author

Is the latest recommended approach to trigger the passed in confmap.WatcherFunc when there is an update? I think our code base is based on a much older architecture that requires custom configProvider.Watch method. If that is the recommended approach, we can update and close this issue.

@evan-bradley
Copy link
Contributor

Is the latest recommended approach to trigger the passed in confmap.WatcherFunc when there is an update?

That's my understanding, yes. I will let others chime in here if they know more about how this may change in the future. At a minimum, I expect reload functionality to be provided through the confmap.Provider interface and not in anything living in otelcol.

I think the design likely needs another review and the docs should be updated, but you can see a description for confmap.WatcherFunc here. None of the default Providers currently take advantage of this, but I have tested this with a custom Provider and it works as expected.

@mx-psi
Copy link
Member

mx-psi commented May 20, 2024

Agreed with @evan-bradley's analysis here. @splunkericl, if there are still remaining features that are not available under the new API, could you write a minimal example that showcases them?

@splunkericl
Copy link
Contributor Author

@mx-psi I think this is fine as long as it is backward compatible. Thanks for the confirmations

@evan-bradley
Copy link
Contributor

@splunkericl It sounds like you're good, so I'm going to close this for now. If you see any other issues with the new API as you adopt it, please let us know.

@evan-bradley evan-bradley closed this as not planned Won't fix, can't repro, duplicate, stale May 22, 2024
@github-project-automation github-project-automation bot moved this from Todo to Done in Collector: v1 May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:confmap bug Something isn't working
Projects
Archived in project
Development

No branches or pull requests

3 participants