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.

Sign up for GitHub

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

Jump to bottom

Remove deprecated comonent.Config.[ID|SetIDName]; Deprecate config.*Settings #6718

Merged
merged 2 commits into from
Dec 12, 2022
Merged

Remove deprecated comonent.Config.[ID|SetIDName]; Deprecate config.*Settings #6718

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
899c7e7
Remove deprecated comonent.Config.[ID|SetIDName]; Deprecate config.*S…
bogdandrutu Dec 9, 2022
2bce1d3
Update .chloggen/rmcfgid-1.yaml
Dec 12, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .chloggen/rmcfgid-1.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# 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: config

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Remove deprecated `comonent.Config.[ID|SetIDName]`.
bogdandrutu marked this conversation as resolved.
Show resolved Hide resolved

# One or more tracking issues or pull requests related to the change
issues: [4714]
11 changes: 11 additions & 0 deletions .chloggen/rmcfgid-2.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# 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: config

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Deprecate `config.[Extension|Exporter|Connector|Receiver|Processor]Settings`.

# One or more tracking issues or pull requests related to the change
issues: [6718]
2 changes: 2 additions & 0 deletions component/componenttest/nop_component.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,5 @@ type nopComponent struct {
component.StartFunc
component.ShutdownFunc
}

type nopConfig struct{}
11 changes: 1 addition & 10 deletions component/componenttest/nop_exporter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"context"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config"
"go.opentelemetry.io/collector/consumer/consumertest"
"go.opentelemetry.io/collector/exporter"
)
Expand All @@ -31,19 +30,11 @@ func NewNopExporterCreateSettings() exporter.CreateSettings {
}
}

type nopExporterConfig struct {
config.ExporterSettings `mapstructure:",squash"` // squash ensures fields are correctly decoded in embedded struct
}

// Deprecated: [v0.67.0] use exportertest.NewNopFactory.
func NewNopExporterFactory() exporter.Factory {
return exporter.NewFactory(
"nop",
func() component.Config {
return &nopExporterConfig{
ExporterSettings: config.NewExporterSettings(component.NewID("nop")),
}
},
func() component.Config { return &nopConfig{} },
exporter.WithTraces(createTracesExporter, component.StabilityLevelStable),
exporter.WithMetrics(createMetricsExporter, component.StabilityLevelStable),
exporter.WithLogs(createLogsExporter, component.StabilityLevelStable),
Expand Down
9 changes: 1 addition & 8 deletions component/componenttest/nop_extension.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"context"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config"
"go.opentelemetry.io/collector/extension"
)

Expand All @@ -30,18 +29,12 @@ func NewNopExtensionCreateSettings() extension.CreateSettings {
}
}

type nopExtensionConfig struct {
config.ExtensionSettings `mapstructure:",squash"` // squash ensures fields are correctly decoded in embedded struct
}

