remove unused extraTags arg from dogstatsd.NewServer (DataDog#10946)

pybalo-nex · Mar 4, 2022 · 476c4b3 · 476c4b3
1 parent 3d8e04d
commit 476c4b3
Show file tree

Hide file tree

Showing 10 changed files with 34 additions and 49 deletions.
diff --git a/cmd/agent/app/run.go b/cmd/agent/app/run.go
@@ -376,7 +376,7 @@ func StartAgent() error {
 	// start dogstatsd
 	if config.Datadog.GetBool("use_dogstatsd") {
 		var err error
-		common.DSD, err = dogstatsd.NewServer(demux, nil)
+		common.DSD, err = dogstatsd.NewServer(demux)
 		if err != nil {
 			log.Errorf("Could not start dogstatsd: %s", err)
 		} else {

diff --git a/cmd/agent/app/settings/runtime_settings_test.go b/cmd/agent/app/settings/runtime_settings_test.go
@@ -23,7 +23,7 @@ func TestDogstatsdMetricsStats(t *testing.T) {
 	opts := aggregator.DefaultDemultiplexerOptions(nil)
 	opts.DontStartForwarders = true
 	demux := aggregator.InitAndStartAgentDemultiplexer(opts, "hostname")
-	common.DSD, err = dogstatsd.NewServer(demux, nil)
+	common.DSD, err = dogstatsd.NewServer(demux)
 	require.Nil(t, err)
 
 	s := DsdStatsRuntimeSetting("dogstatsd_stats")

diff --git a/cmd/dogstatsd/main.go b/cmd/dogstatsd/main.go
@@ -222,7 +222,7 @@ func runAgent(ctx context.Context) (err error) {
 		}
 	}
 
-	statsd, err = dogstatsd.NewServer(demux, nil)
+	statsd, err = dogstatsd.NewServer(demux)
 	if err != nil {
 		log.Criticalf("Unable to start dogstatsd: %s", err)
 		return

diff --git a/pkg/dogstatsd/README.md b/pkg/dogstatsd/README.md
@@ -13,7 +13,7 @@ Usage example:
 // you must first initialize the aggregator, see aggregator.InitAggregator
 
 // This will return an already running statd server ready to receive metrics
-statsd, err := dogstatsd.NewServer(aggregatorInstance.GetBufferedChannels(), nil)
+statsd, err := dogstatsd.NewServer(aggregatorInstance.GetBufferedChannels())
 
 // ...
 

diff --git a/pkg/dogstatsd/server.go b/pkg/dogstatsd/server.go
@@ -198,8 +198,7 @@ type metricsCountBuckets struct {
 }
 
 // NewServer returns a running DogStatsD server.
-// If extraTags is nil, they will be read from DD_DOGSTATSD_TAGS if set.
-func NewServer(demultiplexer aggregator.Demultiplexer, extraTags []string) (*Server, error) {
+func NewServer(demultiplexer aggregator.Demultiplexer) (*Server, error) {
 	// This needs to be done after the configuration is loaded
 	once.Do(initLatencyTelemetry)
 
@@ -284,9 +283,7 @@ func NewServer(demultiplexer aggregator.Demultiplexer, extraTags []string) (*Ser
 	histToDist := config.Datadog.GetBool("histogram_copy_to_distribution")
 	histToDistPrefix := config.Datadog.GetString("histogram_copy_to_distribution_prefix")
 
-	if extraTags == nil {
-		extraTags = config.Datadog.GetStringSlice("dogstatsd_tags")
-	}
+	extraTags := config.Datadog.GetStringSlice("dogstatsd_tags")
 
 	// if the server is running in a context where static tags are required, add those
 	// to extraTags.

diff --git a/pkg/dogstatsd/server_bench_test.go b/pkg/dogstatsd/server_bench_test.go
@@ -50,7 +50,7 @@ func benchParsePackets(b *testing.B, rawPacket []byte) {
 
 	demux := aggregator.InitTestAgentDemultiplexer()
 	defer demux.Stop(false)
-	s, _ := NewServer(demux, nil)
+	s, _ := NewServer(demux)
 	defer s.Stop()
 
 	done := make(chan struct{})
@@ -96,7 +96,7 @@ func BenchmarkPbarseMetricMessage(b *testing.B) {
 	config.SetupLogger("", "off", "", "", false, true, false)
 
 	demux := aggregator.InitTestAgentDemultiplexer()
-	s, _ := NewServer(demux, nil)
+	s, _ := NewServer(demux)
 	defer s.Stop()
 
 	done := make(chan struct{})
@@ -153,7 +153,7 @@ func BenchmarkMapperControl(b *testing.B) {
 	config.SetupLogger("", "off", "", "", false, true, false)
 
 	demux := aggregator.InitTestAgentDemultiplexer()
-	s, _ := NewServer(demux, nil)
+	s, _ := NewServer(demux)
 	defer s.Stop()
 
 	done := make(chan struct{})

diff --git a/pkg/dogstatsd/server_test.go b/pkg/dogstatsd/server_test.go
@@ -53,7 +53,7 @@ func TestNewServer(t *testing.T) {
 
 	demux := aggregator.InitTestAgentDemultiplexerWithFlushInterval(10 * time.Millisecond)
 	defer demux.Stop(false)
-	s, err := NewServer(demux, nil)
+	s, err := NewServer(demux)
 	require.NoError(t, err, "cannot start DSD")
 	assert.NotNil(t, s)
 	assert.True(t, s.Started)
@@ -68,7 +68,7 @@ func TestStopServer(t *testing.T) {
 
 	demux := aggregator.InitTestAgentDemultiplexerWithFlushInterval(10 * time.Millisecond)
 	defer demux.Stop(false)
-	s, err := NewServer(demux, nil)
+	s, err := NewServer(demux)
 	require.NoError(t, err, "cannot start DSD")
 	s.Stop()
 
@@ -94,7 +94,7 @@ func TestUDPReceive(t *testing.T) {
 
 	demux := aggregator.InitTestAgentDemultiplexerWithFlushInterval(10 * time.Millisecond)
 	defer demux.Stop(false)
-	s, err := NewServer(demux, nil)
+	s, err := NewServer(demux)
 	require.NoError(t, err, "cannot start DSD")
 	defer s.Stop()
 
@@ -327,7 +327,7 @@ func TestUDPForward(t *testing.T) {
 
 	demux := mockDemultiplexer()
 	defer demux.Stop(false)
-	s, err := NewServer(demux, nil)
+	s, err := NewServer(demux)
 	require.NoError(t, err, "cannot start DSD")
 	defer s.Stop()
 
@@ -363,7 +363,7 @@ func TestHistToDist(t *testing.T) {
 
 	demux := aggregator.InitTestAgentDemultiplexerWithFlushInterval(10 * time.Millisecond)
 	defer demux.Stop(false)
-	s, err := NewServer(demux, nil)
+	s, err := NewServer(demux)
 	require.NoError(t, err, "cannot start DSD")
 	defer s.Stop()
 
@@ -453,7 +453,7 @@ func TestE2EParsing(t *testing.T) {
 	config.Datadog.SetDefault("dogstatsd_port", port)
 
 	demux := aggregator.InitTestAgentDemultiplexerWithFlushInterval(10 * time.Millisecond)
-	s, err := NewServer(demux, nil)
+	s, err := NewServer(demux)
 	require.NoError(t, err, "cannot start DSD")
 
 	url := fmt.Sprintf("127.0.0.1:%d", config.Datadog.GetInt("dogstatsd_port"))
@@ -475,7 +475,7 @@ func TestE2EParsing(t *testing.T) {
 	defer config.Datadog.SetDefault("dogstatsd_eol_required", []string{})
 
 	demux = aggregator.InitTestAgentDemultiplexerWithFlushInterval(10 * time.Millisecond)
-	s, err = NewServer(demux, nil)
+	s, err = NewServer(demux)
 	require.NoError(t, err, "cannot start DSD")
 
 	// Test metric expecting an EOL
@@ -495,7 +495,7 @@ func TestExtraTags(t *testing.T) {
 	defer config.Datadog.SetDefault("dogstatsd_tags", []string{})
 
 	demux := aggregator.InitTestAgentDemultiplexerWithFlushInterval(10 * time.Millisecond)
-	s, err := NewServer(demux, nil)
+	s, err := NewServer(demux)
 	require.NoError(t, err, "cannot start DSD")
 	defer s.Stop()
 
@@ -527,7 +527,7 @@ func TestStaticTags(t *testing.T) {
 	defer config.Datadog.SetDefault("eks_fargate", false)
 
 	demux := aggregator.InitTestAgentDemultiplexerWithFlushInterval(10 * time.Millisecond)
-	s, err := NewServer(demux, nil)
+	s, err := NewServer(demux)
 	require.NoError(t, err, "cannot start DSD")
 	defer s.Stop()
 
@@ -557,7 +557,7 @@ func TestDebugStatsSpike(t *testing.T) {
 	assert := assert.New(t)
 	demux := mockDemultiplexer()
 	defer demux.Stop(false)
-	s, err := NewServer(demux, nil)
+	s, err := NewServer(demux)
 	require.NoError(t, err, "cannot start DSD")
 	defer s.Stop()
 	require.NoError(t, err, "cannot start DSD")
@@ -600,7 +600,7 @@ func TestDebugStatsSpike(t *testing.T) {
 func TestDebugStats(t *testing.T) {
 	demux := mockDemultiplexer()
 	defer demux.Stop(false)
-	s, err := NewServer(demux, nil)
+	s, err := NewServer(demux)
 	require.NoError(t, err, "cannot start DSD")
 	defer s.Stop()
 
@@ -684,7 +684,7 @@ func TestNoMappingsConfig(t *testing.T) {
 
 	demux := mockDemultiplexer()
 	defer demux.Stop(false)
-	s, err := NewServer(demux, nil)
+	s, err := NewServer(demux)
 	require.NoError(t, err, "cannot start DSD")
 
 	assert.Nil(t, s.mapper)
@@ -798,7 +798,7 @@ dogstatsd_mapper_profiles:
 
 			demux := mockDemultiplexer()
 			defer demux.Stop(false)
-			s, err := NewServer(demux, nil)
+			s, err := NewServer(demux)
 			require.NoError(t, err, "Case `%s` failed. NewServer should not return error %v", scenario.name, err)
 
 			assert.Equal(t, config.Datadog.Get("dogstatsd_mapper_cache_size"), scenario.expectedCacheSize, "Case `%s` failed. cache_size `%s` should be `%s`", scenario.name, config.Datadog.Get("dogstatsd_mapper_cache_size"), scenario.expectedCacheSize)
@@ -841,7 +841,7 @@ func TestNewServerExtraTags(t *testing.T) {
 	config.Datadog.SetDefault("dogstatsd_port", port)
 
 	demux := mockDemultiplexer()
-	s, err := NewServer(demux, nil)
+	s, err := NewServer(demux)
 	require.NoError(err, "starting the DogStatsD server shouldn't fail")
 	require.Len(s.extraTags, 0, "no tags should have been read")
 	s.Stop()
@@ -850,7 +850,7 @@ func TestNewServerExtraTags(t *testing.T) {
 	// when the extraTags parameter isn't used, the DogStatsD server is not reading this env var
 	os.Setenv("DD_TAGS", "hello:world")
 	demux = mockDemultiplexer()
-	s, err = NewServer(demux, nil)
+	s, err = NewServer(demux)
 	require.NoError(err, "starting the DogStatsD server shouldn't fail")
 	require.Len(s.extraTags, 0, "no tags should have been read")
 	s.Stop()
@@ -859,33 +859,21 @@ func TestNewServerExtraTags(t *testing.T) {
 	// when the extraTags parameter isn't used, the DogStatsD server is automatically reading this env var for extra tags
 	os.Setenv("DD_DOGSTATSD_TAGS", "hello:world extra:tags")
 	demux = mockDemultiplexer()
-	s, err = NewServer(demux, nil)
+	s, err = NewServer(demux)
 	require.NoError(err, "starting the DogStatsD server shouldn't fail")
 	require.Len(s.extraTags, 2, "two tags should have been read")
 	require.Equal(s.extraTags[0], "extra:tags", "the tag extra:tags should be set")
 	require.Equal(s.extraTags[1], "hello:world", "the tag hello:world should be set")
 	s.Stop()
 	demux.Stop(false)
-
-	// when the extraTags parameter is used, it should be used as the extraTags for the server
-	// and the DD_DOGSTATSD_TAGS environment var should be ignored.
-	os.Setenv("DD_DOGSTATSD_TAGS", "hello:world") // this should be ignored
-	demux = mockDemultiplexer()
-	s, err = NewServer(demux, []string{"extra:tags", "new:constructor"})
-	require.NoError(err, "starting the DogStatsD server shouldn't fail")
-	require.Len(s.extraTags, 2, "two tags should have been read")
-	require.Equal(s.extraTags[0], "extra:tags", "the tag extra:tags should be set")
-	require.Equal(s.extraTags[1], "new:constructor", "the tag new:constructor should be set")
-	s.Stop()
-	demux.Stop(false)
 }
 
 func TestProcessedMetricsOrigin(t *testing.T) {
 	assert := assert.New(t)
 
 	demux := mockDemultiplexer()
 	defer demux.Stop(false)
-	s, err := NewServer(demux, nil)
+	s, err := NewServer(demux)
 	assert.NoError(err, "starting the DogStatsD server shouldn't fail")
 	s.Stop()
 
@@ -961,7 +949,7 @@ func TestProcessedMetricsOrigin(t *testing.T) {
 func TestContainerIDParsing(t *testing.T) {
 	assert := assert.New(t)
 
-	s, err := NewServer(mockDemultiplexer(), nil)
+	s, err := NewServer(mockDemultiplexer())
 	assert.NoError(err, "starting the DogStatsD server shouldn't fail")
 	s.Stop()
 

diff --git a/pkg/serverless/metrics/metric.go b/pkg/serverless/metrics/metric.go
@@ -38,7 +38,7 @@ type MultipleEndpointConfig interface {
 
 // DogStatsDFactory allows create a new DogStatsD server
 type DogStatsDFactory interface {
-	NewServer(demux aggregator.Demultiplexer, extraTags []string) (*dogstatsd.Server, error)
+	NewServer(demux aggregator.Demultiplexer) (*dogstatsd.Server, error)
 }
 
 const (
@@ -52,8 +52,8 @@ func (m *MetricConfig) GetMultipleEndpoints() (map[string][]string, error) {
 }
 
 // NewServer returns a running DogStatsD server
-func (m *MetricDogStatsD) NewServer(demux aggregator.Demultiplexer, extraTags []string) (*dogstatsd.Server, error) {
-	return dogstatsd.NewServer(demux, extraTags)
+func (m *MetricDogStatsD) NewServer(demux aggregator.Demultiplexer) (*dogstatsd.Server, error) {
+	return dogstatsd.NewServer(demux)
 }
 
 // Start starts the DogStatsD agent
@@ -75,7 +75,7 @@ func (c *ServerlessMetricAgent) Start(forwarderTimeout time.Duration, multipleEn
 	demux := buildDemultiplexer(multipleEndpointConfig, forwarderTimeout)
 
 	if demux != nil {
-		statsd, err := dogstatFactory.NewServer(demux, nil)
+		statsd, err := dogstatFactory.NewServer(demux)
 		if err != nil {
 			log.Errorf("Unable to start the DogStatsD server: %s", err)
 		} else {

diff --git a/pkg/serverless/metrics/metric_test.go b/pkg/serverless/metrics/metric_test.go
@@ -60,7 +60,7 @@ func TestStartInvalidConfig(t *testing.T) {
 type MetricDogStatsDMocked struct {
 }
 
-func (m *MetricDogStatsDMocked) NewServer(demux aggregator.Demultiplexer, extraTags []string) (*dogstatsd.Server, error) {
+func (m *MetricDogStatsDMocked) NewServer(demux aggregator.Demultiplexer) (*dogstatsd.Server, error) {
 	return nil, fmt.Errorf("error")
 }
 
@@ -192,7 +192,7 @@ func TestRaceFlushVersusParsePacket(t *testing.T) {
 	opts.DontStartForwarders = true
 	demux := aggregator.InitAndStartServerlessDemultiplexer(nil, "serverless", time.Second*1000)
 
-	s, err := dogstatsd.NewServer(demux, nil)
+	s, err := dogstatsd.NewServer(demux)
 	require.NoError(t, err, "cannot start DSD")
 	defer s.Stop()
 

diff --git a/test/benchmarks/dogstatsd/main.go b/test/benchmarks/dogstatsd/main.go
@@ -219,7 +219,7 @@ func main() {
 	mockConfig.Set("dogstatsd_stats_buffer", 100)
 	s := serializer.NewSerializer(f, nil)
 	aggr := aggregator.InitAggregator(s, nil, "localhost")
-	statsd, err := dogstatsd.NewServer(aggr.GetBufferedChannels(), nil)
+	statsd, err := dogstatsd.NewServer(aggr.GetBufferedChannels())
 	if err != nil {
 		log.Errorf("Problem allocating dogstatsd server: %s", err)
 		return