From 59f889cd3fe2108c25db2fc722c2eb8acd34265c Mon Sep 17 00:00:00 2001
From: Zakhar Bessarab <z.bessarab@victoriametrics.com>
Date: Thu, 1 Dec 2022 12:57:19 +0400
Subject: [PATCH] app/vmalert: add `remoteWrite.sendTimeout` command-line flag
 to configure timeout for sending data to `remoteWrite.url` (#3423)

* app/vmalert: add `remoteWrite.sendTimeout` command-line flag to configure timeout for sending data to `remoteWrite.url`

* vmalert: remove WriteTimeout from clients Cfg
No need to have it as a part of configuration struct:
* the client isn't used by other packages;
* there are no internal tests to check the WriteTimeout.

* vmalert: remove DisablePathAppend from clients Cfg
No need to have it as a part of configuration struct:
* the client isn't used by other packages;
* there are no internal tests to check the DisablePathAppend.

Co-authored-by: hagen1778 <roman@victoriametrics.com>
---
 app/vmalert/README.md                  |  2 ++
 app/vmalert/remotewrite/init.go        | 15 +++++++--------
 app/vmalert/remotewrite/remotewrite.go | 11 ++---------
 docs/CHANGELOG.md                      |  1 +
 4 files changed, 12 insertions(+), 17 deletions(-)

diff --git a/app/vmalert/README.md b/app/vmalert/README.md
index ac7e8b4da..266924ba8 100644
--- a/app/vmalert/README.md
+++ b/app/vmalert/README.md
@@ -1050,6 +1050,8 @@ The shortlist of configuration flags is the following:
      Optional OAuth2 scopes to use for -notifier.url. Scopes must be delimited by ';'.
   -remoteWrite.oauth2.tokenUrl string
      Optional OAuth2 tokenURL to use for -notifier.url.
+  -remoteWrite.sendTimeout duration
+     Timeout for sending data to the configured -remoteWrite.url. (default 30s)
   -remoteWrite.showURL
      Whether to show -remoteWrite.url in the exported metrics. It is hidden by default, since it can contain sensitive info such as auth key
   -remoteWrite.tlsCAFile string
diff --git a/app/vmalert/remotewrite/init.go b/app/vmalert/remotewrite/init.go
index 27abcbf3f..98030113f 100644
--- a/app/vmalert/remotewrite/init.go
+++ b/app/vmalert/remotewrite/init.go
@@ -77,13 +77,12 @@ func Init(ctx context.Context) (*Client, error) {
 	}
 
 	return NewClient(ctx, Config{
-		Addr:              *addr,
-		AuthCfg:           authCfg,
-		Concurrency:       *concurrency,
-		MaxQueueSize:      *maxQueueSize,
-		MaxBatchSize:      *maxBatchSize,
-		FlushInterval:     *flushInterval,
-		DisablePathAppend: *disablePathAppend,
-		Transport:         t,
+		Addr:          *addr,
+		AuthCfg:       authCfg,
+		Concurrency:   *concurrency,
+		MaxQueueSize:  *maxQueueSize,
+		MaxBatchSize:  *maxBatchSize,
+		FlushInterval: *flushInterval,
+		Transport:     t,
 	})
 }
diff --git a/app/vmalert/remotewrite/remotewrite.go b/app/vmalert/remotewrite/remotewrite.go
index 8aca17992..4fb0db7c6 100644
--- a/app/vmalert/remotewrite/remotewrite.go
+++ b/app/vmalert/remotewrite/remotewrite.go
@@ -22,6 +22,7 @@ import (
 
 var (
 	disablePathAppend = flag.Bool("remoteWrite.disablePathAppend", false, "Whether to disable automatic appending of '/api/v1/write' path to the configured -remoteWrite.url.")
+	sendTimeout       = flag.Duration("remoteWrite.sendTimeout", 30*time.Second, "Timeout for sending data to the configured -remoteWrite.url.")
 )
 
 // Client is an asynchronous HTTP client for writing
@@ -57,13 +58,8 @@ type Config struct {
 	MaxQueueSize int
 	// FlushInterval defines time interval for flushing batches
 	FlushInterval time.Duration
-	// WriteTimeout defines timeout for HTTP write request
-	// to remote storage
-	WriteTimeout time.Duration
 	// Transport will be used by the underlying http.Client
 	Transport *http.Transport
-	// DisablePathAppend can be used to not automatically append '/api/v1/write' to the remote write url
-	DisablePathAppend bool
 }
 
 const (
@@ -89,9 +85,6 @@ func NewClient(ctx context.Context, cfg Config) (*Client, error) {
 	if cfg.FlushInterval == 0 {
 		cfg.FlushInterval = defaultFlushInterval
 	}
-	if cfg.WriteTimeout == 0 {
-		cfg.WriteTimeout = defaultWriteTimeout
-	}
 	if cfg.Transport == nil {
 		cfg.Transport = http.DefaultTransport.(*http.Transport).Clone()
 	}
@@ -101,7 +94,7 @@ func NewClient(ctx context.Context, cfg Config) (*Client, error) {
 	}
 	c := &Client{
 		c: &http.Client{
-			Timeout:   cfg.WriteTimeout,
+			Timeout:   *sendTimeout,
 			Transport: cfg.Transport,
 		},
 		addr:          strings.TrimSuffix(cfg.Addr, "/"),
diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md
index 70ba70750..3aa21d696 100644
--- a/docs/CHANGELOG.md
+++ b/docs/CHANGELOG.md
@@ -19,6 +19,7 @@ The following tip changes can be tested by building VictoriaMetrics components f
 * FEATURE: [vmagent](https://docs.victoriametrics.com/vmagent.html): add `exported_` prefix to metric names exported by scrape targets if these metric names clash with [automatically generated metrics](https://docs.victoriametrics.com/vmagent.html#automatically-generated-metrics) such as `up`, `scrape_samples_scraped`, etc. This prevents from corruption of automatically generated metrics. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3406).
 * FEATURE: [VictoriaMetrics cluster](https://docs.victoriametrics.com/Cluster-VictoriaMetrics.html): improve error message when the requested path cannot be properly parsed, so users could identify the issue and properly fix the path. Now the error message links to [url format docs](https://docs.victoriametrics.com/Cluster-VictoriaMetrics.html#url-format). See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3402).
 * FEATURE: [vmctl](https://docs.victoriametrics.com/vmctl.html): add ability to copy data from sources via Prometheus `remote_read` protocol. See [these docs](https://docs.victoriametrics.com/vmctl.html#migrating-data-by-remote-read-protocol). The related issues: [one](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3132) and [two](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/1101).
+* FEATURE: [vmalert](https://docs.victoriametrics.com/vmalert.html): add `remoteWrite.sendTimeout` command-line flag to configure timeout for sending data to remote write URL. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3408)
 
 * BUGFIX: [vmalert](https://docs.victoriametrics.com/vmalert.html): properly pass HTTP headers during the alert state restore procedure. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3418).