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

Config sources to support full remote config and value substitution #4468

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
b7c405d
Add ConfigSource component type
tigrannajaryan Nov 19, 2021
9c893c0
Add ValueSource
tigrannajaryan Nov 19, 2021
c3fc435
Unmarshal value sources and use
tigrannajaryan Nov 19, 2021
d4a178e
Rearrange root config structure
tigrannajaryan Nov 22, 2021
6363a1c
Improve error message when config source is not ValueProvider
tigrannajaryan Nov 22, 2021
8e2b979
Fix tests
tigrannajaryan Nov 22, 2021
30b13dd
Rename to valueSubstitutor
tigrannajaryan Nov 22, 2021
9f91092
Improve interface names
tigrannajaryan Nov 22, 2021
b6cb36e
Delete unused fields
tigrannajaryan Nov 22, 2021
7959de4
Rename default to local (config map provider)
tigrannajaryan Nov 22, 2021
521b875
Delete unnecessary changes
tigrannajaryan Nov 22, 2021
ef946dc
Add comments
tigrannajaryan Nov 22, 2021
05047ee
Move ConfigSourceSettings to defaultunmarshaler
tigrannajaryan Nov 22, 2021
aca583e
Add support for command line config sources
tigrannajaryan Nov 22, 2021
87415b0
Add http config source
tigrannajaryan Nov 22, 2021
f05015f
Minor refactoring
tigrannajaryan Nov 22, 2021
3d7fb51
Make Retrive and RetriveValue different to allow implementing both
tigrannajaryan Nov 22, 2021
3fb2e33
Allow using value config source in merge_configs section
tigrannajaryan Nov 22, 2021
011c1d4
Add value config support to "file" source
tigrannajaryan Nov 22, 2021
86b69e4
Delete unneeded Manager and Expand provider
tigrannajaryan Nov 22, 2021
1788c26
Config sources to support full remote config and value substitution
tigrannajaryan Nov 22, 2021
e8aeed2
Improve ValueProvider interface comments.
tigrannajaryan Nov 23, 2021
50915c4
Fix linting
tigrannajaryan Nov 23, 2021
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
37 changes: 37 additions & 0 deletions component/configsource.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// 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 component // import "go.opentelemetry.io/collector/component"

import (
"context"

"go.opentelemetry.io/collector/config"
"go.opentelemetry.io/collector/config/configmapprovider"
)

// ConfigSource is a component that provides a configuration map or value.
type ConfigSource interface {
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I saw it. It will be all deleted since it is no longer needed. I deleted some parts of it already in this PR (the Manager).

Choose a reason for hiding this comment

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

Is this feature can be achieved through helm chart

configmapprovider.Provider
Copy link
Member

Choose a reason for hiding this comment

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

Why do you need a ConfigSource and a Provider if they are the same? This is what I was suggesting #3924. I think we should have only one concept.

}

// ConfigSourceCreateSettings is passed to CreateConfigSource functions.
type ConfigSourceCreateSettings struct {
}

type ConfigSourceFactory interface {
Factory
CreateDefaultConfig() config.ConfigSource
CreateConfigSource(ctx context.Context, set ConfigSourceCreateSettings, cfg config.ConfigSource) (configmapprovider.Provider, error)
}
3 changes: 3 additions & 0 deletions component/factories.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ type Factories struct {

// Extensions maps extension type names in the config to the respective factory.
Extensions map[config.Type]ExtensionFactory

// ConfigSources maps config source type names in the config to the respective factory.
ConfigSources map[config.Type]ConfigSourceFactory
}

// MakeReceiverFactoryMap takes a list of receiver factories and returns a map
Expand Down
92 changes: 0 additions & 92 deletions config/configmapprovider/expand.go

This file was deleted.

122 changes: 0 additions & 122 deletions config/configmapprovider/expand_test.go

This file was deleted.

27 changes: 21 additions & 6 deletions config/configmapprovider/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,18 @@ import (
"go.opentelemetry.io/collector/config"
)

