From 057e1f4ea5e0c6779ec2f55225d10828349eb908 Mon Sep 17 00:00:00 2001 From: Pablo Baeyens Date: Fri, 28 Oct 2022 13:13:21 +0200 Subject: [PATCH] [pkg/otlp] Make debug section aligned with logging exporter (#14072) * Make debug section aligned with logging exporter * Add missing files --- pkg/config/config_template.yaml | 19 ++- pkg/config/otlp.go | 33 ++--- pkg/otlp/collector.go | 29 ++-- pkg/otlp/config.go | 3 +- pkg/otlp/config_test.go | 126 +++++++++++++++--- pkg/otlp/map_provider.go | 6 +- .../testdata/debug/empty_but_set_debug.yaml | 2 + pkg/otlp/testdata/debug/loglevel_debug.yaml | 3 + .../testdata/debug/loglevel_disabled.yaml | 3 + pkg/otlp/testdata/debug/verbosity_normal.yaml | 3 + .../loglevel-deprecated-e489aa5d4a79fc3e.yaml | 11 ++ 11 files changed, 174 insertions(+), 64 deletions(-) create mode 100644 pkg/otlp/testdata/debug/empty_but_set_debug.yaml create mode 100644 pkg/otlp/testdata/debug/loglevel_debug.yaml create mode 100644 pkg/otlp/testdata/debug/loglevel_disabled.yaml create mode 100644 pkg/otlp/testdata/debug/verbosity_normal.yaml create mode 100644 releasenotes/notes/loglevel-deprecated-e489aa5d4a79fc3e.yaml diff --git a/pkg/config/config_template.yaml b/pkg/config/config_template.yaml index 0c93ce7ee2d76..19b26301e0876 100644 --- a/pkg/config/config_template.yaml +++ b/pkg/config/config_template.yaml @@ -3400,13 +3400,24 @@ api_key: ## @param debug - custom object - optional ## Debug-specific configuration for OTLP ingest in the Datadog Agent. + ## This template lists the most commonly used settings; see the OpenTelemetry Collector documentation + ## for a full list of available settings: + ## https://github.com/open-telemetry/opentelemetry-collector/tree/main/exporter/loggingexporter#getting-started # # debug: - ## @param loglevel - string - optional - default: info - ## @env DD_OTLP_CONFIG_DEBUG_LOGLEVEL - string - optional - default: info - ## Loglevel for logs when Datadog Agent receives otlp traces/metrics. - ## Options are disabled, debug, info, error, warn. + ## Deprecated (v[6/7].41.0) - use `verbosity` instead + ## @param loglevel - string - optional - default: none + ## @env DD_OTLP_CONFIG_DEBUG_LOGLEVEL - string - optional - default: none + ## Verbosity of debug logs when Datadog Agent receives otlp traces/metrics. + ## Valid values are disabled, debug, info, error, warn. # # loglevel: info + + ## @param verbosity - string - optional - default: normal + ## @env DD_OTLP_CONFIG_DEBUG_VERBOSITY - string - optional - default: normal + ## Verbosity of debug logs when Datadog Agent receives otlp traces/metrics. + ## Valid values are basic, normal, detailed. + # + # verbosity: normal {{end -}} diff --git a/pkg/config/otlp.go b/pkg/config/otlp.go index 0389b2fd4472d..8e7b7fd00c12a 100644 --- a/pkg/config/otlp.go +++ b/pkg/config/otlp.go @@ -19,45 +19,27 @@ const ( OTLPTagCardinalityKey = OTLPMetrics + ".tag_cardinality" OTLPDebugKey = "debug" OTLPDebug = OTLPSection + "." + OTLPDebugKey - OTLPDebugLogLevel = OTLPDebug + ".loglevel" ) -// Following consts define log level of the logging exporter. -// see: https://github.com/open-telemetry/opentelemetry-collector/blob/6fb884b2dbdc37ef2e1aea924040822ce38584bd/exporter/loggingexporter/config.go#L27-L28 -const ( - OTLPDebugLogLevelDisabled = "disabled" - OTLPDebugLogLevelDebug = "debug" - OTLPDebugLogLevelInfo = "info" - OTLPDebugLogLevelWarn = "warn" - OTLPDebugLogLevelError = "error" -) - -// OTLPDebugLogLevelMap TODO -var OTLPDebugLogLevelMap = map[string]struct{}{ - OTLPDebugLogLevelDisabled: {}, - OTLPDebugLogLevelDebug: {}, - OTLPDebugLogLevelInfo: {}, - OTLPDebugLogLevelWarn: {}, - OTLPDebugLogLevelError: {}, -} - // SetupOTLP related configuration. func SetupOTLP(config Config) { config.BindEnvAndSetDefault(OTLPTracePort, 5003) config.BindEnvAndSetDefault(OTLPMetricsEnabled, true) config.BindEnvAndSetDefault(OTLPTracesEnabled, true) - config.BindEnvAndSetDefault(OTLPDebugLogLevel, "info") // NOTE: This only partially works. // The environment variable is also manually checked in pkg/otlp/config.go config.BindEnvAndSetDefault(OTLPTagCardinalityKey, "low", "DD_OTLP_TAG_CARDINALITY") config.SetKnown(OTLPMetrics) - // Set all subkeys of otlp.metrics as known + // Set all subkeys of otlp_config.metrics as known config.SetKnown(OTLPMetrics + ".*") config.SetKnown(OTLPReceiverSection) - // Set all subkeys of otlp.receiver as known + // Set all subkeys of otlp_config.receiver as known config.SetKnown(OTLPReceiverSection + ".*") + config.SetKnown(OTLPDebug) + // Set all subkeys of otlp_config.debug as known + config.SetKnown(OTLPDebug + ".*") // set environment variables for selected fields setupOTLPEnvironmentVariables(config) @@ -100,6 +82,7 @@ func setupOTLPEnvironmentVariables(config Config) { config.BindEnv(OTLPSection + ".metrics.sums.cumulative_monotonic_mode") config.BindEnv(OTLPSection + ".metrics.summaries.mode") - // Debug setting - config.BindEnv(OTLPSection + ".debug.loglevel") + // Debug settings + config.BindEnv(OTLPSection + ".debug.loglevel") // Deprecated + config.BindEnv(OTLPSection + ".debug.verbosity") } diff --git a/pkg/otlp/collector.go b/pkg/otlp/collector.go index e1b2eb70a85a1..6025abb43be5a 100644 --- a/pkg/otlp/collector.go +++ b/pkg/otlp/collector.go @@ -100,23 +100,34 @@ type PipelineConfig struct { TracesEnabled bool // Debug contains debug configurations. Debug map[string]interface{} - // Metrics contains configuration options for the serializer metrics exporter Metrics map[string]interface{} } -// DebugLogEnabled returns whether debug logging is enabled. If invalid loglevel value is set, -// it assume debug logging is disabled. -func (p *PipelineConfig) DebugLogEnabled() bool { +// valid values for debug log level. +var debugLogLevelMap = map[string]struct{}{ + "disabled": {}, + "debug": {}, + "info": {}, + "warn": {}, + "error": {}, +} + +// shouldSetLoggingSection returns whether debug logging is enabled. +// If an invalid loglevel value is set, it assumes debug logging is disabled. +// If the special 'disabled' value is set, it returns false. +// Otherwise it returns true and lets the Collector handle the rest. +func (p *PipelineConfig) shouldSetLoggingSection() bool { + // Legacy behavior: keep it so that we support `loglevel: disabled`. if v, ok := p.Debug["loglevel"]; ok { if s, ok := v.(string); ok { - _, ok := config.OTLPDebugLogLevelMap[s] - if ok { - return s != config.OTLPDebugLogLevelDisabled - } + _, ok := debugLogLevelMap[s] + return ok && s != "disabled" } } - return false + + // If the legacy behavior does not apply, we always want to set the logging section. + return true } // Pipeline is an OTLP pipeline. diff --git a/pkg/otlp/config.go b/pkg/otlp/config.go index 5d5e71c8c244b..ee9bc21100ff0 100644 --- a/pkg/otlp/config.go +++ b/pkg/otlp/config.go @@ -83,6 +83,7 @@ func FromAgentConfig(cfg config.Config) (PipelineConfig, error) { errs = append(errs, fmt.Errorf("at least one OTLP signal needs to be enabled")) } metricsConfig := readConfigSection(cfg, config.OTLPMetrics) + debugConfig := readConfigSection(cfg, config.OTLPDebug) return PipelineConfig{ OTLPReceiverConfig: otlpConfig.ToStringMap(), @@ -90,7 +91,7 @@ func FromAgentConfig(cfg config.Config) (PipelineConfig, error) { MetricsEnabled: metricsEnabled, TracesEnabled: tracesEnabled, Metrics: metricsConfig.ToStringMap(), - Debug: map[string]interface{}{"loglevel": cfg.GetString(config.OTLPDebugLogLevel)}, + Debug: debugConfig.ToStringMap(), }, multierr.Combine(errs...) } diff --git a/pkg/otlp/config_test.go b/pkg/otlp/config_test.go index bb459a36743ae..8980d1afdfc58 100644 --- a/pkg/otlp/config_test.go +++ b/pkg/otlp/config_test.go @@ -62,9 +62,7 @@ func TestFromAgentConfigReceiver(t *testing.T) { "enabled": true, "tag_cardinality": "low", }, - Debug: map[string]interface{}{ - "loglevel": "info", - }, + Debug: map[string]interface{}{}, }, }, { @@ -83,9 +81,7 @@ func TestFromAgentConfigReceiver(t *testing.T) { "enabled": true, "tag_cardinality": "low", }, - Debug: map[string]interface{}{ - "loglevel": "info", - }, + Debug: map[string]interface{}{}, }, }, { @@ -104,9 +100,7 @@ func TestFromAgentConfigReceiver(t *testing.T) { "enabled": true, "tag_cardinality": "low", }, - Debug: map[string]interface{}{ - "loglevel": "info", - }, + Debug: map[string]interface{}{}, }, }, { @@ -140,9 +134,7 @@ func TestFromAgentConfigReceiver(t *testing.T) { "enabled": true, "tag_cardinality": "low", }, - Debug: map[string]interface{}{ - "loglevel": "info", - }, + Debug: map[string]interface{}{}, }, }, } @@ -188,9 +180,7 @@ func TestFromEnvironmentVariables(t *testing.T) { "enabled": true, "tag_cardinality": "low", }, - Debug: map[string]interface{}{ - "loglevel": "info", - }, + Debug: map[string]interface{}{}, }, }, { @@ -217,9 +207,7 @@ func TestFromEnvironmentVariables(t *testing.T) { "enabled": true, "tag_cardinality": "low", }, - Debug: map[string]interface{}{ - "loglevel": "info", - }, + Debug: map[string]interface{}{}, }, }, { @@ -254,9 +242,7 @@ func TestFromEnvironmentVariables(t *testing.T) { "mode": "counters", }, }, - Debug: map[string]interface{}{ - "loglevel": "info", - }, + Debug: map[string]interface{}{}, }, }, { @@ -285,6 +271,32 @@ func TestFromEnvironmentVariables(t *testing.T) { }, }, }, + { + name: "only gRPC, verbosity normal", + env: map[string]string{ + "DD_OTLP_CONFIG_RECEIVER_PROTOCOLS_GRPC_ENDPOINT": "0.0.0.0:9999", + "DD_OTLP_CONFIG_DEBUG_VERBOSITY": "normal", + }, + cfg: PipelineConfig{ + OTLPReceiverConfig: map[string]interface{}{ + "protocols": map[string]interface{}{ + "grpc": map[string]interface{}{ + "endpoint": "0.0.0.0:9999", + }, + }, + }, + MetricsEnabled: true, + TracesEnabled: true, + TracePort: 5003, + Metrics: map[string]interface{}{ + "enabled": true, + "tag_cardinality": "low", + }, + Debug: map[string]interface{}{ + "verbosity": "normal", + }, + }, + }, } for _, testInstance := range tests { t.Run(testInstance.name, func(t *testing.T) { @@ -348,3 +360,75 @@ func TestFromAgentConfigMetrics(t *testing.T) { }) } } + +func TestFromAgentConfigDebug(t *testing.T) { + tests := []struct { + path string + cfg PipelineConfig + shouldSet bool + err string + }{ + { + path: "debug/empty_but_set_debug.yaml", + shouldSet: true, + cfg: PipelineConfig{ + OTLPReceiverConfig: map[string]interface{}{}, + TracePort: 5003, + MetricsEnabled: true, + TracesEnabled: true, + Debug: map[string]interface{}{}, + Metrics: map[string]interface{}{"enabled": true, "tag_cardinality": "low"}, + }, + }, + { + path: "debug/loglevel_debug.yaml", + shouldSet: true, + cfg: PipelineConfig{ + OTLPReceiverConfig: map[string]interface{}{}, + TracePort: 5003, + MetricsEnabled: true, + TracesEnabled: true, + Debug: map[string]interface{}{"loglevel": "debug"}, + Metrics: map[string]interface{}{"enabled": true, "tag_cardinality": "low"}, + }, + }, + { + path: "debug/loglevel_disabled.yaml", + shouldSet: false, + cfg: PipelineConfig{ + OTLPReceiverConfig: map[string]interface{}{}, + TracePort: 5003, + MetricsEnabled: true, + TracesEnabled: true, + Debug: map[string]interface{}{"loglevel": "disabled"}, + Metrics: map[string]interface{}{"enabled": true, "tag_cardinality": "low"}, + }, + }, + { + path: "debug/verbosity_normal.yaml", + shouldSet: true, + cfg: PipelineConfig{ + OTLPReceiverConfig: map[string]interface{}{}, + TracePort: 5003, + MetricsEnabled: true, + TracesEnabled: true, + Debug: map[string]interface{}{"verbosity": "normal"}, + Metrics: map[string]interface{}{"enabled": true, "tag_cardinality": "low"}, + }, + }, + } + + for _, testInstance := range tests { + t.Run(testInstance.path, func(t *testing.T) { + cfg, err := testutil.LoadConfig("./testdata/" + testInstance.path) + require.NoError(t, err) + pcfg, err := FromAgentConfig(cfg) + if err != nil || testInstance.err != "" { + assert.Equal(t, testInstance.err, err.Error()) + } else { + assert.Equal(t, testInstance.cfg, pcfg) + assert.Equal(t, testInstance.shouldSet, pcfg.shouldSetLoggingSection()) + } + }) + } +} diff --git a/pkg/otlp/map_provider.go b/pkg/otlp/map_provider.go index 4769849ee320c..0b3fe6a73add1 100644 --- a/pkg/otlp/map_provider.go +++ b/pkg/otlp/map_provider.go @@ -122,12 +122,10 @@ func buildMap(cfg PipelineConfig) (*confmap.Conf, error) { err = retMap.Merge(metricsMap) errs = append(errs, err) } - if cfg.DebugLogEnabled() { + if cfg.shouldSetLoggingSection() { m := map[string]interface{}{ "exporters": map[string]interface{}{ - "logging": map[string]interface{}{ - "loglevel": cfg.Debug["loglevel"], - }, + "logging": cfg.Debug, }, } if cfg.MetricsEnabled { diff --git a/pkg/otlp/testdata/debug/empty_but_set_debug.yaml b/pkg/otlp/testdata/debug/empty_but_set_debug.yaml new file mode 100644 index 0000000000000..2dce7223d7bbb --- /dev/null +++ b/pkg/otlp/testdata/debug/empty_but_set_debug.yaml @@ -0,0 +1,2 @@ +otlp_config: + debug: diff --git a/pkg/otlp/testdata/debug/loglevel_debug.yaml b/pkg/otlp/testdata/debug/loglevel_debug.yaml new file mode 100644 index 0000000000000..48df64859c790 --- /dev/null +++ b/pkg/otlp/testdata/debug/loglevel_debug.yaml @@ -0,0 +1,3 @@ +otlp_config: + debug: + loglevel: debug diff --git a/pkg/otlp/testdata/debug/loglevel_disabled.yaml b/pkg/otlp/testdata/debug/loglevel_disabled.yaml new file mode 100644 index 0000000000000..92576a13e2c0a --- /dev/null +++ b/pkg/otlp/testdata/debug/loglevel_disabled.yaml @@ -0,0 +1,3 @@ +otlp_config: + debug: + loglevel: disabled diff --git a/pkg/otlp/testdata/debug/verbosity_normal.yaml b/pkg/otlp/testdata/debug/verbosity_normal.yaml new file mode 100644 index 0000000000000..6f50cd5cab824 --- /dev/null +++ b/pkg/otlp/testdata/debug/verbosity_normal.yaml @@ -0,0 +1,3 @@ +otlp_config: + debug: + verbosity: normal diff --git a/releasenotes/notes/loglevel-deprecated-e489aa5d4a79fc3e.yaml b/releasenotes/notes/loglevel-deprecated-e489aa5d4a79fc3e.yaml new file mode 100644 index 0000000000000..d0c62ee57fcd1 --- /dev/null +++ b/releasenotes/notes/loglevel-deprecated-e489aa5d4a79fc3e.yaml @@ -0,0 +1,11 @@ +# Each section from every release note are combined when the +# CHANGELOG.rst is rendered. So the text needs to be worded so that +# it does not depend on any information only available in another +# section. This may mean repeating some details, but each section +# must be readable independently of the other. +# +# Each section note must be formatted as reStructuredText. +--- +deprecations: + - | + The ``otlp_config.debug.loglevel`` setting was deprecated in favor of ``otlp_config.debug.verbosity``.