Skip to content

Commit

Permalink
Deprecate configunmarshaler package, move it to internal (open-teleme…
Browse files Browse the repository at this point in the history
…try#5151)

Folllowup:

* When the deprecated funcs/types are removed the internal package can be moved to service/internal.
* Part of open-telemetry#4936 do not offer ability to configure ConfigUnmarshaler.

Motivation:

* This package is removed because with the latest addition of the `service.ConfigProvider` the usecase to change the unmarshaled config.Config can be achieved by wrapping/implementing that interface, so no clear use-case for this. In the future we can expose it again if we have good reasons.
* During the review of another PR, this was mentioned as something some approvers/maintainers were concerned about what to do with this package. See open-telemetry#4608 (review)
Updates open-telemetry#4605

Signed-off-by: Bogdan Drutu <[email protected]>
  • Loading branch information
bogdandrutu authored and Nicholaswang committed Jun 7, 2022
1 parent a864032 commit dab051d
Show file tree
Hide file tree
Showing 51 changed files with 79 additions and 26 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

### 🚩 Deprecations 🚩

- Deprecate `configunmarshaler` package, move it to internal (#5151)
- Deprecate all API in `model/semconv`. The package is moved to a new `semcomv` module (#5196)

### 💡 Enhancements 💡
Expand Down
23 changes: 23 additions & 0 deletions config/common.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright The OpenTelemetry Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

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

func unmarshal(componentSection *Map, intoCfg interface{}) error {
if cu, ok := intoCfg.(Unmarshallable); ok {
return cu.Unmarshal(componentSection)
}

return componentSection.UnmarshalExact(intoCfg)
}
16 changes: 9 additions & 7 deletions config/configunmarshaler/unmarshaler.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,14 @@
package configunmarshaler // import "go.opentelemetry.io/collector/config/configunmarshaler"

import (
"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config"
"go.opentelemetry.io/collector/internal/configunmarshaler"
)

// ConfigUnmarshaler is the interface that unmarshalls the collector configuration from the config.Map.
type ConfigUnmarshaler interface {
// Unmarshal the configuration from the given parser and factories.
Unmarshal(v *config.Map, factories component.Factories) (*config.Config, error)
}
// Deprecated: [v0.50.0] if you need to update the config.Config implement custom (or wrap) service.ConfigProvider.
type ConfigUnmarshaler = configunmarshaler.ConfigUnmarshaler

// Deprecated: [v0.50.0] not needed since interface will be removed.
var NewDefault = configunmarshaler.NewDefault

// Deprecated: [v0.50.0] use config.UnmarshalReceiver.
var LoadReceiver = configunmarshaler.LoadReceiver
7 changes: 7 additions & 0 deletions config/exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,13 @@ type Exporter interface {
privateConfigExporter()
}

// UnmarshalExporter helper function to unmarshal an Exporter config.
// It checks if the config implements Unmarshallable and uses that if available,
// otherwise uses Map.UnmarshalExact, erroring if a field is nonexistent.
func UnmarshalExporter(cfgMap *Map, cfg Exporter) error {
return unmarshal(cfgMap, cfg)
}

// ExporterSettings defines common settings for a component.Exporter configuration.
// Specific exporters can embed this struct and extend it with more fields if needed.
//
Expand Down
7 changes: 7 additions & 0 deletions config/extension.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,13 @@ type Extension interface {
privateConfigExtension()
}

// UnmarshalExtension helper function to unmarshal an Extension config.
// It checks if the config implements Unmarshallable and uses that if available,
// otherwise uses Map.UnmarshalExact, erroring if a field is nonexistent.
func UnmarshalExtension(cfgMap *Map, cfg Extension) error {
return unmarshal(cfgMap, cfg)
}

// ExtensionSettings defines common settings for a component.Extension configuration.
// Specific processors can embed this struct and extend it with more fields if needed.
//
Expand Down
7 changes: 7 additions & 0 deletions config/processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,13 @@ type Processor interface {
privateConfigProcessor()
}

// UnmarshalProcessor helper function to unmarshal a Processor config.
// It checks if the config implements Unmarshallable and uses that if available,
// otherwise uses Map.UnmarshalExact, erroring if a field is nonexistent.
func UnmarshalProcessor(cfgMap *Map, cfg Processor) error {
return unmarshal(cfgMap, cfg)
}

// ProcessorSettings defines common settings for a component.Processor configuration.
// Specific processors can embed this struct and extend it with more fields if needed.
//
Expand Down
7 changes: 7 additions & 0 deletions config/receiver.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,13 @@ type Receiver interface {
privateConfigReceiver()
}

// UnmarshalReceiver helper function to unmarshal a Receiver config.
// It checks if the config implements Unmarshallable and uses that if available,
// otherwise uses Map.UnmarshalExact, erroring if a field is nonexistent.
func UnmarshalReceiver(cfgMap *Map, cfg Receiver) error {
return unmarshal(cfgMap, cfg)
}

// ReceiverSettings defines common settings for a component.Receiver configuration.
// Specific receivers can embed this struct and extend it with more fields if needed.
//
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

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

import (
"fmt"
Expand All @@ -25,6 +25,12 @@ import (
"go.opentelemetry.io/collector/config/configtelemetry"
)

// ConfigUnmarshaler is the interface that unmarshalls the collector configuration from the config.Map.
type ConfigUnmarshaler interface {
// Unmarshal the configuration from the given parser and factories.
Unmarshal(v *config.Map, factories component.Factories) (*config.Config, error)
}

// These are errors that can be returned by Unmarshal(). Note that error codes are not part
// of Unmarshal()'s public API, they are for internal unit testing only.
type configErrorCode int
Expand Down Expand Up @@ -154,7 +160,7 @@ func unmarshalExtensions(exts map[config.ComponentID]map[string]interface{}, fac

// Now that the default config struct is created we can Unmarshal into it,
// and it will apply user-defined config on top of the default.
if err := unmarshal(config.NewMapFromStringMap(value), extensionCfg); err != nil {
if err := config.UnmarshalExtension(config.NewMapFromStringMap(value), extensionCfg); err != nil {
return nil, errorUnmarshalError(extensionsKeyName, id, err)
}

Expand Down Expand Up @@ -183,7 +189,7 @@ func unmarshalService(srvRaw map[string]interface{}) (config.Service, error) {
},
}

if err := unmarshal(config.NewMapFromStringMap(srvRaw), &srv); err != nil {
if err := config.NewMapFromStringMap(srvRaw).UnmarshalExact(&srv); err != nil {
return srv, fmt.Errorf("error reading service configuration: %w", err)
}

Expand All @@ -210,7 +216,7 @@ func LoadReceiver(componentConfig *config.Map, id config.ComponentID, factory co

// Now that the default config struct is created we can Unmarshal into it,
// and it will apply user-defined config on top of the default.
if err := unmarshal(componentConfig, receiverCfg); err != nil {
if err := config.UnmarshalReceiver(componentConfig, receiverCfg); err != nil {
return nil, errorUnmarshalError(receiversKeyName, id, err)
}

Expand Down Expand Up @@ -259,7 +265,7 @@ func unmarshalExporters(exps map[config.ComponentID]map[string]interface{}, fact

// Now that the default config struct is created we can Unmarshal into it,
// and it will apply user-defined config on top of the default.
if err := unmarshal(config.NewMapFromStringMap(value), exporterCfg); err != nil {
if err := config.UnmarshalExporter(config.NewMapFromStringMap(value), exporterCfg); err != nil {
return nil, errorUnmarshalError(exportersKeyName, id, err)
}

Expand Down Expand Up @@ -287,7 +293,7 @@ func unmarshalProcessors(procs map[config.ComponentID]map[string]interface{}, fa

// Now that the default config struct is created we can Unmarshal into it,
// and it will apply user-defined config on top of the default.
if err := unmarshal(config.NewMapFromStringMap(value), processorCfg); err != nil {
if err := config.UnmarshalProcessor(config.NewMapFromStringMap(value), processorCfg); err != nil {
return nil, errorUnmarshalError(processorsKeyName, id, err)
}

Expand All @@ -297,14 +303,6 @@ func unmarshalProcessors(procs map[config.ComponentID]map[string]interface{}, fa
return processors, nil
}

func unmarshal(componentSection *config.Map, intoCfg interface{}) error {
if cu, ok := intoCfg.(config.Unmarshallable); ok {
return cu.Unmarshal(componentSection)
}

return componentSection.UnmarshalExact(intoCfg)
}

func errorUnknownType(component string, id config.ComponentID, factories []reflect.Value) error {
return fmt.Errorf("unknown %s type %q for %q (valid values: %v)", component, id.Type(), id, factories)
}
Expand Down
7 changes: 4 additions & 3 deletions service/config_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@ import (

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config"
"go.opentelemetry.io/collector/config/configunmarshaler"
"go.opentelemetry.io/collector/config/experimental/configsource"
"go.opentelemetry.io/collector/config/mapconverter/expandmapconverter"
"go.opentelemetry.io/collector/config/mapprovider/envmapprovider"
"go.opentelemetry.io/collector/config/mapprovider/filemapprovider"
"go.opentelemetry.io/collector/config/mapprovider/yamlmapprovider"
"go.opentelemetry.io/collector/internal/configunmarshaler"
)

// ConfigProvider provides the service configuration.
Expand Down Expand Up @@ -91,8 +91,9 @@ type ConfigProviderSettings struct {
// MapConverters is a slice of config.MapConverterFunc.
MapConverters []config.MapConverterFunc

// The configunmarshaler.ConfigUnmarshaler to be used to unmarshal the config.Map into config.Config.
// It is required to not be nil, use configunmarshaler.NewDefault() by default.
// Deprecated: [v0.50.0] because providing custom ConfigUnmarshaler is not necessary since users can wrap/implement
// ConfigProvider if needed to change the resulted config. This functionality will be kept for at least 2 minor versions,
// and if nobody express a need for it will be removed.
Unmarshaler configunmarshaler.ConfigUnmarshaler
}

Expand Down
2 changes: 1 addition & 1 deletion service/config_provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ import (
"go.opentelemetry.io/collector/component/componenttest"
"go.opentelemetry.io/collector/config"
"go.opentelemetry.io/collector/config/configtest"
"go.opentelemetry.io/collector/config/configunmarshaler"
"go.opentelemetry.io/collector/config/experimental/configsource"
"go.opentelemetry.io/collector/config/mapprovider/filemapprovider"
"go.opentelemetry.io/collector/internal/configunmarshaler"
)

type mockProvider struct {
Expand Down
2 changes: 1 addition & 1 deletion service/servicetest/configprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import (
"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config"
"go.opentelemetry.io/collector/config/configtest"
"go.opentelemetry.io/collector/config/configunmarshaler"
"go.opentelemetry.io/collector/internal/configunmarshaler"
)

// LoadConfig loads a config.Config from file, and does NOT validate the configuration.
Expand Down

0 comments on commit dab051d

Please sign in to comment.