Tempo 2.0: Config Cleanup

CHANGELOG.md

@@ -75,6 +75,58 @@ Old config will still work but will be removed in a future release. [#1735](http
   * Upgrade `github.com/grafana/dskit`
   * Upgrade `github.com/grafana/e2e`
   * Upgrade `github.com/minio/minio-go/v7`
+* [CHANGE] Config updates to prepare for Tempo 2.0. [#1978](https://github.com/grafana/tempo/pull/1978) (@joe-elliott)
+  Defaults updated:
+  ```
+  query_frontend:
+    max_oustanding_per_tenant: 2000
+    search:
+        concurrent_jobs: 1000
+        target_bytes_per_job: 104857600
+        max_duration: 168h
+        query_ingesters_until: 30m
+    trace_by_id:
+        query_shards: 50
+  querier:
+      max_concurrent_queries: 20
+      search:
+          prefer_self: 10
+  ingester:
+      concurrent_flushes: 4
+      max_block_duration: 30m
+      max_block_bytes: 524288000
+  storage:
+      trace:
+          pool:
+              max_workers: 400
+              queue_depth: 20000
+          search:
+              read_buffer_count: 32
+              read_buffer_size_bytes: 1048576
+  ```
+  **BREAKING CHANGE** Renamed/removed/moved
+  ```
+  query_frontend:
+    query_shards:                  // removed. use trace_by_id.query_shards
+  querier:
+      query_timeout:               // removed. use trace_by_id.query_timeout
+  compactor:
+      compaction:
+          chunk_size_bytes:        // renamed to v2_in_buffer_bytes
+          flush_size_bytes:        // renamed to v2_out_buffer_bytes
+          iterator_buffer_size:    // renamed to v2_prefetch_traces_count
+  ingester:
+      use_flatbuffer_search:       // removed. automatically set based on block type
+  storage:
+      wal:
+          encoding:                // renamed to v2_encoding
+          version:                 // removed and pinned to block.version
+      block:
+          index_downsample_bytes:  // renamed to v2_index_downsample_bytes
+          index_page_size_bytes:   // renamed to v2_index_page_size_bytes
+          encoding:                // renamed to v2_encoding
+          row_group_size_bytes:    // renamed to parquet_row_group_size_bytes
+  ```
 * [FEATURE] Add capability to configure the used S3 Storage Class [#1697](https://github.com/grafana/tempo/pull/1714) (@amitsetty)
 * [ENHANCEMENT] cache: expose username and sentinel_username redis configuration options for ACL-based Redis Auth support [#1708](https://github.com/grafana/tempo/pull/1708) (@jsievenpiper)
 * [ENHANCEMENT] metrics-generator: expose span size as a metric [#1662](https://github.com/grafana/tempo/pull/1662) (@ie-pham)

cmd/tempo/app/config.go

@@ -20,7 +20,6 @@ import (
 	"github.com/grafana/tempo/pkg/usagestats"
 	"github.com/grafana/tempo/pkg/util"
 	"github.com/grafana/tempo/tempodb"
-	v2 "github.com/grafana/tempo/tempodb/encoding/v2"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/weaveworks/common/server"
 )
@@ -156,14 +155,25 @@ func (c *Config) CheckConfig() []ConfigWarning {
 		warnings = append(warnings, warnStorageTraceBackendLocal)
 	}
 
-	// flatbuffers are configured but we're not using v2
-	if c.Ingester.UseFlatbufferSearch && c.StorageConfig.Trace.Block.Version != v2.VersionString {
-		warnings = append(warnings, warnFlatBuffersNotNecessary)
+	// check v2 specific settings
+	if c.StorageConfig.Trace.Block.Version != "v2" && c.StorageConfig.Trace.Block.IndexDownsampleBytes != storage.DefaultIndexDownSampleBytes {
+		warnings = append(warnings, newV2Warning("v2_index_downsample_bytes"))
 	}
 
-	// we're using v2 but flatbuffers are not configured
-	if !c.Ingester.UseFlatbufferSearch && c.StorageConfig.Trace.Block.Version == v2.VersionString {
-		warnings = append(warnings, warnIngesterSearchWillNotWork)
+	if c.StorageConfig.Trace.Block.Version != "v2" && c.StorageConfig.Trace.Block.IndexPageSizeBytes != storage.DefaultIndexPageSizeBytes {
+		warnings = append(warnings, newV2Warning("v2_index_page_size_bytes"))
+	}
+
+	if c.StorageConfig.Trace.Block.Version != "v2" && c.Compactor.Compactor.ChunkSizeBytes != tempodb.DefaultChunkSizeBytes {
+		warnings = append(warnings, newV2Warning("v2_in_buffer_bytes"))
+	}
+
+	if c.StorageConfig.Trace.Block.Version != "v2" && c.Compactor.Compactor.FlushSizeBytes != tempodb.DefaultFlushSizeBytes {
+		warnings = append(warnings, newV2Warning("v2_out_buffer_bytes"))
+	}
+
+	if c.StorageConfig.Trace.Block.Version != "v2" && c.Compactor.Compactor.IteratorBufferSize != tempodb.DefaultIteratorBufferSize {
+		warnings = append(warnings, newV2Warning("v2_prefetch_traces_count"))
 	}
 
 	return warnings
@@ -235,12 +245,11 @@ var (
 	warnStorageTraceBackendLocal = ConfigWarning{
 		Message: "Local backend will not correctly retrieve traces with a distributed deployment unless all components have access to the same disk. You should probably be using object storage as a backend.",
 	}
-	warnFlatBuffersNotNecessary = ConfigWarning{
-		Message: "Flatbuffers enabled with a block type that supports search.",
-		Explain: "The configured block type supports local search in the ingester. Flatbuffers are not necessary and will consume extra resources.",
-	}
-	warnIngesterSearchWillNotWork = ConfigWarning{
-		Message: "Flatbuffers disabled with a block type that does not support search",
-		Explain: "Flatbuffers are disabled but the configured block type does not support ingester search. This can be ignored if only trace by id lookup is desired.",
-	}
 )
+
+func newV2Warning(setting string) ConfigWarning {
+	return ConfigWarning{
+		Message: "c.StorageConfig.Trace.Block.Version != \"v2\" but " + setting + " is set",
+		Explain: "This setting is only used in v2 blocks",
+	}
+}

cmd/tempo/app/config_test.go

@@ -8,7 +8,8 @@ import (
 	"github.com/grafana/tempo/modules/storage"
 	"github.com/grafana/tempo/tempodb"
 	"github.com/grafana/tempo/tempodb/encoding/common"
-
+	v2 "github.com/grafana/tempo/tempodb/encoding/v2"
+	"github.com/grafana/tempo/tempodb/encoding/vparquet"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -31,7 +32,9 @@ func TestConfig_CheckConfig(t *testing.T) {
 					Trace: tempodb.Config{
 						Backend:       "s3",
 						BlocklistPoll: time.Minute,
-						Block:         &common.BlockConfig{},
+						Block: &common.BlockConfig{
+							Version: "v2",
+						},
 					},
 				},
 				Distributor: distributor.Config{
@@ -55,30 +58,48 @@ func TestConfig_CheckConfig(t *testing.T) {
 				cfg.StorageConfig.Trace = tempodb.Config{
 					Backend:                  "local",
 					BlocklistPollConcurrency: 1,
-					Block:                    &common.BlockConfig{},
+					Block: &common.BlockConfig{
+						Version: "v2",
+					},
 				}
 				cfg.Target = "something"
 				return cfg
 			}(),
 			expect: []ConfigWarning{warnStorageTraceBackendLocal},
 		},
 		{
-			name: "warn ingester search",
+			name: "warnings for v2 settings when they drift from default",
 			config: func() *Config {
 				cfg := newDefaultConfig()
-				cfg.StorageConfig.Trace.Block.Version = "v2"
+				cfg.StorageConfig.Trace.Block.Version = vparquet.VersionString
+				cfg.StorageConfig.Trace.Block.IndexDownsampleBytes = 1
+				cfg.StorageConfig.Trace.Block.IndexPageSizeBytes = 1
+				cfg.Compactor.Compactor.ChunkSizeBytes = 1
+				cfg.Compactor.Compactor.FlushSizeBytes = 1
+				cfg.Compactor.Compactor.IteratorBufferSize = 1
 				return cfg
 			}(),
-			expect: []ConfigWarning{warnIngesterSearchWillNotWork},
+			expect: []ConfigWarning{
+				newV2Warning("v2_index_downsample_bytes"),
+				newV2Warning("v2_index_page_size_bytes"),
+				newV2Warning("v2_in_buffer_bytes"),
+				newV2Warning("v2_out_buffer_bytes"),
+				newV2Warning("v2_prefetch_traces_count"),
+			},
 		},
 		{
-			name: "warn flatbuffers not necessary",
+			name: "no warnings for v2 settings when they drift from default and v2 is the block version",
 			config: func() *Config {
 				cfg := newDefaultConfig()
-				cfg.Ingester.UseFlatbufferSearch = true
+				cfg.StorageConfig.Trace.Block.Version = v2.VersionString
+				cfg.StorageConfig.Trace.Block.IndexDownsampleBytes = 1
+				cfg.StorageConfig.Trace.Block.IndexPageSizeBytes = 1
+				cfg.Compactor.Compactor.ChunkSizeBytes = 1
+				cfg.Compactor.Compactor.FlushSizeBytes = 1
+				cfg.Compactor.Compactor.IteratorBufferSize = 1
 				return cfg
 			}(),
-			expect: []ConfigWarning{warnFlatBuffersNotNecessary},
+			expect: nil,
 		},
 	}
 

cmd/tempo/app/modules.go

@@ -38,6 +38,7 @@ import (
 	"github.com/grafana/tempo/tempodb/backend/gcs"
 	"github.com/grafana/tempo/tempodb/backend/local"
 	"github.com/grafana/tempo/tempodb/backend/s3"
+	v2 "github.com/grafana/tempo/tempodb/encoding/v2"
 )
 
 // The various modules that make up tempo.
@@ -145,6 +146,9 @@ func (t *App) initDistributor() (services.Service, error) {
 }
 
 func (t *App) initIngester() (services.Service, error) {
+	// always use flatbuffer search if we're using the v2 blocks. todo: in 2.1 remove flatbuffer search altogether
+	t.cfg.Ingester.UseFlatbufferSearch = (t.cfg.StorageConfig.Trace.Block.Version == v2.VersionString)
+
 	t.cfg.Ingester.LifecyclerConfig.ListenPort = t.cfg.Server.GRPCListenPort
 	ingester, err := ingester.New(t.cfg.Ingester, t.store, t.overrides, prometheus.DefaultRegisterer)
 	if err != nil {