Support default configs for storage backends (jaegertracing#5691)

## Which problem is this PR solving?
- Many OTEL Collector components provide default configs such that users
do not need to provide most settings.
- However, due to our organization of the storage extension config,
there was no way to provide defaults

## Description of the changes
- 🛑 **breaking** Change the main config inside out: instead of static
separation by storage types followed by a map of custom names, move the
map to the top level followed by static separation by storage type. E.g.
```yaml
# before
    memory:
      some_storage:
        max_traces: 100000
# after
    backends:
      some_storage:
        memory:
          max_traces: 100000
```
- This required an introduction of an extra nesting via `backends`,
which is a bit unfortunate, but the OTEL framework requires config to be
a struct, it did not work with a map at the top level. Having a struct
might actually be beneficial in the future if we need some top-level
settings
- Add custom marshaling code where if a specific storage type is
detected as set-by-user, then a default is created first (similar
mechanism to how extension default configs is created by OTEL Collector,
but unfortunately that mechanism is rigid, non-recursive)
- Change sample config files to the new format
- Use names `some_store` and `another_store` in configs to emphasize
that those are custom names, not part of the config struct
- Add more unit tests for config

## How was this change tested?
- Unit tests and CI

## Checklist
- [ ] I have read
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
- [ ] I have signed all commits
- [ ] I have added unit tests for the new functionality
- [ ] I have run lint and test steps successfully
  - for `jaeger`: `make lint test`
  - for `jaeger-ui`: `yarn lint` and `yarn test`

---------

Signed-off-by: Yuri Shkuro <[email protected]>

Author: yurishkuro

cmd/jaeger/config-badger.yaml

@@ -8,26 +8,22 @@ service:
 
 extensions:
   jaeger_query:
-    trace_storage: badger_main
-    trace_storage_archive: badger_archive
+    trace_storage: some_store
+    trace_storage_archive: another_store
     ui_config: ./cmd/jaeger/config-ui.json
 
   jaeger_storage:
-    badger:
-      badger_main:
-        directory_key: "/tmp/jaeger/"
-        directory_value: "/tmp/jaeger/"
-        ephemeral: false
-        maintenance_interval: 5
-        metrics_update_interval: 10
-        span_store_ttl: 72h
-      badger_archive:
-        directory_key: "/tmp/jaeger_archive/"
-        directory_value: "/tmp/jaeger_archive/"
-        ephemeral: false
-        maintenance_interval: 5
-        metrics_update_interval: 10
-        span_store_ttl: 720h
+    backends:
+      some_store:
+        badger:
+          directory_key: "/tmp/jaeger/"
+          directory_value: "/tmp/jaeger/"
+          ephemeral: false
+      another_store:
+        badger:
+          directory_key: "/tmp/jaeger_archive/"
+          directory_value: "/tmp/jaeger_archive/"
+          ephemeral: false
 
 receivers:
   otlp:
@@ -40,4 +36,4 @@ processors:
 
 exporters:
   jaeger_storage_exporter:
-    trace_storage: badger_main
+    trace_storage: some_store

cmd/jaeger/config-cassandra.yaml

@@ -8,30 +8,18 @@ service:
 
 extensions:
   jaeger_query:
-    trace_storage: cassandra_main
-    trace_storage_archive: cassandra_archive
+    trace_storage: some_storage
+    trace_storage_archive: another_storage
     ui_config: ./cmd/jaeger/config-ui.json
 
   jaeger_storage:
-    cassandra:
-      cassandra_main:
-        servers: 127.0.0.1
-        port: 9042
-        keyspace: "jaeger_v1_dc1"
-        connections_per_host: 2
-        index:
-          tags: true
-          logs: true
-          process_tags: true
-      cassandra_archive:
-        servers: 127.0.0.1
-        port: 9042
-        keyspace: "jaeger_v1_dc1"
-        connections_per_host: 2
-        index:
-          tags: true
-          logs: true
-          process_tags: true
+    backends:
+      some_storage:
+        cassandra:
+          keyspace: "jaeger_v1_dc1"
+      another_storage:
+        cassandra:
+          keyspace: "jaeger_v1_dc1"
 receivers:
   otlp:
     protocols:
@@ -50,4 +38,4 @@ processors:
 
 exporters:
   jaeger_storage_exporter:
-    trace_storage: cassandra_main
+    trace_storage: some_storage

cmd/jaeger/config-elasticsearch.yaml

@@ -8,24 +8,18 @@ service:
 
 extensions:
   jaeger_query:
-    trace_storage: es_main
-    trace_storage_archive: es_archive
+    trace_storage: some_storage
+    trace_storage_archive: another_storage
     ui_config: ./cmd/jaeger/config-ui.json
 
   jaeger_storage:
-    elasticsearch:
-      es_main:
-        server_urls: http://localhost:9200
-        log_level: "error"
-        index_prefix: "jaeger-main"
-        use_aliases: false
-        create_mappings: true
-      es_archive:
-        server_urls: http://localhost:9200
-        log_level: "error"
-        index_prefix: "jaeger-archive"
-        use_aliases: false
-        create_mappings: true
+    backends:
+      some_storage:
+        elasticsearch:
+          index_prefix: "jaeger-main"
+      another_storage:
+        elasticsearch:
+          index_prefix: "jaeger-archive"
 
 receivers:
   otlp:
@@ -38,4 +32,4 @@ processors:
 
 exporters:
   jaeger_storage_exporter:
-    trace_storage: es_main
+    trace_storage: some_storage

cmd/jaeger/config-opensearch.yaml

@@ -8,25 +8,19 @@ service:
 
 extensions:
   jaeger_query:
-    trace_storage: os_main
-    trace_storage_archive: os_archive
+    trace_storage: some_storage
+    trace_storage_archive: another_storage
     ui_config: ./cmd/jaeger/config-ui.json
 
   jaeger_storage:
-    opensearch:
-      os_main:
-        server_urls: http://localhost:9200
-        log_level: "error"
-        index_prefix: "jaeger-main"
-        use_aliases: false
-        create_mappings: true
+    backends:
+      some_storage:
+        opensearch:
+          index_prefix: "jaeger-main"
 
-      os_archive:
-        server_urls: http://localhost:9200
-        log_level: "error"
-        index_prefix: "jaeger-main"
-        use_aliases: false
-        create_mappings: true
+      another_storage:
+        opensearch:
+          index_prefix: "jaeger-main"
 
 receivers:
   otlp:
@@ -39,4 +33,4 @@ processors:
 
 exporters:
   jaeger_storage_exporter:
-    trace_storage: os_main
+    trace_storage: some_storage

cmd/jaeger/config-remote-storage.yaml

@@ -8,16 +8,16 @@ service:
 
 extensions:
   jaeger_query:
-    trace_storage: external-storage
+    trace_storage: some-storage
     ui_config: ./cmd/jaeger/config-ui.json
 
   jaeger_storage:
-    grpc:
-      external-storage:
-        endpoint: localhost:17271
-        timeout: 5s
-        tls:
-          insecure: true
+    backends:
+      some-storage:
+        grpc:
+          endpoint: localhost:17271
+          tls:
+            insecure: true
 
 receivers:
   otlp:
@@ -30,4 +30,4 @@ processors:
 
 exporters:
   jaeger_storage_exporter:
-    trace_storage: external-storage
+    trace_storage: some-storage

cmd/jaeger/internal/all-in-one.yaml

@@ -8,12 +8,13 @@ service:
 
 extensions:
   jaeger_query:
-    trace_storage: memstore
+    trace_storage: some_storage
 
   jaeger_storage:
-    memory:
-      memstore:
-        max_traces: 100000
+    backends:
+      some_storage:
+        memory:
+          max_traces: 100000
 
 receivers:
   otlp:
@@ -35,4 +36,4 @@ processors:
 
 exporters:
   jaeger_storage_exporter:
-    trace_storage: memstore
+    trace_storage: some_storage

cmd/jaeger/internal/exporters/storageexporter/exporter_test.go

@@ -29,7 +29,7 @@ import (
 	"go.opentelemetry.io/collector/pdata/ptrace"
 	noopmetric "go.opentelemetry.io/otel/metric/noop"
 	nooptrace "go.opentelemetry.io/otel/trace/noop"
-	"go.uber.org/zap"
+	"go.uber.org/zap/zaptest"
 
 	"github.com/jaegertracing/jaeger/cmd/jaeger/internal/extension/jaegerstorage"
 	"github.com/jaegertracing/jaeger/model"
@@ -103,7 +103,7 @@ func TestExporter(t *testing.T) {
 
 	ctx := context.Background()
 	telemetrySettings := component.TelemetrySettings{
-		Logger:         zap.L(),
+		Logger:         zaptest.NewLogger(t),
 		TracerProvider: nooptrace.NewTracerProvider(),
 		MeterProvider:  noopmetric.NewMeterProvider(),
 	}
@@ -157,18 +157,21 @@ func TestExporter(t *testing.T) {
 }
 
 func makeStorageExtension(t *testing.T, memstoreName string) component.Host {
+	telemetrySettings := component.TelemetrySettings{
+		Logger:         zaptest.NewLogger(t),
+		TracerProvider: nooptrace.NewTracerProvider(),
+		MeterProvider:  noopmetric.NewMeterProvider(),
+	}
 	extensionFactory := jaegerstorage.NewFactory()
 	storageExtension, err := extensionFactory.CreateExtension(
 		context.Background(),
 		extension.Settings{
-			TelemetrySettings: component.TelemetrySettings{
-				Logger:         zap.L(),
-				TracerProvider: nooptrace.NewTracerProvider(),
-			},
+			TelemetrySettings: telemetrySettings,
 		},
-		&jaegerstorage.Config{Memory: map[string]memory.Configuration{
-			memstoreName: {MaxTraces: 10000},
-		}})
+		&jaegerstorage.Config{Backends: map[string]jaegerstorage.Backend{
+			memstoreName: {Memory: &memory.Configuration{MaxTraces: 10000}},
+		}},
+	)
 	require.NoError(t, err)
 
 	host := storagetest.NewStorageHost()

cmd/jaeger/internal/extension/jaegerstorage/config.go

@@ -6,31 +6,95 @@ package jaegerstorage
 import (
 	"fmt"
 	"reflect"
+	"time"
 
+	"go.opentelemetry.io/collector/component"
+	"go.opentelemetry.io/collector/confmap"
+
+	casCfg "github.com/jaegertracing/jaeger/pkg/cassandra/config"
 	esCfg "github.com/jaegertracing/jaeger/pkg/es/config"
 	"github.com/jaegertracing/jaeger/plugin/storage/badger"
 	"github.com/jaegertracing/jaeger/plugin/storage/cassandra"
+	"github.com/jaegertracing/jaeger/plugin/storage/es"
 	"github.com/jaegertracing/jaeger/plugin/storage/grpc"
 	"github.com/jaegertracing/jaeger/plugin/storage/memory"
 )
 
-// Config has the configuration for jaeger-query,
+var (
+	_ component.ConfigValidator = (*Config)(nil)
+	_ confmap.Unmarshaler       = (*Backend)(nil)
+)
+
+// Config contains configuration(s) for jaeger trace storage.
+// Keys in the map are storage names that can be used to refer to them
+// from other components, e.g. from jaeger_storage_exporter or jaeger_query.
+// We tried to alias this type directly to a map, but conf did not populated it correctly.
+// Note also that the Backend struct has a custom unmarshaler.
 type Config struct {
-	Memory        map[string]memory.Configuration   `mapstructure:"memory"`
-	Badger        map[string]badger.NamespaceConfig `mapstructure:"badger"`
-	GRPC          map[string]grpc.ConfigV2          `mapstructure:"grpc"`
-	Opensearch    map[string]esCfg.Configuration    `mapstructure:"opensearch"`
-	Elasticsearch map[string]esCfg.Configuration    `mapstructure:"elasticsearch"`
-	Cassandra     map[string]cassandra.Options      `mapstructure:"cassandra"`
-	// TODO add other storage types here
-	// TODO how will this work with 3rd party storage implementations?
-	//      Option: instead of looking for specific name, check interface.
+	Backends map[string]Backend `mapstructure:"backends"`
+}
+
+type Backend struct {
+	Memory        *memory.Configuration   `mapstructure:"memory"`
+	Badger        *badger.NamespaceConfig `mapstructure:"badger"`
+	GRPC          *grpc.ConfigV2          `mapstructure:"grpc"`
+	Cassandra     *cassandra.Options      `mapstructure:"cassandra"`
+	Elasticsearch *esCfg.Configuration    `mapstructure:"elasticsearch"`
+	Opensearch    *esCfg.Configuration    `mapstructure:"opensearch"`
+}
+
+// Unmarshal implements confmap.Unmarshaler. This allows us to provide
+// defaults for different configs. It cannot be done in createDefaultConfig()
+// because at that time we don't know which backends the user wants to use.
+func (cfg *Backend) Unmarshal(conf *confmap.Conf) error {
+	// apply defaults
+	if conf.IsSet("memory") {
+		cfg.Memory = &memory.Configuration{
+			MaxTraces: 1_000_000,
+		}
+	}
+	if conf.IsSet("badger") {
+		v := badger.DefaultNamespaceConfig()
+		cfg.Badger = &v
+	}
+	if conf.IsSet("grpc") {
+		v := grpc.DefaultConfigV2()
+		cfg.GRPC = &v
+	}
+	if conf.IsSet("cassandra") {
+		cfg.Cassandra = &cassandra.Options{
+			Primary: cassandra.NamespaceConfig{
+				Configuration: casCfg.DefaultConfiguration(),
+				Enabled:       true,
+			},
+			SpanStoreWriteCacheTTL: 12 * time.Hour,
+			Index: cassandra.IndexConfig{
+				Tags:        true,
+				ProcessTags: true,
+				Logs:        true,
+			},
+		}
+	}
+	if conf.IsSet("elasticsearch") {
+		v := es.DefaultConfig()
+		cfg.Elasticsearch = &v
+	}
+	if conf.IsSet("opensearch") {
+		v := es.DefaultConfig()
+		cfg.Opensearch = &v
+	}
+	return conf.Unmarshal(cfg)
 }
 
 func (cfg *Config) Validate() error {
-	emptyCfg := createDefaultConfig().(*Config)
-	if reflect.DeepEqual(*cfg, *emptyCfg) {
-		return fmt.Errorf("%s: no storage type present in config", ID)
+	if len(cfg.Backends) == 0 {
+		return fmt.Errorf("at least one storage is required")
+	}
+	for name, b := range cfg.Backends {
+		empty := Backend{}
+		if reflect.DeepEqual(b, empty) {
+			return fmt.Errorf("no backend defined for storage '%s'", name)
+		}
 	}
 	return nil
 }