Add mechanism for choosing between the two metadata-sending methods, defaulting to the new one

tpaschalis · tpaschalis · commit 42fc28821aaf · 2023-04-28T11:55:34.000+03:00
Signed-off-by: Paschalis Tsilias &lt;paschalist0@gmail.com&gt;
diff --git a/cmd/prometheus/main.go b/cmd/prometheus/main.go
@@ -413,7 +413,7 @@ func main() {
 	a.Flag("scrape.discovery-reload-interval", "Interval used by scrape manager to throttle target groups updates.").
 		Hidden().Default("5s").SetValue(&cfg.scrape.DiscoveryReloadInterval)
 
-	a.Flag("enable-feature", "Comma separated feature names to enable. Valid options: agent, exemplar-storage, expand-external-labels, memory-snapshot-on-shutdown, promql-at-modifier, promql-negative-offset, promql-per-step-stats, remote-write-receiver (DEPRECATED), extra-scrape-metrics, new-service-discovery-manager, auto-gomaxprocs, no-default-scrape-port, native-histograms. See https://prometheus.io/docs/prometheus/latest/feature_flags/ for more details.").
+	a.Flag("enable-feature", "Comma separated feature names to enable. Valid options: agent, exemplar-storage, expand-external-labels, memory-snapshot-on-shutdown, promql-at-modifier, promql-negative-offset, promql-per-step-stats, remote-write-receiver (DEPRECATED), extra-scrape-metrics, new-service-discovery-manager, auto-gomaxprocs, no-default-scrape-port, native-histograms, metadata-storage. See https://prometheus.io/docs/prometheus/latest/feature_flags/ for more details.").
 		Default("").StringsVar(&cfg.featureList)
 
 	promlogflag.AddFlags(a, &cfg.promlogConfig)
@@ -500,6 +500,12 @@ func main() {
 		cfg.tsdb.OutOfOrderTimeWindow = cfgFile.StorageConfig.TSDBConfig.OutOfOrderTimeWindow
 	}
 
+	for _, rwc := range cfgFile.RemoteWriteConfigs {
+		if rwc.SendWALMetadata && !cfg.scrape.EnableMetadataStorage {
+			level.Warn(logger).Log("msg", "the 'send_metadata' remote_write parameter must be set along the --enable-features=metadata-storage flag to take effect")
+		}
+	}
+
 	// Now that the validity of the config is established, set the config
 	// success metrics accordingly, although the config isn't really loaded
 	// yet. This will happen later (including setting these metrics again),
diff --git a/docs/feature_flags.md b/docs/feature_flags.md
@@ -126,3 +126,12 @@ still ingest those conventional histograms that do not come with a
 corresponding native histogram. However, if a native histogram is present,
 Prometheus will ignore the corresponding conventional histogram, with the
 notable exception of exemplars, which are always ingested.
+
+## Metadata Storage
+`--enable-features=metadata-storage`
+
+When enabled, Prometheus will store metadata in-memory and keep track of
+metadata changes as WAL records. This should be used along the remote write's
+`send_metadata` parameter.
+This new way of storing and sending metadata is mutually exclusive with the
+`metadata_config.send` field and the current way of sending metadata.
diff --git a/storage/remote/queue_manager_test.go b/storage/remote/queue_manager_test.go
@@ -861,8 +861,6 @@ func (c *TestWriteClient) waitForExpectedData(tb testing.TB) {
 	}
 }
 
-var emptyMetadata prompb.Metadata
-
 func (c *TestWriteClient) Store(_ context.Context, req []byte) error {
 	c.mtx.Lock()
 	defer c.mtx.Unlock()
diff --git a/storage/remote/write.go b/storage/remote/write.go
@@ -21,6 +21,7 @@ import (
 	"time"
 
 	"github.com/go-kit/log"
+	"github.com/go-kit/log/level"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/promauto"
 
@@ -145,6 +146,15 @@ func (rws *WriteStorage) ApplyConfig(conf *config.Config) error {
 			return fmt.Errorf("duplicate remote write configs are not allowed, found duplicate for URL: %s", rwConf.URL)
 		}
 
+		// The current MetadataWatcher implementation is mutually exclusive
+		// with the new approach, which stores metadata as WAL records and
+		// ships them alongside series. If both mechanisms are set, the new one
+		// takes precedence by implicitly disabling the older one.
+		if rwConf.MetadataConfig.Send && rwConf.SendWALMetadata {
+			level.Warn(rws.logger).Log("msg", "the 'send_metadata' and 'metadata_config.send' parameters are mutually exclusive; defaulting to use 'send_metadata'")
+			rwConf.MetadataConfig.Send = false
+		}
+
 		// Set the queue name to the config hash if the user has not set
 		// a name in their remote write config so we can still differentiate
 		// between queues that have the same remote write endpoint.
diff --git a/storage/remote/write_test.go b/storage/remote/write_test.go
@@ -373,3 +373,43 @@ func TestWriteStorageApplyConfigsPartialUpdate(t *testing.T) {
 	err = s.Close()
 	require.NoError(t, err)
 }
+
+func TestMetadataStorageMechanismPreference(t *testing.T) {
+	dir := t.TempDir()
+	s := NewWriteStorage(nil, nil, dir, defaultFlushDeadline, nil)
+
+	c0 := &config.RemoteWriteConfig{
+		RemoteTimeout: model.Duration(10 * time.Second),
+		QueueConfig:   config.DefaultQueueConfig,
+		WriteRelabelConfigs: []*relabel.Config{
+			{
+				Regex: relabel.MustNewRegexp(".+"),
+			},
+		},
+		SendWALMetadata: true,
+		MetadataConfig:  config.DefaultMetadataConfig, // MetadataConfig.Send defaults to true
+	}
+	conf := &config.Config{
+		GlobalConfig:       config.GlobalConfig{},
+		RemoteWriteConfigs: []*config.RemoteWriteConfig{c0},
+	}
+	// We need to set URL's so that metric creation doesn't panic.
+	for i := range conf.RemoteWriteConfigs {
+		conf.RemoteWriteConfigs[i].URL = &common_config.URL{
+			URL: &url.URL{
+				Host: "http://test-storage.com",
+			},
+		}
+	}
+	hash, err := toHash(conf.RemoteWriteConfigs[0])
+	require.NoError(t, err)
+
+	require.NoError(t, s.ApplyConfig(conf))
+
+	require.Len(t, s.queues, 1)
+
+	// When both SendWALMetadata and MetadataConfig.Send are set to true,
+	// default to only using the new mechanism.
+	require.True(t, s.queues[hash].sendMetadata)
+	require.False(t, s.queues[hash].mcfg.Send)
+}

Original file line number	Diff line number	Diff line change
`@@ -861,8 +861,6 @@ func (c *TestWriteClient) waitForExpectedData(tb testing.TB) {`
`861`	`861`	`}`
`862`	`862`	`}`
`863`	`863`
`864`		`-var emptyMetadata prompb.Metadata`
`865`		`-`
`866`	`864`	`func (c *TestWriteClient) Store(_ context.Context, req []byte) error {`
`867`	`865`	`c.mtx.Lock()`
`868`	`866`	`defer c.mtx.Unlock()`