[exporter/datadog] Remove automatic env variable detection

CHANGELOG.md

@@ -5,6 +5,9 @@
 ### 🛑 Breaking changes 🛑
 
 - `stackdriverexporter`: Remove the stackdriver exporter in favor of the identical googlecloud exporter (#9274)
+- `datadogexporter`: Remove `GetHotsTags` method from `TagsConfig` struct (#9249)
+- `datadogexporter`: Remove automatic environment variable detection (#9249)
+   - Check [#8396](https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/8396) for upgrade instructions.
 
 ### 🚩 Deprecations 🚩
 

exporter/datadogexporter/config/config.go

@@ -18,12 +18,14 @@ import (
 	"encoding"
 	"errors"
 	"fmt"
+	"os"
 	"regexp"
 	"strings"
 
 	"go.opentelemetry.io/collector/config"
 	"go.opentelemetry.io/collector/config/confignet"
 	"go.opentelemetry.io/collector/exporter/exporterhelper"
+	"go.uber.org/multierr"
 	"go.uber.org/zap"
 
 	"github.com/open-telemetry/opentelemetry-collector-contrib/exporter/datadogexporter/internal/metadata/valid"
@@ -50,12 +52,9 @@ const (
 type APIConfig struct {
 	// Key is the Datadog API key to associate your Agent's data with your organization.
 	// Create a new API key here: https://app.datadoghq.com/account/settings
-	//
-	// It can also be set through the `DD_API_KEY` environment variable (Deprecated: [v0.47.0] set environment variable explicitly on configuration instead).
 	Key string `mapstructure:"key"`
 
 	// Site is the site of the Datadog intake to send data to.
-	// It can also be set through the `DD_SITE` environment variable (Deprecated: [v0.47.0] set environment variable explicitly on configuration instead).
 	// The default value is "datadoghq.com".
 	Site string `mapstructure:"site"`
 }
@@ -76,7 +75,6 @@ type MetricsConfig struct {
 	DeltaTTL int64 `mapstructure:"delta_ttl"`
 
 	// TCPAddr.Endpoint is the host of the Datadog intake server to send metrics to.
-	// It can also be set through the `DD_URL` environment variable (Deprecated: [v0.47.0] set environment variable explicitly on configuration instead)..
 	// If unset, the value is obtained from the Site.
 	confignet.TCPAddr `mapstructure:",squash"`
 
@@ -167,7 +165,6 @@ type MetricsExporterConfig struct {
 // TracesConfig defines the traces exporter specific configuration options
 type TracesConfig struct {
 	// TCPAddr.Endpoint is the host of the Datadog intake server to send traces to.
-	// It can also be set through the `DD_APM_URL` environment variable (Deprecated: [v0.47.0] set environment variable explicitly on configuration instead).
 	// If unset, the value is obtained from the Site.
 	confignet.TCPAddr `mapstructure:",squash"`
 
@@ -200,7 +197,6 @@ type TracesConfig struct {
 // It is embedded in the configuration
 type TagsConfig struct {
 	// Hostname is the host name for unified service tagging.
-	// It can also be set through the `DD_HOST` environment variable (Deprecated: [v0.47.0] set environment variable explicitly on configuration instead).
 	// If unset, it is determined automatically.
 	// See https://docs.datadoghq.com/agent/faq/how-datadog-agent-determines-the-hostname
 	// for more details.
@@ -209,48 +205,23 @@ type TagsConfig struct {
 	// Env is the environment for unified service tagging.
 	// Deprecated: [v0.49.0] Set `deployment.environment` semconv instead, see https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/9016 for details.
 	// This option will be removed in v0.52.0.
-	// It can also be set through the `DD_ENV` environment variable (Deprecated: [v0.47.0] set environment variable explicitly on configuration instead).
 	Env string `mapstructure:"env"`
 
 	// Service is the service for unified service tagging.
 	// Deprecated: [v0.49.0] Set `service.name` semconv instead, see https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/8781 for details.
 	// This option will be removed in v0.52.0.
-	// It can also be set through the `DD_SERVICE` environment variable (Deprecated: [v0.47.0] set environment variable explicitly on configuration instead).
 	Service string `mapstructure:"service"`
 
 	// Version is the version for unified service tagging.
 	// Deprecated: [v0.49.0] Set `service.version` semconv instead, see https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/8783 for details.
 	// This option will be removed in v0.52.0.
-	// It can also be set through the `DD_VERSION` environment variable (Deprecated: [v0.47.0] set environment variable explicitly on configuration instead).
 	Version string `mapstructure:"version"`
 
-	// EnvVarTags is the list of space-separated tags passed by the `DD_TAGS` environment variable
-	// Superseded by Tags if the latter is set.
-	// Should not be set in the user-provided config.
-	//
-	// Deprecated: [v0.47.0] Use `host_metadata::tags` HostMetadataConfig.Tags instead.
-	EnvVarTags string `mapstructure:"envvartags"`
-
 	// Tags is the list of default tags to add to every metric or trace.
 	// Deprecated: [v0.49.0] Use `host_metadata::tags` (HostMetadataConfig.Tags)
 	Tags []string `mapstructure:"tags"`
 }
 
-// GetHostTags gets the host tags extracted from the configuration
-// Deprecated: [v0.49.0] Access fields explicitly instead.
-func (t *TagsConfig) GetHostTags() []string {
-	tags := t.Tags
-
-	if len(tags) == 0 {
-		tags = strings.Split(t.EnvVarTags, " ")
-	}
-
-	if t.Env != "none" {
-		tags = append(tags, fmt.Sprintf("env:%s", t.Env))
-	}
-	return tags
-}
-
 // HostnameSource is the source for the hostname of host metadata.
 type HostnameSource string
 
@@ -387,7 +358,11 @@ func (c *Config) Sanitize(logger *zap.Logger) error {
 	}
 
 	if c.API.Key == "" {
-		return errUnsetAPIKey
+		err := errUnsetAPIKey
+		if val, ok := os.LookupEnv("DD_API_KEY"); ok && val != "" {
+			err = multierr.Append(err, errUsedEnvVar("api::key", "DD_API_KEY"))
+		}
+		return err
 	}
 
 	c.API.Key = strings.TrimSpace(c.API.Key)
@@ -407,7 +382,7 @@ func (c *Config) Sanitize(logger *zap.Logger) error {
 	}
 
 	for _, err := range c.warnings {
-		logger.Warn(fmt.Sprintf("Deprecated: %v", err))
+		logger.Warn(fmt.Sprintf("%v", err))
 	}
 
 	return nil

exporter/datadogexporter/config/config_test.go

@@ -24,63 +24,6 @@ import (
 	"go.uber.org/zap"
 )
 
-func TestHostTags(t *testing.T) {
-	tc := TagsConfig{
-		Hostname: "customhost",
-		Env:      "customenv",
-		// Service and version should be only used for traces
-		Service: "customservice",
-		Version: "customversion",
-		Tags:    []string{"key1:val1", "key2:val2"},
-	}
-
-	assert.ElementsMatch(t,
-		[]string{
-			"env:customenv",
-			"key1:val1",
-			"key2:val2",
-		},
-		tc.GetHostTags(),
-	)
-
-	tc = TagsConfig{
-		Hostname: "customhost",
-		Env:      "customenv",
-		// Service and version should be only used for traces
-		Service:    "customservice",
-		Version:    "customversion",
-		Tags:       []string{"key1:val1", "key2:val2"},
-		EnvVarTags: "key3:val3 key4:val4",
-	}
-
-	assert.ElementsMatch(t,
-		[]string{
-			"env:customenv",
-			"key1:val1",
-			"key2:val2",
-		},
-		tc.GetHostTags(),
-	)
-
-	tc = TagsConfig{
-		Hostname: "customhost",
-		Env:      "customenv",
-		// Service and version should be only used for traces
-		Service:    "customservice",
-		Version:    "customversion",
-		EnvVarTags: "key3:val3 key4:val4",
-	}
-
-	assert.ElementsMatch(t,
-		[]string{
-			"env:customenv",
-			"key3:val3",
-			"key4:val4",
-		},
-		tc.GetHostTags(),
-	)
-}
-
 // TestOverrideMetricsURL tests that the metrics URL is overridden
 // correctly when set manually.
 func TestOverrideMetricsURL(t *testing.T) {
@@ -211,7 +154,7 @@ func TestUnmarshal(t *testing.T) {
 
 	for _, testInstance := range tests {
 		t.Run(testInstance.name, func(t *testing.T) {
-			cfg := futureDefaultConfig()
+			cfg := oldDefaultConfig()
 			err := cfg.Unmarshal(testInstance.configMap)
 			if err != nil || testInstance.err != "" {
 				assert.EqualError(t, err, testInstance.err)

exporter/datadogexporter/config/warn_envvars.go

@@ -16,22 +16,47 @@ package config // import "github.com/open-telemetry/opentelemetry-collector-cont
 
 import (
 	"fmt"
+	"os"
+	"strings"
 	"time"
 
 	"go.opentelemetry.io/collector/config"
+	"go.opentelemetry.io/collector/config/confignet"
 	"go.opentelemetry.io/collector/exporter/exporterhelper"
 )
 
-// futureDefaultConfig returns the future default config we will use from v0.50 onwards.
-func futureDefaultConfig() *Config {
+// oldDefaultConfig returns the old default config we used to have before v0.50.
+func oldDefaultConfig() *Config {
+	// This used to be done at 'GetHostTags', done here now
+	var tags []string
+	if os.Getenv("DD_TAGS") != "" {
+		tags = strings.Split(os.Getenv("DD_TAGS"), " ")
+	}
+
 	return &Config{
 		ExporterSettings: config.NewExporterSettings(config.NewComponentID("datadog")),
 		TimeoutSettings: exporterhelper.TimeoutSettings{
 			Timeout: 15 * time.Second,
 		},
 		RetrySettings: exporterhelper.NewDefaultRetrySettings(),
 		QueueSettings: exporterhelper.NewDefaultQueueSettings(),
+		API: APIConfig{
+			Key:  os.Getenv("DD_API_KEY"), // Must be set if using API
+			Site: os.Getenv("DD_SITE"),    // If not provided, set during config sanitization
+		},
+
+		TagsConfig: TagsConfig{
+			Hostname: os.Getenv("DD_HOST"),
+			Env:      os.Getenv("DD_ENV"),
+			Service:  os.Getenv("DD_SERVICE"),
+			Version:  os.Getenv("DD_VERSION"),
+			Tags:     tags,
+		},
+
 		Metrics: MetricsConfig{
+			TCPAddr: confignet.TCPAddr{
+				Endpoint: os.Getenv("DD_URL"), // If not provided, set during config sanitization
+			},
 			SendMonotonic: true,
 			DeltaTTL:      3600,
 			Quantiles:     true,
@@ -48,10 +73,14 @@ func futureDefaultConfig() *Config {
 			},
 		},
 		Traces: TracesConfig{
-			SampleRate:      1,
+			SampleRate: 1,
+			TCPAddr: confignet.TCPAddr{
+				Endpoint: os.Getenv("DD_APM_URL"), // If not provided, set during config sanitization
+			},
 			IgnoreResources: []string{},
 		},
 		HostMetadata: HostMetadataConfig{
+			Tags:           tags,
 			Enabled:        true,
 			HostnameSource: HostnameSourceFirstResource,
 		},
@@ -65,17 +94,17 @@ func futureDefaultConfig() *Config {
 // explicitly on configuration instead.
 func errUsedEnvVar(settingName, envVarName string) error {
 	return fmt.Errorf(
-		"%q will not default to %q's value starting on v0.50.0. Set %s: ${%s} to remove this warning",
-		settingName,
+		"%q env var is set but not used explicitly on %q. %q does not default to %q's value since v0.50.0",
 		envVarName,
 		settingName,
+		settingName,
 		envVarName,
 	)
 }
 
 // tagsDiffer checks if the host tags from each configuration are different.
-func tagsDiffer(cfgTags []string, futureTags []string) bool {
-	if len(cfgTags) != len(futureTags) {
+func tagsDiffer(cfgTags []string, oldTags []string) bool {
+	if len(cfgTags) != len(oldTags) {
 		return true
 	}
 
@@ -84,9 +113,9 @@ func tagsDiffer(cfgTags []string, futureTags []string) bool {
 		oldCfgSet[tag] = struct{}{}
 	}
 
-	for _, tag := range futureTags {
+	for _, tag := range cfgTags {
 		if _, ok := oldCfgSet[tag]; !ok {
-			// tag is in old but not new
+			// tag is in new but not old
 			return true
 		}
 	}
@@ -95,14 +124,14 @@ func tagsDiffer(cfgTags []string, futureTags []string) bool {
 }
 
 // warnUseOfEnvVars returns warnings related to automatic environment variable detection.
-// Right now, we automatically get the value for e.g. the API key from DD_API_KEY, even if
-// the user is not doing `api.key: ${DD_API_KEY}`. We are going to remove this functionality.
+// On v0.50.0 we removed automatic environment variable detection, we warn users if the
+// pre-v0.50.0 configuration would have different values.
 func warnUseOfEnvVars(configMap *config.Map, cfg *Config) (warnings []error) {
-	// We don't see the raw YAML contents from our exporter so, to compare with the new default,
-	// we unmarshal the config map on the future default config and compare the two structs.
+	// We don't see the raw YAML contents from our exporter so, to compare with the old default,
+	// we unmarshal the config map on the old default config and compare the two structs.
 	// Any differences will be due to the change in default configuration.
-	futureCfg := futureDefaultConfig()
-	err := configMap.UnmarshalExact(futureCfg)
+	oldCfg := oldDefaultConfig()
+	err := configMap.UnmarshalExact(oldCfg)
 
 	errIssue := fmt.Errorf("see github.com/open-telemetry/opentelemetry-collector-contrib/issues/8396 for more details")
 
@@ -117,32 +146,33 @@ func warnUseOfEnvVars(configMap *config.Map, cfg *Config) (warnings []error) {
 
 	// We could probably do this with reflection but I don't want to risk
 	// an accidental panic so we check each field manually.
-	if cfg.API.Key != futureCfg.API.Key {
+	if cfg.API.Key != oldCfg.API.Key {
 		warnings = append(warnings, errUsedEnvVar("api.key", "DD_API_KEY"))
 	}
-	if cfg.API.Site != futureCfg.API.Site {
+	if cfg.API.Site != oldCfg.API.Site {
 		warnings = append(warnings, errUsedEnvVar("api.site", "DD_SITE"))
 	}
-	if cfg.Hostname != futureCfg.Hostname {
+	if cfg.Hostname != oldCfg.Hostname {
 		warnings = append(warnings, errUsedEnvVar("hostname", "DD_HOST"))
 	}
-	if cfg.Env != futureCfg.Env {
+	if cfg.Env != oldCfg.Env {
 		warnings = append(warnings, errUsedEnvVar("env", "DD_ENV"))
 	}
-	if cfg.Service != futureCfg.Service {
+	if cfg.Service != oldCfg.Service {
 		warnings = append(warnings, errUsedEnvVar("service", "DD_SERVICE"))
 	}
-	if cfg.Version != futureCfg.Version {
+	if cfg.Version != oldCfg.Version {
 		warnings = append(warnings, errUsedEnvVar("version", "DD_VERSION"))
 	}
-	if cfg.Metrics.Endpoint != futureCfg.Metrics.Endpoint {
+	if cfg.Metrics.Endpoint != oldCfg.Metrics.Endpoint {
 		warnings = append(warnings, errUsedEnvVar("metrics.endpoint", "DD_URL"))
 	}
-	if cfg.Traces.Endpoint != futureCfg.Traces.Endpoint {
+	if cfg.Traces.Endpoint != oldCfg.Traces.Endpoint {
 		warnings = append(warnings, errUsedEnvVar("traces.endpoint", "DD_APM_URL"))
 	}
-	if tagsDiffer(cfg.GetHostTags(), futureCfg.GetHostTags()) {
-		warnings = append(warnings, fmt.Errorf("\"tags\" will not default to \"DD_TAGS\"'s value starting on v0.50.0. Use 'env' configuration source instead to remove this warning"))
+	if tagsDiffer(cfg.HostMetadata.Tags, oldCfg.HostMetadata.Tags) {
+		warnings = append(warnings, fmt.Errorf("\"tags\" does not default to \"DD_TAGS\"'s value since v0.50.0. Use 'env' configuration source instead"))
+
 	}
 
 	if len(warnings) > 0 {

exporter/datadogexporter/config/warning_deprecated_test.go

@@ -85,7 +85,7 @@ func TestDeprecationSendMonotonic(t *testing.T) {
 
 	for _, testInstance := range tests {
 		t.Run(testInstance.name, func(t *testing.T) {
-			cfg := futureDefaultConfig()
+			cfg := oldDefaultConfig()
 			err := cfg.Unmarshal(testInstance.cfgMap)
 			if err != nil || testInstance.err != "" {
 				assert.EqualError(t, err, testInstance.err)