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

…9837)

**Description:** 
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.

**Link to tracking Issue:**
[5615](#5615)

**Testing:** 
I wrote unit tests that ensured 

1. logging occurred when an environment variable was unset 
2. logging occcured when the env var was empty.  
3. there was no log when an env var was used correctly

I also started the otel collector with the sample config - and added an
env var reference in the sample config. I then inserted a print
statement next to each log call to see whether my code paths were hit in
the live application. I then went through the 3 cases mentioned above
and ensured that logging behavior was accurate.

.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: [api]

cmd/mdatagen/loader.go

@@ -12,6 +12,7 @@ import (
 	"strings"
 
 	"go.opentelemetry.io/collector/confmap"
+	"go.opentelemetry.io/collector/confmap/confmaptest"
 	"go.opentelemetry.io/collector/confmap/provider/fileprovider"
 	"go.opentelemetry.io/collector/pdata/pcommon"
 )
@@ -239,7 +240,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(confmaptest.NewNopProviderSettings()).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/confmaptest/provider_settings.go

@@ -0,0 +1,14 @@
+// Copyright The OpenTelemetry Authors
+// SPDX-License-Identifier: Apache-2.0
+
+package confmaptest // import "go.opentelemetry.io/collector/confmap/confmaptest"
+
+import (
+	"go.uber.org/zap"
+
+	"go.opentelemetry.io/collector/confmap"
+)
+
+func NewNopProviderSettings() confmap.ProviderSettings {
+	return confmap.ProviderSettings{Logger: zap.NewNop()}
+}

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,15 @@ 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
+}
 
 // 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(ps confmap.ProviderSettings) confmap.Provider {
+	return &provider{
+		logger: ps.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(confmaptest.NewNopProviderSettings())))
 }
 
 func TestEmptyName(t *testing.T) {
-	env := NewWithSettings(confmap.ProviderSettings{})
+	env := NewWithSettings(confmaptest.NewNopProviderSettings())
 	_, 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(confmaptest.NewNopProviderSettings())
 	_, 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(confmaptest.NewNopProviderSettings())
 	_, 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(confmaptest.NewNopProviderSettings())
 	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
 )