Skip to content

Commit

Permalink
[chore][exporter/datadog] Only start hostmetadata.Reporter if `host…
Browse files Browse the repository at this point in the history
…_metadata.enabled` (open-telemetry#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 open-telemetry#36522
  • Loading branch information
jade-guiton-dd authored and sbylica-splunk committed Dec 17, 2024
1 parent 54c2a24 commit 48cc95d
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 36 deletions.
36 changes: 24 additions & 12 deletions exporter/datadogexporter/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down
6 changes: 0 additions & 6 deletions exporter/datadogexporter/logs_exporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,9 +230,6 @@ func TestLogsExporter(t *testing.T) {
Endpoint: server.URL,
},
},
HostMetadata: HostMetadataConfig{
ReporterPeriod: 30 * time.Minute,
},
}

params := exportertest.NewNopSettings()
Expand Down Expand Up @@ -598,9 +595,6 @@ func TestLogsAgentExporter(t *testing.T) {
CompressionLevel: 6,
BatchWait: 1,
},
HostMetadata: HostMetadataConfig{
ReporterPeriod: 30 * time.Minute,
},
}
params := exportertest.NewNopSettings()
f := NewFactory()
Expand Down
9 changes: 5 additions & 4 deletions exporter/datadogexporter/metrics_exporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand All @@ -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)
Expand Down Expand Up @@ -440,7 +441,9 @@ func TestNewExporter_Zorkian(t *testing.T) {
},
},
HostMetadata: HostMetadataConfig{
Enabled: true,
ReporterPeriod: 30 * time.Minute,
HostnameSource: HostnameSourceFirstResource,
},
}
params := exportertest.NewNopSettings()
Expand All @@ -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)
Expand Down
15 changes: 1 addition & 14 deletions exporter/datadogexporter/traces_exporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,9 +145,6 @@ func TestTracesSource(t *testing.T) {
IgnoreResources: []string{},
},
},
HostMetadata: HostMetadataConfig{
ReporterPeriod: 30 * time.Minute,
},
}

assert := assert.New(t)
Expand Down Expand Up @@ -270,9 +267,6 @@ func TestTraceExporter(t *testing.T) {
},
TraceBuffer: 2,
},
HostMetadata: HostMetadataConfig{
ReporterPeriod: 30 * time.Minute,
},
}
cfg.Traces.SetFlushInterval(0.1)

Expand All @@ -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()
Expand Down Expand Up @@ -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)

Expand Down

0 comments on commit 48cc95d

Please sign in to comment.