Skip to content

Commit

Permalink
Create configload.Parser struct to abstract away from Viper (open-tel…
Browse files Browse the repository at this point in the history
…emetry#2728)

* Make ConfigUnmarshaler use `map[string]interface{}`

* Remove viper references on comments

* Encapsulate in `Loader` struct

This commit also moves the `ConfigUnmarshaler` and `CustomUnmarshaler` types
into the config package, since otherwise an import cycle would happen.

* Missing `config.NewLoader()`

* Move `Loader` to `configload` package

The `config` package indirectly depends on `exporterhelper`, so
`Loader` can't be on it.

* Empty commit to retrigger Github

For some reason my commit is not showing up on the PR

* Fix merge commit

* More merge fixes

* Address review comments

- s/instanceg/instance
- Add period at end of doc comments
- Reword NewViper comment
- Rename ViperDelimiter to KeyDelimiter

* Rename from Loader to Parser

* Add tests for `ToStringMap` and bring back old implementation

* Revert changes on component/component.go and related changes

This commit was done by running

git diff origin/main component/component.go exporter/ extension/ \
internal/testcomponents/example_exporter.go processor/ receiver/ | git apply -R

* Manual fixes after reverting component files

* s/componentViperSection/componentSection/
  • Loading branch information
mx-psi authored Mar 25, 2021
1 parent 6ead842 commit 06ea2c6
Show file tree
Hide file tree
Showing 12 changed files with 251 additions and 29 deletions.
111 changes: 111 additions & 0 deletions config/configload/configload.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
// 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 configload implements the configuration Parser.
package configload

import (
"strings"

"github.com/spf13/viper"
)

const (
// KeyDelimiter is used as the default key delimiter in the default viper instance.
KeyDelimiter = "::"
)

// NewViper creates a new Viper instance with key delimiter KeyDelimiter instead of the
// default ".". This way configs can have keys that contain ".".
func NewViper() *viper.Viper {
return viper.NewWithOptions(viper.KeyDelimiter(KeyDelimiter))
}

// NewParser creates a new Parser instance.
func NewParser() *Parser {
return &Parser{
v: NewViper(),
}
}

// FromViper creates a Parser from a Viper instance.
func FromViper(v *viper.Viper) *Parser {
return &Parser{v: v}
}

// Parser loads configuration.
type Parser struct {
v *viper.Viper
}

// Viper returns the underlying Viper instance.
func (l *Parser) Viper() *viper.Viper {
return l.v
}

// UnmarshalExact unmarshals the config into a struct, erroring if a field is nonexistent.
func (l *Parser) UnmarshalExact(intoCfg interface{}) error {
return l.v.UnmarshalExact(intoCfg)
}

// 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.
//
// In case intermediate keys do not exist, or map to a non-map value,
// a new map is created and inserted, and the search continues from there:
// the initial map "m" may be modified!
// This function comes from Viper code https://github.com/spf13/viper/blob/5253694/util.go#L201-L230
// It is used here because of https://github.com/spf13/viper/issues/819
func deepSearch(m map[string]interface{}, path []string) map[string]interface{} {
for _, k := range path {
m2, ok := m[k]
if !ok {
// intermediate key does not exist
// => create it and continue from there
m3 := make(map[string]interface{})
m[k] = m3
m = m3
continue
}
m3, ok := m2.(map[string]interface{})
if !ok {
// intermediate key is a value
// => replace with a new map
m3 = make(map[string]interface{})
m[k] = m3
}
// continue search from here
m = m3
}
return m
}

// ToStringMap creates a map[string]interface{} from a Parser.
func (l *Parser) ToStringMap() map[string]interface{} {
// This is equivalent to l.v.AllSettings() but it maps nil values
// We can't use AllSettings here because of https://github.com/spf13/viper/issues/819

m := map[string]interface{}{}
// start from the list of keys, and construct the map one value at a time
for _, k := range l.v.AllKeys() {
value := l.v.Get(k)
path := strings.Split(k, KeyDelimiter)
lastKey := strings.ToLower(path[len(path)-1])
deepestMap := deepSearch(m, path[0:len(path)-1])
// set innermost value
deepestMap[lastKey] = value
}
return m
}
88 changes: 88 additions & 0 deletions config/configload/configload_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
// 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 configload

import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestToStringMap(t *testing.T) {
tests := []struct {
name string
fileName string
stringMap map[string]interface{}
}{
{
name: "Sample Collector configuration",
fileName: "testdata/config.yaml",
stringMap: map[string]interface{}{
"receivers": map[string]interface{}{
"nop": nil,
"nop/myreceiver": nil,
},

"processors": map[string]interface{}{
"nop": nil,
"nop/myprocessor": nil,
},

"exporters": map[string]interface{}{
"nop": nil,
"nop/myexporter": nil,
},

"extensions": map[string]interface{}{
"nop": nil,
"nop/myextension": nil,
},

"service": map[string]interface{}{
"extensions": []interface{}{"nop"},
"pipelines": map[string]interface{}{
"traces": map[string]interface{}{
"receivers": []interface{}{"nop"},
"processors": []interface{}{"nop"},
"exporters": []interface{}{"nop"},
},
},
},
},
},
{
name: "Sample types",
fileName: "testdata/basic_types.yaml",
stringMap: map[string]interface{}{
"typed.options": map[string]interface{}{
"floating.point.example": 3.14,
"integer.example": 1234,
"bool.example": false,
"string.example": "this is a string",
},
},
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
v := NewViper()
v.SetConfigFile(test.fileName)
require.NoError(t, v.ReadInConfig(), "Unable to read configuration file '%s'", test.fileName)
parser := FromViper(v)
assert.Equal(t, test.stringMap, parser.ToStringMap())
})
}
}
5 changes: 5 additions & 0 deletions config/configload/testdata/basic_types.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
typed.options:
floating.point.example: 3.14
integer.example: 1234
bool.example: no
string.example: this is a string
23 changes: 23 additions & 0 deletions config/configload/testdata/config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
receivers:
nop:
nop/myreceiver:

processors:
nop:
nop/myprocessor:

exporters:
nop:
nop/myexporter:

extensions:
nop:
nop/myextension:

service:
extensions: [nop]
pipelines:
traces:
receivers: [nop]
processors: [nop]
exporters: [nop]
18 changes: 4 additions & 14 deletions config/configparser/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"github.com/spf13/viper"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config/configload"
"go.opentelemetry.io/collector/config/configmodels"
)

Expand All @@ -44,11 +45,6 @@ const (
errUnmarshalTopLevelStructureError
)

const (
// ViperDelimiter is used as the default key delimiter in the default viper instance
ViperDelimiter = "::"
)

type configError struct {
msg string // human readable error message.
code configErrorCode // internal error code.
Expand Down Expand Up @@ -98,12 +94,6 @@ type pipelineSettings struct {
// typeAndNameSeparator is the separator that is used between type and name in type/name composite keys.
const typeAndNameSeparator = "/"

// Creates a new Viper instance with a different key-delimitor "::" instead of the
// default ".". This way configs can have keys that contain ".".
func NewViper() *viper.Viper {
return viper.NewWithOptions(viper.KeyDelimiter(ViperDelimiter))
}

// Load loads a Config from Viper.
// After loading the config, need to check if it is valid by calling `ValidateConfig`.
func Load(
Expand Down Expand Up @@ -566,11 +556,11 @@ func defaultUnmarshaler(componentViperSection *viper.Viper, intoCfg interface{})
func ViperSubExact(v *viper.Viper, key string) (*viper.Viper, error) {
data := v.Get(key)
if data == nil {
return NewViper(), nil
return configload.NewViper(), nil
}

if reflect.TypeOf(data).Kind() == reflect.Map {
subv := NewViper()
subv := configload.NewViper()
// Cannot return error because the subv is empty.
_ = subv.MergeConfigMap(cast.ToStringMap(data))
return subv, nil
Expand All @@ -582,7 +572,7 @@ func ViperSubExact(v *viper.Viper, key string) (*viper.Viper, error) {
}

func viperFromStringMap(data map[string]interface{}) *viper.Viper {
v := NewViper()
v := configload.NewViper()
// Cannot return error because the subv is empty.
_ = v.MergeConfigMap(cast.ToStringMap(data))
return v
Expand Down
3 changes: 2 additions & 1 deletion config/configparser/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"github.com/stretchr/testify/require"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config/configload"
"go.opentelemetry.io/collector/config/configmodels"
"go.opentelemetry.io/collector/config/confignet"
"go.opentelemetry.io/collector/internal/testcomponents"
Expand Down Expand Up @@ -495,7 +496,7 @@ func TestLoadEmptyAllSections(t *testing.T) {

func loadConfigFile(t *testing.T, fileName string, factories component.Factories) (*configmodels.Config, error) {
// Read yaml config from file
v := NewViper()
v := configload.NewViper()
v.SetConfigFile(fileName)
require.NoErrorf(t, v.ReadInConfig(), "unable to read the file %v", fileName)

Expand Down
3 changes: 2 additions & 1 deletion config/configtest/configtest.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"github.com/stretchr/testify/require"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config/configload"
"go.opentelemetry.io/collector/config/configmodels"
"go.opentelemetry.io/collector/config/configparser"
)
Expand All @@ -30,7 +31,7 @@ import (
// Example usage for testing can be found in configtest_test.go
func NewViperFromYamlFile(t *testing.T, fileName string) *viper.Viper {
// Read yaml config from file
v := configparser.NewViper()
v := configload.NewViper()
v.SetConfigFile(fileName)
require.NoErrorf(t, v.ReadInConfig(), "unable to read the file %v", fileName)

Expand Down
6 changes: 3 additions & 3 deletions receiver/jaegerreceiver/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ import (
"go.opentelemetry.io/collector/config/configerror"
"go.opentelemetry.io/collector/config/configgrpc"
"go.opentelemetry.io/collector/config/confighttp"
"go.opentelemetry.io/collector/config/configload"
"go.opentelemetry.io/collector/config/confignet"
"go.opentelemetry.io/collector/config/configparser"
"go.opentelemetry.io/collector/config/configtest"
"go.opentelemetry.io/collector/config/configtls"
)
Expand Down Expand Up @@ -344,9 +344,9 @@ func TestCustomUnmarshalErrors(t *testing.T) {
fu, ok := factory.(component.ConfigUnmarshaler)
assert.True(t, ok)

err := fu.Unmarshal(configparser.NewViper(), nil)
err := fu.Unmarshal(configload.NewViper(), nil)
assert.Error(t, err, "should not have been able to marshal to a nil config")

err = fu.Unmarshal(configparser.NewViper(), &RemoteSamplingConfig{})
err = fu.Unmarshal(configload.NewViper(), &RemoteSamplingConfig{})
assert.Error(t, err, "should not have been able to marshal to a non-jaegerreceiver config")
}
3 changes: 2 additions & 1 deletion service/application.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config/configcheck"
"go.opentelemetry.io/collector/config/configload"
"go.opentelemetry.io/collector/config/configmodels"
"go.opentelemetry.io/collector/config/configparser"
"go.opentelemetry.io/collector/config/configtelemetry"
Expand Down Expand Up @@ -236,7 +237,7 @@ func (app *Application) setupConfigurationComponents(ctx context.Context, factor

app.logger.Info("Loading configuration...")

cfg, err := factory(configparser.NewViper(), app.rootCmd, app.factories)
cfg, err := factory(configload.NewViper(), app.rootCmd, app.factories)
if err != nil {
return fmt.Errorf("cannot load configuration: %w", err)
}
Expand Down
Loading

0 comments on commit 06ea2c6

Please sign in to comment.