Skip to content

Commit

Permalink
[confmap] Add logger to ConverterSettings (#10135)
Browse files Browse the repository at this point in the history
#### Description
Adds a Logger to ConverterSettings to enable logging from within
converters. Also update the expand converter to log a warning if the env
var is empty or missing.

#### Link to tracking issue
Closes
#9162
Closes
#5615

#### Testing

Unit tests and local testing


![image](https://github.com/open-telemetry/opentelemetry-collector/assets/12352919/af5dd1e2-62f9-4272-97c7-da57166ef07e)

```yaml
receivers:
  nop:

exporters:
  otlphttp:
    endpoint: http://0.0.0.0:4317
    headers:
      # Not set
      x-test: $TEMP3
  debug:
    # set to "detailed"
    verbosity: $TEMP

service:
  telemetry:
    logs:
      level: info
  pipelines:
    traces:
      receivers:
        - nop
      exporters:
        - otlphttp
        - debug
```

#### Documentation
Added godoc comments
  • Loading branch information
TylerHelmuth committed May 10, 2024
1 parent 2f87518 commit 1d52fb9
Show file tree
Hide file tree
Showing 7 changed files with 55 additions and 7 deletions.
25 changes: 25 additions & 0 deletions .chloggen/confmap-converter-logger.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# 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: Allow Converters to write logs during startup

# One or more tracking issues or pull requests related to the change
issues: [10135]

# (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: []
10 changes: 9 additions & 1 deletion confmap/converter.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,18 @@ package confmap // import "go.opentelemetry.io/collector/confmap"

import (
"context"

"go.uber.org/zap"
)

// ConverterSettings are the settings to initialize a Converter.
type ConverterSettings struct{}
type ConverterSettings struct {
// Logger is a zap.Logger that will be passed to Converters.
// Converters should be able to rely on the Logger being non-nil;
// when instantiating a Converter with a ConverterFactory,
// nil Logger references should be replaced with a no-op Logger.
Logger *zap.Logger
}

// ConverterFactory defines a factory that can be used to instantiate
// new instances of a Converter.
Expand Down
12 changes: 9 additions & 3 deletions confmap/converter/expandconverter/expand.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ func NewFactory() confmap.ConverterFactory {
return confmap.NewConverterFactory(newConverter)
}

func newConverter(_ confmap.ConverterSettings) confmap.Converter {
func newConverter(set confmap.ConverterSettings) confmap.Converter {
return converter{
loggedDeprecations: make(map[string]struct{}),
logger: zap.NewNop(), // TODO: pass logger in ConverterSettings
logger: set.Logger,
}
}

Expand Down Expand Up @@ -80,6 +80,12 @@ func (c converter) expandEnv(s string) string {
if str == "$" {
return "$"
}
return os.Getenv(str)
val, exists := os.LookupEnv(str)
if !exists {
c.logger.Warn("Configuration references unset environment variable", zap.String("name", str))
} else if len(val) == 0 {
c.logger.Info("Configuration references empty environment variable", zap.String("name", str))
}
return val
})
}
2 changes: 1 addition & 1 deletion confmap/converter/expandconverter/expand_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -254,5 +254,5 @@ func TestDeprecatedWarning(t *testing.T) {
}

func createConverter() confmap.Converter {
return NewFactory().Create(confmap.ConverterSettings{})
return NewFactory().Create(confmap.ConverterSettings{Logger: zap.NewNop()})
}
4 changes: 4 additions & 0 deletions confmap/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,10 @@ func NewResolver(set ResolverSettings) (*Resolver, error) {
set.ProviderSettings.Logger = zap.NewNop()
}

if set.ConverterSettings.Logger == nil {
set.ConverterSettings.Logger = zap.NewNop()
}

var providers map[string]Provider
var converters []Converter

Expand Down
4 changes: 4 additions & 0 deletions confmap/resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,10 @@ func TestProvidesDefaultLogger(t *testing.T) {
_, err := NewResolver(ResolverSettings{
URIs: []string{filepath.Join("testdata", "config.yaml")},
ProviderFactories: []ProviderFactory{factory},
ConverterFactories: []ConverterFactory{NewConverterFactory(func(set ConverterSettings) Converter {
assert.NotNil(t, set.Logger)
return &mockConverter{}
})},
})
require.NoError(t, err)
require.NotNil(t, provider.logger)
Expand Down
5 changes: 3 additions & 2 deletions otelcol/collector.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,9 @@ func NewCollector(set CollectorSettings) (*Collector, error) {
bc := newBufferedCore(zapcore.DebugLevel)
cc := &collectorCore{core: bc}
options := append([]zap.Option{zap.WithCaller(true)}, set.LoggingOptions...)
set.ConfigProviderSettings.ResolverSettings.ProviderSettings = confmap.ProviderSettings{Logger: zap.New(cc, options...)}
set.ConfigProviderSettings.ResolverSettings.ConverterSettings = confmap.ConverterSettings{}
logger := zap.New(cc, options...)
set.ConfigProviderSettings.ResolverSettings.ProviderSettings = confmap.ProviderSettings{Logger: logger}
set.ConfigProviderSettings.ResolverSettings.ConverterSettings = confmap.ConverterSettings{Logger: logger}

if configProvider == nil {
configProvider, err = NewConfigProvider(set.ConfigProviderSettings)
Expand Down

0 comments on commit 1d52fb9

Please sign in to comment.