diff --git a/confmap/converter/expandconverter/expand.go b/confmap/converter/expandconverter/expand.go index dc3a89812c4..4ea461fcf7e 100644 --- a/confmap/converter/expandconverter/expand.go +++ b/confmap/converter/expandconverter/expand.go @@ -5,42 +5,54 @@ package expandconverter // import "go.opentelemetry.io/collector/confmap/convert import ( "context" + "fmt" "os" + "regexp" + + "go.uber.org/zap" "go.opentelemetry.io/collector/confmap" ) -type converter struct{} +type converter struct { + logger *zap.Logger + + // Record of which env vars we have logged a warning for + loggedDeprecations map[string]struct{} +} // New returns a confmap.Converter, that expands all environment variables for a given confmap.Conf. // // Notice: This API is experimental. -func New(confmap.ConverterSettings) confmap.Converter { - return converter{} +func New(_ confmap.ConverterSettings) confmap.Converter { + return converter{ + loggedDeprecations: make(map[string]struct{}), + logger: zap.NewNop(), // TODO: pass logger in ConverterSettings + } } -func (converter) Convert(_ context.Context, conf *confmap.Conf) error { +func (c converter) Convert(_ context.Context, conf *confmap.Conf) error { out := make(map[string]any) for _, k := range conf.AllKeys() { - out[k] = expandStringValues(conf.Get(k)) + out[k] = c.expandStringValues(conf.Get(k)) } return conf.Merge(confmap.NewFromStringMap(out)) } -func expandStringValues(value any) any { +func (c converter) expandStringValues(value any) any { switch v := value.(type) { case string: - return expandEnv(v) + return c.expandEnv(v) case []any: nslice := make([]any, 0, len(v)) for _, vint := range v { - nslice = append(nslice, expandStringValues(vint)) + nslice = append(nslice, c.expandStringValues(vint)) } return nslice case map[string]any: nmap := map[string]any{} for mk, mv := range v { - nmap[mk] = expandStringValues(mv) + nmap[mk] = c.expandStringValues(mv) } return nmap default: @@ -48,8 +60,16 @@ func expandStringValues(value any) any { } } -func expandEnv(s string) string { +func (c converter) expandEnv(s string) string { return os.Expand(s, func(str string) string { + // Matches on $VAR style environment variables + // in order to make sure we don't log a warning for ${VAR} + var regex = regexp.MustCompile(fmt.Sprintf(`\$%s`, regexp.QuoteMeta(str))) + if _, exists := c.loggedDeprecations[str]; !exists && regex.MatchString(s) { + msg := fmt.Sprintf("Variable substitution using $VAR will be deprecated in favor of ${VAR} and ${env:VAR}, please update $%s", str) + c.logger.Warn(msg, zap.String("variable", str)) + c.loggedDeprecations[str] = struct{}{} + } // This allows escaping environment variable substitution via $$, e.g. // - $FOO will be substituted with env var FOO // - $$FOO will be replaced with $FOO diff --git a/confmap/converter/expandconverter/expand_test.go b/confmap/converter/expandconverter/expand_test.go index 731dd52e9b6..7837c2de912 100644 --- a/confmap/converter/expandconverter/expand_test.go +++ b/confmap/converter/expandconverter/expand_test.go @@ -5,11 +5,15 @@ package expandconverter import ( "context" + "fmt" "path/filepath" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.uber.org/zap" + "go.uber.org/zap/zapcore" + "go.uber.org/zap/zaptest/observer" "go.opentelemetry.io/collector/confmap" "go.opentelemetry.io/collector/confmap/confmaptest" @@ -159,3 +163,92 @@ func TestNewExpandConverterHostPort(t *testing.T) { }) } } + +func NewTestConverter() (confmap.Converter, *observer.ObservedLogs) { + core, logs := observer.New(zapcore.InfoLevel) + conv := converter{loggedDeprecations: make(map[string]struct{}), logger: zap.New(core)} + return conv, logs +} + +func TestDeprecatedWarning(t *testing.T) { + msgTemplate := `Variable substitution using $VAR will be deprecated in favor of ${VAR} and ${env:VAR}, please update $%s` + t.Setenv("HOST", "127.0.0.1") + t.Setenv("PORT", "4317") + + t.Setenv("HOST_NAME", "127.0.0.2") + t.Setenv("HOST.NAME", "127.0.0.3") + + var testCases = []struct { + name string + input map[string]any + expectedOutput map[string]any + expectedWarnings []string + }{ + { + name: "no warning", + input: map[string]any{ + "test": "${HOST}:${PORT}", + }, + expectedOutput: map[string]any{ + "test": "127.0.0.1:4317", + }, + expectedWarnings: []string{}, + }, + { + name: "one deprecated var", + input: map[string]any{ + "test": "${HOST}:$PORT", + }, + expectedOutput: map[string]any{ + "test": "127.0.0.1:4317", + }, + expectedWarnings: []string{"PORT"}, + }, + { + name: "two deprecated vars", + input: map[string]any{ + "test": "$HOST:$PORT", + }, + expectedOutput: map[string]any{ + "test": "127.0.0.1:4317", + }, + expectedWarnings: []string{"HOST", "PORT"}, + }, + { + name: "one depracated serveral times", + input: map[string]any{ + "test": "$HOST,$HOST", + "test2": "$HOST", + }, + expectedOutput: map[string]any{ + "test": "127.0.0.1,127.0.0.1", + "test2": "127.0.0.1", + }, + expectedWarnings: []string{"HOST"}, + }, + { + name: "one warning", + input: map[string]any{ + "test": "$HOST_NAME,${HOST.NAME}", + }, + expectedOutput: map[string]any{ + "test": "127.0.0.2,127.0.0.3", + }, + expectedWarnings: []string{"HOST_NAME"}, + }, + } + for _, tt := range testCases { + t.Run(tt.name, func(t *testing.T) { + conf := confmap.NewFromStringMap(tt.input) + conv, logs := NewTestConverter() + require.NoError(t, conv.Convert(context.Background(), conf)) + + assert.Equal(t, tt.expectedOutput, conf.ToStringMap()) + assert.Equal(t, len(tt.expectedWarnings), len(logs.All())) + for i, variable := range tt.expectedWarnings { + errorMsg := fmt.Sprintf(msgTemplate, variable) + assert.Equal(t, errorMsg, logs.All()[i].Message) + } + }) + } +} diff --git a/confmap/converter/expandconverter/go.mod b/confmap/converter/expandconverter/go.mod index 00240e71b7d..e484513b998 100644 --- a/confmap/converter/expandconverter/go.mod +++ b/confmap/converter/expandconverter/go.mod @@ -6,6 +6,7 @@ require ( github.com/stretchr/testify v1.9.0 go.opentelemetry.io/collector/confmap v0.97.0 go.uber.org/goleak v1.3.0 + go.uber.org/zap v1.26.0 ) require ( diff --git a/confmap/converter/expandconverter/go.sum b/confmap/converter/expandconverter/go.sum index 5cb8d751cee..3b64c43d41f 100644 --- a/confmap/converter/expandconverter/go.sum +++ b/confmap/converter/expandconverter/go.sum @@ -24,6 +24,8 @@ go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto= go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE= go.uber.org/multierr v1.11.0 h1:blXXJkSxSSfBVBlC76pxqeO+LN3aDfLQo+309xJstO0= go.uber.org/multierr v1.11.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y= +go.uber.org/zap v1.26.0 h1:sI7k6L95XOKS281NhVKOFCUNIvv9e0w4BF8N3u+tCRo= +go.uber.org/zap v1.26.0/go.mod h1:dtElttAiwGvoJ/vj4IwHBS/gXsEu/pZ50mUIRWuG0so= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 h1:qIbj1fsPNlZgppZ+VLlY7N33q108Sa+fhmuc+sWQYwY= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=