[jaeger-v2] Refactor Configuration For Query Service (#5998)

## Which problem is this PR solving? - Part of #5996 ## Description of the changes - Reworked the configuration for `jaeger-query` to have more logical groupings. - [Migration document](https://docs.google.com/document/d/1-01QRgMFWbHGXXJqs5cL4kA-P10BfU0TD9Kvp3rEw3M/edit?usp=sharing) ## How was this change tested? - CI ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` --------- Signed-off-by: Mahad Zaryab <[email protected]>
jaegertracing · Sep 25, 2024 · d7f01d1 · d7f01d1
1 parent 7c6ed87
commit d7f01d1
Show file tree

Hide file tree

Showing 21 changed files with 160 additions and 108 deletions.
diff --git a/cmd/jaeger/config-badger.yaml b/cmd/jaeger/config-badger.yaml
@@ -12,9 +12,11 @@ extensions:
     http:
 
   jaeger_query:
-    trace_storage: some_store
-    trace_storage_archive: another_store
-    ui_config: ./cmd/jaeger/config-ui.json
+    storage:
+      traces: some_store
+      traces_archive: another_store
+    ui:
+      config_file: ./cmd/jaeger/config-ui.json
 
   jaeger_storage:
     backends:

diff --git a/cmd/jaeger/config-cassandra.yaml b/cmd/jaeger/config-cassandra.yaml
@@ -12,9 +12,11 @@ extensions:
     http:
 
   jaeger_query:
-    trace_storage: some_storage
-    trace_storage_archive: another_storage
-    ui_config: ./cmd/jaeger/config-ui.json
+    storage:
+      traces: some_storage
+      traces_archive: another_storage
+    ui:
+      config_file: ./cmd/jaeger/config-ui.json
 
   jaeger_storage:
     backends:

diff --git a/cmd/jaeger/config-elasticsearch.yaml b/cmd/jaeger/config-elasticsearch.yaml
@@ -12,9 +12,11 @@ extensions:
     http:
 
   jaeger_query:
-    trace_storage: some_storage
-    trace_storage_archive: another_storage
-    ui_config: ./cmd/jaeger/config-ui.json
+    storage:
+      traces: some_storage
+      traces_archive: another_storage
+    ui:
+      config_file: ./cmd/jaeger/config-ui.json
 
   jaeger_storage:
     backends:

diff --git a/cmd/jaeger/config-kafka-ingester.yaml b/cmd/jaeger/config-kafka-ingester.yaml
@@ -19,7 +19,8 @@ extensions:
       endpoint: 0.0.0.0:14133
 
   jaeger_query:
-    trace_storage: some_storage
+    storage:
+      traces: some_storage
 
   jaeger_storage:
     backends:

diff --git a/cmd/jaeger/config-opensearch.yaml b/cmd/jaeger/config-opensearch.yaml
@@ -12,9 +12,11 @@ extensions:
     http:
 
   jaeger_query:
-    trace_storage: some_storage
-    trace_storage_archive: another_storage
-    ui_config: ./cmd/jaeger/config-ui.json
+    storage:
+      traces: some_storage
+      traces_archive: another_storage
+    ui:
+      config_file: ./cmd/jaeger/config-ui.json
 
   jaeger_storage:
     backends:

diff --git a/cmd/jaeger/config-remote-storage.yaml b/cmd/jaeger/config-remote-storage.yaml
@@ -12,8 +12,10 @@ extensions:
     http:
 
   jaeger_query:
-    trace_storage: some-storage
-    ui_config: ./cmd/jaeger/config-ui.json
+    storage:
+      traces: some-storage
+    ui:
+      config_file: ./cmd/jaeger/config-ui.json
 
   jaeger_storage:
     backends:

diff --git a/cmd/jaeger/config-tail-sampling-always-sample.yaml b/cmd/jaeger/config-tail-sampling-always-sample.yaml
@@ -14,7 +14,8 @@ extensions:
     use_v2: true
     http:
   jaeger_query:
-    trace_storage: some_storage
+    storage:
+      traces: some_storage
   jaeger_storage:
     backends:
       some_storage:

diff --git a/cmd/jaeger/config-tail-sampling-service-name-policy.yaml b/cmd/jaeger/config-tail-sampling-service-name-policy.yaml
@@ -14,7 +14,8 @@ extensions:
     use_v2: true
     http:
   jaeger_query:
-    trace_storage: some_storage
+    storage:
+      traces: some_storage
   jaeger_storage:
     backends:
       some_storage:

diff --git a/cmd/jaeger/config.yaml b/cmd/jaeger/config.yaml
@@ -17,9 +17,11 @@ extensions:
   #   endpoint: 0.0.0.0:55679
 
   jaeger_query:
-    trace_storage: some_store
-    trace_storage_archive: another_store
-    ui_config: ./cmd/jaeger/config-ui.json
+    storage:
+      traces: some_store
+      traces_archive: another_store
+    ui:
+      config_file: ./cmd/jaeger/config-ui.json
 
   jaeger_storage:
     backends:

diff --git a/cmd/jaeger/internal/all-in-one.yaml b/cmd/jaeger/internal/all-in-one.yaml
@@ -16,7 +16,8 @@ service:
 
 extensions:
   jaeger_query:
-    trace_storage: some_storage
+    storage:
+      traces: some_storage
 
   jaeger_storage:
     backends:

diff --git a/cmd/jaeger/internal/extension/jaegerquery/config.go b/cmd/jaeger/internal/extension/jaegerquery/config.go
@@ -10,20 +10,28 @@ import (
 	"go.opentelemetry.io/collector/config/confighttp"
 
 	queryApp "github.com/jaegertracing/jaeger/cmd/query/app"
-	"github.com/jaegertracing/jaeger/pkg/tenancy"
 )
 
 var _ component.ConfigValidator = (*Config)(nil)
 
 // Config represents the configuration for jaeger-query,
 type Config struct {
 	queryApp.QueryOptionsBase `mapstructure:",squash"`
-	TraceStoragePrimary       string                  `valid:"required" mapstructure:"trace_storage"`
-	TraceStorageArchive       string                  `valid:"optional" mapstructure:"trace_storage_archive"`
-	MetricStorage             string                  `valid:"optional" mapstructure:"metric_storage"`
-	HTTP                      confighttp.ServerConfig `mapstructure:",squash"`
-	GRPC                      configgrpc.ServerConfig `mapstructure:",squash"`
-	Tenancy                   tenancy.Options         `mapstructure:"multi_tenancy"`
+	// Storage holds configuration related to the various data stores that are to be queried.
+	Storage Storage `mapstructure:"storage"`
+	// HTTP holds the HTTP configuration that the query service uses to serve requests.
+	HTTP confighttp.ServerConfig `mapstructure:"http"`
+	// GRPC holds the GRPC configuration that the query service uses to serve requests.
+	GRPC configgrpc.ServerConfig `mapstructure:"grpc"`
+}
+
+type Storage struct {
+	// TracesPrimary contains the name of the primary trace storage that is being queried.
+	TracesPrimary string `mapstructure:"traces" valid:"required"`
+	// TracesArchive contains the name of the archive trace storage that is being queried.
+	TracesArchive string `mapstructure:"traces_archive" valid:"optional"`
+	// Metrics contains the name of the metric storage that is being queried.
+	Metrics string `mapstructure:"metrics" valid:"optional"`
 }
 
 func (cfg *Config) Validate() error {

diff --git a/cmd/jaeger/internal/extension/jaegerquery/config_test.go b/cmd/jaeger/internal/extension/jaegerquery/config_test.go
@@ -19,12 +19,14 @@ func Test_Validate(t *testing.T) {
 		{
 			name:        "Empty config",
 			config:      &Config{},
-			expectedErr: "TraceStoragePrimary: non zero value required",
+			expectedErr: "Storage.TracesPrimary: non zero value required",
 		},
 		{
 			name: "Non empty-config",
 			config: &Config{
-				TraceStoragePrimary: "some-storage",
+				Storage: Storage{
+					TracesPrimary: "some-storage",
+				},
 			},
 			expectedErr: "",
 		},

diff --git a/cmd/jaeger/internal/extension/jaegerquery/server.go b/cmd/jaeger/internal/extension/jaegerquery/server.go
@@ -55,9 +55,9 @@ func (s *server) Start(_ context.Context, host component.Host) error {
 	mf := otelmetrics.NewFactory(s.telset.MeterProvider)
 	baseFactory := mf.Namespace(metrics.NSOptions{Name: "jaeger"})
 	queryMetricsFactory := baseFactory.Namespace(metrics.NSOptions{Name: "query"})
-	f, err := jaegerstorage.GetStorageFactory(s.config.TraceStoragePrimary, host)
+	f, err := jaegerstorage.GetStorageFactory(s.config.Storage.TracesPrimary, host)
 	if err != nil {
-		return fmt.Errorf("cannot find primary storage %s: %w", s.config.TraceStoragePrimary, err)
+		return fmt.Errorf("cannot find primary storage %s: %w", s.config.Storage.TracesPrimary, err)
 	}
 
 	spanReader, err := f.CreateSpanReader()
@@ -124,12 +124,12 @@ func (s *server) Start(_ context.Context, host component.Host) error {
 }
 
 func (s *server) addArchiveStorage(opts *querysvc.QueryServiceOptions, host component.Host) error {
-	if s.config.TraceStorageArchive == "" {
+	if s.config.Storage.TracesArchive == "" {
 		s.telset.Logger.Info("Archive storage not configured")
 		return nil
 	}
 
-	f, err := jaegerstorage.GetStorageFactory(s.config.TraceStorageArchive, host)
+	f, err := jaegerstorage.GetStorageFactory(s.config.Storage.TracesArchive, host)
 	if err != nil {
 		return fmt.Errorf("cannot find archive storage factory: %w", err)
 	}
@@ -141,12 +141,12 @@ func (s *server) addArchiveStorage(opts *querysvc.QueryServiceOptions, host comp
 }
 
 func (s *server) createMetricReader(host component.Host) (metricsstore.Reader, error) {
-	if s.config.MetricStorage == "" {
+	if s.config.Storage.Metrics == "" {
 		s.telset.Logger.Info("Metric storage not configured")
 		return disabled.NewMetricsReader()
 	}
 
-	mf, err := jaegerstorage.GetMetricsFactory(s.config.MetricStorage, host)
+	mf, err := jaegerstorage.GetMetricsFactory(s.config.Storage.Metrics, host)
 	if err != nil {
 		return nil, fmt.Errorf("cannot find metrics storage factory: %w", err)
 	}

diff --git a/cmd/jaeger/internal/extension/jaegerquery/server_test.go b/cmd/jaeger/internal/extension/jaegerquery/server_test.go
@@ -134,53 +134,67 @@ func TestServerStart(t *testing.T) {
 		{
 			name: "Non-empty config with fake storage host",
 			config: &Config{
-				TraceStorageArchive: "jaeger_storage",
-				TraceStoragePrimary: "jaeger_storage",
-				MetricStorage:       "jaeger_metrics_storage",
+				Storage: Storage{
+					TracesArchive: "jaeger_storage",
+					TracesPrimary: "jaeger_storage",
+					Metrics:       "jaeger_metrics_storage",
+				},
 			},
 		},
 		{
 			name: "factory error",
 			config: &Config{
-				TraceStoragePrimary: "need-factory-error",
+				Storage: Storage{
+					TracesPrimary: "need-factory-error",
+				},
 			},
 			expectedErr: "cannot find primary storage",
 		},
 		{
 			name: "span reader error",
 			config: &Config{
-				TraceStoragePrimary: "need-span-reader-error",
+				Storage: Storage{
+					TracesPrimary: "need-span-reader-error",
+				},
 			},
 			expectedErr: "cannot create span reader",
 		},
 		{
 			name: "dependency error",
 			config: &Config{
-				TraceStoragePrimary: "need-dependency-reader-error",
+				Storage: Storage{
+					TracesPrimary: "need-dependency-reader-error",
+				},
 			},
 			expectedErr: "cannot create dependencies reader",
 		},
 		{
 			name: "storage archive error",
 			config: &Config{
-				TraceStorageArchive: "need-factory-error",
-				TraceStoragePrimary: "jaeger_storage",
+				Storage: Storage{
+					TracesArchive: "need-factory-error",
+					TracesPrimary: "jaeger_storage",
+				},
 			},
 			expectedErr: "cannot find archive storage factory",
 		},
 		{
 			name: "metrics storage error",
 			config: &Config{
-				MetricStorage:       "need-factory-error",
-				TraceStoragePrimary: "jaeger_storage",
+				Storage: Storage{
+					Metrics:       "need-factory-error",
+					TracesPrimary: "jaeger_storage",
+				},
 			},
 			expectedErr: "cannot find metrics storage factory",
 		},
 		{
 			name: " metrics reader error",
 			config: &Config{
-				MetricStorage:       "need-metrics-reader-error",
-				TraceStoragePrimary: "jaeger_storage",
+				Storage: Storage{
+					Metrics:       "need-metrics-reader-error",
+					TracesPrimary: "jaeger_storage",
+				},
 			},
 			expectedErr: "cannot create metrics reader",
 		},
@@ -254,7 +268,9 @@ func TestServerAddArchiveStorage(t *testing.T) {
 		{
 			name: "Archive storage set",
 			config: &Config{
-				TraceStorageArchive: "random-value",
+				Storage: Storage{
+					TracesArchive: "random-value",
+				},
 			},
 			qSvcOpts:       &querysvc.QueryServiceOptions{},
 			expectedOutput: "",
@@ -263,7 +279,9 @@ func TestServerAddArchiveStorage(t *testing.T) {
 		{
 			name: "Archive storage not supported",
 			config: &Config{
-				TraceStorageArchive: "badger",
+				Storage: Storage{
+					TracesArchive: "badger",
+				},
 			},
 			qSvcOpts:       &querysvc.QueryServiceOptions{},
 			extension:      fakeStorageExt{},
@@ -313,7 +331,9 @@ func TestServerAddMetricsStorage(t *testing.T) {
 		{
 			name: "Metrics storage set",
 			config: &Config{
-				MetricStorage: "random-value",
+				Storage: Storage{
+					Metrics: "random-value",
+				},
 			},
 			expectedOutput: "",
 			expectedErr:    "cannot find metrics storage factory: cannot find extension",

diff --git a/cmd/jaeger/internal/integration/e2e_integration.go b/cmd/jaeger/internal/integration/e2e_integration.go
@@ -126,7 +126,7 @@ func (s *E2EStorageIntegration) e2eInitialize(t *testing.T, storage string) {
 			Path: "./cmd/jaeger/jaeger",
 			Args: []string{"jaeger", "--config", configFile},
 			// Change the working directory to the root of this project
-			// since the binary config file jaeger_query's ui_config points to
+			// since the binary config file jaeger_query's ui.config_file points to
 			// "./cmd/jaeger/config-ui.json"
 			Dir: "../../../..",
 		},
@@ -223,7 +223,9 @@ func createStorageCleanerConfig(t *testing.T, configFile string, storage string)
 	extensions := extensionsAny.(map[string]any)
 	queryAny, ok := extensions["jaeger_query"]
 	require.True(t, ok)
-	traceStorageAny, ok := queryAny.(map[string]any)["trace_storage"]
+	storageAny, ok := queryAny.(map[string]any)["storage"]
+	require.True(t, ok)
+	traceStorageAny, ok := storageAny.(map[string]any)["traces"]
 	require.True(t, ok)
 	traceStorage := traceStorageAny.(string)
 	extensions["storage_cleaner"] = map[string]string{"trace_storage": traceStorage}