Remove autocomplete_filtering_enabled config flag (#3729)

* Remove autocomplete_filtering_enabled config flag * Remove missed reference * IT'S A BREAKING CHANGE * Last two fixes
grafana · Jun 21, 2024 · 2a38bd1 · 2a38bd1
1 parent 480e967
commit 2a38bd1
Show file tree

Hide file tree

Showing 16 changed files with 20 additions and 49 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,4 +1,6 @@
 ## main / unreleased
+
+* [CHANGE] **BREAKING CHANGE** Remove `autocomplete_filtering_enabled` feature flag [#3729](https://github.com/grafana/tempo/pull/3729) (@mapno)
 * [CHANGE] Bump opentelemetry-collector to 0.102.1 [#3784](https://github.com/grafana/tempo/pull/3784) (@debasishbsws)
 * [CHANGE] Bump Jaeger query docker image to 1.57.0 [#3652](https://github.com/grafana/tempo/issues/3652) (@iblancasa)
 * [CHANGE] Update Go to 1.22.4 [#3757](https://github.com/grafana/tempo/pull/3757) [#3793](https://github.com/grafana/tempo/pull/3793) (@joe-elliott, @mapno)

diff --git a/cmd/tempo/app/app.go b/cmd/tempo/app/app.go
@@ -60,9 +60,8 @@ var (
 		nil,
 	)
 
-	statFeatureEnabledAuth                  = usagestats.NewInt("feature_enabled_auth_stats")
-	statFeatureEnabledMultitenancy          = usagestats.NewInt("feature_enabled_multitenancy")
-	statFeatureAutocompleteFilteringEnabled = usagestats.NewInt("feature_enabled_autocomplete_filtering")
+	statFeatureEnabledAuth         = usagestats.NewInt("feature_enabled_auth_stats")
+	statFeatureEnabledMultitenancy = usagestats.NewInt("feature_enabled_multitenancy")
 )
 
 // App is the root datastructure.
@@ -113,11 +112,6 @@ func New(cfg Config) (*App, error) {
 		statFeatureEnabledMultitenancy.Set(1)
 	}
 
-	statFeatureAutocompleteFilteringEnabled.Set(0)
-	if cfg.AutocompleteFilteringEnabled {
-		statFeatureAutocompleteFilteringEnabled.Set(1)
-	}
-
 	app.setupAuthMiddleware()
 
 	if err := app.setupModuleManager(); err != nil {

diff --git a/cmd/tempo/app/config.go b/cmd/tempo/app/config.go
@@ -31,15 +31,14 @@ import (
 
 // Config is the root config for App.
 type Config struct {
-	Target                       string        `yaml:"target,omitempty"`
-	AuthEnabled                  bool          `yaml:"auth_enabled,omitempty"`
-	MultitenancyEnabled          bool          `yaml:"multitenancy_enabled,omitempty"`
-	ShutdownDelay                time.Duration `yaml:"shutdown_delay,omitempty"`
-	StreamOverHTTPEnabled        bool          `yaml:"stream_over_http_enabled,omitempty"`
-	HTTPAPIPrefix                string        `yaml:"http_api_prefix"`
-	UseOTelTracer                bool          `yaml:"use_otel_tracer,omitempty"`
-	EnableGoRuntimeMetrics       bool          `yaml:"enable_go_runtime_metrics,omitempty"`
-	AutocompleteFilteringEnabled bool          `yaml:"autocomplete_filtering_enabled,omitempty"`
+	Target                 string        `yaml:"target,omitempty"`
+	AuthEnabled            bool          `yaml:"auth_enabled,omitempty"`
+	MultitenancyEnabled    bool          `yaml:"multitenancy_enabled,omitempty"`
+	ShutdownDelay          time.Duration `yaml:"shutdown_delay,omitempty"`
+	StreamOverHTTPEnabled  bool          `yaml:"stream_over_http_enabled,omitempty"`
+	HTTPAPIPrefix          string        `yaml:"http_api_prefix"`
+	UseOTelTracer          bool          `yaml:"use_otel_tracer,omitempty"`
+	EnableGoRuntimeMetrics bool          `yaml:"enable_go_runtime_metrics,omitempty"`
 
 	Server          server.Config           `yaml:"server,omitempty"`
 	InternalServer  internalserver.Config   `yaml:"internal_server,omitempty"`
@@ -76,7 +75,6 @@ func (c *Config) RegisterFlagsAndApplyDefaults(prefix string, f *flag.FlagSet) {
 	f.StringVar(&c.HTTPAPIPrefix, "http-api-prefix", "", "String prefix for all http api endpoints.")
 	f.BoolVar(&c.UseOTelTracer, "use-otel-tracer", false, "Set to true to replace the OpenTracing tracer with the OpenTelemetry tracer")
 	f.BoolVar(&c.EnableGoRuntimeMetrics, "enable-go-runtime-metrics", false, "Set to true to enable all Go runtime metrics")
-	f.BoolVar(&c.AutocompleteFilteringEnabled, "autocomplete-filtering.enabled", true, "Set to false to disable autocomplete filtering")
 	f.DurationVar(&c.ShutdownDelay, "shutdown-delay", 0, "How long to wait between SIGTERM and shutdown. After receiving SIGTERM, Tempo will report not-ready status via /ready endpoint.")
 
 	// Server settings

diff --git a/cmd/tempo/app/modules.go b/cmd/tempo/app/modules.go
@@ -247,7 +247,6 @@ func (t *App) initDistributor() (services.Service, error) {
 
 func (t *App) initIngester() (services.Service, error) {
 	t.cfg.Ingester.LifecyclerConfig.ListenPort = t.cfg.Server.GRPCListenPort
-	t.cfg.Ingester.AutocompleteFilteringEnabled = t.cfg.AutocompleteFilteringEnabled
 	t.cfg.Ingester.DedicatedColumns = t.cfg.StorageConfig.Trace.Block.DedicatedColumns
 	ingester, err := ingester.New(t.cfg.Ingester, t.store, t.Overrides, prometheus.DefaultRegisterer)
 	if err != nil {
@@ -313,8 +312,6 @@ func (t *App) initQuerier() (services.Service, error) {
 		ingesterRings = append(ingesterRings, ring)
 	}
 
-	t.cfg.Querier.AutocompleteFilteringEnabled = t.cfg.AutocompleteFilteringEnabled
-
 	querier, err := querier.New(
 		t.cfg.Querier,
 		t.cfg.IngesterClient,

diff --git a/docs/sources/tempo/api_docs/_index.md b/docs/sources/tempo/api_docs/_index.md
@@ -448,7 +448,7 @@ This endpoint can also receive `start` and `end` optional parameters. These para
 
 #### Filtered tag values
 
-The `autocomplete_filtering_enabled` configuration parameter is set to `true` by default. This provides an optional URL query parameter, `q`, to your request.
+You can pass an optional URL query parameter, `q`, to your request.
 The `q` parameter is a URL-encoded [TraceQL query]({{< relref "../traceql" >}}).
 If provided, the tag values returned by the API are filtered to only return values seen on spans matching your filter parameters.
 

diff --git a/docs/sources/tempo/configuration/_index.md b/docs/sources/tempo/configuration/_index.md
@@ -74,11 +74,6 @@ Tempo uses the server from `dskit/server`. For more information on configuration
 # Optional. Setting to true enables multitenancy and requires X-Scope-OrgID header on all requests.
 [multitenancy_enabled: <bool> | default = false]
 
-# Optional. Setting to true enables query filtering in tag value search API `/api/v2/search/<tag>/values`.
-# If filtering is enabled, the API accepts a query parameter `q` containing a TraceQL query,
-# and returns only tag values that match the query.
-[autocomplete_filtering_enabled: <bool> | default = true]
-
 # Optional. String prefix for all http api endpoints. Must include beginning slash.
 [http_api_prefix: <string>]
 

diff --git a/docs/sources/tempo/configuration/manifest.md b/docs/sources/tempo/configuration/manifest.md
@@ -24,7 +24,6 @@ This manifest was generated on 2023-11-13.
 ```yaml
 target: all
 http_api_prefix: ""
-autocomplete_filtering_enabled: true
 server:
     http_listen_network: tcp
     http_listen_address: ""

diff --git a/docs/sources/tempo/setup/upgrade.md b/docs/sources/tempo/setup/upgrade.md
@@ -157,16 +157,10 @@ cache:
 <table>
   <tr>
    <td>Parameter
-   </td>
+   </td
    <td>Comments
    </td>
   </tr>
-  <tr>
-   <td><code>autocomplete_filtering_enabled</code>
-   </td>
-   <td>Set to <code>true</code> by default [PR <a href="https://github.com/grafana/tempo/pull/3178">3178</a>]
-   </td>
-  </tr>
   <tr>
    <td><code>distributor.log_received_traces</code>
    </td>

diff --git a/integration/e2e/api_test.go b/integration/e2e/api_test.go
@@ -30,7 +30,7 @@ func TestSearchTagValuesV2(t *testing.T) {
 	defer s.Close()
 
 	require.NoError(t, util.CopyFileToSharedDir(s, configAllInOneLocal, "config.yaml"))
-	tempo := util.NewTempoAllInOne("-autocomplete-filtering.enabled=true")
+	tempo := util.NewTempoAllInOne()
 	require.NoError(t, s.StartAndWaitReady(tempo))
 
 	jaegerClient, err := util.NewJaegerGRPCClient(tempo.Endpoint(14250))

diff --git a/modules/ingester/config.go b/modules/ingester/config.go
@@ -28,8 +28,7 @@ type Config struct {
 	OverrideRingKey      string        `yaml:"override_ring_key"`
 	FlushAllOnShutdown   bool          `yaml:"flush_all_on_shutdown"`
 
-	DedicatedColumns             backend.DedicatedColumns `yaml:"-"`
-	AutocompleteFilteringEnabled bool                     `yaml:"-"`
+	DedicatedColumns backend.DedicatedColumns `yaml:"-"`
 }
 
 // RegisterFlagsAndApplyDefaults registers the flags.

diff --git a/modules/ingester/ingester.go b/modules/ingester/ingester.go
@@ -319,7 +319,7 @@ func (i *Ingester) getOrCreateInstance(instanceID string) (*instance, error) {
 	inst, ok = i.instances[instanceID]
 	if !ok {
 		var err error
-		inst, err = newInstance(instanceID, i.limiter, i.overrides, i.store, i.local, i.cfg.AutocompleteFilteringEnabled, i.cfg.DedicatedColumns)
+		inst, err = newInstance(instanceID, i.limiter, i.overrides, i.store, i.local, i.cfg.DedicatedColumns)
 		if err != nil {
 			return nil, err
 		}

diff --git a/modules/ingester/instance.go b/modules/ingester/instance.go
@@ -111,11 +111,9 @@ type instance struct {
 	localWriter backend.Writer
 
 	hash hash.Hash32
-
-	autocompleteFilteringEnabled bool
 }
 
-func newInstance(instanceID string, limiter *Limiter, overrides ingesterOverrides, writer tempodb.Writer, l *local.Backend, autocompleteFiltering bool, dedicatedColumns backend.DedicatedColumns) (*instance, error) {
+func newInstance(instanceID string, limiter *Limiter, overrides ingesterOverrides, writer tempodb.Writer, l *local.Backend, dedicatedColumns backend.DedicatedColumns) (*instance, error) {
 	i := &instance{
 		traces:     map[uint32]*liveTrace{},
 		traceSizes: map[uint32]uint32{},
@@ -134,8 +132,6 @@ func newInstance(instanceID string, limiter *Limiter, overrides ingesterOverride
 		localWriter: backend.NewWriter(l),
 
 		hash: fnv.New32(),
-
-		autocompleteFilteringEnabled: autocompleteFiltering,
 	}
 	err := i.resetHeadBlock()
 	if err != nil {

diff --git a/modules/ingester/instance_search.go b/modules/ingester/instance_search.go
@@ -403,7 +403,7 @@ func (i *instance) SearchTagValuesV2(ctx context.Context, req *tempopb.SearchTag
 		}
 
 		// if the query is empty or autocomplete filtering is disabled, use the old search
-		if !i.autocompleteFilteringEnabled || traceql.IsEmptyQuery(query) {
+		if traceql.IsEmptyQuery(query) {
 			return s.SearchTagValuesV2(ctx, tag, traceql.MakeCollectTagValueFunc(valueCollector.Collect), common.DefaultSearchOptions())
 		}
 

diff --git a/modules/ingester/instance_search_test.go b/modules/ingester/instance_search_test.go
@@ -300,7 +300,6 @@ func testSearchTagsAndValues(t *testing.T, ctx context.Context, i *instance, tag
 
 func TestInstanceSearchTagAndValuesV2(t *testing.T) {
 	i, _ := defaultInstance(t)
-	i.autocompleteFilteringEnabled = true
 
 	// add dummy search data
 	var (

diff --git a/modules/querier/config.go b/modules/querier/config.go
@@ -24,8 +24,6 @@ type Config struct {
 	ShuffleShardingIngestersLookbackPeriod time.Duration `yaml:"shuffle_sharding_ingesters_lookback_period"`
 	QueryRelevantIngesters                 bool          `yaml:"query_relevant_ingesters"`
 	SecondaryIngesterRing                  string        `yaml:"secondary_ingester_ring,omitempty"`
-
-	AutocompleteFilteringEnabled bool `yaml:"-"`
 }
 
 type SearchConfig struct {

diff --git a/modules/querier/querier.go b/modules/querier/querier.go
@@ -995,7 +995,7 @@ func (q *Querier) internalTagValuesSearchBlockV2(ctx context.Context, req *tempo
 	opts.TotalPages = int(req.PagesToSearch)
 
 	query := traceql.ExtractMatchers(req.SearchReq.Query)
-	if !q.cfg.AutocompleteFilteringEnabled || traceql.IsEmptyQuery(query) {
+	if traceql.IsEmptyQuery(query) {
 		return q.store.SearchTagValuesV2(ctx, meta, req.SearchReq, common.DefaultSearchOptions())
 	}