From 098744ae03e4a83f7b300febc88062e6e7520f12 Mon Sep 17 00:00:00 2001
From: Andrey Krylov <andrey.krylov@pm.me>
Date: Thu, 30 Jan 2025 09:54:14 +0300
Subject: [PATCH] fix config key collision: un-squash client config (issue
 37332)

---
 .../prometheusremotewriteexporter/README.md   | 21 ++++++---
 .../prometheusremotewriteexporter/config.go   | 43 +++++++++++++++++++
 .../config_test.go                            | 34 +++++++++++++++
 .../prometheusremotewriteexporter/exporter.go |  9 +++-
 .../exporter_test.go                          | 30 +++++++++++++
 .../prometheusremotewriteexporter/factory.go  | 16 +++----
 .../testdata/config.yaml                      |  5 +++
 7 files changed, 141 insertions(+), 17 deletions(-)

diff --git a/exporter/prometheusremotewriteexporter/README.md b/exporter/prometheusremotewriteexporter/README.md
index 360871c236ed..55481c70edec 100644
--- a/exporter/prometheusremotewriteexporter/README.md
+++ b/exporter/prometheusremotewriteexporter/README.md
@@ -29,14 +29,16 @@ how this exporter works.
 
 The following settings are required:
 
-- `endpoint` (no default): The remote write URL to send remote write samples.
+- `prw_client`
+  - `endpoint` (no default): The remote write URL to send remote write samples.
+- `endpoint`: DEPRECATED
 
-By default, TLS is enabled and must be configured under `tls:`:
+By default, TLS is enabled and must be configured under `prw_client.tls:`:
 
 - `insecure` (default = `false`): whether to enable client transport security for
   the exporter's connection.
 
-As a result, the following parameters are also required under `tls:`:
+As a result, the following parameters are also required under `prw_client.tls:`:
 
 - `cert_file` (no default): path to the TLS cert to use for TLS required connections. Should
   only be used if `insecure` is set to false.
@@ -46,8 +48,11 @@ As a result, the following parameters are also required under `tls:`:
 The following settings can be optionally configured:
 
 - `external_labels`: map of labels names and values to be attached to each metric data point
-- `headers`: additional headers attached to each HTTP request.
-  - *Note the following headers cannot be changed: `Content-Encoding`, `Content-Type`, `X-Prometheus-Remote-Write-Version`, and `User-Agent`.*
+- `prw_client`
+  - `headers`: additional headers attached to each HTTP request.
+    - *Note the following headers cannot be changed: `Content-Encoding`, `Content-Type`, `X-Prometheus-Remote-Write-Version`, and `User-Agent`.*
+  - `timeout`: sets timeout for HTTP requests to Prometheus remote write endpoint (default 5s).
+- `headers`: DEPRECATED
 - `namespace`: prefix attached to each exported metric name.
 - `add_metric_suffixes`: If set to false, type and unit suffixes will not be added to metrics. Default: true.
 - `send_metadata`: If set to true, prometheus metadata will be generated and sent. Default: false.
@@ -73,7 +78,8 @@ Example:
 ```yaml
 exporters:
   prometheusremotewrite:
-    endpoint: "https://my-cortex:7900/api/v1/push"
+    prw_client:
+      endpoint: "https://my-cortex:7900/api/v1/push"
     wal: # Enabling the Write-Ahead-Log for the exporter.
       directory: ./prom_rw # The directory to store the WAL in
       buffer_size: 100 # Optional count of elements to be read from the WAL before truncating; default of 300
@@ -87,7 +93,8 @@ Example:
 ```yaml
 exporters:
   prometheusremotewrite:
-    endpoint: "https://my-cortex:7900/api/v1/push"
+    prw_client:
+      endpoint: "https://my-cortex:7900/api/v1/push"
     external_labels:
       label_name1: label_value1
       label_name2: label_value2
