Skip to content

Commit

Permalink
refactor isEnabled in separate package configcheck
Browse files Browse the repository at this point in the history
  • Loading branch information
mackjmr committed Nov 27, 2024
1 parent 6033a62 commit 8398c38
Show file tree
Hide file tree
Showing 9 changed files with 100 additions and 80 deletions.
4 changes: 2 additions & 2 deletions comp/metadata/host/hostimpl/utils/host.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (
"time"

"github.com/DataDog/datadog-agent/comp/metadata/host/hostimpl/hosttags"
"github.com/DataDog/datadog-agent/comp/otelcol/otlp"
"github.com/DataDog/datadog-agent/comp/otelcol/otlp/configcheck"
"github.com/DataDog/datadog-agent/pkg/collector/python"
"github.com/DataDog/datadog-agent/pkg/config/model"
"github.com/DataDog/datadog-agent/pkg/logs/status"
Expand All @@ -36,7 +36,7 @@ var (
hostInfoCacheKey = cache.BuildAgentKey("host", "utils", "hostInfo")

// for testing
otlpIsEnabled = otlp.IsEnabled
otlpIsEnabled = configcheck.IsEnabled
installinfoGet = installinfo.Get
)

Expand Down
3 changes: 2 additions & 1 deletion comp/otelcol/collector/impl-pipeline/pipeline.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
collector "github.com/DataDog/datadog-agent/comp/otelcol/collector/def"
"github.com/DataDog/datadog-agent/comp/otelcol/logsagentpipeline"
"github.com/DataDog/datadog-agent/comp/otelcol/otlp"
"github.com/DataDog/datadog-agent/comp/otelcol/otlp/configcheck"
"github.com/DataDog/datadog-agent/comp/otelcol/otlp/datatype"
apiutil "github.com/DataDog/datadog-agent/pkg/api/util"
"github.com/DataDog/datadog-agent/pkg/logs/message"
Expand Down Expand Up @@ -87,7 +88,7 @@ type collectorImpl struct {
}

func (c *collectorImpl) start(context.Context) error {
on := otlp.IsEnabled(c.config)
on := configcheck.IsEnabled(c.config)
c.inventoryAgent.Set(otlpEnabled, on)
if !on {
return nil
Expand Down
3 changes: 2 additions & 1 deletion comp/otelcol/collector/impl-pipeline/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (

"github.com/DataDog/datadog-agent/comp/core/status"
"github.com/DataDog/datadog-agent/comp/otelcol/otlp"
"github.com/DataDog/datadog-agent/comp/otelcol/otlp/configcheck"
)

//go:embed status_templates
Expand All @@ -28,7 +29,7 @@ func (c *collectorImpl) getStatusInfo() map[string]interface{} {

func (c *collectorImpl) populateStatus(stats map[string]interface{}) {
otlpStatus := make(map[string]interface{})
otlpIsEnabled := otlp.IsEnabled(c.config)
otlpIsEnabled := configcheck.IsEnabled(c.config)

var otlpCollectorStatus otlp.CollectorStatus

Expand Down
74 changes: 4 additions & 70 deletions comp/otelcol/otlp/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,11 @@ import (
"fmt"
"strings"

"github.com/mohae/deepcopy"
"go.opentelemetry.io/collector/confmap"
"go.uber.org/multierr"

"github.com/DataDog/datadog-agent/comp/core/config"
"github.com/DataDog/datadog-agent/comp/otelcol/otlp/components/exporter/serializerexporter"
"github.com/DataDog/datadog-agent/comp/otelcol/otlp/configcheck"
coreconfig "github.com/DataDog/datadog-agent/pkg/config/setup"
"github.com/DataDog/datadog-agent/pkg/util"
"github.com/go-viper/mapstructure/v2"
Expand All @@ -31,50 +30,10 @@ func portToUint(v int) (port uint, err error) {
return
}

// readConfigSection from a config.Component object.
func readConfigSection(cfg config.Reader, section string) *confmap.Conf {
// Viper doesn't work well when getting subsections, since it
// ignores environment variables and nil-but-present sections.
// To work around this, we do the following two steps:

// Step one works around https://github.com/spf13/viper/issues/819
// If we only had the stuff below, the nil sections would be ignored.
// We want to take into account nil-but-present sections.
//
// Furthermore, Viper returns an `interface{}` nil in the case where
// `section` is present but empty: e.g. we want to read
// "otlp_config.receiver", but we have
//
// otlp_config:
// receiver:
//
// `GetStringMap` it will fail to cast `interface{}` nil to
// `map[string]interface{}` nil; we use `Get` and cast manually.
rawVal := cfg.Get(section)
stringMap := map[string]interface{}{}
if val, ok := rawVal.(map[string]interface{}); ok {
// deep copy since `cfg.Get` returns a reference
stringMap = deepcopy.Copy(val).(map[string]interface{})
}

// Step two works around https://github.com/spf13/viper/issues/1012
// we check every key manually, and if it belongs to the OTLP receiver section,
// we set it. We need to do this to account for environment variable values.
prefix := section + "."
for _, key := range cfg.AllKeysLowercased() {
if strings.HasPrefix(key, prefix) && cfg.IsSet(key) {
mapKey := strings.ReplaceAll(key[len(prefix):], ".", confmap.KeyDelimiter)
// deep copy since `cfg.Get` returns a reference
stringMap[mapKey] = deepcopy.Copy(cfg.Get(key))
}
}
return confmap.NewFromStringMap(stringMap)
}

// FromAgentConfig builds a pipeline configuration from an Agent configuration.
func FromAgentConfig(cfg config.Reader) (PipelineConfig, error) {
var errs []error
otlpConfig := readConfigSection(cfg, coreconfig.OTLPReceiverSection)
otlpConfig := configcheck.ReadConfigSection(cfg, coreconfig.OTLPReceiverSection)
tracePort, err := portToUint(cfg.GetInt(coreconfig.OTLPTracePort))
if err != nil {
errs = append(errs, fmt.Errorf("internal trace port is invalid: %w", err))
Expand All @@ -85,7 +44,7 @@ func FromAgentConfig(cfg config.Reader) (PipelineConfig, error) {
if !metricsEnabled && !tracesEnabled && !logsEnabled {
errs = append(errs, fmt.Errorf("at least one OTLP signal needs to be enabled"))
}
metricsConfig := readConfigSection(cfg, coreconfig.OTLPMetrics)
metricsConfig := configcheck.ReadConfigSection(cfg, coreconfig.OTLPMetrics)
metricsConfigMap := metricsConfig.ToStringMap()

if _, ok := metricsConfigMap["apm_stats_receiver_addr"]; !ok {
Expand All @@ -101,7 +60,7 @@ func FromAgentConfig(cfg config.Reader) (PipelineConfig, error) {
errs = append(errs, fmt.Errorf("failed to normalize metrics config: %w", err))
}

debugConfig := readConfigSection(cfg, coreconfig.OTLPDebug)
debugConfig := configcheck.ReadConfigSection(cfg, coreconfig.OTLPDebug)

return PipelineConfig{
OTLPReceiverConfig: otlpConfig.ToStringMap(),
Expand Down Expand Up @@ -141,28 +100,3 @@ func normalizeMetricsConfig(metricsConfigMap map[string]interface{}, strict bool
}
return mc, nil
}

// IsEnabled checks if OTLP pipeline is enabled in a given config.
func IsEnabled(cfg config.Reader) bool {
return hasSection(cfg, coreconfig.OTLPReceiverSubSectionKey)
}

// HasLogsSectionEnabled checks if OTLP logs are explicitly enabled in a given config.
func HasLogsSectionEnabled(cfg config.Reader) bool {
return hasSection(cfg, coreconfig.OTLPLogsEnabled) && cfg.GetBool(coreconfig.OTLPLogsEnabled)
}

func hasSection(cfg config.Reader, section string) bool {
// HACK: We want to mark as enabled if the section is present, even if empty, so that we get errors
// from unmarshaling/validation done by the Collector code.
//
// IsSet won't work here: it will return false if the section is present but empty.
// To work around this, we check if the receiver key is present in the string map, which does the 'correct' thing.
_, ok := readConfigSection(cfg, coreconfig.OTLPSection).ToStringMap()[section]
return ok
}

// IsDisplayed checks if the OTLP section should be rendered in the Agent
func IsDisplayed() bool {
return true
}
5 changes: 3 additions & 2 deletions comp/otelcol/otlp/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/DataDog/datadog-agent/comp/otelcol/otlp/configcheck"
"github.com/DataDog/datadog-agent/comp/otelcol/otlp/testutil"
)

Expand All @@ -32,7 +33,7 @@ func TestIsEnabled(t *testing.T) {
t.Run(testInstance.path, func(t *testing.T) {
cfg, err := testutil.LoadConfig(t, "./testdata/"+testInstance.path)
require.NoError(t, err)
assert.Equal(t, testInstance.enabled, IsEnabled(cfg))
assert.Equal(t, testInstance.enabled, configcheck.IsEnabled(cfg))
})
}
}
Expand All @@ -41,7 +42,7 @@ func TestIsEnabledEnv(t *testing.T) {
t.Setenv("DD_OTLP_CONFIG_RECEIVER_PROTOCOLS_GRPC_ENDPOINT", "0.0.0.0:9993")
cfg, err := testutil.LoadConfig(t, "./testdata/empty.yaml")
require.NoError(t, err)
assert.True(t, IsEnabled(cfg))
assert.True(t, configcheck.IsEnabled(cfg))
}

func TestFromAgentConfigReceiver(t *testing.T) {
Expand Down
81 changes: 81 additions & 0 deletions comp/otelcol/otlp/configcheck/configcheck.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
// Unless explicitly stated otherwise all files in this repository are licensed
// under the Apache License Version 2.0.
// This product includes software developed at Datadog (https://www.datadoghq.com/).
// Copyright 2021-present Datadog, Inc.

package configcheck

import (
"strings"

"github.com/mohae/deepcopy"
"go.opentelemetry.io/collector/confmap"

"github.com/DataDog/datadog-agent/comp/core/config"
coreconfig "github.com/DataDog/datadog-agent/pkg/config/setup"
)

// ReadConfigSection from a config.Component object.
func ReadConfigSection(cfg config.Reader, section string) *confmap.Conf {
// Viper doesn't work well when getting subsections, since it
// ignores environment variables and nil-but-present sections.
// To work around this, we do the following two steps:

// Step one works around https://github.com/spf13/viper/issues/819
// If we only had the stuff below, the nil sections would be ignored.
// We want to take into account nil-but-present sections.
//
// Furthermore, Viper returns an `interface{}` nil in the case where
// `section` is present but empty: e.g. we want to read
// "otlp_config.receiver", but we have
//
// otlp_config:
// receiver:
//
// `GetStringMap` it will fail to cast `interface{}` nil to
// `map[string]interface{}` nil; we use `Get` and cast manually.
rawVal := cfg.Get(section)
stringMap := map[string]interface{}{}
if val, ok := rawVal.(map[string]interface{}); ok {
// deep copy since `cfg.Get` returns a reference
stringMap = deepcopy.Copy(val).(map[string]interface{})
}

// Step two works around https://github.com/spf13/viper/issues/1012
// we check every key manually, and if it belongs to the OTLP receiver section,
// we set it. We need to do this to account for environment variable values.
prefix := section + "."
for _, key := range cfg.AllKeysLowercased() {
if strings.HasPrefix(key, prefix) && cfg.IsSet(key) {
mapKey := strings.ReplaceAll(key[len(prefix):], ".", confmap.KeyDelimiter)
// deep copy since `cfg.Get` returns a reference
stringMap[mapKey] = deepcopy.Copy(cfg.Get(key))
}
}
return confmap.NewFromStringMap(stringMap)
}

// IsEnabled checks if OTLP pipeline is enabled in a given config.
func IsEnabled(cfg config.Reader) bool {
return hasSection(cfg, coreconfig.OTLPReceiverSubSectionKey)
}

// HasLogsSectionEnabled checks if OTLP logs are explicitly enabled in a given config.
func HasLogsSectionEnabled(cfg config.Reader) bool {
return hasSection(cfg, coreconfig.OTLPLogsEnabled) && cfg.GetBool(coreconfig.OTLPLogsEnabled)
}

func hasSection(cfg config.Reader, section string) bool {
// HACK: We want to mark as enabled if the section is present, even if empty, so that we get errors
// from unmarshaling/validation done by the Collector code.
//
// IsSet won't work here: it will return false if the section is present but empty.
// To work around this, we check if the receiver key is present in the string map, which does the 'correct' thing.
_, ok := ReadConfigSection(cfg, coreconfig.OTLPSection).ToStringMap()[section]
return ok
}

// IsDisplayed checks if the OTLP section should be rendered in the Agent
func IsDisplayed() bool {
return true
}
3 changes: 2 additions & 1 deletion comp/otelcol/otlp/pipeline_validator_serverless.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,12 @@ import (
"fmt"

"github.com/DataDog/datadog-agent/comp/core/config"
"github.com/DataDog/datadog-agent/comp/otelcol/otlp/configcheck"
"github.com/DataDog/datadog-agent/pkg/logs/message"
)

func checkAndUpdateCfg(cfg config.Component, pcfg PipelineConfig, logsAgentChannel chan *message.Message) error {
if HasLogsSectionEnabled(cfg) {
if configcheck.HasLogsSectionEnabled(cfg) {
pipelineError.Store(fmt.Errorf("Cannot enable OTLP log ingestion for serverless"))
return pipelineError.Load()
}
Expand Down
4 changes: 2 additions & 2 deletions comp/trace/config/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (
corecompcfg "github.com/DataDog/datadog-agent/comp/core/config"
tagger "github.com/DataDog/datadog-agent/comp/core/tagger/def"
"github.com/DataDog/datadog-agent/comp/core/tagger/types"
"github.com/DataDog/datadog-agent/comp/otelcol/otlp"
"github.com/DataDog/datadog-agent/comp/otelcol/otlp/configcheck"
"github.com/DataDog/datadog-agent/pkg/config/env"
"github.com/DataDog/datadog-agent/pkg/config/model"
pkgconfigsetup "github.com/DataDog/datadog-agent/pkg/config/setup"
Expand Down Expand Up @@ -355,7 +355,7 @@ func applyDatadogConfig(c *config.AgentConfig, core corecompcfg.Component) error
c.GUIPort = core.GetString("GUI_port")

var grpcPort int
if otlp.IsEnabled(pkgconfigsetup.Datadog()) {
if configcheck.IsEnabled(pkgconfigsetup.Datadog()) {
grpcPort = core.GetInt(pkgconfigsetup.OTLPTracePort)
}

Expand Down
3 changes: 2 additions & 1 deletion pkg/serverless/otlp/otlp.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"go.opentelemetry.io/collector/otelcol"

coreOtlp "github.com/DataDog/datadog-agent/comp/otelcol/otlp"
"github.com/DataDog/datadog-agent/comp/otelcol/otlp/configcheck"
pkgconfigsetup "github.com/DataDog/datadog-agent/pkg/config/setup"
"github.com/DataDog/datadog-agent/pkg/serializer"
"github.com/DataDog/datadog-agent/pkg/util/log"
Expand Down Expand Up @@ -61,7 +62,7 @@ func (o *ServerlessOTLPAgent) Stop() {

// IsEnabled returns true if the OTLP endpoint should be enabled.
func IsEnabled() bool {
return coreOtlp.IsEnabled(pkgconfigsetup.Datadog())
return configcheck.IsEnabled(pkgconfigsetup.Datadog())
}

var (
Expand Down

0 comments on commit 8398c38

Please sign in to comment.