type fileMapProvider struct {
type FileMapProvider struct {
Copy link
Member

Choose a reason for hiding this comment

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

This provides both so probably FileProvider is better name

fileName string
}

// NewFile returns a new Provider that reads the configuration from the given file.
func NewFile(fileName string) Provider {
return &fileMapProvider{
// NewFile returns a new MapProvider that reads the configuration from the given file.
func NewFile(fileName string) *FileMapProvider {
return &FileMapProvider{
fileName: fileName,
}
}

func (fmp *fileMapProvider) Retrieve(_ context.Context, _ func(*ChangeEvent)) (Retrieved, error) {
func (fmp *FileMapProvider) Retrieve(_ context.Context, _ func(*ChangeEvent)) (RetrievedMap, error) {
Copy link
Member

Choose a reason for hiding this comment

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

Why do you need this if the other call can retrieve a map? That was my whole concern that we don't need 2 concepts we just need to find the "middle ground" between the full flexibility ConfigSource have and restricted scope of MapProvider. The reality is that the "ConfigSources" for example do not use params except for the "template" case for example, so we need to answer is that a reasonable use-case so we need to support params?

if fmp.fileName == "" {
return nil, errors.New("config file not specified")
}
Expand All @@ -46,6 +46,21 @@ func (fmp *fileMapProvider) Retrieve(_ context.Context, _ func(*ChangeEvent)) (R
return &simpleRetrieved{confMap: cp}, nil
}

func (*fileMapProvider) Shutdown(context.Context) error {
func (fmp *FileMapProvider) RetrieveValue(_ context.Context, _ func(*ChangeEvent), selector string, paramsConfigMap *config.Map) (RetrievedValue, error) {
if selector == "" {
return nil, errors.New("config file not specified")
}

// TODO: support multiple file paths in selector (use some sort of delimiter).

cp, err := config.NewMapFromFile(selector)
if err != nil {
return nil, fmt.Errorf("error loading config file %q: %w", fmp.fileName, err)
}

return &simpleRetrievedValue{value: cp}, nil
}

func (*FileMapProvider) Shutdown(context.Context) error {
return nil
}
4 changes: 2 additions & 2 deletions config/configmapprovider/inmemory.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,11 @@ type inMemoryMapProvider struct {
}

// NewInMemory returns a new Provider that reads the configuration, from the provided buffer, as YAML.
func NewInMemory(buf io.Reader) Provider {
func NewInMemory(buf io.Reader) MapProvider {
return &inMemoryMapProvider{buf: buf}
}

func (inp *inMemoryMapProvider) Retrieve(_ context.Context, onChange func(*ChangeEvent)) (Retrieved, error) {
func (inp *inMemoryMapProvider) Retrieve(_ context.Context, onChange func(*ChangeEvent)) (RetrievedMap, error) {
cfg, err := config.NewMapFromBuffer(inp.buf)
if err != nil {
return nil, err
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,10 @@

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

// NewDefault returns the default Provider, and it creates configuration from a file
// NewLocal returns a local MapProvider, and it creates configuration from a file
// defined by the given configFile and overwrites fields using properties.
func NewDefault(configFileName string, properties []string) Provider {
return NewExpand(
NewMerge(
NewFile(configFileName),
NewProperties(properties)))
func NewLocal(configFileName string, properties []string) MapProvider {
return NewMerge(
NewFile(configFileName),
NewProperties(properties))
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ import (
"go.opentelemetry.io/collector/config"
)

func TestDefaultMapProvider(t *testing.T) {
mp := NewDefault("testdata/default-config.yaml", nil)
func TestLocalMapProvider(t *testing.T) {
mp := NewLocal("testdata/default-config.yaml", nil)
retr, err := mp.Retrieve(context.Background(), nil)
require.NoError(t, err)

Expand All @@ -44,8 +44,8 @@ exporters:
assert.NoError(t, mp.Shutdown(context.Background()))
}

func TestDefaultMapProvider_AddNewConfig(t *testing.T) {
mp := NewDefault("testdata/default-config.yaml", []string{"processors.batch.timeout=2s"})
func TestLocalMapProvider_AddNewConfig(t *testing.T) {
mp := NewLocal("testdata/default-config.yaml", []string{"processors.batch.timeout=2s"})
cp, err := mp.Retrieve(context.Background(), nil)
require.NoError(t, err)

Expand All @@ -64,8 +64,8 @@ exporters:
assert.NoError(t, mp.Shutdown(context.Background()))
}

func TestDefaultMapProvider_OverwriteConfig(t *testing.T) {
mp := NewDefault(
func TestLocalMapProvider_OverwriteConfig(t *testing.T) {
mp := NewLocal(
"testdata/default-config.yaml",
[]string{"processors.batch.timeout=2s", "exporters.otlp.endpoint=localhost:1234"})
cp, err := mp.Retrieve(context.Background(), nil)
Expand All @@ -86,17 +86,17 @@ exporters:
assert.NoError(t, mp.Shutdown(context.Background()))
}

func TestDefaultMapProvider_InexistentFile(t *testing.T) {
mp := NewDefault("testdata/otelcol-config.yaml", nil)
func TestLocalMapProvider_InexistentFile(t *testing.T) {
mp := NewLocal("testdata/otelcol-config.yaml", nil)
require.NotNil(t, mp)
_, err := mp.Retrieve(context.Background(), nil)
require.Error(t, err)

assert.NoError(t, mp.Shutdown(context.Background()))
}

func TestDefaultMapProvider_EmptyFileName(t *testing.T) {
mp := NewDefault("", nil)
func TestLocalMapProvider_EmptyFileName(t *testing.T) {
mp := NewLocal("", nil)
_, err := mp.Retrieve(context.Background(), nil)
require.Error(t, err)

Expand Down
Loading