Clean up OTEL metadata option and code to be removed in 2.14 (#9069)

* Clean up OTEL metadata option and code to be removed in 2.14

The configuration was deprecated in 2.12 and set to be removed in 2.14. We
no longer use it anywhere internally.

Signed-off-by: Nick Pillitteri <[email protected]>

* Fix flakey API tests

Signed-off-by: Nick Pillitteri <[email protected]>

* Update integration test

Signed-off-by: Nick Pillitteri <[email protected]>

* Update CHANGELOG.md

Co-authored-by: Taylor C <[email protected]>

---------

Signed-off-by: Nick Pillitteri <[email protected]>
Co-authored-by: Taylor C <[email protected]>

CHANGELOG.md

@@ -26,6 +26,7 @@
 * [CHANGE] Distributor: Promote replying with `Retry-After` header on retryable errors to stable and set `-distributor.retry-after-header.enabled=true` by default. #8694
 * [CHANGE] Distributor: Replace `-distributor.retry-after-header.max-backoff-exponent` and `-distributor.retry-after-header.base-seconds` with `-distributor.retry-after-header.min-backoff` and `-distributor.retry-after-header.max-backoff` for easier configuration. #8694
 * [CHANGE] Ingester: increase the default inactivity timeout of active series (`-ingester.active-series-metrics-idle-timeout`) from `10m` to `20m`. #8975
+* [CHANGE] Distributor: Remove `-distributor.enable-otlp-metadata-storage` flag, which was deprecated in version 2.12. #9069
 * [FEATURE] Querier: add experimental streaming PromQL engine, enabled with `-querier.query-engine=mimir`. #8422 #8430 #8454 #8455 #8360 #8490 #8508 #8577 #8660 #8671 #8677 #8747 #8850 #8872 #8838 #8911 #8909 #8923 #8924 #8925 #8932 #8933 #8934 #8962 #8986 #8993 #8995 #9017 #9018
 * [FEATURE] Experimental Kafka-based ingest storage. #6888 #6894 #6929 #6940 #6951 #6974 #6982 #7029 #7030 #7091 #7142 #7147 #7148 #7153 #7160 #7193 #7349 #7376 #7388 #7391 #7393 #7394 #7402 #7404 #7423 #7424 #7437 #7486 #7503 #7508 #7540 #7621 #7682 #7685 #7694 #7695 #7696 #7697 #7701 #7733 #7734 #7741 #7752 #7838 #7851 #7871 #7877 #7880 #7882 #7887 #7891 #7925 #7955 #7967 #8031 #8063 #8077 #8088 #8135 #8176 #8184 #8194 #8216 #8217 #8222 #8233 #8503 #8542 #8579 #8657 #8686 #8688 #8703 #8706 #8708 #8738 #8750 #8778 #8808 #8809 #8841 #8842 #8845 #8853 #8886 #8988
   * What it is:

cmd/mimir/config-descriptor.json

@@ -85,17 +85,6 @@
           "fieldType": "boolean",
           "fieldCategory": "advanced"
         },
