-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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 confmap provider to registry.json #5637
base: main
Are you sure you want to change the base?
Conversation
I will also plan to submit a separate PR to add the existing five confmap.providers to the registry once this change is approved. |
Can you put all of them in one PR? Makes it easier to validate things and how we are going to name those values. |
Yes, done. |
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
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.
Thanks, see some questions and suggestions inlined
Co-authored-by: Severin Neumann <[email protected]>
As mentioned in #5387, there are more components that are compatible with the OpenTelemetry Collector.
These include
confmap.Converter
andconfmap.Provider
types.confmap.Converter: open-telemetry/opentelemetry-collector#11582
confmap.Provider: open-telemetry/opentelemetry-collector#4567
There are currently five confmap.Providers that are maintained in the OpenTelemetry Collector core repository and while there is only one Converter there and it is deprecated, there are vendors that make use of converters and it is possible they would choose to publish them in the registry once this is available.
As I am not super familiar with the registry and changes necessary to update the same, I may have missed file(s) that need to be changed and will happily make these changes if suggested.
Additionally, I don't feel strongly about calling them "confmap converter" vs. "converter" or provider, but it seemed slightly more descriptive. Open to feedback there as well.