[otel-agent] address broken log level assignment (#28743)

cmd/otel-agent/config/agent_config.go

@@ -25,6 +25,40 @@ import (
 	"go.opentelemetry.io/collector/service"
 )
 
+type logLevel int
+
+const (
+	trace logLevel = iota - 1
+	debug
+	info
+	warn
+	err
+	critical
+	off
+)
+
+// datadog agent log levels: trace, debug, info, warn, error, critical, and off
+// otel log levels: disabled, debug, info, warn, error
+var logLevelMap = map[string]logLevel{
+	"off":      off,
+	"disabled": off,
+	"trace":    trace,
+	"debug":    debug,
+	"info":     info,
+	"warn":     warn,
+	"error":    err,
+	"critical": critical,
+}
+
+var logLevelReverseMap = func(src map[string]logLevel) map[logLevel]string {
+	reverse := map[logLevel]string{}
+	for k, v := range src {
+		reverse[v] = k
+	}
+
+	return reverse
+}(logLevelMap)
+
 // NewConfigComponent creates a new config component from the given URIs
 func NewConfigComponent(ctx context.Context, ddCfg string, uris []string) (config.Component, error) {
 	// Load the configuration from the fileName
@@ -60,6 +94,13 @@ func NewConfigComponent(ctx context.Context, ddCfg string, uris []string) (confi
 	apiKey := string(ddc.API.Key)
 	// Set the global agent config
 	pkgconfig := pkgconfigsetup.Datadog()
+
+	pkgconfig.SetConfigName("OTel")
+	pkgconfig.SetEnvPrefix("DD")
+	pkgconfig.SetEnvKeyReplacer(strings.NewReplacer(".", "_"))
+	pkgconfig.BindEnvAndSetDefault("log_level", "info")
+
+	activeLogLevel := critical
 	if len(ddCfg) != 0 {
 		// if the configuration file path was supplied via CLI flags or env vars,
 		// add that first so it's first in line
@@ -73,49 +114,63 @@ func NewConfigComponent(ctx context.Context, ddCfg string, uris []string) (confi
 		if err != nil {
 			return nil, err
 		}
+		var ok bool
+		activeLogLevel, ok = logLevelMap[pkgconfig.GetString("log_level")]
+		if !ok {
+			return nil, fmt.Errorf("invalid log level (%v) set in the Datadog Agent configuration", pkgconfig.GetString("log_level"))
+		}
 	}
 
-	pkgconfig.SetConfigName("OTel")
-	pkgconfig.SetEnvPrefix("DD")
-	pkgconfig.SetEnvKeyReplacer(strings.NewReplacer(".", "_"))
+	// Set the right log level. The most verbose setting takes precedence.
+	telemetryLogLevel := sc.Telemetry.Logs.Level
+	telemetryLogMapping, ok := logLevelMap[telemetryLogLevel.String()]
+	if !ok {
+		return nil, fmt.Errorf("invalid log level (%v) set in the OTel Telemetry configuration", telemetryLogLevel.String())
+	}
+	if telemetryLogMapping < activeLogLevel {
+		activeLogLevel = telemetryLogMapping
+	}
 
 	// Override config read (if any) with Default values
 	pkgconfigsetup.InitConfig(pkgconfig)
-	pkgconfig.Set("api_key", apiKey, pkgconfigmodel.SourceLocalConfigProcess)
-	pkgconfig.Set("site", site, pkgconfigmodel.SourceLocalConfigProcess)
+	pkgconfigmodel.ApplyOverrideFuncs(pkgconfig)
+
+	pkgconfig.Set("log_level", logLevelReverseMap[activeLogLevel], pkgconfigmodel.SourceFile)
+
+	pkgconfig.Set("api_key", apiKey, pkgconfigmodel.SourceFile)
+	pkgconfig.Set("site", site, pkgconfigmodel.SourceFile)
 
-	pkgconfig.Set("dd_url", ddc.Metrics.Endpoint, pkgconfigmodel.SourceLocalConfigProcess)
+	pkgconfig.Set("dd_url", ddc.Metrics.Endpoint, pkgconfigmodel.SourceFile)
 
 	// Log configs
-	pkgconfig.Set("logs_enabled", true, pkgconfigmodel.SourceLocalConfigProcess)
-	pkgconfig.Set("logs_config.force_use_http", true, pkgconfigmodel.SourceLocalConfigProcess)
-	pkgconfig.Set("logs_config.logs_dd_url", ddc.Logs.Endpoint, pkgconfigmodel.SourceLocalConfigProcess)
-	pkgconfig.Set("logs_config.batch_wait", ddc.Logs.BatchWait, pkgconfigmodel.SourceLocalConfigProcess)
-	pkgconfig.Set("logs_config.use_compression", ddc.Logs.UseCompression, pkgconfigmodel.SourceLocalConfigProcess)
-	pkgconfig.Set("logs_config.compression_level", ddc.Logs.CompressionLevel, pkgconfigmodel.SourceLocalConfigProcess)
-	pkgconfig.Set("log_level", sc.Telemetry.Logs.Level, pkgconfigmodel.SourceLocalConfigProcess)
+	pkgconfig.Set("logs_enabled", true, pkgconfigmodel.SourceDefault)
+	pkgconfig.Set("logs_config.force_use_http", true, pkgconfigmodel.SourceDefault)
+	pkgconfig.Set("logs_config.logs_dd_url", ddc.Logs.Endpoint, pkgconfigmodel.SourceFile)
+	pkgconfig.Set("logs_config.batch_wait", ddc.Logs.BatchWait, pkgconfigmodel.SourceFile)
+	pkgconfig.Set("logs_config.use_compression", ddc.Logs.UseCompression, pkgconfigmodel.SourceFile)
+	pkgconfig.Set("logs_config.compression_level", ddc.Logs.CompressionLevel, pkgconfigmodel.SourceFile)
 
 	// APM & OTel trace configs
-	pkgconfig.Set("apm_config.enabled", true, pkgconfigmodel.SourceLocalConfigProcess)
-	pkgconfig.Set("apm_config.apm_non_local_traffic", true, pkgconfigmodel.SourceLocalConfigProcess)
+	pkgconfig.Set("apm_config.enabled", true, pkgconfigmodel.SourceDefault)
+	pkgconfig.Set("apm_config.apm_non_local_traffic", true, pkgconfigmodel.SourceDefault)
 
-	pkgconfig.Set("apm_config.debug.port", 0, pkgconfigmodel.SourceLocalConfigProcess)      // Disabled in the otel-agent
-	pkgconfig.Set(pkgconfigsetup.OTLPTracePort, 0, pkgconfigmodel.SourceLocalConfigProcess) // Disabled in the otel-agent
+	pkgconfig.Set("apm_config.debug.port", 0, pkgconfigmodel.SourceDefault)      // Disabled in the otel-agent
+	pkgconfig.Set(pkgconfigsetup.OTLPTracePort, 0, pkgconfigmodel.SourceDefault) // Disabled in the otel-agent
 
-	pkgconfig.Set("otlp_config.traces.span_name_as_resource_name", ddc.Traces.SpanNameAsResourceName, pkgconfigmodel.SourceLocalConfigProcess)
-	pkgconfig.Set("otlp_config.traces.span_name_remappings", ddc.Traces.SpanNameRemappings, pkgconfigmodel.SourceLocalConfigProcess)
+	pkgconfig.Set("otlp_config.traces.span_name_as_resource_name", ddc.Traces.SpanNameAsResourceName, pkgconfigmodel.SourceFile)
+	pkgconfig.Set("otlp_config.traces.span_name_remappings", ddc.Traces.SpanNameRemappings, pkgconfigmodel.SourceFile)
 
-	pkgconfig.Set("apm_config.receiver_enabled", false, pkgconfigmodel.SourceLocalConfigProcess) // disable HTTP receiver
-	pkgconfig.Set("apm_config.ignore_resources", ddc.Traces.IgnoreResources, pkgconfigmodel.SourceLocalConfigProcess)
-	pkgconfig.Set("apm_config.skip_ssl_validation", ddc.ClientConfig.TLSSetting.InsecureSkipVerify, pkgconfigmodel.SourceLocalConfigProcess)
+	pkgconfig.Set("apm_config.receiver_enabled", false, pkgconfigmodel.SourceDefault) // disable HTTP receiver
+	pkgconfig.Set("apm_config.ignore_resources", ddc.Traces.IgnoreResources, pkgconfigmodel.SourceFile)
+	pkgconfig.Set("apm_config.skip_ssl_validation", ddc.ClientConfig.TLSSetting.InsecureSkipVerify, pkgconfigmodel.SourceFile)
 	if v := ddc.Traces.TraceBuffer; v > 0 {
-		pkgconfig.Set("apm_config.trace_buffer", v, pkgconfigmodel.SourceLocalConfigProcess)
+		pkgconfig.Set("apm_config.trace_buffer", v, pkgconfigmodel.SourceFile)
 	}
 	if addr := ddc.Traces.Endpoint; addr != "" {
-		pkgconfig.Set("apm_config.apm_dd_url", addr, pkgconfigmodel.SourceLocalConfigProcess)
+		pkgconfig.Set("apm_config.apm_dd_url", addr, pkgconfigmodel.SourceFile)
 	}
 	if ddc.Traces.ComputeTopLevelBySpanKind {
-		pkgconfig.Set("apm_config.features", []string{"enable_otlp_compute_top_level_by_span_kind"}, pkgconfigmodel.SourceLocalConfigProcess)
+		pkgconfig.Set("apm_config.features", []string{"enable_otlp_compute_top_level_by_span_kind"}, pkgconfigmodel.SourceFile)
 	}
 
 	return pkgconfig, nil

cmd/otel-agent/config/agent_config_test.go

@@ -7,12 +7,28 @@ package config
 
 import (
 	"context"
+	"os"
+	"strings"
 	"testing"
 
+	pkgconfigmodel "github.com/DataDog/datadog-agent/pkg/config/model"
+	pkgconfigsetup "github.com/DataDog/datadog-agent/pkg/config/setup"
+
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/suite"
 )
 
-func TestAgentConfig(t *testing.T) {
+type ConfigTestSuite struct {
+	suite.Suite
+}
+
+func (suite *ConfigTestSuite) SetupTest() {
+	datadog := pkgconfigmodel.NewConfig("datadog", "DD", strings.NewReplacer(".", "_"))
+	pkgconfigsetup.SetDatadog(datadog)
+}
+
+func (suite *ConfigTestSuite) TestAgentConfig() {
+	t := suite.T()
 	fileName := "testdata/config.yaml"
 	c, err := NewConfigComponent(context.Background(), "", []string{fileName})
 	if err != nil {
@@ -37,7 +53,8 @@ func TestAgentConfig(t *testing.T) {
 	assert.Equal(t, nil, c.Get("apm_config.features"))
 }
 
-func TestAgentConfigDefaults(t *testing.T) {
+func (suite *ConfigTestSuite) TestAgentConfigDefaults() {
+	t := suite.T()
 	fileName := "testdata/config_default.yaml"
 	c, err := NewConfigComponent(context.Background(), "", []string{fileName})
 	if err != nil {
@@ -58,14 +75,131 @@ func TestAgentConfigDefaults(t *testing.T) {
 	assert.Equal(t, []string{"enable_otlp_compute_top_level_by_span_kind"}, c.Get("apm_config.features"))
 }
 
-func TestNoDDExporter(t *testing.T) {
+func (suite *ConfigTestSuite) TestAgentConfigWithDatadogYamlDefaults() {
+	t := suite.T()
+	fileName := "testdata/config_default.yaml"
+	ddFileName := "testdata/datadog.yaml"
+	c, err := NewConfigComponent(context.Background(), ddFileName, []string{fileName})
+	if err != nil {
+		t.Errorf("Failed to load agent config: %v", err)
+	}
+
+	// all expected defaults
+	assert.Equal(t, "DATADOG_API_KEY", c.Get("api_key"))
+	assert.Equal(t, "datadoghq.com", c.Get("site"))
+	assert.Equal(t, "https://api.datadoghq.com", c.Get("dd_url"))
+	assert.Equal(t, true, c.Get("logs_enabled"))
+	assert.Equal(t, "https://agent-http-intake.logs.datadoghq.com", c.Get("logs_config.logs_dd_url"))
+	assert.Equal(t, 5, c.Get("logs_config.batch_wait"))
+	assert.Equal(t, true, c.Get("logs_config.use_compression"))
+	assert.Equal(t, true, c.Get("logs_config.force_use_http"))
+	assert.Equal(t, 6, c.Get("logs_config.compression_level"))
+	assert.Equal(t, "https://trace.agent.datadoghq.com", c.Get("apm_config.apm_dd_url"))
+	assert.Equal(t, false, c.Get("apm_config.receiver_enabled"))
+	assert.Equal(t, true, c.Get("otlp_config.traces.span_name_as_resource_name"))
+	assert.Equal(t, []string{"enable_otlp_compute_top_level_by_span_kind"}, c.Get("apm_config.features"))
+
+	// log_level from datadog.yaml takes precedence -> more verbose
+	assert.Equal(t, "debug", c.Get("log_level"))
+}
+
+func (suite *ConfigTestSuite) TestAgentConfigWithDatadogYamlKeysAvailable() {
+	t := suite.T()
+	fileName := "testdata/config_default.yaml"
+	ddFileName := "testdata/datadog.yaml"
+	c, err := NewConfigComponent(context.Background(), ddFileName, []string{fileName})
+	if err != nil {
+		t.Errorf("Failed to load agent config: %v", err)
+	}
+
+	// log_level from datadog.yaml takes precedence -> more verbose
+	assert.Equal(t, "debug", c.Get("log_level"))
+	assert.True(t, c.GetBool("otelcollector.enabled"))
+	assert.Equal(t, "https://localhost:7777", c.GetString("otelcollector.extension_url"))
+	assert.Equal(t, 5009, c.GetInt("agent_ipc.port"))
+	assert.Equal(t, 60, c.GetInt("agent_ipc.config_refresh_interval"))
+}
+
+func (suite *ConfigTestSuite) TestLogLevelPrecedence() {
+	t := suite.T()
+	fileName := "testdata/config_default.yaml"
+	ddFileName := "testdata/datadog_low_log_level.yaml"
+	c, err := NewConfigComponent(context.Background(), ddFileName, []string{fileName})
+	if err != nil {
+		t.Errorf("Failed to load agent config: %v", err)
+	}
+
+	// log_level from service config takes precedence -> more verbose
+	// ddFlleName configures level warn, Telemetry defaults to info
+	assert.Equal(t, "info", c.Get("log_level"))
+}
+
+func (suite *ConfigTestSuite) TestEnvLogLevelPrecedence() {
+	t := suite.T()
+	oldval, exists := os.LookupEnv("DD_LOG_LEVEL")
+	os.Setenv("DD_LOG_LEVEL", "debug")
+	defer func() {
+		if !exists {
+			os.Unsetenv("DD_LOG_LEVEL")
+		} else {
+			os.Setenv("DD_LOG_LEVEL", oldval)
+		}
+	}()
+	fileName := "testdata/config_default.yaml"
+	ddFileName := "testdata/datadog_low_log_level.yaml"
+	c, err := NewConfigComponent(context.Background(), ddFileName, []string{fileName})
+	if err != nil {
+		t.Errorf("Failed to load agent config: %v", err)
+	}
+
+	// log_level from service config takes precedence -> more verbose
+	// ddFlleName configures level warn, Telemetry defaults to info, env sets debug
+	assert.Equal(t, "debug", c.Get("log_level"))
+}
+
+func (suite *ConfigTestSuite) TestEnvBadLogLevel() {
+	t := suite.T()
+	oldval, exists := os.LookupEnv("DD_LOG_LEVEL")
+	os.Setenv("DD_LOG_LEVEL", "yabadabadooo")
+	defer func() {
+		if !exists {
+			os.Unsetenv("DD_LOG_LEVEL")
+		} else {
+			os.Setenv("DD_LOG_LEVEL", oldval)
+		}
+	}()
+	fileName := "testdata/config_default.yaml"
+	ddFileName := "testdata/datadog_low_log_level.yaml"
+	_, err := NewConfigComponent(context.Background(), ddFileName, []string{fileName})
+	assert.Error(t, err)
+}
+
+func (suite *ConfigTestSuite) TestBadLogLevel() {
+	t := suite.T()
+	fileName := "testdata/config_default.yaml"
+	ddFileName := "testdata/datadog_bad_log_level.yaml"
+	_, err := NewConfigComponent(context.Background(), ddFileName, []string{fileName})
+
+	// log_level from service config takes precedence -> more verbose
+	// ddFlleName configures level warn, Telemetry defaults to info
+	assert.Error(t, err)
+}
+
+func (suite *ConfigTestSuite) TestNoDDExporter() {
+	t := suite.T()
 	fileName := "testdata/config_no_dd_exporter.yaml"
 	_, err := NewConfigComponent(context.Background(), "", []string{fileName})
 	assert.EqualError(t, err, "no datadog exporter found")
 }
 
-func TestMultipleDDExporters(t *testing.T) {
+func (suite *ConfigTestSuite) TestMultipleDDExporters() {
+	t := suite.T()
 	fileName := "testdata/config_multiple_dd_exporters.yaml"
 	_, err := NewConfigComponent(context.Background(), "", []string{fileName})
 	assert.EqualError(t, err, "multiple datadog exporters found")
 }
+
+// TestSuite runs the CalculatorTestSuite
+func TestSuite(t *testing.T) {
+	suite.Run(t, new(ConfigTestSuite))
+}

cmd/otel-agent/config/testdata/datadog.yaml

@@ -0,0 +1,12 @@
+api_key: deadbeef
+
+log_level: debug
+
+otelcollector:
+  enabled: true
+  extension_url: "https://localhost:7777"
+
+agent_ipc:
+  port: 5009
+  config_refresh_interval: 60
+

cmd/otel-agent/config/testdata/datadog_low_log_level.yaml

@@ -0,0 +1,10 @@
+log_level: warn
+
+otelcollector:
+  enabled: true
+  extension_url: "https://localhost:7777"
+
+agent_ipc:
+  port: 5009
+  config_refresh_interval: 60
+