diff --git a/exporter/prometheusremotewriteexporter/config.go b/exporter/prometheusremotewriteexporter/config.go
index 34a35fabd41f..e24fddf27962 100644
--- a/exporter/prometheusremotewriteexporter/config.go
+++ b/exporter/prometheusremotewriteexporter/config.go
@@ -9,6 +9,7 @@ import (
 	"go.opentelemetry.io/collector/component"
 	"go.opentelemetry.io/collector/config/confighttp"
 	"go.opentelemetry.io/collector/config/configretry"
+	"go.opentelemetry.io/collector/confmap"
 	"go.opentelemetry.io/collector/exporter/exporterhelper"
 
 	"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/resourcetotelemetry"
@@ -31,6 +32,8 @@ type Config struct {
 	ExternalLabels map[string]string `mapstructure:"external_labels"`
 
 	ClientConfig confighttp.ClientConfig `mapstructure:",squash"` // squash ensures fields are correctly decoded in embedded struct.
+	// PRWClient configures HTTP client for Prometheus remote write
+	PRWClient *confighttp.ClientConfig `mapstructure:"prw_client,omitempty"`
 
 	// maximum size in bytes of time series batch sent to remote storage
 	MaxBatchSizeBytes int `mapstructure:"max_batch_size_bytes"`
@@ -59,6 +62,16 @@ type Config struct {
 	SendMetadata bool `mapstructure:"send_metadata"`
 }
 
+func (cfg *Config) Unmarshal(c *confmap.Conf) error {
+	if err := c.Unmarshal(cfg); err != nil {
+		return err
+	}
+	if cfg.PRWClient != nil {
+		setDefaultsPRWClientConfig(cfg.PRWClient)
+	}
+	return nil
+}
+
 type CreatedMetric struct {
 	// Enabled if true the _created metrics could be exported
 	Enabled bool `mapstructure:"enabled"`
@@ -126,3 +139,33 @@ func (cfg *Config) Validate() error {
 
 	return nil
 }
+
+func setDefaultsPRWClientConfig(cfg *confighttp.ClientConfig) {
+	if cfg.Endpoint == "" {
+		cfg.Endpoint = "http://some.url:9411/api/prom/push"
+	}
+	// We almost read 0 bytes, so no need to tune ReadBufferSize.
+	if cfg.WriteBufferSize == 0 {
+		cfg.WriteBufferSize = 512 * 1024
+	}
+	if cfg.Timeout == 0 {
+		cfg.Timeout = exporterhelper.NewDefaultTimeoutConfig().Timeout
+	}
+
+	defaultCfg := confighttp.NewDefaultClientConfig()
+	if cfg.Headers == nil {
+		cfg.Headers = defaultCfg.Headers
+	}
+	if cfg.MaxIdleConns == nil {
+		cfg.MaxIdleConns = defaultCfg.MaxIdleConns
+	}
+	if cfg.MaxIdleConnsPerHost == nil {
+		cfg.MaxIdleConnsPerHost = defaultCfg.MaxIdleConnsPerHost
+	}
+	if cfg.MaxConnsPerHost == nil {
+		cfg.MaxConnsPerHost = defaultCfg.MaxConnsPerHost
+	}
+	if cfg.IdleConnTimeout == nil {
+		cfg.IdleConnTimeout = defaultCfg.IdleConnTimeout
+	}
+}
diff --git a/exporter/prometheusremotewriteexporter/config_test.go b/exporter/prometheusremotewriteexporter/config_test.go
index cc851f68fb33..49f7262091f6 100644
--- a/exporter/prometheusremotewriteexporter/config_test.go
+++ b/exporter/prometheusremotewriteexporter/config_test.go
@@ -44,6 +44,9 @@ func TestLoadConfig(t *testing.T) {
 		"Prometheus-Remote-Write-Version": "0.1.0",
 		"X-Scope-OrgID":                   "234",
 	}
+	prwClientConfig := newDefaultPRWClientConfig()
+	prwClientConfig.Endpoint = "localhost:8888"
+	prwClientConfig.Timeout = 11 * time.Second
 	tests := []struct {
 		id           component.ID
 		expected     component.Config
@@ -76,6 +79,7 @@ func TestLoadConfig(t *testing.T) {
 				Namespace:                   "test-space",
 				ExternalLabels:              map[string]string{"key1": "value1", "key2": "value2"},
 				ClientConfig:                clientConfig,
+				PRWClient:                   nil,
 				ResourceToTelemetrySettings: resourcetotelemetry.Settings{Enabled: true},
 				TargetInfo: &TargetInfo{
 					Enabled: true,
@@ -83,6 +87,36 @@ func TestLoadConfig(t *testing.T) {
 				CreatedMetric: &CreatedMetric{Enabled: true},
 			},
 		},
+		{
+			id: component.NewIDWithName(metadata.Type, "prw_client"),
+			expected: &Config{
+				MaxBatchSizeBytes: 3000000,
+				TimeoutSettings:   exporterhelper.NewDefaultTimeoutConfig(),
+				BackOffConfig: configretry.BackOffConfig{
+					Enabled:             true,
+					InitialInterval:     50 * time.Millisecond,
+					MaxInterval:         30 * time.Second,
+					MaxElapsedTime:      5 * time.Minute,
+					RandomizationFactor: backoff.DefaultRandomizationFactor,
+					Multiplier:          backoff.DefaultMultiplier,
+				},
+				RemoteWriteQueue: RemoteWriteQueue{
+					Enabled:      true,
+					QueueSize:    10000,
+					NumConsumers: 5,
+				},
+				AddMetricSuffixes:           true,
+				Namespace:                   "",
+				ExternalLabels:              make(map[string]string),
+				ClientConfig:                newDefaultPRWClientConfig(),
+				PRWClient:                   &prwClientConfig,
+				ResourceToTelemetrySettings: resourcetotelemetry.Settings{Enabled: false},
+				TargetInfo: &TargetInfo{
+					Enabled: true,
+				},
+				CreatedMetric: &CreatedMetric{Enabled: false},
+			},
+		},
 		{
 			id:           component.NewIDWithName(metadata.Type, "negative_queue_size"),
 			errorMessage: "remote write queue size can't be negative",
diff --git a/exporter/prometheusremotewriteexporter/exporter.go b/exporter/prometheusremotewriteexporter/exporter.go
index 6ad7d31981c0..99b138864c69 100644
--- a/exporter/prometheusremotewriteexporter/exporter.go
+++ b/exporter/prometheusremotewriteexporter/exporter.go
@@ -112,7 +112,12 @@ func newPRWExporter(cfg *Config, set exporter.Settings) (*prwExporter, error) {
 		return nil, err
 	}
 
-	endpointURL, err := url.ParseRequestURI(cfg.ClientConfig.Endpoint)
+	clientSettings := &cfg.ClientConfig
+	if cfg.PRWClient != nil {
+		clientSettings = cfg.PRWClient
+	}
+
+	endpointURL, err := url.ParseRequestURI(clientSettings.Endpoint)
 	if err != nil {
 		return nil, errors.New("invalid endpoint")
 	}
@@ -139,7 +144,7 @@ func newPRWExporter(cfg *Config, set exporter.Settings) (*prwExporter, error) {
 		userAgentHeader:   userAgentHeader,
 		maxBatchSizeBytes: cfg.MaxBatchSizeBytes,
 		concurrency:       concurrency,
-		clientSettings:    &cfg.ClientConfig,
+		clientSettings:    clientSettings,
 		settings:          set.TelemetrySettings,
 		retrySettings:     cfg.BackOffConfig,
 		retryOnHTTP429:    retryOn429FeatureGate.IsEnabled(),
diff --git a/exporter/prometheusremotewriteexporter/exporter_test.go b/exporter/prometheusremotewriteexporter/exporter_test.go
index bd2e32644cac..3a27c5b3c410 100644
--- a/exporter/prometheusremotewriteexporter/exporter_test.go
+++ b/exporter/prometheusremotewriteexporter/exporter_test.go
@@ -138,6 +138,36 @@ func Test_NewPRWExporter(t *testing.T) {
 	}
 }
 
+func Test_NewPRWExporter_ClientConfig(t *testing.T) {
+	prwClientConfig := confighttp.NewDefaultClientConfig()
+	prwClientConfig.Endpoint = "overridden.endpoint:8080"
+	cfg := &Config{
+		TimeoutSettings: exporterhelper.TimeoutConfig{},
+		BackOffConfig:   configretry.BackOffConfig{},
+		Namespace:       "",
+		ExternalLabels:  map[string]string{},
+		ClientConfig:    confighttp.NewDefaultClientConfig(),
+		PRWClient:       &prwClientConfig,
+		TargetInfo: &TargetInfo{
+			Enabled: true,
+		},
+		CreatedMetric: &CreatedMetric{
+			Enabled: false,
+		},
+	}
+	buildInfo := component.BuildInfo{
+		Description: "OpenTelemetry Collector",
+		Version:     "1.0",
+	}
+	set := exportertest.NewNopSettings()
+	set.BuildInfo = buildInfo
+
+	prwe, err := newPRWExporter(cfg, set)
+	assert.NoError(t, err)
+
+	assert.Equal(t, "overridden.endpoint:8080", prwe.endpointURL.String())
+}
+
 // Test_Start checks if the client is properly created as expected.
 func Test_Start(t *testing.T) {
 	cfg := &Config{
diff --git a/exporter/prometheusremotewriteexporter/factory.go b/exporter/prometheusremotewriteexporter/factory.go
index 4333d89396de..e3edce286165 100644
--- a/exporter/prometheusremotewriteexporter/factory.go
+++ b/exporter/prometheusremotewriteexporter/factory.go
@@ -82,16 +82,15 @@ func createMetricsExporter(ctx context.Context, set exporter.Settings,
 	return resourcetotelemetry.WrapMetricsExporter(prwCfg.ResourceToTelemetrySettings, exporter), nil
 }
 
+func newDefaultPRWClientConfig() confighttp.ClientConfig {
+	cfg := confighttp.NewDefaultClientConfig()
+	setDefaultsPRWClientConfig(&cfg)
+	return cfg
+}
+
 func createDefaultConfig() component.Config {
 	retrySettings := configretry.NewDefaultBackOffConfig()
 	retrySettings.InitialInterval = 50 * time.Millisecond
-	clientConfig := confighttp.NewDefaultClientConfig()
-	clientConfig.Endpoint = "http://some.url:9411/api/prom/push"
-	// We almost read 0 bytes, so no need to tune ReadBufferSize.
-	clientConfig.ReadBufferSize = 0
-	clientConfig.WriteBufferSize = 512 * 1024
-	clientConfig.Timeout = exporterhelper.NewDefaultTimeoutConfig().Timeout
-
 	numConsumers := 5
 	if enableMultipleWorkersFeatureGate.IsEnabled() {
 		numConsumers = 1
@@ -106,7 +105,8 @@ func createDefaultConfig() component.Config {
 		BackOffConfig:     retrySettings,
 		AddMetricSuffixes: true,
 		SendMetadata:      false,
-		ClientConfig:      clientConfig,
+		ClientConfig:      newDefaultPRWClientConfig(),
+		PRWClient:         nil,
 		// TODO(jbd): Adjust the default queue size.
 		RemoteWriteQueue: RemoteWriteQueue{
 			Enabled:      true,
diff --git a/exporter/prometheusremotewriteexporter/testdata/config.yaml b/exporter/prometheusremotewriteexporter/testdata/config.yaml
index 8c80bf21409d..51a5755f6980 100644
--- a/exporter/prometheusremotewriteexporter/testdata/config.yaml
+++ b/exporter/prometheusremotewriteexporter/testdata/config.yaml
@@ -27,6 +27,11 @@ prometheusremotewrite/2:
     queue_size: 2000
     num_consumers: 10
 
+prometheusremotewrite/prw_client:
+  prw_client:
+    endpoint: "localhost:8888"
+    timeout: "11s"
+
 prometheusremotewrite/negative_queue_size:
   endpoint: "localhost:8888"
   remote_write_queue: