Skip to content

Commit

Permalink
fix: Prefer environment variables over code configuration (#106)
Browse files Browse the repository at this point in the history 
## Which problem is this PR solving?
When setting configuration values by code and environment variables,
environment variables should win. This is a design choice so that
operators of the library can change behaviour without requiring
rebuilding an application. This PR updates the sequence of applying
configuration options so that env vars are applied last and will replace
any code specificied options.

Tests are updated to reflect this, and includes adding additional tests
to verify expected behaviour of setting generic, trace and metrics
headers via env var which was added in the following P;
- Follow up to #99 

## Short description of the changes
- Updates all string config options to use the `overwrite` go-envconfig
option so env var values replace code-set options
- Remove unused ResourceAttributesFromEnv config option
- Move applying env var options to the config struct after applying code
options
- Update tests to reflect env vars have priority over code-provided
options, includes fixing a couple of tests where env vars were being
used by error

## How to verify that this has the expected result
Unit tests exercise expected behaviour and help prevent future
regressions.

---------

Co-authored-by: Steve Moyer <smoyer1@selesy.com>
Co-authored-by: Robb Kidd <robbkidd@honeycomb.io>
  • Loading branch information
@MikeGoldsmith @smoyer64 @robbkidd
3 people authored Mar 1, 2024
1 parent 9a50544 commit 6c48c9a
Show file tree
Hide file tree
Showing 5 changed files with 320 additions and 235 deletions.
3 changes: 2 additions & 1 deletion .gitignore
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
## smoke test artifacts
/smoke-tests/collector/**/**
/smoke-tests/report.xml
/smoke-tests/report.xml
/test_results/**/**
116 changes: 54 additions & 62 deletions otelconfig/otelconfig.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -301,27 +301,26 @@ func (l *defaultHandler) Handle(err error) {
// vary depending on the protocol chosen. If not overridden by explicit configuration, it will
// be overridden with an appropriate default upon initialization.
type Config struct {
ExporterEndpoint string `env:"OTEL_EXPORTER_OTLP_ENDPOINT"`
ExporterEndpoint string `env:"OTEL_EXPORTER_OTLP_ENDPOINT,overwrite"`
ExporterEndpointInsecure bool `env:"OTEL_EXPORTER_OTLP_INSECURE,default=false"`
TracesExporterEndpoint string `env:"OTEL_EXPORTER_OTLP_TRACES_ENDPOINT"`
TracesExporterEndpoint string `env:"OTEL_EXPORTER_OTLP_TRACES_ENDPOINT,overwrite"`
TracesExporterEndpointInsecure bool `env:"OTEL_EXPORTER_OTLP_TRACES_INSECURE"`
TracesEnabled bool `env:"OTEL_TRACES_ENABLED,default=true"`
ServiceName string `env:"OTEL_SERVICE_NAME"`
ServiceVersion string `env:"OTEL_SERVICE_VERSION,default=unknown"`
MetricsExporterEndpoint string `env:"OTEL_EXPORTER_OTLP_METRICS_ENDPOINT"`
ServiceName string `env:"OTEL_SERVICE_NAME,overwrite"`
ServiceVersion string `env:"OTEL_SERVICE_VERSION,overwrite,default=unknown"`
MetricsExporterEndpoint string `env:"OTEL_EXPORTER_OTLP_METRICS_ENDPOINT,overwrite"`
MetricsExporterEndpointInsecure bool `env:"OTEL_EXPORTER_OTLP_METRICS_INSECURE"`
MetricsEnabled bool `env:"OTEL_METRICS_ENABLED,default=true"`
MetricsReportingPeriod string `env:"OTEL_EXPORTER_OTLP_METRICS_PERIOD,default=30s"`
LogLevel string `env:"OTEL_LOG_LEVEL,default=info"`
Propagators []string `env:"OTEL_PROPAGATORS,default=tracecontext,baggage"`
ResourceAttributesFromEnv string `env:"OTEL_RESOURCE_ATTRIBUTES"`
ExporterProtocol Protocol `env:"OTEL_EXPORTER_OTLP_PROTOCOL,default=grpc"`
TracesExporterProtocol Protocol `env:"OTEL_EXPORTER_OTLP_TRACES_PROTOCOL"`
MetricsExporterProtocol Protocol `env:"OTEL_EXPORTER_OTLP_METRICS_PROTOCOL"`
MetricsReportingPeriod string `env:"OTEL_EXPORTER_OTLP_METRICS_PERIOD,overwrite,default=30s"`
LogLevel string `env:"OTEL_LOG_LEVEL,overwrite,default=info"`
Propagators []string `env:"OTEL_PROPAGATORS,overwrite,default=tracecontext,baggage"`
ExporterProtocol Protocol `env:"OTEL_EXPORTER_OTLP_PROTOCOL,overwrite,default=grpc"`
TracesExporterProtocol Protocol `env:"OTEL_EXPORTER_OTLP_TRACES_PROTOCOL,overwrite"`
MetricsExporterProtocol Protocol `env:"OTEL_EXPORTER_OTLP_METRICS_PROTOCOL,overwrite"`
Headers map[string]string `env:"OTEL_EXPORTER_OTLP_HEADERS,overwrite,separator=="`
TracesHeaders map[string]string `env:"OTEL_EXPORTER_OTLP_TRACES_HEADERS,overwrite,separator=="`
MetricsHeaders map[string]string `env:"OTEL_EXPORTER_OTLP_METRICS_HEADERS,overwrite,separator=="`
ResourceAttributes map[string]string
ResourceAttributes map[string]string `env:"OTEL_RESOURCE_ATTRIBUTES,overwrite,separator=="`
SpanProcessors []trace.SpanProcessor
Sampler trace.Sampler
ResourceOptions []resource.Option
Expand All @@ -341,12 +340,6 @@ func newConfig(opts ...Option) (*Config, error) {
errorHandler: &defaultHandler{logger: defLogger},
Sampler: trace.AlwaysSample(),
}
envError := envconfig.Process(context.Background(), c)
if envError != nil {
c.Logger.Fatalf("environment error: %v", envError)
// if our logger implementation doesn't os.Exit, we want to return here
return nil, fmt.Errorf("environment error: %w", envError)
}
// if exporter endpoint is not set using an env var, use the configured default
if c.ExporterEndpoint == "" {
c.ExporterEndpoint = DefaultExporterEndpoint
Expand All @@ -368,6 +361,14 @@ func newConfig(opts ...Option) (*Config, error) {
l.logLevel = c.LogLevel
}

// apply environment variables last to override any vendor or user options
envError := envconfig.Process(context.Background(), c)
if envError != nil {
c.Logger.Fatalf("environment error: %v", envError)
// if our logger implementation doesn't os.Exit, we want to return here
return nil, fmt.Errorf("environment error: %w", envError)
}

var err error
c.Resource, err = newResource(c)
return c, err
Expand All @@ -380,55 +381,35 @@ type OtelConfig struct {
}

func newResource(c *Config) (*resource.Resource, error) {
r := resource.Environment()

hostnameSet := false
for iter := r.Iter(); iter.Next(); {
if iter.Attribute().Key == semconv.HostNameKey && len(iter.Attribute().Value.Emit()) > 0 {
hostnameSet = true
}
}

attributes := []attribute.KeyValue{
semconv.TelemetrySDKNameKey.String("otelconfig"),
semconv.TelemetrySDKLanguageGo,
semconv.TelemetrySDKVersionKey.String(version),
}

if len(c.ServiceName) > 0 {
attributes = append(attributes, semconv.ServiceNameKey.String(c.ServiceName))
}

if len(c.ServiceVersion) > 0 {
attributes = append(attributes, semconv.ServiceVersionKey.String(c.ServiceVersion))
options := []resource.Option{
resource.WithSchemaURL(semconv.SchemaURL),
}

for key, value := range c.ResourceAttributes {
if len(value) > 0 {
if key == string(semconv.HostNameKey) {
hostnameSet = true
if c.ResourceAttributes != nil {
attrs := make([]attribute.KeyValue, 0, len(c.ResourceAttributes))
for k, v := range c.ResourceAttributes {
if len(v) > 0 {
attrs = append(attrs, attribute.String(k, v))
}
attributes = append(attributes, attribute.String(key, value))
}
options = append(options, resource.WithAttributes(attrs...))
}

if !hostnameSet {
hostname, err := os.Hostname()
if err != nil {
c.Logger.Debugf("unable to set host.name. Set OTEL_RESOURCE_ATTRIBUTES=\"host.name=<your_host_name>\" env var or configure WithResourceAttributes in code: %v", err)
} else {
attributes = append(attributes, semconv.HostNameKey.String(hostname))
}
options = append(options, c.ResourceOptions...)
if c.ServiceName != "" {
options = append(options, resource.WithAttributes(semconv.ServiceNameKey.String(c.ServiceName)))
}

attributes = append(r.Attributes(), attributes...)

baseOptions := []resource.Option{
resource.WithSchemaURL(semconv.SchemaURL),
resource.WithAttributes(attributes...),
if c.ServiceVersion != "" {
options = append(options, resource.WithAttributes(semconv.ServiceVersionKey.String(c.ServiceVersion)))
}

options := append(baseOptions, c.ResourceOptions...)
options = append(options, resource.WithHost())
options = append(options, resource.WithAttributes(
semconv.TelemetrySDKNameKey.String("otelconfig"),
semconv.TelemetrySDKLanguageGo,
semconv.TelemetrySDKVersionKey.String(version),
))
// OTEL_RESOURCE_ATTRIBUTES wins over anything from code
options = append(options, resource.WithFromEnv())
// OTEL_SERVICE_VERSION beats service.version from OTEL_RESOURCE_ATTRIBUTES, though
options = append(options, resource.WithDetectors(serviceVersionDetector{}))

// Note: There are new detectors we may wish to take advantage
// of, now available in the default SDK (e.g., WithProcess(),
Expand All @@ -437,7 +418,18 @@ func newResource(c *Config) (*resource.Resource, error) {
context.Background(),
options...,
)
}

type serviceVersionDetector struct{}

var _ resource.Detector = serviceVersionDetector{}

func (serviceVersionDetector) Detect(ctx context.Context) (*resource.Resource, error) {
serviceVersion := strings.TrimSpace(os.Getenv("OTEL_SERVICE_VERSION"))
if serviceVersion == "" {
return resource.Empty(), nil
}
return resource.NewSchemaless(semconv.ServiceVersionKey.String(serviceVersion)), nil
}

type setupFunc func(*Config) (func() error, error)
Expand Down
Loading

0 comments on commit 6c48c9a

Please sign in to comment.