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

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.NewProviderSettingsNoopLogger()).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,23 @@ 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 NewProviderSettings(logger *zap.Logger) ProviderSettings {
+	return ProviderSettings{Logger: logger}
+}
+
+func NewProviderSettingsNoopLogger() ProviderSettings {
+	return NewProviderSettings(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(fmt.Sprintf("Environment variable %s is used in configuration but is unset", envVarName))
+	} else if len(val) == 0 {
+		emp.logger.Warn(fmt.Sprintf("Environment variable %s is used in configuration but is empty", 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

@@ -7,6 +7,9 @@ import (
 	"context"
 	"testing"
 
+	"go.uber.org/zap"
+	"go.uber.org/zap/zaptest/observer"
+
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
@@ -25,18 +28,18 @@ exporters:
 `
 
 func TestValidateProviderScheme(t *testing.T) {
-	assert.NoError(t, confmaptest.ValidateProviderScheme(NewWithSettings(confmap.ProviderSettings{})))
+	assert.NoError(t, confmaptest.ValidateProviderScheme(NewWithSettings(confmap.NewProviderSettingsNoopLogger())))
 }
 
 func TestEmptyName(t *testing.T) {
-	env := NewWithSettings(confmap.ProviderSettings{})
+	env := NewWithSettings(confmap.NewProviderSettingsNoopLogger())
 	_, 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.NewProviderSettingsNoopLogger())
 	_, err := env.Retrieve(context.Background(), "https://", nil)
 	assert.Error(t, err)
 	assert.NoError(t, env.Shutdown(context.Background()))
@@ -45,7 +48,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.NewProviderSettingsNoopLogger())
 	_, err := env.Retrieve(context.Background(), envSchemePrefix+envName, nil)
 	assert.Error(t, err)
 	assert.NoError(t, env.Shutdown(context.Background()))
@@ -55,7 +58,27 @@ func TestEnv(t *testing.T) {
 	const envName = "default-config"
 	t.Setenv(envName, validYAML)
 
-	env := NewWithSettings(confmap.ProviderSettings{})
+	env := NewWithSettings(confmap.NewProviderSettingsNoopLogger())
+	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()))
+}
+
+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()
@@ -67,4 +90,43 @@ func TestEnv(t *testing.T) {
 	assert.Equal(t, expectedMap.ToStringMap(), retMap.ToStringMap())
 
 	assert.NoError(t, env.Shutdown(context.Background()))
+	assert.Equal(t, ol.Len(), 0)
+}
+
+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, ol.Len(), 1)
+	assert.Equal(t, ol.All()[0].Message, "Environment variable default-config is used in configuration but is unset")
+}
+
+func TestEmptyEnvWithLoggerWarn(t *testing.T) {
+	const envName = "default-config"
+	t.Setenv(envName, "")
+
+	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, ol.Len(), 1)
+	assert.Equal(t, ol.All()[0].Message, "Environment variable default-config is used in configuration but is empty")
 }

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.NewProviderSettingsNoopLogger())))
 }
 
 func TestEmptyName(t *testing.T) {
-	fp := NewWithSettings(confmap.ProviderSettings{})
+	fp := NewWithSettings(confmap.NewProviderSettingsNoopLogger())
 	_, 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.NewProviderSettingsNoopLogger())
 	_, 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.NewProviderSettingsNoopLogger())
 	_, 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.NewProviderSettingsNoopLogger())
 	_, 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.NewProviderSettingsNoopLogger())
 	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.NewProviderSettingsNoopLogger())
 	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
 )
 

confmap/provider/httpprovider/provider_test.go

@@ -14,7 +14,7 @@ import (
 )
 
 func TestSupportedScheme(t *testing.T) {
-	fp := NewWithSettings(confmap.ProviderSettings{})
+	fp := NewWithSettings(confmap.NewProviderSettingsNoopLogger())
 	assert.Equal(t, "http", fp.Scheme())
 	require.NoError(t, fp.Shutdown(context.Background()))
 }

confmap/provider/httpsprovider/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
 )