Skip to content

Commit

Permalink
[exporter/datadog] Respect confighttp configs and use better defaults (
Browse files Browse the repository at this point in the history
…open-telemetry#31733)

**Description:** <Describe what has changed.>
Datadog exporter respects a subset of settings in confighttp client
configs and uses a consistent default HTTP transport settings as Datadog
Agent
(https://github.com/DataDog/datadog-agent/blob/f9ae7f4b842f83b23b2dfe3f15d31f9e6b12e857/pkg/util/http/transport.go#L91-L106).
  • Loading branch information
songy23 committed Mar 14, 2024
1 parent 4ca7530 commit 135d723
Show file tree
Hide file tree
Showing 21 changed files with 387 additions and 81 deletions.
27 changes: 27 additions & 0 deletions .chloggen/ddog-config-change.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# Use this changelog template to create an entry for release notes.

# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: breaking

# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
component: datadogexporter

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: "Remove config structs `LimitedClientConfig` and `LimitedTLSClientSettings`"

# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
issues: [31733]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext: This should have no impact to end users as long as they do not import Datadog exporter config structs in their source code.

# If your change doesn't affect end users or the exported elements of any package,
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
# Optional: The change log or logs in which this entry should be included.
# e.g. '[user]' or '[user, api]'
# Include 'user' if the change is relevant to end users.
# Include 'api' if there is a change to a library API.
# Default: '[user]'
change_logs: [api]
27 changes: 27 additions & 0 deletions .chloggen/ddog-http-defaults.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# Use this changelog template to create an entry for release notes.

# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: enhancement

# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
component: datadogexporter

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Datadog exporter uses the same default HTTP settings as Datadog Agent HTTP transport

# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
issues: [31733]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext:

# If your change doesn't affect end users or the exported elements of any package,
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
# Optional: The change log or logs in which this entry should be included.
# e.g. '[user]' or '[user, api]'
# Include 'user' if the change is relevant to end users.
# Include 'api' if there is a change to a library API.
# Default: '[user]'
change_logs: []
27 changes: 27 additions & 0 deletions .chloggen/ddog-respect-confighttp.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# Use this changelog template to create an entry for release notes.

# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: enhancement

# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
component: datadogexporter

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Datadog exporter respects a subset of settings in confighttp client configs

# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
issues: [31733]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext: "Currently the following configs are respected: `read_buffer_size`, `write_buffer_size`, `timeout`, `max_idle_conns`, `max_idle_conns_per_host`, `max_conns_per_host`, `idle_conn_timeout`, `disable_keep_alives` and `tls.insecure_skip_verify`."

# If your change doesn't affect end users or the exported elements of any package,
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
# Optional: The change log or logs in which this entry should be included.
# e.g. '[user]' or '[user, api]'
# Include 'user' if the change is relevant to end users.
# Include 'api' if there is a change to a library API.
# Default: '[user]'
change_logs: []
54 changes: 38 additions & 16 deletions exporter/datadogexporter/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"strings"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config/confighttp"
"go.opentelemetry.io/collector/config/confignet"
"go.opentelemetry.io/collector/config/configopaque"
"go.opentelemetry.io/collector/config/configretry"
Expand Down Expand Up @@ -390,24 +391,11 @@ type HostMetadataConfig struct {
Tags []string `mapstructure:"tags"`
}

// LimitedTLSClientSetting is a subset of TLSClientSetting, see LimitedClientConfig for more details
type LimitedTLSClientSettings struct {
// InsecureSkipVerify controls whether a client verifies the server's
// certificate chain and host name.
InsecureSkipVerify bool `mapstructure:"insecure_skip_verify"`
}

type LimitedClientConfig struct {
TLSSetting LimitedTLSClientSettings `mapstructure:"tls,omitempty"`
}

// Config defines configuration for the Datadog exporter.
type Config struct {
exporterhelper.TimeoutSettings `mapstructure:",squash"` // squash ensures fields are correctly decoded in embedded struct.
exporterhelper.QueueSettings `mapstructure:"sending_queue"`
configretry.BackOffConfig `mapstructure:"retry_on_failure"`

LimitedClientConfig `mapstructure:",squash"`
confighttp.ClientConfig `mapstructure:",squash"` // squash ensures fields are correctly decoded in embedded struct.
exporterhelper.QueueSettings `mapstructure:"sending_queue"`
configretry.BackOffConfig `mapstructure:"retry_on_failure"`

TagsConfig `mapstructure:",squash"`

Expand Down Expand Up @@ -450,6 +438,10 @@ var _ component.Config = (*Config)(nil)

// Validate the configuration for errors. This is required by component.Config.
func (c *Config) Validate() error {
if err := validateClientConfig(c.ClientConfig); err != nil {
return err
}

if c.OnlyMetadata && (!c.HostMetadata.Enabled || c.HostMetadata.HostnameSource != HostnameSourceFirstResource) {
return errNoMetadata
}
Expand Down Expand Up @@ -490,6 +482,36 @@ func (c *Config) Validate() error {
return nil
}

func validateClientConfig(cfg confighttp.ClientConfig) error {
var unsupported []string
if cfg.Auth != nil {
unsupported = append(unsupported, "auth")
}
if cfg.Endpoint != "" {
unsupported = append(unsupported, "endpoint")
}
if cfg.Compression != "" {
unsupported = append(unsupported, "compression")
}
if cfg.ProxyURL != "" {
unsupported = append(unsupported, "proxy_url")
}
if cfg.Headers != nil {
unsupported = append(unsupported, "headers")
}
if cfg.HTTP2ReadIdleTimeout != 0 {
unsupported = append(unsupported, "http2_read_idle_timeout")
}
if cfg.HTTP2PingTimeout != 0 {
unsupported = append(unsupported, "http2_ping_timeout")
}

if len(unsupported) > 0 {
return fmt.Errorf("these confighttp client configs are currently not respected by Datadog exporter: %s", strings.Join(unsupported, ", "))
}
return nil
}

var _ error = (*renameError)(nil)

// renameError is an error related to a renamed setting.
Expand Down
84 changes: 81 additions & 3 deletions exporter/datadogexporter/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,25 @@ package datadogexporter

import (
"testing"
"time"

"github.com/stretchr/testify/assert"
"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config/configauth"
"go.opentelemetry.io/collector/config/confighttp"
"go.opentelemetry.io/collector/config/configopaque"
"go.opentelemetry.io/collector/config/configtls"
"go.opentelemetry.io/collector/confmap"
)

func TestValidate(t *testing.T) {
idleConnTimeout := 30 * time.Second
maxIdleConn := 300
maxIdleConnPerHost := 150
maxConnPerHost := 250
ty, err := component.NewType("ty")
assert.NoError(t, err)
auth := configauth.Authentication{AuthenticatorID: component.NewID(ty)}

tests := []struct {
name string
Expand Down Expand Up @@ -94,8 +107,8 @@ func TestValidate(t *testing.T) {
name: "TLS settings are valid",
cfg: &Config{
API: APIConfig{Key: "notnull"},
LimitedClientConfig: LimitedClientConfig{
TLSSetting: LimitedTLSClientSettings{
ClientConfig: confighttp.ClientConfig{
TLSSetting: configtls.ClientConfig{
InsecureSkipVerify: true,
},
},
Expand All @@ -115,6 +128,40 @@ func TestValidate(t *testing.T) {
Traces: TracesConfig{PeerTags: []string{"tag1", "tag2"}},
},
},
{
name: "With confighttp client configs",
cfg: &Config{
API: APIConfig{Key: "notnull"},
ClientConfig: confighttp.ClientConfig{
ReadBufferSize: 100,
WriteBufferSize: 200,
Timeout: 10 * time.Second,
IdleConnTimeout: &idleConnTimeout,
MaxIdleConns: &maxIdleConn,
MaxIdleConnsPerHost: &maxIdleConnPerHost,
MaxConnsPerHost: &maxConnPerHost,
DisableKeepAlives: true,
TLSSetting: configtls.ClientConfig{InsecureSkipVerify: true},
},
},
},

{
name: "unsupported confighttp client configs",
cfg: &Config{
API: APIConfig{Key: "notnull"},
ClientConfig: confighttp.ClientConfig{
Endpoint: "endpoint",
Compression: "gzip",
ProxyURL: "proxy",
Auth: &auth,
Headers: map[string]configopaque.String{"key": "val"},
HTTP2ReadIdleTimeout: 250,
HTTP2PingTimeout: 200,
},
},
err: "these confighttp client configs are currently not respected by Datadog exporter: auth, endpoint, compression, proxy_url, headers, http2_read_idle_timeout, http2_ping_timeout",
},
}
for _, testInstance := range tests {
t.Run(testInstance.name, func(t *testing.T) {
Expand All @@ -129,10 +176,26 @@ func TestValidate(t *testing.T) {
}

func TestUnmarshal(t *testing.T) {
cfgWithHTTPConfigs := NewFactory().CreateDefaultConfig().(*Config)
idleConnTimeout := 30 * time.Second
maxIdleConn := 300
maxIdleConnPerHost := 150
maxConnPerHost := 250
cfgWithHTTPConfigs.ReadBufferSize = 100
cfgWithHTTPConfigs.WriteBufferSize = 200
cfgWithHTTPConfigs.Timeout = 10 * time.Second
cfgWithHTTPConfigs.MaxIdleConns = &maxIdleConn
cfgWithHTTPConfigs.MaxIdleConnsPerHost = &maxIdleConnPerHost
cfgWithHTTPConfigs.MaxConnsPerHost = &maxConnPerHost
cfgWithHTTPConfigs.IdleConnTimeout = &idleConnTimeout
cfgWithHTTPConfigs.DisableKeepAlives = true
cfgWithHTTPConfigs.TLSSetting.InsecureSkipVerify = true
cfgWithHTTPConfigs.warnings = nil

tests := []struct {
name string
configMap *confmap.Conf
cfg Config
cfg *Config
err string
}{
{
Expand Down Expand Up @@ -264,6 +327,21 @@ func TestUnmarshal(t *testing.T) {
}),
err: "\"metrics::sums::initial_cumulative_monotonic_value\" can only be configured when \"metrics::sums::cumulative_monotonic_mode\" is set to \"to_delta\"",
},
{
name: "unmarshall confighttp client configs",
configMap: confmap.NewFromStringMap(map[string]any{
"read_buffer_size": 100,
"write_buffer_size": 200,
"timeout": "10s",
"max_idle_conns": 300,
"max_idle_conns_per_host": 150,
"max_conns_per_host": 250,
"disable_keep_alives": true,
"idle_conn_timeout": "30s",
"tls": map[string]any{"insecure_skip_verify": true},
}),
cfg: cfgWithHTTPConfigs,
},
}

f := NewFactory()
Expand Down
21 changes: 21 additions & 0 deletions exporter/datadogexporter/examples/collector.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,27 @@ exporters:
## @param tls - boolean - optional - default: false
# insecure_skip_verify: false

## @param read_buffer_size - integer - optional
## @param write_buffer_size - integer - optional
## @param timeout - duration - optional
## @param max_idle_conns - integer - optional
## @param max_idle_conns_per_host - integer - optional
## @param max_conns_per_host - integer - optional
## @param idle_conn_timeout - duration - optional
## @param disable_keep_alives - boolean - optional
##
## Below are a subset of configs in confighttp that are respected by Datadog exporter.
## See https://pkg.go.dev/go.opentelemetry.io/collector/config/confighttp for details on these configs.
##
# read_buffer_size: 0
# write_buffer_size: 0
# timeout: 15s
# max_idle_conns: 100
# max_idle_conns_per_host: 0
# max_conns_per_host: 0
# idle_conn_timeout: 0s
# disable_keep_alives: false

## @param metrics - custom object - optional
## Metric exporter specific configuration.
#
Expand Down
1 change: 1 addition & 0 deletions exporter/datadogexporter/examples/logs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ processors:

exporters:
datadog:
idle_conn_timeout: 10s
api:
site: ${env:DD_SITE}
key: ${env:DD_API_KEY}
Expand Down
12 changes: 7 additions & 5 deletions exporter/datadogexporter/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"github.com/DataDog/opentelemetry-mapping-go/pkg/otlp/attributes"
"github.com/DataDog/opentelemetry-mapping-go/pkg/otlp/attributes/source"
"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config/confighttp"
"go.opentelemetry.io/collector/config/confignet"
"go.opentelemetry.io/collector/config/configretry"
"go.opentelemetry.io/collector/consumer"
Expand Down Expand Up @@ -170,18 +171,19 @@ func NewFactory() exporter.Factory {
return newFactoryWithRegistry(featuregate.GlobalRegistry())
}

func defaulttimeoutSettings() exporterhelper.TimeoutSettings {
return exporterhelper.TimeoutSettings{
func defaultClientConfig() confighttp.ClientConfig {
// do not use NewDefaultClientConfig for backwards-compatibility
return confighttp.ClientConfig{
Timeout: 15 * time.Second,
}
}

// createDefaultConfig creates the default exporter configuration
func (f *factory) createDefaultConfig() component.Config {
return &Config{
TimeoutSettings: defaulttimeoutSettings(),
BackOffConfig: configretry.NewDefaultBackOffConfig(),
QueueSettings: exporterhelper.NewDefaultQueueSettings(),
ClientConfig: defaultClientConfig(),
BackOffConfig: configretry.NewDefaultBackOffConfig(),
QueueSettings: exporterhelper.NewDefaultQueueSettings(),

API: APIConfig{
Site: "datadoghq.com",
Expand Down
Loading

0 comments on commit 135d723

Please sign in to comment.