Log when Environment Provider tries to pull unset or empty env var

.chloggen/log-env-empty.yaml

@@ -0,0 +1,26 @@
+# Use this changelog template to create an entry for release notes.
+
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: enhancement
+
+# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
+component: confmap
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Creates a logger in the confmap.ProviderSettings and uses it to log when there is a missing or blank environment variable referenced in config. For now the noop logger is used everywhere except tests.
+
+
+# One or more tracking issues or pull requests related to the change
+issues: [5615]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext:
+
+# Optional: The change log or logs in which this entry should be included.
+# e.g. '[user]' or '[user, api]'
+# Include 'user' if the change is relevant to end users.
+# Include 'api' if there is a change to a library API.
+# Default: '[user]'
+change_logs: [user]

cmd/mdatagen/loader.go

@@ -239,7 +239,7 @@ type templateContext struct {
 }
 
 func loadMetadata(filePath string) (metadata, error) {
-	cp, err := fileprovider.NewWithSettings(confmap.ProviderSettings{}).Retrieve(context.Background(), "file:"+filePath, nil)
+	cp, err := fileprovider.NewWithSettings(confmap.NewProviderSettingsNopLogger()).Retrieve(context.Background(), "file:"+filePath, nil)
 	if err != nil {
 		return metadata{}, err
 	}

config/configopaque/go.mod

@@ -21,6 +21,7 @@ require (
 	github.com/pmezard/go-difflib v1.0.0 // indirect
 	github.com/rogpeppe/go-internal v1.10.0 // indirect
 	go.uber.org/multierr v1.11.0 // indirect
+	go.uber.org/zap v1.27.0 // indirect
 	gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect
 )
 

confmap/converter/expandconverter/go.mod

@@ -6,7 +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
+	go.uber.org/zap v1.27.0
 )
 
 require (

confmap/go.mod

@@ -10,12 +10,12 @@ require (
 	github.com/stretchr/testify v1.9.0
 	go.uber.org/goleak v1.3.0
 	go.uber.org/multierr v1.11.0
+	go.uber.org/zap v1.27.0
 	gopkg.in/yaml.v3 v3.0.1
 )
 
 require (
 	github.com/davecgh/go-spew v1.1.1 // indirect
-	github.com/kr/text v0.2.0 // indirect
 	github.com/mitchellh/copystructure v1.2.0 // indirect
 	github.com/mitchellh/reflectwalk v1.0.2 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect

confmap/provider.go

@@ -6,11 +6,19 @@ package confmap // import "go.opentelemetry.io/collector/confmap"
 import (
 	"context"
 	"fmt"
+
+	"go.uber.org/zap"
 )
 
 // ProviderSettings are the settings to initialize a Provider.
 // Any Provider should take this as a parameter in its constructor.
-type ProviderSettings struct{}
+type ProviderSettings struct {
+	Logger *zap.Logger
+}
+
+func NewProviderSettingsNopLogger() ProviderSettings {
+	return ProviderSettings{zap.NewNop()}
+}
 
 // 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,

confmap/provider/envprovider/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.27.0
 )
 
 require (

confmap/provider/envprovider/provider.go

@@ -9,28 +9,41 @@ import (
 	"os"
 	"strings"
 
+	"go.uber.org/zap"
+
 	"go.opentelemetry.io/collector/confmap"
 	"go.opentelemetry.io/collector/confmap/provider/internal"
 )
 
 const schemeName = "env"
 
-type provider struct{}
+type provider struct {
+	logger *zap.Logger
+}
 
 // NewWithSettings returns a new confmap.Provider 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 NewWithSettings(confmap.ProviderSettings) confmap.Provider {
-	return &provider{}
+func NewWithSettings(a confmap.ProviderSettings) confmap.Provider {
+	return &provider{
+		logger: a.Logger,
+	}
 }
 
 func (emp *provider) Retrieve(_ context.Context, uri string, _ confmap.WatcherFunc) (*confmap.Retrieved, error) {
 	if !strings.HasPrefix(uri, schemeName+":") {
 		return nil, fmt.Errorf("%q uri is not supported by %q provider", uri, schemeName)
 	}
+	envVarName := uri[len(schemeName)+1:]
+	val, exists := os.LookupEnv(envVarName)
+	if !exists {
+		emp.logger.Warn("Configuration references unset environment variable", zap.String("name", envVarName))
+	} else if len(val) == 0 {
+		emp.logger.Info("Configuration references empty environment variable", zap.String("name", envVarName))
+	}
 
-	return internal.NewRetrievedFromYAML([]byte(os.Getenv(uri[len(schemeName)+1:])))
+	return internal.NewRetrievedFromYAML([]byte(val))
 }
 
 func (*provider) Scheme() string {

confmap/provider/envprovider/provider_test.go

@@ -9,6 +9,8 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"go.uber.org/zap"
+	"go.uber.org/zap/zaptest/observer"
 
 	"go.opentelemetry.io/collector/confmap"
 	"go.opentelemetry.io/collector/confmap/confmaptest"
@@ -25,18 +27,18 @@ exporters:
 `
 
 func TestValidateProviderScheme(t *testing.T) {
-	assert.NoError(t, confmaptest.ValidateProviderScheme(NewWithSettings(confmap.ProviderSettings{})))
+	assert.NoError(t, confmaptest.ValidateProviderScheme(NewWithSettings(confmap.NewProviderSettingsNopLogger())))
 }
 
 func TestEmptyName(t *testing.T) {
-	env := NewWithSettings(confmap.ProviderSettings{})
+	env := NewWithSettings(confmap.NewProviderSettingsNopLogger())
 	_, err := env.Retrieve(context.Background(), "", nil)
 	require.Error(t, err)
 	assert.NoError(t, env.Shutdown(context.Background()))
 }
 
 func TestUnsupportedScheme(t *testing.T) {
-	env := NewWithSettings(confmap.ProviderSettings{})
+	env := NewWithSettings(confmap.NewProviderSettingsNopLogger())
 	_, err := env.Retrieve(context.Background(), "https://", nil)
 	assert.Error(t, err)
 	assert.NoError(t, env.Shutdown(context.Background()))
@@ -45,7 +47,7 @@ func TestUnsupportedScheme(t *testing.T) {
 func TestInvalidYAML(t *testing.T) {
 	const envName = "invalid-yaml"
 	t.Setenv(envName, "[invalid,")
-	env := NewWithSettings(confmap.ProviderSettings{})
+	env := NewWithSettings(confmap.NewProviderSettingsNopLogger())
 	_, err := env.Retrieve(context.Background(), envSchemePrefix+envName, nil)
 	assert.Error(t, err)
 	assert.NoError(t, env.Shutdown(context.Background()))
@@ -55,7 +57,7 @@ func TestEnv(t *testing.T) {
 	const envName = "default-config"
 	t.Setenv(envName, validYAML)
 
-	env := NewWithSettings(confmap.ProviderSettings{})
+	env := NewWithSettings(confmap.NewProviderSettingsNopLogger())
 	ret, err := env.Retrieve(context.Background(), envSchemePrefix+envName, nil)
 	require.NoError(t, err)
 	retMap, err := ret.AsConf()
@@ -68,3 +70,70 @@ func TestEnv(t *testing.T) {
 
 	assert.NoError(t, env.Shutdown(context.Background()))
 }
+
+func TestEnvWithLogger(t *testing.T) {
+	const envName = "default-config"
+	t.Setenv(envName, validYAML)
+	core, ol := observer.New(zap.WarnLevel)
+	logger := zap.New(core)
+
+	env := NewWithSettings(confmap.ProviderSettings{Logger: logger})
+	ret, err := env.Retrieve(context.Background(), envSchemePrefix+envName, nil)
+	require.NoError(t, err)
+	retMap, err := ret.AsConf()
+	assert.NoError(t, err)
+	expectedMap := confmap.NewFromStringMap(map[string]any{
+		"processors::batch":         nil,
+		"exporters::otlp::endpoint": "localhost:4317",
+	})
+	assert.Equal(t, expectedMap.ToStringMap(), retMap.ToStringMap())
+
+	assert.NoError(t, env.Shutdown(context.Background()))
+	assert.Equal(t, 0, ol.Len())
+}
+
+func TestUnsetEnvWithLoggerWarn(t *testing.T) {
+	const envName = "default-config"
+	core, ol := observer.New(zap.WarnLevel)
+	logger := zap.New(core)
+
+	env := NewWithSettings(confmap.ProviderSettings{Logger: logger})
+	ret, err := env.Retrieve(context.Background(), envSchemePrefix+envName, nil)
+	require.NoError(t, err)
+	retMap, err := ret.AsConf()
+	assert.NoError(t, err)
+	expectedMap := confmap.NewFromStringMap(map[string]any{})
+	assert.Equal(t, expectedMap.ToStringMap(), retMap.ToStringMap())
+
+	assert.NoError(t, env.Shutdown(context.Background()))
+
+	assert.Equal(t, 1, ol.Len())
+	logLine := ol.All()[0]
+	assert.Equal(t, "Configuration references unset environment variable", logLine.Message)
+	assert.Equal(t, zap.WarnLevel, logLine.Level)
+	assert.Equal(t, envName, logLine.Context[0].String)
+}
+
+func TestEmptyEnvWithLoggerWarn(t *testing.T) {
+	const envName = "default-config"
+	t.Setenv(envName, "")
+
+	core, ol := observer.New(zap.InfoLevel)
+	logger := zap.New(core)
+
+	env := NewWithSettings(confmap.ProviderSettings{Logger: logger})
+	ret, err := env.Retrieve(context.Background(), envSchemePrefix+envName, nil)
+	require.NoError(t, err)
+	retMap, err := ret.AsConf()
+	assert.NoError(t, err)
+	expectedMap := confmap.NewFromStringMap(map[string]any{})
+	assert.Equal(t, expectedMap.ToStringMap(), retMap.ToStringMap())
+
+	assert.NoError(t, env.Shutdown(context.Background()))
+
+	assert.Equal(t, 1, ol.Len())
+	logLine := ol.All()[0]
+	assert.Equal(t, "Configuration references empty environment variable", logLine.Message)
+	assert.Equal(t, zap.InfoLevel, logLine.Level)
+	assert.Equal(t, envName, logLine.Context[0].String)
+}

confmap/provider/fileprovider/go.mod

@@ -18,6 +18,7 @@ require (
 	github.com/mitchellh/reflectwalk v1.0.2 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
 	go.uber.org/multierr v1.11.0 // indirect
+	go.uber.org/zap v1.27.0 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 )
 

confmap/provider/fileprovider/provider_test.go

@@ -19,25 +19,25 @@ import (
 const fileSchemePrefix = schemeName + ":"
 
 func TestValidateProviderScheme(t *testing.T) {
-	assert.NoError(t, confmaptest.ValidateProviderScheme(NewWithSettings(confmap.ProviderSettings{})))
+	assert.NoError(t, confmaptest.ValidateProviderScheme(NewWithSettings(confmap.NewProviderSettingsNopLogger())))
 }
 
 func TestEmptyName(t *testing.T) {
-	fp := NewWithSettings(confmap.ProviderSettings{})
+	fp := NewWithSettings(confmap.NewProviderSettingsNopLogger())
 	_, err := fp.Retrieve(context.Background(), "", nil)
 	require.Error(t, err)
 	require.NoError(t, fp.Shutdown(context.Background()))
 }
 
 func TestUnsupportedScheme(t *testing.T) {
-	fp := NewWithSettings(confmap.ProviderSettings{})
+	fp := NewWithSettings(confmap.NewProviderSettingsNopLogger())
 	_, err := fp.Retrieve(context.Background(), "https://", nil)
 	assert.Error(t, err)
 	assert.NoError(t, fp.Shutdown(context.Background()))
 }
 
 func TestNonExistent(t *testing.T) {
-	fp := NewWithSettings(confmap.ProviderSettings{})
+	fp := NewWithSettings(confmap.NewProviderSettingsNopLogger())
 	_, 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)
@@ -46,7 +46,7 @@ func TestNonExistent(t *testing.T) {
 }
 
 func TestInvalidYAML(t *testing.T) {
-	fp := NewWithSettings(confmap.ProviderSettings{})
+	fp := NewWithSettings(confmap.NewProviderSettingsNopLogger())
 	_, 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)
@@ -55,7 +55,7 @@ func TestInvalidYAML(t *testing.T) {
 }
 
 func TestRelativePath(t *testing.T) {
-	fp := NewWithSettings(confmap.ProviderSettings{})
+	fp := NewWithSettings(confmap.NewProviderSettingsNopLogger())
 	ret, err := fp.Retrieve(context.Background(), fileSchemePrefix+filepath.Join("testdata", "default-config.yaml"), nil)
 	require.NoError(t, err)
 	retMap, err := ret.AsConf()
@@ -69,7 +69,7 @@ func TestRelativePath(t *testing.T) {
 }
 
 func TestAbsolutePath(t *testing.T) {
-	fp := NewWithSettings(confmap.ProviderSettings{})
+	fp := NewWithSettings(confmap.NewProviderSettingsNopLogger())
 	ret, err := fp.Retrieve(context.Background(), fileSchemePrefix+absolutePath(t, filepath.Join("testdata", "default-config.yaml")), nil)
 	require.NoError(t, err)
 	retMap, err := ret.AsConf()

confmap/provider/httpprovider/go.mod

@@ -18,6 +18,7 @@ require (
 	github.com/mitchellh/reflectwalk v1.0.2 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
 	go.uber.org/multierr v1.11.0 // indirect
+	go.uber.org/zap v1.27.0 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 )