Skip to content

Commit

Permalink
Change CustomUnmarshaler implementations to use config.Parser, prove …
Browse files Browse the repository at this point in the history
…the interface (#2810)

Signed-off-by: Bogdan Drutu <[email protected]>
  • Loading branch information
bogdandrutu authored Mar 26, 2021
1 parent 3f83ad0 commit acb3ae1
Show file tree
Hide file tree
Showing 8 changed files with 51 additions and 37 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
- Rename `NewComponent()` to `New()`
- obsReport.NewExporter accepts a settings struct (#2668)
- Remove ErrorWaitingHost from `componenttest` (#2582)
- Move `config.Load` to use `configparser.Load` (#2796)
- Remove `configtest.NewViperFromYamlFile()`, use `config.Parser.NewParserFromFile()` (#2806)
- Move `config.ViperSubExact()` to use `config.Parser.Sub()` (#2806)

## 💡 Enhancements 💡

Expand Down
30 changes: 12 additions & 18 deletions config/configparser/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
"strings"

"github.com/spf13/cast"
"github.com/spf13/viper"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config"
Expand Down Expand Up @@ -225,7 +224,7 @@ func loadExtensions(exts map[string]interface{}, factories map[configmodels.Type

// Iterate over extensions and create a config for each.
for key, value := range exts {
componentConfig := viperFromStringMap(cast.ToStringMap(value))
componentConfig := config.NewParserFromStringMap(cast.ToStringMap(value))
expandEnvConfig(componentConfig)

// Decode the key into type and fullName components.
Expand Down Expand Up @@ -275,7 +274,7 @@ func loadService(rawService serviceSettings) (configmodels.Service, error) {
}

// LoadReceiver loads a receiver config from componentConfig using the provided factories.
func LoadReceiver(componentConfig *viper.Viper, typeStr configmodels.Type, fullName string, factory component.ReceiverFactory) (configmodels.Receiver, error) {
func LoadReceiver(componentConfig *config.Parser, typeStr configmodels.Type, fullName string, factory component.ReceiverFactory) (configmodels.Receiver, error) {
// Create the default config for this receiver.
receiverCfg := factory.CreateDefaultConfig()
receiverCfg.SetName(fullName)
Expand All @@ -297,7 +296,7 @@ func loadReceivers(recvs map[string]interface{}, factories map[configmodels.Type

// Iterate over input map and create a config for each.
for key, value := range recvs {
componentConfig := viperFromStringMap(cast.ToStringMap(value))
componentConfig := config.NewParserFromStringMap(cast.ToStringMap(value))
expandEnvConfig(componentConfig)

// Decode the key into type and fullName components.
Expand Down Expand Up @@ -334,7 +333,7 @@ func loadExporters(exps map[string]interface{}, factories map[configmodels.Type]

// Iterate over Exporters and create a config for each.
for key, value := range exps {
componentConfig := viperFromStringMap(cast.ToStringMap(value))
componentConfig := config.NewParserFromStringMap(cast.ToStringMap(value))
expandEnvConfig(componentConfig)

// Decode the key into type and fullName components.
Expand Down Expand Up @@ -377,7 +376,7 @@ func loadProcessors(procs map[string]interface{}, factories map[configmodels.Typ

// Iterate over processors and create a config for each.
for key, value := range procs {
componentConfig := viperFromStringMap(cast.ToStringMap(value))
componentConfig := config.NewParserFromStringMap(cast.ToStringMap(value))
expandEnvConfig(componentConfig)

// Decode the key into type and fullName components.
Expand Down Expand Up @@ -456,7 +455,7 @@ func loadPipelines(pipelinesConfig map[string]pipelineSettings) (configmodels.Pi

// expandEnvConfig creates a new viper config with expanded values for all the values (simple, list or map value).
// It does not expand the keys.
func expandEnvConfig(v *viper.Viper) {
func expandEnvConfig(v *config.Parser) {
for _, k := range v.AllKeys() {
v.Set(k, expandStringValues(v.Get(k)))
}
Expand Down Expand Up @@ -535,20 +534,15 @@ func expandEnv(s string) string {
})
}

func unmarshaler(factory component.Factory) component.CustomUnmarshaler {
func unmarshaler(factory component.Factory) func(componentViperSection *config.Parser, intoCfg interface{}) error {
if fu, ok := factory.(component.ConfigUnmarshaler); ok {
return fu.Unmarshal
return func(componentParser *config.Parser, intoCfg interface{}) error {
return fu.Unmarshal(componentParser.Viper(), intoCfg)
}
}
return defaultUnmarshaler
}

func defaultUnmarshaler(componentViperSection *viper.Viper, intoCfg interface{}) error {
return componentViperSection.UnmarshalExact(intoCfg)
}

func viperFromStringMap(data map[string]interface{}) *viper.Viper {
v := config.NewViper()
// Cannot return error because the subv is empty.
_ = v.MergeConfigMap(cast.ToStringMap(data))
return v
func defaultUnmarshaler(componentParser *config.Parser, intoCfg interface{}) error {
return componentParser.UnmarshalExact(intoCfg)
}
12 changes: 12 additions & 0 deletions config/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,13 @@ func ParserFromViper(v *viper.Viper) *Parser {
return &Parser{v: v}
}

func NewParserFromStringMap(data map[string]interface{}) *Parser {
v := NewViper()
// Cannot return error because the subv is empty.
_ = v.MergeConfigMap(data)
return ParserFromViper(v)
}

// Parser loads configuration.
type Parser struct {
v *viper.Viper
Expand Down Expand Up @@ -111,6 +118,11 @@ func (l *Parser) Sub(key string) (*Parser, error) {
return nil, fmt.Errorf("unexpected sub-config value kind for key:%s value:%v kind:%v)", key, data, reflect.TypeOf(data).Kind())
}

// Viper returns the viper.Viper implementation of this Parser.
func (l *Parser) Viper() *viper.Viper {
return l.v
}

// deepSearch scans deep maps, following the key indexes listed in the
// sequence "path".
// The last value is expected to be another map, and is returned.
Expand Down
11 changes: 5 additions & 6 deletions receiver/hostmetricsreceiver/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"errors"
"fmt"

"github.com/spf13/cast"
"github.com/spf13/viper"
"go.uber.org/zap"

Expand Down Expand Up @@ -76,11 +77,9 @@ func NewFactory() component.ReceiverFactory {

// customUnmarshaler returns custom unmarshaler for this config.
func customUnmarshaler(componentViperSection *viper.Viper, intoCfg interface{}) error {

componentParser := config.ParserFromViper(componentViperSection)
// load the non-dynamic config normally
cp := config.ParserFromViper(componentViperSection)

err := cp.Unmarshal(intoCfg)
err := componentParser.Unmarshal(intoCfg)
if err != nil {
return err
}
Expand All @@ -94,15 +93,15 @@ func customUnmarshaler(componentViperSection *viper.Viper, intoCfg interface{})

cfg.Scrapers = map[string]internal.Config{}

scrapersSection, err := cp.Sub(scrapersKey)
scrapersSection, err := componentParser.Sub(scrapersKey)
if err != nil {
return err
}
if len(scrapersSection.AllKeys()) == 0 {
return errors.New("must specify at least one scraper when using hostmetrics receiver")
}

for key := range componentViperSection.GetStringMap(scrapersKey) {
for key := range cast.ToStringMap(componentParser.Get(scrapersKey)) {
factory, ok := getScraperFactory(key)
if !ok {
return fmt.Errorf("invalid scraper key: %s", key)
Expand Down
11 changes: 6 additions & 5 deletions receiver/jaegerreceiver/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,11 @@ import (
"net"
"strconv"

"github.com/spf13/cast"
"github.com/spf13/viper"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config"
"go.opentelemetry.io/collector/config/configgrpc"
"go.opentelemetry.io/collector/config/confighttp"
"go.opentelemetry.io/collector/config/configmodels"
Expand Down Expand Up @@ -61,22 +63,21 @@ func NewFactory() component.ReceiverFactory {

// customUnmarshaler is used to add defaults for named but empty protocols
func customUnmarshaler(componentViperSection *viper.Viper, intoCfg interface{}) error {
if componentViperSection == nil || len(componentViperSection.AllKeys()) == 0 {
componentParser := config.ParserFromViper(componentViperSection)
if componentParser == nil || len(componentParser.AllKeys()) == 0 {
return fmt.Errorf("empty config for Jaeger receiver")
}

componentViperSection.SetConfigType("yaml")

// UnmarshalExact will not set struct properties to nil even if no key is provided,
// so set the protocol structs to nil where the keys were omitted.
err := componentViperSection.UnmarshalExact(intoCfg)
err := componentParser.UnmarshalExact(intoCfg)
if err != nil {
return err
}

receiverCfg := intoCfg.(*Config)

protocols := componentViperSection.GetStringMap(protocolsFieldName)
protocols := cast.ToStringMap(componentParser.Get(protocolsFieldName))
if len(protocols) == 0 {
return fmt.Errorf("must specify at least one protocol when using the Jaeger receiver")
}
Expand Down
9 changes: 6 additions & 3 deletions receiver/otlpreceiver/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,12 @@ import (
"context"
"fmt"

"github.com/spf13/cast"
"github.com/spf13/viper"
"go.uber.org/zap"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config"
"go.opentelemetry.io/collector/config/configgrpc"
"go.opentelemetry.io/collector/config/confighttp"
"go.opentelemetry.io/collector/config/configmodels"
Expand Down Expand Up @@ -79,18 +81,19 @@ func createDefaultConfig() configmodels.Receiver {

// customUnmarshaler is used to add defaults for named but empty protocols
func customUnmarshaler(componentViperSection *viper.Viper, intoCfg interface{}) error {
if componentViperSection == nil || len(componentViperSection.AllKeys()) == 0 {
componentParser := config.ParserFromViper(componentViperSection)
if componentParser == nil || len(componentParser.AllKeys()) == 0 {
return fmt.Errorf("empty config for OTLP receiver")
}
// first load the config normally
err := componentViperSection.UnmarshalExact(intoCfg)
err := componentParser.UnmarshalExact(intoCfg)
if err != nil {
return err
}

receiverCfg := intoCfg.(*Config)
// next manually search for protocols in viper, if a protocol is not present it means it is disable.
protocols := componentViperSection.GetStringMap(protocolsFieldName)
protocols := cast.ToStringMap(componentParser.Get(protocolsFieldName))

// UnmarshalExact will ignore empty entries like a protocol with no values, so if a typo happened
// in the protocol that is intended to be enabled will not be enabled. So check if the protocols
Expand Down
1 change: 0 additions & 1 deletion receiver/prometheusreceiver/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ func TestLoadConfig(t *testing.T) {
factory := NewFactory()
factories.Receivers[typeStr] = factory
cfg, err := configtest.LoadConfigFile(t, path.Join(".", "testdata", "config.yaml"), factories)

require.NoError(t, err)
require.NotNil(t, cfg)

Expand Down
11 changes: 7 additions & 4 deletions receiver/prometheusreceiver/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,12 @@ import (
"fmt"

_ "github.com/prometheus/prometheus/discovery/install" // init() of this package registers service discovery impl.
"github.com/spf13/cast"
"github.com/spf13/viper"
"gopkg.in/yaml.v2"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config"
"go.opentelemetry.io/collector/config/configmodels"
"go.opentelemetry.io/collector/consumer"
"go.opentelemetry.io/collector/receiver/receiverhelper"
Expand Down Expand Up @@ -52,22 +54,23 @@ func NewFactory() component.ReceiverFactory {
}

func customUnmarshaler(componentViperSection *viper.Viper, intoCfg interface{}) error {
if componentViperSection == nil {
componentParser := config.ParserFromViper(componentViperSection)
if componentParser == nil {
return nil
}
// We need custom unmarshaling because prometheus "config" subkey defines its own
// YAML unmarshaling routines so we need to do it explicitly.

err := componentViperSection.UnmarshalExact(intoCfg)
err := componentParser.UnmarshalExact(intoCfg)
if err != nil {
return fmt.Errorf("prometheus receiver failed to parse config: %s", err)
}

// Unmarshal prometheus's config values. Since prometheus uses `yaml` tags, so use `yaml`.
if !componentViperSection.IsSet(prometheusConfigKey) {
promCfgMap := cast.ToStringMap(componentParser.Get(prometheusConfigKey))
if len(promCfgMap) == 0 {
return nil
}
promCfgMap := componentViperSection.Sub(prometheusConfigKey).AllSettings()
out, err := yaml.Marshal(promCfgMap)
if err != nil {
return fmt.Errorf("prometheus receiver failed to marshal config to yaml: %s", err)
Expand Down

0 comments on commit acb3ae1

Please sign in to comment.