app/vmagent: substitute -remoteWrite.url with secret-url value in logs, since it may contain sensitive info such as passwords or auth tokens

Pass `-remoteWrite.showURL` command-line flag in order to see real `-remoteWrite.url` values in logs and at `/metrics` page.

Updates https://github.com/VictoriaMetrics/VictoriaMetrics/issues/773

app/vmagent/README.md

@@ -228,6 +228,10 @@ If you have suggestions, improvements or found a bug - feel free to open an issu
   The directory can grow large when remote storage is unavailable for extended periods of time and if `-remoteWrite.maxDiskUsagePerURL` isn't set.
   If you don't want to send all the data from the directory to remote storage, simply stop `vmagent` and delete the directory.
 
+* By default `vmagent` masks `-remoteWrite.url` with `secret-url` values in logs and at `/metrics` page because
+  the url may contain sensitive information such as auth tokens or passwords.
+  Pass `-remoteWrite.showURL` command-line flag when starting `vmagent` in order to see all the valid urls.
+
 * If you see `skipping duplicate scrape target with identical labels` errors when scraping Kubernetes pods, then it is likely these pods listen multiple ports
   or they use init container.
 

app/vmagent/remotewrite/client.go

@@ -44,7 +44,7 @@ var (
 )
 
 type client struct {
-	urlLabelValue  string
+	sanitizedURL   string
 	remoteWriteURL string
 	authHeader     string
 	fq             *persistentqueue.FastQueue
@@ -59,7 +59,7 @@ type client struct {
 	stopCh chan struct{}
 }
 
-func newClient(argIdx int, remoteWriteURL, urlLabelValue string, fq *persistentqueue.FastQueue, concurrency int) *client {
+func newClient(argIdx int, remoteWriteURL, sanitizedURL string, fq *persistentqueue.FastQueue, concurrency int) *client {
 	tlsCfg, err := getTLSConfig(argIdx)
 	if err != nil {
 		logger.Panicf("FATAL: cannot initialize TLS config: %s", err)
@@ -101,7 +101,7 @@ func newClient(argIdx int, remoteWriteURL, urlLabelValue string, fq *persistentq
 		authHeader = "Bearer " + token
 	}
 	c := &client{
-		urlLabelValue:  urlLabelValue,
+		sanitizedURL:   sanitizedURL,
 		remoteWriteURL: remoteWriteURL,
 		authHeader:     authHeader,
 		fq:             fq,
@@ -111,10 +111,10 @@ func newClient(argIdx int, remoteWriteURL, urlLabelValue string, fq *persistentq
 		},
 		stopCh: make(chan struct{}),
 	}
-	c.requestDuration = metrics.GetOrCreateHistogram(fmt.Sprintf(`vmagent_remotewrite_duration_seconds{url=%q}`, c.urlLabelValue))
+	c.requestDuration = metrics.GetOrCreateHistogram(fmt.Sprintf(`vmagent_remotewrite_duration_seconds{url=%q}`, c.sanitizedURL))
-	c.requestsOKCount = metrics.GetOrCreateCounter(fmt.Sprintf(`vmagent_remotewrite_requests_total{url=%q, status_code="2XX"}`, c.urlLabelValue))
+	c.requestsOKCount = metrics.GetOrCreateCounter(fmt.Sprintf(`vmagent_remotewrite_requests_total{url=%q, status_code="2XX"}`, c.sanitizedURL))
-	c.errorsCount = metrics.GetOrCreateCounter(fmt.Sprintf(`vmagent_remotewrite_errors_total{url=%q}`, c.urlLabelValue))
+	c.errorsCount = metrics.GetOrCreateCounter(fmt.Sprintf(`vmagent_remotewrite_errors_total{url=%q}`, c.sanitizedURL))
-	c.retriesCount = metrics.GetOrCreateCounter(fmt.Sprintf(`vmagent_remotewrite_retries_count_total{url=%q}`, c.urlLabelValue))
+	c.retriesCount = metrics.GetOrCreateCounter(fmt.Sprintf(`vmagent_remotewrite_retries_count_total{url=%q}`, c.sanitizedURL))
 	for i := 0; i < concurrency; i++ {
 		c.wg.Add(1)
 		go func() {
@@ -122,14 +122,14 @@ func newClient(argIdx int, remoteWriteURL, urlLabelValue string, fq *persistentq
 			c.runWorker()
 		}()
 	}
-	logger.Infof("initialized client for -remoteWrite.url=%q", c.remoteWriteURL)
+	logger.Infof("initialized client for -remoteWrite.url=%q", c.sanitizedURL)
 	return c
 }
 
 func (c *client) MustStop() {
 	close(c.stopCh)
 	c.wg.Wait()
-	logger.Infof("stopped client for -remoteWrite.url=%q", c.remoteWriteURL)
+	logger.Infof("stopped client for -remoteWrite.url=%q", c.sanitizedURL)
 }
 
 func getTLSConfig(argIdx int) (*tls.Config, error) {
@@ -176,7 +176,7 @@ func (c *client) runWorker() {
 				// The block has been sent successfully.
 			case <-time.After(graceDuration):
 				logger.Errorf("couldn't sent block with size %d bytes to %q in %.3f seconds during shutdown; dropping it",
-					len(block), c.remoteWriteURL, graceDuration.Seconds())
+					len(block), c.sanitizedURL, graceDuration.Seconds())
 			}
 			return
 		}
@@ -190,7 +190,7 @@ func (c *client) sendBlock(block []byte) {
 again:
 	req, err := http.NewRequest("POST", c.remoteWriteURL, bytes.NewBuffer(block))
 	if err != nil {
-		logger.Panicf("BUG: unexected error from http.NewRequest(%q): %s", c.remoteWriteURL, err)
+		logger.Panicf("BUG: unexected error from http.NewRequest(%q): %s", c.sanitizedURL, err)
 	}
 	h := req.Header
 	h.Set("User-Agent", "vmagent")
@@ -211,7 +211,7 @@ again:
 			retryDuration = time.Minute
 		}
 		logger.Errorf("couldn't send a block with size %d bytes to %q: %s; re-sending the block in %.3f seconds",
-			len(block), c.remoteWriteURL, err, retryDuration.Seconds())
+			len(block), c.sanitizedURL, err, retryDuration.Seconds())
 		t := time.NewTimer(retryDuration)
 		select {
 		case <-c.stopCh:
@@ -231,7 +231,7 @@ again:
 
 	// Unexpected status code returned
 	retriesCount++
-	metrics.GetOrCreateCounter(fmt.Sprintf(`vmagent_remotewrite_requests_total{url=%q, status_code="%d"}`, c.urlLabelValue, statusCode)).Inc()
+	metrics.GetOrCreateCounter(fmt.Sprintf(`vmagent_remotewrite_requests_total{url=%q, status_code="%d"}`, c.sanitizedURL, statusCode)).Inc()
 	retryDuration *= 2
 	if retryDuration > time.Minute {
 		retryDuration = time.Minute
@@ -239,10 +239,10 @@ again:
 	body, err := ioutil.ReadAll(resp.Body)
 	_ = resp.Body.Close()
 	if err != nil {
-		logger.Errorf("cannot read response body from %q during retry #%d: %s", c.remoteWriteURL, retriesCount, err)
+		logger.Errorf("cannot read response body from %q during retry #%d: %s", c.sanitizedURL, retriesCount, err)
 	} else {
 		logger.Errorf("unexpected status code received after sending a block with size %d bytes to %q during retry #%d: %d; response body=%q; "+
-			"re-sending the block in %.3f seconds", len(block), c.remoteWriteURL, retriesCount, statusCode, body, retryDuration.Seconds())
+			"re-sending the block in %.3f seconds", len(block), c.sanitizedURL, retriesCount, statusCode, body, retryDuration.Seconds())
 	}
 	t := time.NewTimer(retryDuration)
 	select {

app/vmagent/remotewrite/remotewrite.go

@@ -9,7 +9,6 @@ import (
 
 	"github.com/VictoriaMetrics/VictoriaMetrics/lib/decimal"
 	"github.com/VictoriaMetrics/VictoriaMetrics/lib/flagutil"
-	"github.com/VictoriaMetrics/VictoriaMetrics/lib/httpserver"
 	"github.com/VictoriaMetrics/VictoriaMetrics/lib/logger"
 	"github.com/VictoriaMetrics/VictoriaMetrics/lib/memory"
 	"github.com/VictoriaMetrics/VictoriaMetrics/lib/persistentqueue"
@@ -63,7 +62,7 @@ func Init() {
 	}
 	if !*showRemoteWriteURL {
 		// remoteWrite.url can contain authentication codes, so hide it at `/metrics` output.
-		httpserver.RegisterSecretFlag("remoteWrite.url")
+		flagutil.RegisterSecretFlag("remoteWrite.url")
 	}
 	initLabelsGlobal()
 	rcs, err := loadRelabelConfigs()
@@ -83,11 +82,11 @@ func Init() {
 		maxInmemoryBlocks = 2
 	}
 	for i, remoteWriteURL := range *remoteWriteURLs {
-		urlLabelValue := fmt.Sprintf("secret-url-%d", i+1)
+		sanitizedURL := fmt.Sprintf("%d:secret-url", i+1)
 		if *showRemoteWriteURL {
-			urlLabelValue = fmt.Sprintf("%d:%s", i+1, remoteWriteURL)
+			sanitizedURL = fmt.Sprintf("%d:%s", i+1, remoteWriteURL)
 		}
-		rwctx := newRemoteWriteCtx(i, remoteWriteURL, maxInmemoryBlocks, urlLabelValue)
+		rwctx := newRemoteWriteCtx(i, remoteWriteURL, maxInmemoryBlocks, sanitizedURL)
 		rwctxs = append(rwctxs, rwctx)
 	}
 
@@ -190,17 +189,17 @@ type remoteWriteCtx struct {
 	relabelMetricsDropped *metrics.Counter
 }
 
-func newRemoteWriteCtx(argIdx int, remoteWriteURL string, maxInmemoryBlocks int, urlLabelValue string) *remoteWriteCtx {
+func newRemoteWriteCtx(argIdx int, remoteWriteURL string, maxInmemoryBlocks int, sanitizedURL string) *remoteWriteCtx {
 	h := xxhash.Sum64([]byte(remoteWriteURL))
 	path := fmt.Sprintf("%s/persistent-queue/%d_%016X", *tmpDataPath, argIdx+1, h)
-	fq := persistentqueue.MustOpenFastQueue(path, remoteWriteURL, maxInmemoryBlocks, maxPendingBytesPerURL.N)
+	fq := persistentqueue.MustOpenFastQueue(path, sanitizedURL, maxInmemoryBlocks, maxPendingBytesPerURL.N)
-	_ = metrics.GetOrCreateGauge(fmt.Sprintf(`vmagent_remotewrite_pending_data_bytes{path=%q, url=%q}`, path, urlLabelValue), func() float64 {
+	_ = metrics.GetOrCreateGauge(fmt.Sprintf(`vmagent_remotewrite_pending_data_bytes{path=%q, url=%q}`, path, sanitizedURL), func() float64 {
 		return float64(fq.GetPendingBytes())
 	})
-	_ = metrics.GetOrCreateGauge(fmt.Sprintf(`vmagent_remotewrite_pending_inmemory_blocks{path=%q, url=%q}`, path, urlLabelValue), func() float64 {
+	_ = metrics.GetOrCreateGauge(fmt.Sprintf(`vmagent_remotewrite_pending_inmemory_blocks{path=%q, url=%q}`, path, sanitizedURL), func() float64 {
 		return float64(fq.GetInmemoryQueueLen())
 	})
-	c := newClient(argIdx, remoteWriteURL, urlLabelValue, fq, *queues)
+	c := newClient(argIdx, remoteWriteURL, sanitizedURL, fq, *queues)
 	pss := make([]*pendingSeries, *queues)
 	for i := range pss {
 		pss[i] = newPendingSeries(fq.MustWriteBlock)
@@ -211,7 +210,7 @@ func newRemoteWriteCtx(argIdx int, remoteWriteURL string, maxInmemoryBlocks int,
 		c:   c,
 		pss: pss,
 
-		relabelMetricsDropped: metrics.GetOrCreateCounter(fmt.Sprintf(`vmagent_remotewrite_relabel_metrics_dropped_total{path=%q, url=%q}`, path, urlLabelValue)),
+		relabelMetricsDropped: metrics.GetOrCreateCounter(fmt.Sprintf(`vmagent_remotewrite_relabel_metrics_dropped_total{path=%q, url=%q}`, path, sanitizedURL)),
 	}
 }
 

docs/vmagent.md

@@ -228,6 +228,10 @@ If you have suggestions, improvements or found a bug - feel free to open an issu
   The directory can grow large when remote storage is unavailable for extended periods of time and if `-remoteWrite.maxDiskUsagePerURL` isn't set.
   If you don't want to send all the data from the directory to remote storage, simply stop `vmagent` and delete the directory.
 
+* By default `vmagent` masks `-remoteWrite.url` with `secret-url` values in logs and at `/metrics` page because
+  the url may contain sensitive information such as auth tokens or passwords.
+  Pass `-remoteWrite.showURL` command-line flag when starting `vmagent` in order to see all the valid urls.
+
 * If you see `skipping duplicate scrape target with identical labels` errors when scraping Kubernetes pods, then it is likely these pods listen multiple ports
   or they use init container.
 

lib/flagutil/secret.go

@@ -0,0 +1,26 @@
+package flagutil
+
+import (
+	"strings"
+)
+
+// RegisterSecretFlag registers flagName as secret.
+//
+// This function must be called before starting logging.
+// It cannot be called from concurrent goroutines.
+//
+// Secret flags aren't exported at `/metrics` page.
+func RegisterSecretFlag(flagName string) {
+	lname := strings.ToLower(flagName)
+	secretFlags[lname] = true
+}
+
+var secretFlags = make(map[string]bool)
+
+// IsSecretFlag returns true of s contains flag name with secret value, which shouldn't be exposed.
+func IsSecretFlag(s string) bool {
+	if strings.Contains(s, "pass") || strings.Contains(s, "key") || strings.Contains(s, "secret") || strings.Contains(s, "token") {
+		return true
+	}
+	return secretFlags[s]
+}

lib/httpserver/metrics.go

@@ -8,6 +8,7 @@ import (
 	"time"
 
 	"github.com/VictoriaMetrics/VictoriaMetrics/lib/buildinfo"
+	"github.com/VictoriaMetrics/VictoriaMetrics/lib/flagutil"
 	"github.com/VictoriaMetrics/VictoriaMetrics/lib/memory"
 	"github.com/VictoriaMetrics/metrics"
 )
@@ -27,7 +28,7 @@ func WritePrometheusMetrics(w io.Writer) {
 	flag.VisitAll(func(f *flag.Flag) {
 		lname := strings.ToLower(f.Name)
 		value := f.Value.String()
-		if isSecretFlag(lname) {
+		if flagutil.IsSecretFlag(lname) {
 			// Do not expose passwords and keys to prometheus.
 			value = "secret"
 		}
@@ -36,23 +37,3 @@ func WritePrometheusMetrics(w io.Writer) {
 }
 
 var startTime = time.Now()
-
-// RegisterSecretFlag registers flagName as secret.
-//
-// This function must be called before starting httpserver.
-// It cannot be called from concurrent goroutines.
-//
-// Secret flags aren't exported at `/metrics` page.
-func RegisterSecretFlag(flagName string) {
-	lname := strings.ToLower(flagName)
-	secretFlags[lname] = true
-}
-
-var secretFlags = make(map[string]bool)
-
-func isSecretFlag(s string) bool {
-	if strings.Contains(s, "pass") || strings.Contains(s, "key") || strings.Contains(s, "secret") || strings.Contains(s, "token") {
-		return true
-	}
-	return secretFlags[s]
-}

lib/logger/flag.go

@@ -5,6 +5,7 @@ import (
 	"strings"
 
 	"github.com/VictoriaMetrics/VictoriaMetrics/lib/buildinfo"
+	"github.com/VictoriaMetrics/VictoriaMetrics/lib/flagutil"
 )
 
 func logAllFlags() {
@@ -13,7 +14,7 @@ func logAllFlags() {
 	flag.VisitAll(func(f *flag.Flag) {
 		lname := strings.ToLower(f.Name)
 		value := f.Value.String()
-		if strings.Contains(lname, "pass") || strings.Contains(lname, "key") || strings.Contains(lname, "secret") {
+		if flagutil.IsSecretFlag(lname) {
 			// Do not expose passwords and keys to prometheus.
 			value = "secret"
 		}