[exporter/datadog] Respect confighttp configs and use better defaults #31733
Conversation
| // Config defines configuration for the Datadog exporter. | ||
| type Config struct { | ||
| exporterhelper.TimeoutSettings `mapstructure:",squash"` // squash ensures fields are correctly decoded in embedded struct. |
There was a problem hiding this comment.
Note: need to get rid of TimeoutSettings because TimeoutSettings.Timeout and ClientConfig.Timeout have the same mapstructure key, which is not allowed. Same reason applies to LimitedClientConfig / tls
| @@ -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 { | |||
There was a problem hiding this comment.
I am undecided on whether this should be on Unmarshal or on Validate, do you have a strong opinion on this?
There was a problem hiding this comment.
I don't feel strongly either. Those are valid (masharlable) confighttp options but we manually apply restriction from datadog exporter side, so I put it in Validate.
One benefit of putting in Unmarshal is that we can distinguish between not set vs. set to empty / 0, although I don't see it being very meaningful in this case.
| @@ -490,6 +482,36 @@ func (c *Config) Validate() error { | |||
| return nil | |||
| } | |||
|
|
|||
| func validateClientConfig(cfg confighttp.ClientConfig) error { | |||
There was a problem hiding this comment.
A downside of this is that if new options are added to confighttp.ClientConfig, they won't be caught by this check. I guess using reflection would work here, but it's a bit of a pain
There was a problem hiding this comment.
I'd avoid using reflection too. In this case reflection also won't give us the mapstructure key, so we either can only return a generic error on new options or can only use the struct field name (which is different from mapstructure key), neither seems ideal.
Description:
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).