-
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 support for providers in components command #11900
base: main
Are you sure you want to change the base?
Add support for providers in components command #11900
Conversation
Sample output./bin/otelcorecol_linux_amd64 components
buildinfo:
command: otelcorecol
description: Local OpenTelemetry Collector binary, testing only.
version: 0.115.0-dev
receivers:
- name: nop
module: go.opentelemetry.io/collector/receiver/nopreceiver v0.115.0
stability:
logs: Beta
metrics: Beta
traces: Beta
- name: otlp
module: go.opentelemetry.io/collector/receiver/otlpreceiver v0.115.0
stability:
logs: Beta
metrics: Stable
traces: Stable
processors:
- name: batch
module: go.opentelemetry.io/collector/processor/batchprocessor v0.115.0
stability:
logs: Beta
metrics: Beta
traces: Beta
- name: memory_limiter
module: go.opentelemetry.io/collector/processor/memorylimiterprocessor v0.115.0
stability:
logs: Beta
metrics: Beta
traces: Beta
exporters:
- name: debug
module: go.opentelemetry.io/collector/exporter/debugexporter v0.115.0
stability:
logs: Development
metrics: Development
traces: Development
- name: nop
module: go.opentelemetry.io/collector/exporter/nopexporter v0.115.0
stability:
logs: Beta
metrics: Beta
traces: Beta
- name: otlp
module: go.opentelemetry.io/collector/exporter/otlpexporter v0.115.0
stability:
logs: Beta
metrics: Stable
traces: Stable
- name: otlphttp
module: go.opentelemetry.io/collector/exporter/otlphttpexporter v0.115.0
stability:
logs: Beta
metrics: Stable
traces: Stable
connectors:
- name: forward
module: go.opentelemetry.io/collector/connector/forwardconnector v0.115.0
stability:
logs-to-logs: Beta
logs-to-metrics: Undefined
logs-to-traces: Undefined
metrics-to-logs: Undefined
metrics-to-metrics: Beta
metrics-to-traces: Undefined
traces-to-logs: Undefined
traces-to-metrics: Undefined
traces-to-traces: Beta
extensions:
- name: memory_limiter
module: go.opentelemetry.io/collector/extension/memorylimiterextension v0.115.0
stability:
extension: Development
- name: zpages
module: go.opentelemetry.io/collector/extension/zpagesextension v0.115.0
stability:
extension: Beta
providers:
- name: env
module: go.opentelemetry.io/collector/confmap/provider/envprovider v1.21.0
- name: file
module: go.opentelemetry.io/collector/confmap/provider/fileprovider v1.21.0
- name: http
module: go.opentelemetry.io/collector/confmap/provider/httpprovider v1.21.0
- name: https
module: go.opentelemetry.io/collector/confmap/provider/httpsprovider v1.21.0
- name: yaml
module: go.opentelemetry.io/collector/confmap/provider/yamlprovider v1.21.0 |
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.
@ChrsMark sorry for the delay. I would love to see this merged. Can you please merge main and run make genotelcorecol
to update the provider versions in the generated main.go
file?
e022af7
to
86ecb23
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #11900 +/- ##
=======================================
Coverage 91.60% 91.61%
=======================================
Files 461 461
Lines 24678 24685 +7
=======================================
+ Hits 22607 22614 +7
Misses 1689 1689
Partials 382 382 ☔ View full report in Codecov by Sentry. |
Thank's for reviewing this @andrzej-stencel! The patch is up to date now. /cc @mx-psi |
confmap/resolver.go
Outdated
@@ -39,6 +39,9 @@ type ResolverSettings struct { | |||
// It is required to have at least one factory. | |||
ProviderFactories []ProviderFactory | |||
|
|||
// ProviderModules maps provider types to their respective go modules. | |||
ProviderModules map[string]string |
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 PR in general LGTM but I don't feel like this is the best place for this field. The resolver does not really use this field. Should this be in otelcol.CollectorSettings
maybe?
772f5b9
to
74b548c
Compare
d79ea67
to
ef97da9
Compare
otelcol/command_components.go
Outdated
for _, confmapProvider := range confmapProviderFactories { | ||
provider := confmapProvider.Create(set.ConfigProviderSettings.ResolverSettings.ProviderSettings) | ||
scheme := provider.Scheme() | ||
module := set.ProviderModules[scheme+"provider"] |
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.
Using the scheme as the name for the provider feels like it's not guaranteed to always work, I'm a bit concerned some providers may have a difference in the future even if all upstream providers have matching names and schemes.
Could we do the following instead:
- In the builder, set up all the metadata we need given the Go module metadata. We should probably just do all the transformations we need here.
- Loop through
set.providerModules
and retrieve this data directly.
What do you think?
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.
Thinking on it more, we should call out the scheme instead of or in addition to the module name since the scheme is the part users actually care about. So we'll need to still read from confmapProviderFactories
to get the scheme.
However I still don't think we can guarantee that future providers won't have names like acmecorpsecretsprovider
that read URIs with schemes like acs://corp.my/secret
. We could potentially use reflection to find the module for a given provider and match them up, but I'm not sure about the best way to do this given the provider interfaces don't give a module name.
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 see! Would that make sense to extend the provider interface so as to be able to provide the package or module name?
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.
Unfortunately we likely can't expand the interface any further now that confmap is stable. However it looks like we can do something like this in the map inside main.go.tmpl:
{{.Name}}.NewFactory().Create(confmap.ProviderSettings{}).Scheme()
If we key on the scheme like this and have the go module as the value, I think we only need a few changes inside the main.go template.
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.
That works, indeed! Thank's for the suggestion!
Signed-off-by: ChrsMark <[email protected]>
Signed-off-by: ChrsMark <[email protected]>
Signed-off-by: ChrsMark <[email protected]>
7bb33f3
to
8cc1fde
Compare
Signed-off-by: ChrsMark <[email protected]>
8cc1fde
to
da97c18
Compare
@@ -41,3 +41,6 @@ extensions: | |||
module: go.opentelemetry.io/collector/extension/extensiontest v1.2.3 | |||
stability: | |||
extension: Stable | |||
providers: | |||
- name: nop |
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.
One last thing, should we call this scheme
instead of name
?
Signed-off-by: ChrsMark <[email protected]>
Signed-off-by: ChrsMark <[email protected]>
Description
This PR adds support for printing config providers with the
components
command.Link to tracking issue
Related to #11570
Testing
Documentation