Skip to content

Commit

Permalink
Move MapProvider to config, split providers per own package
Browse files Browse the repository at this point in the history
This PR:
1. moves the `configmapprovider.Provider` to `config.MapProvider` and related structs.
2. every provider (env, file, yaml) are split in their own packages to help open-telemetry#4759.

Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
  • Loading branch information
bogdandrutu committed Mar 17, 2022
1 parent f3fd237 commit 0e06154
Show file tree
Hide file tree
Showing 17 changed files with 224 additions and 166 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

### 🚩 Deprecations 🚩

- Move MapProvider to config, split providers in their own package (#5030)
- API related to `pdata.AttributeValue` is deprecated in favor of `pdata.Value` (#4975)
- `pdata.AttributeValue` struct is deprecated in favor of `pdata.Value`
- `pdata.AttributeValueType` type is deprecated in favor of `pdata.ValueType`
Expand Down
3 changes: 2 additions & 1 deletion config/configmapprovider/expand_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/config"
"go.opentelemetry.io/collector/config/filemapprovider"
)

func TestNewExpandConverter(t *testing.T) {
Expand Down Expand Up @@ -116,7 +117,7 @@ func TestNewExpandConverter_EscapedEnvVars(t *testing.T) {
}

func loadConfigMap(fileName string) (*config.Map, error) {
ret, err := NewFile().Retrieve(context.Background(), "file:"+fileName, nil)
ret, err := filemapprovider.New().Retrieve(context.Background(), "file:"+fileName, nil)
if err != nil {
return nil, err
}
Expand Down
93 changes: 20 additions & 73 deletions config/configmapprovider/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,85 +15,32 @@
package configmapprovider // import "go.opentelemetry.io/collector/config/configmapprovider"

import (
"context"

"go.opentelemetry.io/collector/config"
"go.opentelemetry.io/collector/config/envmapprovider"
"go.opentelemetry.io/collector/config/filemapprovider"
"go.opentelemetry.io/collector/config/yamlmapprovider"
)

// Provider is an interface that helps to retrieve a config map and watch for any
// changes to the config map. Implementations may load the config from a file,
// a database or any other source.
//
// The typical usage is the following:
//
// r, err := mapProvider.Retrieve("file:/path/to/config")
// // Use r.Map; wait for onChange() to be called.
// r.Close()
// r, err = mapProvider.Retrieve("file:/path/to/config")
// // Use r.Map; wait for onChange() to be called.
// r.Close()
// // repeat retrieve/wait/close cycle until it is time to shut down the Collector process.
// // ...
// mapProvider.Shutdown()
type Provider interface {
// Retrieve goes to the configuration source and retrieves the selected data which
// contains the value to be injected in the configuration and the corresponding watcher that
// will be used to monitor for updates of the retrieved value.
//
// `location` must follow the "<scheme>:<opaque_data>" format. This format is compatible
// with the URI definition (see https://datatracker.ietf.org/doc/html/rfc3986). The "<scheme>"
// must be always included in the `location`. The scheme supported by any provider MUST be at
// least 2 characters long to avoid conflicting with a driver-letter identifier as specified
// in https://tools.ietf.org/id/draft-kerwin-file-scheme-07.html#syntax.
//
// `watcher` callback is called when the config changes. watcher may be called from
// a different go routine. After watcher is called Retrieved.Get should be called
// to get the new config. See description of Retrieved for more details.
// watcher may be nil, which indicates that the caller is not interested in
// knowing about the changes.
//
// If ctx is cancelled should return immediately with an error.
// Should never be called concurrently with itself or with Shutdown.
Retrieve(ctx context.Context, location string, watcher WatcherFunc) (Retrieved, error)
// Deprecated: [v0.48.0] use envmapprovider.New
var NewFile = envmapprovider.New

// Shutdown signals that the configuration for which this Provider was used to
// retrieve values is no longer in use and the Provider should close and release
// any resources that it may have created.
//
// This method must be called when the Collector service ends, either in case of
// success or error. Retrieve cannot be called after Shutdown.
//
// Should never be called concurrently with itself or with Retrieve.
// If ctx is cancelled should return immediately with an error.
Shutdown(ctx context.Context) error
}
// Deprecated: [v0.48.0] use filemapprovider.New
var NewEnv = filemapprovider.New

type WatcherFunc func(*ChangeEvent)
// Deprecated: [v0.48.0] use yamlmapprovider.New
var NewYAML = yamlmapprovider.New

// ChangeEvent describes the particular change event that happened with the config.
// TODO: see if this can be eliminated.
type ChangeEvent struct {
// Error is nil if the config is changed and needs to be re-fetched.
// Any non-nil error indicates that there was a problem with watching the config changes.
Error error
}
// Deprecated: [v0.48.0] use config.MapProvider
type Provider = config.MapProvider

// Retrieved holds the result of a call to the Retrieve method of a Provider object.
type Retrieved struct {
Map *config.Map
// Deprecated: [v0.48.0] use config.WatcherFunc
type WatcherFunc = config.WatcherFunc

// CloseFunc specifies a function to be invoked when the configuration for which it was
// used to retrieve values is no longer in use and should close and release any watchers
// that it may have created.
//
// If nil, then nothing to be closed.
CloseFunc
}
// Deprecated: [v0.48.0] use config.ChangeEvent
type ChangeEvent = config.ChangeEvent

// CloseFunc a function to close and release any watchers that it may have created.
//
// Should block until all resources are closed, and guarantee that `onChange` is not
// going to be called after it returns except when `ctx` is cancelled.
//
// Should never be called concurrently with itself.
type CloseFunc func(context.Context) error
// Deprecated: [v0.48.0] use config.Retrieved
type Retrieved = config.Retrieved

// Deprecated: [v0.48.0] use config.CloseFunc
type CloseFunc = config.CloseFunc
4 changes: 2 additions & 2 deletions config/configtest/configtest.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,15 @@ import (
"go.uber.org/multierr"

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

// The regular expression for valid config field tag.
var configFieldTagRegExp = regexp.MustCompile("^[a-z0-9][a-z0-9_]*$")

// LoadConfigMap loads a config.Map from file, and does NOT validate the configuration.
func LoadConfigMap(fileName string) (*config.Map, error) {
ret, err := configmapprovider.NewFile().Retrieve(context.Background(), "file:"+fileName, nil)
ret, err := filemapprovider.New().Retrieve(context.Background(), "file:"+fileName, nil)
return ret.Map, err
}

Expand Down
14 changes: 7 additions & 7 deletions config/configmapprovider/env.go → config/envmapprovider/env.go
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 configmapprovider // import "go.opentelemetry.io/collector/config/configmapprovider"
package envmapprovider // import "go.opentelemetry.io/collector/config/envmapprovider"

import (
"context"
Expand All @@ -29,26 +29,26 @@ const envSchemeName = "env"

type envMapProvider struct{}

// NewEnv returns a new Provider that reads the configuration from the given environment variable.
// New returns a new config.MapProvider that reads the configuration from the given environment variable.
//
// This Provider supports "env" scheme, and can be called with a selector:
// `env:NAME_OF_ENVIRONMENT_VARIABLE`
func NewEnv() Provider {
func New() config.MapProvider {
return &envMapProvider{}
}

func (emp *envMapProvider) Retrieve(_ context.Context, location string, _ WatcherFunc) (Retrieved, error) {
func (emp *envMapProvider) Retrieve(_ context.Context, location string, _ config.WatcherFunc) (config.Retrieved, error) {
if !strings.HasPrefix(location, envSchemeName+":") {
return Retrieved{}, fmt.Errorf("%v location is not supported by %v provider", location, envSchemeName)
return config.Retrieved{}, fmt.Errorf("%v location is not supported by %v provider", location, envSchemeName)
}

content := os.Getenv(location[len(envSchemeName)+1:])
var data map[string]interface{}
if err := yaml.Unmarshal([]byte(content), &data); err != nil {
return Retrieved{}, fmt.Errorf("unable to parse yaml: %w", err)
return config.Retrieved{}, fmt.Errorf("unable to parse yaml: %w", err)
}

return Retrieved{Map: config.NewMapFromStringMap(data)}, nil
return config.Retrieved{Map: config.NewMapFromStringMap(data)}, nil
}

func (*envMapProvider) Shutdown(context.Context) error {
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 configmapprovider
package envmapprovider

import (
"context"
Expand All @@ -28,26 +28,26 @@ import (

const envSchemePrefix = envSchemeName + ":"

func TestEnv_EmptyName(t *testing.T) {
env := NewEnv()
func TestEmptyName(t *testing.T) {
env := New()
_, err := env.Retrieve(context.Background(), "", nil)
require.Error(t, err)
assert.NoError(t, env.Shutdown(context.Background()))
}

func TestEnv_UnsupportedScheme(t *testing.T) {
env := NewEnv()
_, err := env.Retrieve(context.Background(), "http://", nil)
func TestUnsupportedScheme(t *testing.T) {
env := New()
_, err := env.Retrieve(context.Background(), "https://", nil)
assert.Error(t, err)
assert.NoError(t, env.Shutdown(context.Background()))
}

func TestEnv_InvalidYaml(t *testing.T) {
func TestInvalidYaml(t *testing.T) {
bytes, err := os.ReadFile(filepath.Join("testdata", "invalid-yaml.yaml"))
require.NoError(t, err)
const envName = "invalid-yaml"
t.Setenv(envName, string(bytes))
env := NewEnv()
env := New()
_, err = env.Retrieve(context.Background(), envSchemePrefix+envName, nil)
assert.Error(t, err)
assert.NoError(t, env.Shutdown(context.Background()))
Expand All @@ -59,7 +59,7 @@ func TestEnv(t *testing.T) {
const envName = "default-config"
t.Setenv(envName, string(bytes))

env := NewEnv()
env := New()
ret, err := env.Retrieve(context.Background(), envSchemePrefix+envName, nil)
require.NoError(t, err)
expectedMap := config.NewMapFromStringMap(map[string]interface{}{
Expand Down
5 changes: 5 additions & 0 deletions config/envmapprovider/testdata/default-config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
processors:
batch:
exporters:
otlp:
endpoint: "localhost:4317"
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 configmapprovider // import "go.opentelemetry.io/collector/config/configmapprovider"
package filemapprovider // import "go.opentelemetry.io/collector/config/filemapprovider"

import (
"context"
Expand All @@ -30,7 +30,7 @@ const fileSchemeName = "file"

type fileMapProvider struct{}

// NewFile returns a new Provider that reads the configuration from a file.
// New returns a new config.MapProvider that reads the configuration from a file.
//
// This Provider supports "file" scheme, and can be called with a "location" that follows:
// file-location = "file:" local-path
Expand All @@ -43,27 +43,27 @@ type fileMapProvider struct{}
// `file:/path/to/file` - absolute path (unix, windows)
// `file:c:/path/to/file` - absolute path including drive-letter (windows)
// `file:c:\path\to\file` - absolute path including drive-letter (windows)
func NewFile() Provider {
func New() config.MapProvider {
return &fileMapProvider{}
}

func (fmp *fileMapProvider) Retrieve(_ context.Context, location string, _ WatcherFunc) (Retrieved, error) {
func (fmp *fileMapProvider) Retrieve(_ context.Context, location string, _ config.WatcherFunc) (config.Retrieved, error) {
if !strings.HasPrefix(location, fileSchemeName+":") {
return Retrieved{}, fmt.Errorf("%v location is not supported by %v provider", location, fileSchemeName)
return config.Retrieved{}, fmt.Errorf("%v location is not supported by %v provider", location, fileSchemeName)
}

// Clean the path before using it.
content, err := ioutil.ReadFile(filepath.Clean(location[len(fileSchemeName)+1:]))
if err != nil {
return Retrieved{}, fmt.Errorf("unable to read the file %v: %w", location, err)
return config.Retrieved{}, fmt.Errorf("unable to read the file %v: %w", location, err)
}

var data map[string]interface{}
if err = yaml.Unmarshal(content, &data); err != nil {
return Retrieved{}, fmt.Errorf("unable to parse yaml: %w", err)
return config.Retrieved{}, fmt.Errorf("unable to parse yaml: %w", err)
}

return Retrieved{Map: config.NewMapFromStringMap(data)}, nil
return config.Retrieved{Map: config.NewMapFromStringMap(data)}, nil
}

func (*fileMapProvider) Shutdown(context.Context) error {
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 configmapprovider
package filemapprovider

import (
"context"
Expand All @@ -28,40 +28,40 @@ import (

const fileSchemePrefix = fileSchemeName + ":"

func TestFile_EmptyName(t *testing.T) {
fp := NewFile()
func TestEmptyName(t *testing.T) {
fp := New()
_, err := fp.Retrieve(context.Background(), "", nil)
require.Error(t, err)
require.NoError(t, fp.Shutdown(context.Background()))
}

func TestFile_UnsupportedScheme(t *testing.T) {
fp := NewFile()
_, err := fp.Retrieve(context.Background(), "http://", nil)
func TestUnsupportedScheme(t *testing.T) {
fp := New()
_, err := fp.Retrieve(context.Background(), "https://", nil)
assert.Error(t, err)
assert.NoError(t, fp.Shutdown(context.Background()))
}

func TestFile_NonExistent(t *testing.T) {
fp := NewFile()
func TestNonExistent(t *testing.T) {
fp := New()
_, err := fp.Retrieve(context.Background(), fileSchemePrefix+filepath.Join("testdata", "non-existent.yaml"), nil)
assert.Error(t, err)
_, err = fp.Retrieve(context.Background(), fileSchemePrefix+absolutePath(t, filepath.Join("testdata", "non-existent.yaml")), nil)
assert.Error(t, err)
require.NoError(t, fp.Shutdown(context.Background()))
}

func TestFile_InvalidYaml(t *testing.T) {
fp := NewFile()
func TestInvalidYaml(t *testing.T) {
fp := New()
_, err := fp.Retrieve(context.Background(), fileSchemePrefix+filepath.Join("testdata", "invalid-yaml.yaml"), nil)
assert.Error(t, err)
_, err = fp.Retrieve(context.Background(), fileSchemePrefix+absolutePath(t, filepath.Join("testdata", "invalid-yaml.yaml")), nil)
assert.Error(t, err)
require.NoError(t, fp.Shutdown(context.Background()))
}

func TestFile_RelativePath(t *testing.T) {
fp := NewFile()
func TestRelativePath(t *testing.T) {
fp := New()
ret, err := fp.Retrieve(context.Background(), fileSchemePrefix+filepath.Join("testdata", "default-config.yaml"), nil)
require.NoError(t, err)
expectedMap := config.NewMapFromStringMap(map[string]interface{}{
Expand All @@ -72,8 +72,8 @@ func TestFile_RelativePath(t *testing.T) {
assert.NoError(t, fp.Shutdown(context.Background()))
}

func TestFile_AbsolutePath(t *testing.T) {
fp := NewFile()
func TestAbsolutePath(t *testing.T) {
fp := New()
ret, err := fp.Retrieve(context.Background(), fileSchemePrefix+absolutePath(t, filepath.Join("testdata", "default-config.yaml")), nil)
require.NoError(t, err)
expectedMap := config.NewMapFromStringMap(map[string]interface{}{
Expand Down
5 changes: 5 additions & 0 deletions config/filemapprovider/testdata/default-config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
processors:
batch:
exporters:
otlp:
endpoint: "localhost:4317"
Loading

0 comments on commit 0e06154

Please sign in to comment.