-
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
[otelcol] Obtain the Collector's effective configuration from otelcol.Config
#10139
Changes from 5 commits
9694609
dbb2850
a803cca
a008ba1
b8a2f6a
c063bc2
a708ac8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
# Use this changelog template to create an entry for release notes. | ||
|
||
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' | ||
change_type: breaking | ||
|
||
# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver) | ||
component: otelcol | ||
|
||
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). | ||
note: Obtain the Collector's effective config from otelcol.Config | ||
|
||
# One or more tracking issues or pull requests related to the change | ||
issues: [10139] | ||
|
||
# (Optional) One or more lines of additional information to render under the primary note. | ||
# These lines will be padded with 2 spaces and then inserted directly into the document. | ||
# Use pipe (|) for multiline entries. | ||
subtext: "`otelcol.Collector` will now marshal `confmap.Conf` objects from `otelcol.Config` itself." | ||
|
||
# Optional: The change log or logs in which this entry should be included. | ||
# e.g. '[user]' or '[user, api]' | ||
# Include 'user' if the change is relevant to end users. | ||
# Include 'api' if there is a change to a library API. | ||
# Default: '[user]' | ||
change_logs: [api] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
# Use this changelog template to create an entry for release notes. | ||
|
||
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' | ||
change_type: deprecation | ||
|
||
# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver) | ||
component: otelcol | ||
|
||
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). | ||
note: Deprecate `otelcol.ConfmapProvider` | ||
|
||
# One or more tracking issues or pull requests related to the change | ||
issues: [10139] | ||
|
||
# (Optional) One or more lines of additional information to render under the primary note. | ||
# These lines will be padded with 2 spaces and then inserted directly into the document. | ||
# Use pipe (|) for multiline entries. | ||
subtext: "`otelcol.Collector` will now marshal `confmap.Conf` objects from `otelcol.Config` itself." | ||
|
||
# Optional: The change log or logs in which this entry should be included. | ||
# e.g. '[user]' or '[user, api]' | ||
# Include 'user' if the change is relevant to end users. | ||
# Include 'api' if there is a change to a library API. | ||
# Default: '[user]' | ||
change_logs: [api] |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -58,6 +58,9 @@ type ConfigProvider interface { | |
// | ||
// The purpose of this interface is that otelcol.ConfigProvider structs do not | ||
// necessarily need to use confmap.Conf as their underlying config structure. | ||
// | ||
// Deprecated: [v0.105.0] This interface is deprecated. otelcol.Collector will now obtain | ||
// a confmap.Conf object from the unmarshaled config itself. | ||
type ConfmapProvider interface { | ||
// GetConfmap resolves the Collector's configuration and provides it as a confmap.Conf object. | ||
// | ||
|
@@ -70,7 +73,6 @@ type configProvider struct { | |
} | ||
|
||
var _ ConfigProvider = (*configProvider)(nil) | ||
var _ ConfmapProvider = (*configProvider)(nil) | ||
|
||
// ConfigProviderSettings are the settings to configure the behavior of the ConfigProvider. | ||
type ConfigProviderSettings struct { | ||
|
@@ -142,12 +144,3 @@ func (cm *configProvider) Watch() <-chan error { | |
func (cm *configProvider) Shutdown(ctx context.Context) error { | ||
return cm.mapResolver.Shutdown(ctx) | ||
} | ||
|
||
func (cm *configProvider) GetConfmap(ctx context.Context) (*confmap.Conf, error) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we keep a dummy implementation of this method for one or more releases that returns an error saying that this functionality is deprecated? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I removed this because I don't expect anyone else is using it, but thinking on it, this should go through the normal deprecation process. I've added a deprecation notice to this method and we can just remove the implementation when we remove the interface. I would feel comfortable removing both one release after deprecating them. |
||
conf, err := cm.mapResolver.Resolve(ctx) | ||
if err != nil { | ||
return nil, fmt.Errorf("cannot resolve the configuration: %w", err) | ||
} | ||
|
||
return conf, nil | ||
} |
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.
Another option could be to make this only a warning log, if we're comfortable that all extensions asking for the effective configuration will get an empty map.