// Deprecated: [v0.67.0] use extensiontest.NewNopFactory.
func NewNopExtensionFactory() extension.Factory {
return extension.NewFactory(
"nop",
func() component.Config {
return &nopExtensionConfig{
ExtensionSettings: config.NewExtensionSettings(component.NewID("nop")),
}
return &nopConfig{}
},
func(context.Context, component.ExtensionCreateSettings, component.Config) (component.Extension, error) {
return nopExtensionInstance, nil
Expand Down
11 changes: 1 addition & 10 deletions component/componenttest/nop_processor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"context"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config"
"go.opentelemetry.io/collector/consumer"
"go.opentelemetry.io/collector/consumer/consumertest"
"go.opentelemetry.io/collector/processor"
Expand All @@ -32,19 +31,11 @@ func NewNopProcessorCreateSettings() processor.CreateSettings {
}
}

type nopProcessorConfig struct {
config.ProcessorSettings `mapstructure:",squash"` // squash ensures fields are correctly decoded in embedded struct
}

// Deprecated: [v0.68.0] use processortest.NewNopFactory.
func NewNopProcessorFactory() processor.Factory {
return processor.NewFactory(
"nop",
func() component.Config {
return &nopProcessorConfig{
ProcessorSettings: config.NewProcessorSettings(component.NewID("nop")),
}
},
func() component.Config { return &nopConfig{} },
processor.WithTraces(createTracesProcessor, component.StabilityLevelStable),
processor.WithMetrics(createMetricsProcessor, component.StabilityLevelStable),
processor.WithLogs(createLogsProcessor, component.StabilityLevelStable),
Expand Down
11 changes: 1 addition & 10 deletions component/componenttest/nop_receiver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"context"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config"
"go.opentelemetry.io/collector/consumer"
"go.opentelemetry.io/collector/receiver"
)
Expand All @@ -31,19 +30,11 @@ func NewNopReceiverCreateSettings() receiver.CreateSettings {
}
}

type nopReceiverConfig struct {
config.ReceiverSettings `mapstructure:",squash"` // squash ensures fields are correctly decoded in embedded struct
}

// Deprecated: [v0.67.0] use receivertest.NewNopFactory
func NewNopReceiverFactory() receiver.Factory {
return receiver.NewFactory(
"nop",
func() component.Config {
return &nopReceiverConfig{
ReceiverSettings: config.NewReceiverSettings(component.NewID("nop")),
}
},
func() component.Config { return &nopConfig{} },
receiver.WithTraces(createTraces, component.StabilityLevelStable),
receiver.WithMetrics(createMetrics, component.StabilityLevelStable),
receiver.WithLogs(createLogs, component.StabilityLevelStable))
Expand Down
10 changes: 1 addition & 9 deletions component/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,7 @@ import (
// (e.g. check if a required field is present).
//
// A valid implementation MUST pass the check componenttest.CheckConfigStruct (return nil error).
type Config interface {
// Deprecated: [v0.67.0] use Settings.ID.
ID() ID

// Deprecated: [v0.67.0] use Settings.ID.
SetIDName(idName string)

privateConfig()
}
type Config interface{}
Copy link
Member

@dmitryax dmitryax Dec 11, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// Config defines the configuration for a component.Component.

This description is probably not correct anymore, it can be applied to any configuration part now, not component.Component only, right?

Do you have any further plans for this type? Do we even need it if it's just interface{} now?

Also UnmarshalConfig doesn't seems to be applicable to compoonent module anymore. Do you think we should keep it here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This description is probably not correct anymore, it can be applied to any configuration part now, not component.Component only, right?

Not sure I understand. The "component.Factory.CreateDefaultConfig" returns this object. If others are using it somewhere else that is their problem... What are you looking for here?

Do you have any further plans for this type? Do we even need it if it's just interface{} now?

Good discussion if we need it. We can think in a future PR about that.

Also UnmarshalConfig doesn't seems to be applicable to compoonent module anymore. Do you think we should keep it here?

Same as previous, we can followup with more changes, for the moment the purpose of #4714 was to make sure we don't mutate the Config and that becomes an opaque object from the core collector perspective.

Copy link
Member

@dmitryax dmitryax Dec 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I understand. The "component.Factory.CreateDefaultConfig" returns this object. If others are using it somewhere else that is their problem... What are you looking for here?

Makes sense, so we still expect the Config to be mostly applied to Component even if it can be used with any other sub-configuration. That's what I was looking for


// As interface types are only used for static typing, a common idiom to find the reflection Type
// for an interface type Foo is to use a *Foo value.
Expand Down
17 changes: 5 additions & 12 deletions config/connector.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,10 @@ import (
"go.opentelemetry.io/collector/component"
)

// ConnectorSettings defines common settings for a component.Connector configuration.
// Specific connectors can embed this struct and extend it with more fields if needed.
//
// When embedded in the exporter config, it must be with `mapstructure:",squash"` tag.
type ConnectorSettings struct {
settings
}
// Deprecated: [v0.68.0] will be removed soon, Config no longer requires to embed this.
type ConnectorSettings struct{}

// NewConnectorSettings return a new ConnectorSettings with the given ComponentID.
func NewConnectorSettings(id component.ID) ConnectorSettings {
return ConnectorSettings{settings: newSettings(id)}
// Deprecated: [v0.68.0] will be removed soon, Config no longer requires to embed this.
func NewConnectorSettings(component.ID) ConnectorSettings {
return ConnectorSettings{}
}

var _ component.Config = (*ConnectorSettings)(nil)
15 changes: 4 additions & 11 deletions config/exporter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,10 @@ import (
"go.opentelemetry.io/collector/component"
)

// ExporterSettings defines common settings for a component.Exporter configuration.
// Specific exporters can embed this struct and extend it with more fields if needed.
//
// When embedded in the exporter config, it must be with `mapstructure:",squash"` tag.
type ExporterSettings struct {
settings
}
// Deprecated: [v0.68.0] will be removed soon, Config no longer requires to embed this.
type ExporterSettings struct{}

// NewExporterSettings return a new ExporterSettings with the given ComponentID.
// Deprecated: [v0.68.0] will be removed soon, Config no longer requires to embed this.
func NewExporterSettings(id component.ID) ExporterSettings {
return ExporterSettings{settings: newSettings(id)}
return ExporterSettings{}
}

var _ component.Config = (*ExporterSettings)(nil)
15 changes: 5 additions & 10 deletions config/extension.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,16 @@
// limitations under the License.

package config // import "go.opentelemetry.io/collector/config"

import (
"go.opentelemetry.io/collector/component"
)

// ExtensionSettings defines common settings for a component.Extension configuration.
// Specific processors can embed this struct and extend it with more fields if needed.
//
// When embedded in the extension config, it must be with `mapstructure:",squash"` tag.
// Deprecated: [v0.68.0] will be removed soon, Config no longer requires to embed this.
type ExtensionSettings struct {
settings
}

// NewExtensionSettings return a new ExtensionSettings with the given ID.
func NewExtensionSettings(id component.ID) ExtensionSettings {
return ExtensionSettings{settings: newSettings(id)}
// Deprecated: [v0.68.0] will be removed soon, Config no longer requires to embed this.
func NewExtensionSettings(component.ID) ExtensionSettings {
return ExtensionSettings{}
}

var _ component.Config = (*ExtensionSettings)(nil)
18 changes: 6 additions & 12 deletions config/processor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,15 @@
// limitations under the License.

package config // import "go.opentelemetry.io/collector/config"

import (
"go.opentelemetry.io/collector/component"
)

// ProcessorSettings defines common settings for a component.Processor configuration.
// Specific processors can embed this struct and extend it with more fields if needed.
//
// When embedded in the processor config it must be with `mapstructure:",squash"` tag.
type ProcessorSettings struct {
settings
}
// Deprecated: [v0.68.0] will be removed soon, Config no longer requires to embed this.
type ProcessorSettings struct{}

// NewProcessorSettings return a new ProcessorSettings with the given ComponentID.
func NewProcessorSettings(id component.ID) ProcessorSettings {
return ProcessorSettings{settings: newSettings(id)}
// Deprecated: [v0.68.0] will be removed soon, Config no longer requires to embed this.
func NewProcessorSettings(component.ID) ProcessorSettings {
return ProcessorSettings{}
}

var _ component.Config = (*ProcessorSettings)(nil)
18 changes: 6 additions & 12 deletions config/receiver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,15 @@
// limitations under the License.

package config // import "go.opentelemetry.io/collector/config"

import (
"go.opentelemetry.io/collector/component"
)

// ReceiverSettings defines common settings for a receiver.Receiver configuration.
// Specific receivers can embed this struct and extend it with more fields if needed.
//
// When embedded in the receiver config it must be with `mapstructure:",squash"` tag.
type ReceiverSettings struct {
settings
}
// Deprecated: [v0.68.0] will be removed soon, Config no longer requires to embed this.
type ReceiverSettings struct{}

// NewReceiverSettings return a new ReceiverSettings with the given ID.
func NewReceiverSettings(id component.ID) ReceiverSettings {
return ReceiverSettings{settings: newSettings(id)}
// Deprecated: [v0.68.0] will be removed soon, Config no longer requires to embed this.
func NewReceiverSettings(component.ID) ReceiverSettings {
return ReceiverSettings{}
}

var _ component.Config = (*ReceiverSettings)(nil)
40 changes: 0 additions & 40 deletions config/settings.go
View file
Open in desktop

This file was deleted.

9 changes: 4 additions & 5 deletions connector/connector_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,12 @@ import (
"github.com/stretchr/testify/assert"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config"
"go.opentelemetry.io/collector/consumer"
)

func TestNewConnectorFactory_NoOptions(t *testing.T) {
const typeStr = "test"
defaultCfg := config.NewConnectorSettings(component.NewID(typeStr))
defaultCfg := struct{}{}
factory := NewFactory(typeStr, func() component.Config { return &defaultCfg })
assert.EqualValues(t, typeStr, factory.Type())
assert.EqualValues(t, &defaultCfg, factory.CreateDefaultConfig())
Expand Down Expand Up @@ -56,7 +55,7 @@ func TestNewConnectorFactory_NoOptions(t *testing.T) {

func TestNewConnectorFactory_WithSameTypes(t *testing.T) {
const typeStr = "test"
defaultCfg := config.NewConnectorSettings(component.NewID(typeStr))
defaultCfg := struct{}{}
factory := NewFactory(typeStr, func() component.Config { return &defaultCfg },
WithTracesToTraces(createTracesToTraces, component.StabilityLevelAlpha),
WithMetricsToMetrics(createMetricsToMetrics, component.StabilityLevelBeta),
Expand Down Expand Up @@ -94,7 +93,7 @@ func TestNewConnectorFactory_WithSameTypes(t *testing.T) {

func TestNewConnectorFactory_WithTranslateTypes(t *testing.T) {
const typeStr = "test"
defaultCfg := config.NewConnectorSettings(component.NewID(typeStr))
defaultCfg := struct{}{}
factory := NewFactory(typeStr, func() component.Config { return &defaultCfg },
WithTracesToMetrics(createTracesToMetrics, component.StabilityLevelDevelopment),
WithTracesToLogs(createTracesToLogs, component.StabilityLevelAlpha),
Expand Down Expand Up @@ -139,7 +138,7 @@ func TestNewConnectorFactory_WithTranslateTypes(t *testing.T) {

func TestNewConnectorFactory_WithAllTypes(t *testing.T) {
const typeStr = "test"
defaultCfg := config.NewConnectorSettings(component.NewID(typeStr))
defaultCfg := struct{}{}
factory := NewFactory(typeStr, func() component.Config { return &defaultCfg },
WithTracesToTraces(createTracesToTraces, component.StabilityLevelAlpha),
WithTracesToMetrics(createTracesToMetrics, component.StabilityLevelDevelopment),
Expand Down
Loading