-        {
-          "kind": "field",
-          "name": "enable_otel_metadata_translation",
-          "required": false,
-          "desc": "If true, store metadata when ingesting metrics via OTLP. This makes metric descriptions and types available for metrics ingested via OTLP.",
-          "fieldValue": null,
-          "fieldDefaultValue": true,
-          "fieldFlag": "distributor.enable-otlp-metadata-storage",
-          "fieldType": "boolean",
-          "fieldCategory": "deprecated"
-        },
         {
           "kind": "field",
           "name": "get_request_for_ingester_shutdown_enabled",

cmd/mimir/help-all.txt.tmpl

@@ -1119,8 +1119,6 @@ Usage of ./cmd/mimir/mimir:
     	[experimental] When enabled, OTLP write requests are directly translated to Mimir equivalents, for optimum performance. (default true)
   -distributor.drop-label string
     	This flag can be used to specify label names that to drop during sample ingestion within the distributor and can be repeated in order to drop multiple labels.
-  -distributor.enable-otlp-metadata-storage
-    	[deprecated] If true, store metadata when ingesting metrics via OTLP. This makes metric descriptions and types available for metrics ingested via OTLP. (default true)
   -distributor.ha-tracker.cluster string
     	Prometheus label to look for in samples to identify a Prometheus HA cluster. (default "cluster")
   -distributor.ha-tracker.consul.acl-token string

docs/sources/mimir/configure/configuration-parameters/index.md

@@ -141,11 +141,6 @@ api:
   # CLI flag: -api.skip-label-name-validation-header-enabled
   [skip_label_name_validation_header_enabled: <boolean> | default = false]
 
-  # (deprecated) If true, store metadata when ingesting metrics via OTLP. This
-  # makes metric descriptions and types available for metrics ingested via OTLP.
-  # CLI flag: -distributor.enable-otlp-metadata-storage
-  [enable_otel_metadata_translation: <boolean> | default = true]
-
   # (deprecated) Enable GET requests to the /ingester/shutdown endpoint to
   # trigger an ingester shutdown. This is a potentially dangerous operation and
   # should only be enabled consciously.

integration/otlp_ingestion_test.go

@@ -6,6 +6,7 @@ package integration
 import (
 	"fmt"
 	"io"
+	"strconv"
 	"testing"
 	"time"
 
@@ -48,17 +49,12 @@ func testOTLPIngestion(t *testing.T, enableSuffixes bool) {
 
 	// Start Mimir in single binary mode, reading the config from file and overwriting
 	// the backend config to make it work with Minio.
-	enable := "false"
-	if enableSuffixes {
-		enable = "true"
-	}
 	flags := mergeFlags(
 		DefaultSingleBinaryFlags(),
 		BlocksStorageFlags(),
 		BlocksStorageS3Flags(),
 		map[string]string{
-			"-distributor.enable-otlp-metadata-storage": "true",
-			"-distributor.otel-metric-suffixes-enabled": enable,
+			"-distributor.otel-metric-suffixes-enabled": strconv.FormatBool(enableSuffixes),
 		},
 	)
 

pkg/api/api.go

@@ -50,9 +50,6 @@ type ConfigHandler func(actualCfg interface{}, defaultCfg interface{}) http.Hand
 type Config struct {
 	SkipLabelNameValidationHeader bool `yaml:"skip_label_name_validation_header_enabled" category:"advanced"`
 
-	// TODO: Remove option in Mimir 2.14.
-	EnableOtelMetadataStorage bool `yaml:"enable_otel_metadata_translation" category:"deprecated"`
-
 	// TODO: Remove option in Mimir 2.15.
 	GETRequestForIngesterShutdownEnabled bool `yaml:"get_request_for_ingester_shutdown_enabled" category:"deprecated"`
 
@@ -73,7 +70,6 @@ type Config struct {
 // RegisterFlags adds the flags required to config this to the given FlagSet.
 func (cfg *Config) RegisterFlags(f *flag.FlagSet) {
 	f.BoolVar(&cfg.SkipLabelNameValidationHeader, "api.skip-label-name-validation-header-enabled", false, "Allows to skip label name validation via X-Mimir-SkipLabelNameValidation header on the http write path. Use with caution as it breaks PromQL. Allowing this for external clients allows any client to send invalid label names. After enabling it, requests with a specific HTTP header set to true will not have label names validated.")
-	f.BoolVar(&cfg.EnableOtelMetadataStorage, "distributor.enable-otlp-metadata-storage", true, "If true, store metadata when ingesting metrics via OTLP. This makes metric descriptions and types available for metrics ingested via OTLP.")
 	f.BoolVar(&cfg.GETRequestForIngesterShutdownEnabled, "api.get-request-for-ingester-shutdown-enabled", false, "Enable GET requests to the /ingester/shutdown endpoint to trigger an ingester shutdown. This is a potentially dangerous operation and should only be enabled consciously.")
 	cfg.RegisterFlagsWithPrefix("", f)
 }
@@ -267,7 +263,7 @@ func (a *API) RegisterDistributor(d *distributor.Distributor, pushConfig distrib
 	distributorpb.RegisterDistributorServer(a.server.GRPC, d)
 
 	a.RegisterRoute(PrometheusPushEndpoint, distributor.Handler(pushConfig.MaxRecvMsgSize, d.RequestBufferPool, a.sourceIPs, a.cfg.SkipLabelNameValidationHeader, limits, pushConfig.RetryConfig, d.PushWithMiddlewares, d.PushMetrics, a.logger), true, false, "POST")
-	a.RegisterRoute(OTLPPushEndpoint, distributor.OTLPHandler(pushConfig.MaxOTLPRequestSize, d.RequestBufferPool, a.sourceIPs, a.cfg.EnableOtelMetadataStorage, limits, pushConfig.RetryConfig, d.PushWithMiddlewares, d.PushMetrics, reg, a.logger, pushConfig.DirectOTLPTranslationEnabled), true, false, "POST")
+	a.RegisterRoute(OTLPPushEndpoint, distributor.OTLPHandler(pushConfig.MaxOTLPRequestSize, d.RequestBufferPool, a.sourceIPs, limits, pushConfig.RetryConfig, d.PushWithMiddlewares, d.PushMetrics, reg, a.logger, pushConfig.DirectOTLPTranslationEnabled), true, false, "POST")
 
 	a.indexPage.AddLinks(defaultWeight, "Distributor", []IndexPageLink{
 		{Desc: "Ring status", Path: "/distributor/ring"},

pkg/api/api_test.go

@@ -31,12 +31,10 @@ func (fl *FakeLogger) Log(...interface{}) error {
 
 func TestNewApiWithoutSourceIPExtractor(t *testing.T) {
 	cfg := Config{}
-	serverCfg := server.Config{
-		HTTPListenAddress: "localhost",
-		GRPCListenAddress: "localhost",
-		MetricsNamespace:  "without_source_ip_extractor",
-	}
+	serverCfg := getServerConfig(t)
+	serverCfg.MetricsNamespace = "without_source_ip_extractor"
 	federationCfg := tenantfederation.Config{}
+
 	require.NoError(t, serverCfg.LogLevel.Set("info"))
 	srv, err := server.New(serverCfg)
 	require.NoError(t, err)
@@ -48,13 +46,11 @@ func TestNewApiWithoutSourceIPExtractor(t *testing.T) {
 
 func TestNewApiWithSourceIPExtractor(t *testing.T) {
 	cfg := Config{}
-	serverCfg := server.Config{
-		LogSourceIPs:      true,
-		HTTPListenAddress: "localhost",
-		GRPCListenAddress: "localhost",
-		MetricsNamespace:  "with_source_ip_extractor",
-	}
+	serverCfg := getServerConfig(t)
+	serverCfg.LogSourceIPs = true
+	serverCfg.MetricsNamespace = "with_source_ip_extractor"
 	federationCfg := tenantfederation.Config{}
+
 	require.NoError(t, serverCfg.LogLevel.Set("info"))
 	srv, err := server.New(serverCfg)
 	require.NoError(t, err)
@@ -69,12 +65,11 @@ func TestNewApiWithInvalidSourceIPExtractor(t *testing.T) {
 	s := server.Server{
 		HTTP: &mux.Router{},
 	}
-	serverCfg := server.Config{
-		LogSourceIPs:       true,
-		LogSourceIPsHeader: "SomeHeader",
-		LogSourceIPsRegex:  "[*",
-		MetricsNamespace:   "with_invalid_source_ip_extractor",
-	}
+	serverCfg := getServerConfig(t)
+	serverCfg.LogSourceIPs = true
+	serverCfg.LogSourceIPsHeader = "SomeHeader"
+	serverCfg.LogSourceIPsRegex = "[*"
+	serverCfg.MetricsNamespace = "with_invalid_source_ip_extractor"
 	federationCfg := tenantfederation.Config{}
 
 	api, err := New(cfg, federationCfg, serverCfg, &s, &FakeLogger{})
@@ -229,14 +224,16 @@ func TestApiIngesterShutdown(t *testing.T) {
 			srv, err := server.New(serverCfg)
 			require.NoError(t, err)
 
-			go func() { _ = srv.Run() }()
-			t.Cleanup(srv.Stop)
-
 			api, err := New(cfg, federationCfg, serverCfg, srv, log.NewNopLogger())
 			require.NoError(t, err)
-
 			api.RegisterIngester(&MockIngester{})
 
+			go func() { _ = srv.Run() }()
+			t.Cleanup(func() {
+				srv.Stop()
+				srv.Shutdown()
+			})
+
 			req := httptest.NewRequest("GET", "/ingester/shutdown", nil)
 			w := httptest.NewRecorder()
 			api.server.HTTP.ServeHTTP(w, req)

pkg/distributor/otel.go

@@ -56,7 +56,6 @@ func OTLPHandler(
 	maxRecvMsgSize int,
 	requestBufferPool util.Pool,
 	sourceIPs *middleware.SourceIPExtractor,
-	enableOtelMetadataStorage bool,
 	limits OTLPHandlerLimits,
 	retryCfg RetryConfig,
 	push PushFunc,
@@ -200,11 +199,7 @@ func OTLPHandler(
 		)
 
 		req.Timeseries = metrics
-
-		if enableOtelMetadataStorage {
-			metadata := otelMetricsToMetadata(addSuffixes, otlpReq.Metrics())
-			req.Metadata = metadata
-		}
+		req.Metadata = otelMetricsToMetadata(addSuffixes, otlpReq.Metrics())
 
 		return nil
 	})

pkg/distributor/otel_test.go

@@ -78,7 +78,7 @@ func BenchmarkOTLPHandler(b *testing.B) {
 		validation.NewMockTenantLimits(map[string]*validation.Limits{}),
 	)
 	require.NoError(b, err)
-	handler := OTLPHandler(100000, nil, nil, true, limits, RetryConfig{}, pushFunc, nil, nil, log.NewNopLogger(), true)
+	handler := OTLPHandler(100000, nil, nil, limits, RetryConfig{}, pushFunc, nil, nil, log.NewNopLogger(), true)
 
 	b.Run("protobuf", func(b *testing.B) {
 		req := createOTLPProtoRequest(b, exportReq, false)
@@ -218,25 +218,6 @@ func TestHandlerOTLPPush(t *testing.T) {
 		return nil
 	}
 
-	samplesVerifierFuncDisabledMetadataIngest := func(t *testing.T, pushReq *Request) error {
-		request, err := pushReq.WriteRequest()
-		require.NoError(t, err)
-
-		series := request.Timeseries
-		require.Len(t, series, 1)
-
-		samples := series[0].Samples
-		require.Equal(t, 1, len(samples))
-		assert.Equal(t, float64(1), samples[0].Value)
-		assert.Equal(t, "__name__", series[0].Labels[0].Name)
-		assert.Equal(t, "foo", series[0].Labels[0].Value)
-
-		metadata := request.Metadata
-		assert.Equal(t, []*mimirpb.MetricMetadata(nil), metadata)
-
-		return nil
-	}
-
 	tests := []struct {
 		name     string
 		series   []prompb.TimeSeries
@@ -246,41 +227,29 @@ func TestHandlerOTLPPush(t *testing.T) {
 		encoding    string
 		maxMsgSize  int
 
-		verifyFunc                func(*testing.T, *Request) error
-		responseCode              int
-		errMessage                string
-		enableOtelMetadataStorage bool
+		verifyFunc   func(*testing.T, *Request) error
+		responseCode int
+		errMessage   string
 
 		expectedLogs        []string
 		expectedRetryHeader bool
 	}{
 		{
-			name:                      "Write samples. No compression",
-			maxMsgSize:                100000,
-			verifyFunc:                samplesVerifierFunc,
-			series:                    sampleSeries,
-			metadata:                  sampleMetadata,
-			responseCode:              http.StatusOK,
-			enableOtelMetadataStorage: true,
-		},
-		{
-			name:                      "Write samples. Not enabled metadata ingest",
-			maxMsgSize:                100000,
-			verifyFunc:                samplesVerifierFuncDisabledMetadataIngest,
-			series:                    sampleSeries,
-			metadata:                  sampleMetadata,
-			responseCode:              http.StatusOK,
-			enableOtelMetadataStorage: false,
+			name:         "Write samples. No compression",
+			maxMsgSize:   100000,
+			verifyFunc:   samplesVerifierFunc,
+			series:       sampleSeries,
+			metadata:     sampleMetadata,
+			responseCode: http.StatusOK,
 		},
 		{
-			name:                      "Write samples. With compression",
-			compression:               true,
-			maxMsgSize:                100000,
-			verifyFunc:                samplesVerifierFunc,
-			series:                    sampleSeries,
-			metadata:                  sampleMetadata,
-			responseCode:              http.StatusOK,
-			enableOtelMetadataStorage: true,
+			name:         "Write samples. With compression",
+			compression:  true,
+			maxMsgSize:   100000,
+			verifyFunc:   samplesVerifierFunc,
+			series:       sampleSeries,
+			metadata:     sampleMetadata,
+			responseCode: http.StatusOK,
 		},
 		{
 			name:        "Write samples. No compression, request too big",
@@ -376,8 +345,7 @@ func TestHandlerOTLPPush(t *testing.T) {
 				pushReq.CleanUp()
 				return nil
 			},
-			responseCode:              http.StatusOK,
-			enableOtelMetadataStorage: true,
+			responseCode: http.StatusOK,
 		},
 	}
 	for _, tt := range tests {
@@ -401,7 +369,7 @@ func TestHandlerOTLPPush(t *testing.T) {
 
 			logs := &concurrency.SyncBuffer{}
 			retryConfig := RetryConfig{Enabled: true, MinBackoff: 5 * time.Second, MaxBackoff: 5 * time.Second}
-			handler := OTLPHandler(tt.maxMsgSize, nil, nil, tt.enableOtelMetadataStorage, limits, retryConfig, pusher, nil, nil, level.NewFilter(log.NewLogfmtLogger(logs), level.AllowInfo()), true)
+			handler := OTLPHandler(tt.maxMsgSize, nil, nil, limits, retryConfig, pusher, nil, nil, level.NewFilter(log.NewLogfmtLogger(logs), level.AllowInfo()), true)
 
 			resp := httptest.NewRecorder()
 			handler.ServeHTTP(resp, req)
@@ -474,7 +442,7 @@ func TestHandler_otlpDroppedMetricsPanic(t *testing.T) {
 
 	req := createOTLPProtoRequest(t, pmetricotlp.NewExportRequestFromMetrics(md), false)
 	resp := httptest.NewRecorder()
-	handler := OTLPHandler(100000, nil, nil, true, limits, RetryConfig{}, func(_ context.Context, pushReq *Request) error {
+	handler := OTLPHandler(100000, nil, nil, limits, RetryConfig{}, func(_ context.Context, pushReq *Request) error {
 		request, err := pushReq.WriteRequest()
 		assert.NoError(t, err)
 		assert.Len(t, request.Timeseries, 3)
@@ -520,7 +488,7 @@ func TestHandler_otlpDroppedMetricsPanic2(t *testing.T) {
 
 	req := createOTLPProtoRequest(t, pmetricotlp.NewExportRequestFromMetrics(md), false)
 	resp := httptest.NewRecorder()
-	handler := OTLPHandler(100000, nil, nil, true, limits, RetryConfig{}, func(_ context.Context, pushReq *Request) error {
+	handler := OTLPHandler(100000, nil, nil, limits, RetryConfig{}, func(_ context.Context, pushReq *Request) error {
 		request, err := pushReq.WriteRequest()
 		t.Cleanup(pushReq.CleanUp)
 		require.NoError(t, err)
@@ -546,7 +514,7 @@ func TestHandler_otlpDroppedMetricsPanic2(t *testing.T) {
 
 	req = createOTLPProtoRequest(t, pmetricotlp.NewExportRequestFromMetrics(md), false)
 	resp = httptest.NewRecorder()
-	handler = OTLPHandler(100000, nil, nil, true, limits, RetryConfig{}, func(_ context.Context, pushReq *Request) error {
+	handler = OTLPHandler(100000, nil, nil, limits, RetryConfig{}, func(_ context.Context, pushReq *Request) error {
 		request, err := pushReq.WriteRequest()
 		t.Cleanup(pushReq.CleanUp)
 		require.NoError(t, err)
@@ -574,7 +542,7 @@ func TestHandler_otlpWriteRequestTooBigWithCompression(t *testing.T) {
 
 	resp := httptest.NewRecorder()
 
-	handler := OTLPHandler(140, nil, nil, true, nil, RetryConfig{}, readBodyPushFunc(t), nil, nil, log.NewNopLogger(), true)
+	handler := OTLPHandler(140, nil, nil, nil, RetryConfig{}, readBodyPushFunc(t), nil, nil, log.NewNopLogger(), true)
 	handler.ServeHTTP(resp, req)
 	assert.Equal(t, http.StatusRequestEntityTooLarge, resp.Code)
 	body, err := io.ReadAll(resp.Body)

pkg/distributor/push_test.go

@@ -1081,7 +1081,7 @@ func TestOTLPPushHandlerErrorsAreReportedCorrectlyViaHttpgrpc(t *testing.T) {
 
 		return nil
 	}
-	h := OTLPHandler(200, util.NewBufferPool(), nil, false, otlpLimitsMock{}, RetryConfig{}, push, newPushMetrics(reg), reg, log.NewNopLogger(), true)
+	h := OTLPHandler(200, util.NewBufferPool(), nil, otlpLimitsMock{}, RetryConfig{}, push, newPushMetrics(reg), reg, log.NewNopLogger(), true)
 	srv.HTTP.Handle("/otlp", h)
 
 	// start the server

pkg/frontend/querymiddleware/roundtrip.go

@@ -39,10 +39,6 @@ const (
 	labelNamesPathSuffix                              = "/api/v1/labels"
 	remoteReadPathSuffix                              = "/api/v1/read"
 
-	// DefaultDeprecatedAlignQueriesWithStep is the default value for the deprecated querier frontend config DeprecatedAlignQueriesWithStep
-	// which has been moved to a per-tenant limit; TODO remove in Mimir 2.14
-	DefaultDeprecatedAlignQueriesWithStep = false
-
 	queryTypeInstant                      = "query"
 	queryTypeRange                        = "query_range"
 	queryTypeRemoteRead                   = "remote_read"