From 48cc95d5821d9241353df3509e629b4601439480 Mon Sep 17 00:00:00 2001 From: Jade Guiton Date: Tue, 10 Dec 2024 11:59:28 +0100 Subject: [PATCH] [chore][exporter/datadog] Only start `hostmetadata.Reporter` if `host_metadata.enabled` (#36669) #### Description This PR avoids creating an `inframetadata.Reporter` and starting the metadata-sending goroutine when `host_metadata.enabled` is `false`, and removes some code that was used to work around the previous behavior. See tracking issue for context. When `enabled` is `false`, we leave the `exporter.metadataReporter` field as `nil`. The parts of the code accessing this field were already gated behind `enabled: true`, or `only_metadata: true` (which, in a valid config, implies `enabled: false`), so it should not cause issues. However, tests that modify the config after the exporter is created, or which manually create invalid `Config` structs may encounter segfaults. I assumed we do not mind segfaults in cases of incorrect internal use, and fixed the one test that was failing because of this. #### Link to tracking issue Fixes #36522 --- exporter/datadogexporter/factory.go | 36 ++++++++++++------- .../datadogexporter/logs_exporter_test.go | 6 ---- .../datadogexporter/metrics_exporter_test.go | 9 ++--- .../datadogexporter/traces_exporter_test.go | 15 +------- 4 files changed, 30 insertions(+), 36 deletions(-) diff --git a/exporter/datadogexporter/factory.go b/exporter/datadogexporter/factory.go index 3ba017623954..5726d889d403 100644 --- a/exporter/datadogexporter/factory.go +++ b/exporter/datadogexporter/factory.go @@ -270,10 +270,14 @@ func (f *factory) createMetricsExporter( statsv := set.BuildInfo.Command + set.BuildInfo.Version f.consumeStatsPayload(ctx, &wg, statsIn, statsWriter, statsv, acfg.AgentVersion, set.Logger) pcfg := newMetadataConfigfromConfig(cfg) - metadataReporter, err := f.Reporter(set, pcfg) - if err != nil { - cancel() - return nil, fmt.Errorf("failed to build host metadata reporter: %w", err) + // Don't start a `Reporter` if host metadata is disabled. + var metadataReporter *inframetadata.Reporter + if cfg.HostMetadata.Enabled { + metadataReporter, err = f.Reporter(set, pcfg) + if err != nil { + cancel() + return nil, fmt.Errorf("failed to build host metadata reporter: %w", err) + } } if cfg.OnlyMetadata { @@ -375,10 +379,14 @@ func (f *factory) createTracesExporter( } pcfg := newMetadataConfigfromConfig(cfg) - metadataReporter, err := f.Reporter(set, pcfg) - if err != nil { - cancel() - return nil, fmt.Errorf("failed to build host metadata reporter: %w", err) + // Don't start a `Reporter` if host metadata is disabled. + var metadataReporter *inframetadata.Reporter + if cfg.HostMetadata.Enabled { + metadataReporter, err = f.Reporter(set, pcfg) + if err != nil { + cancel() + return nil, fmt.Errorf("failed to build host metadata reporter: %w", err) + } } if cfg.OnlyMetadata { @@ -463,10 +471,14 @@ func (f *factory) createLogsExporter( // cancel() runs on shutdown pcfg := newMetadataConfigfromConfig(cfg) - metadataReporter, err := f.Reporter(set, pcfg) - if err != nil { - cancel() - return nil, fmt.Errorf("failed to build host metadata reporter: %w", err) + // Don't start a `Reporter` if host metadata is disabled. + var metadataReporter *inframetadata.Reporter + if cfg.HostMetadata.Enabled { + metadataReporter, err = f.Reporter(set, pcfg) + if err != nil { + cancel() + return nil, fmt.Errorf("failed to build host metadata reporter: %w", err) + } } attributesTranslator, err := f.AttributesTranslator(set.TelemetrySettings) diff --git a/exporter/datadogexporter/logs_exporter_test.go b/exporter/datadogexporter/logs_exporter_test.go index a2837353def9..941561890853 100644 --- a/exporter/datadogexporter/logs_exporter_test.go +++ b/exporter/datadogexporter/logs_exporter_test.go @@ -230,9 +230,6 @@ func TestLogsExporter(t *testing.T) { Endpoint: server.URL, }, }, - HostMetadata: HostMetadataConfig{ - ReporterPeriod: 30 * time.Minute, - }, } params := exportertest.NewNopSettings() @@ -598,9 +595,6 @@ func TestLogsAgentExporter(t *testing.T) { CompressionLevel: 6, BatchWait: 1, }, - HostMetadata: HostMetadataConfig{ - ReporterPeriod: 30 * time.Minute, - }, } params := exportertest.NewNopSettings() f := NewFactory() diff --git a/exporter/datadogexporter/metrics_exporter_test.go b/exporter/datadogexporter/metrics_exporter_test.go index 7d973171c850..95935fd053a4 100644 --- a/exporter/datadogexporter/metrics_exporter_test.go +++ b/exporter/datadogexporter/metrics_exporter_test.go @@ -59,10 +59,13 @@ func TestNewExporter(t *testing.T) { }, }, HostMetadata: HostMetadataConfig{ + Enabled: true, ReporterPeriod: 30 * time.Minute, + HostnameSource: HostnameSourceFirstResource, }, } cfg.HostMetadata.SetSourceTimeout(50 * time.Millisecond) + params := exportertest.NewNopSettings() f := NewFactory() @@ -76,8 +79,6 @@ func TestNewExporter(t *testing.T) { require.NoError(t, err) assert.Empty(t, server.MetadataChan) - cfg.HostMetadata.Enabled = true - cfg.HostMetadata.HostnameSource = HostnameSourceFirstResource testMetrics = pmetric.NewMetrics() testutil.TestMetrics.CopyTo(testMetrics) err = exp.ConsumeMetrics(context.Background(), testMetrics) @@ -440,7 +441,9 @@ func TestNewExporter_Zorkian(t *testing.T) { }, }, HostMetadata: HostMetadataConfig{ + Enabled: true, ReporterPeriod: 30 * time.Minute, + HostnameSource: HostnameSourceFirstResource, }, } params := exportertest.NewNopSettings() @@ -456,8 +459,6 @@ func TestNewExporter_Zorkian(t *testing.T) { require.NoError(t, err) assert.Empty(t, server.MetadataChan) - cfg.HostMetadata.Enabled = true - cfg.HostMetadata.HostnameSource = HostnameSourceFirstResource testMetrics = pmetric.NewMetrics() testutil.TestMetrics.CopyTo(testMetrics) err = exp.ConsumeMetrics(context.Background(), testMetrics) diff --git a/exporter/datadogexporter/traces_exporter_test.go b/exporter/datadogexporter/traces_exporter_test.go index 68aff8b3e1ea..54e4aaf73e4e 100644 --- a/exporter/datadogexporter/traces_exporter_test.go +++ b/exporter/datadogexporter/traces_exporter_test.go @@ -145,9 +145,6 @@ func TestTracesSource(t *testing.T) { IgnoreResources: []string{}, }, }, - HostMetadata: HostMetadataConfig{ - ReporterPeriod: 30 * time.Minute, - }, } assert := assert.New(t) @@ -270,9 +267,6 @@ func TestTraceExporter(t *testing.T) { }, TraceBuffer: 2, }, - HostMetadata: HostMetadataConfig{ - ReporterPeriod: 30 * time.Minute, - }, } cfg.Traces.SetFlushInterval(0.1) @@ -298,11 +292,7 @@ func TestNewTracesExporter(t *testing.T) { metricsServer := testutil.DatadogServerMock() defer metricsServer.Close() - cfg := &Config{ - HostMetadata: HostMetadataConfig{ - ReporterPeriod: 30 * time.Minute, - }, - } + cfg := &Config{} cfg.API.Key = "ddog_32_characters_long_api_key1" cfg.Metrics.TCPAddrConfig.Endpoint = metricsServer.URL params := exportertest.NewNopSettings() @@ -368,9 +358,6 @@ func TestPushTraceData_NewEnvConvention(t *testing.T) { Traces: TracesConfig{ TCPAddrConfig: confignet.TCPAddrConfig{Endpoint: server.URL}, }, - HostMetadata: HostMetadataConfig{ - ReporterPeriod: 30 * time.Minute, - }, } cfg.Traces.SetFlushInterval(0.1)