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.

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

Change CustomUnmarshaler implementations to use config.Parser, prove the interface #2810

Merged
merged 1 commit into from
Mar 26, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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 💡
- `batch` processor: - Support max batch size for logs (#2736)
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)) {
bogdandrutu marked this conversation as resolved.
Show resolved Hide resolved
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