jaegertracing / jaeger

CNCF Jaeger, a Distributed Tracing Platform
https://www.jaegertracing.io/
Apache License 2.0
20.54k stars 2.44k forks source link

[v2] Consolidate storage selection into jaeger_storage extension #6225

Open yurishkuro opened 3 days ago

yurishkuro commented 3 days ago

I think I made a small mistake in the design of the v2 storage configuration. The jaeger_storage extension defines configurations for all storage backends, but does not actually takes a position which ones are which. This leads to two problems:

  1. The other components need to make this choice and be in sync. Specifically jaeger_storage_exporter and jaeger_query both require TraceStorage name which they use to retrieve a storage factory from jaeger_storage
  2. The primary/archive storage distinction work #6156 revealed that storage factory needs the is_archive input parameter to customize the backend implementation specifically for archive storage needs. However, in the current setup jaeger_storage does not know this distinction, so in #6156 a new config flag is being proposed, which is technically redundant.

Proposal

Move selection of specific storage implementations into jaeger_storage extension as well. Right now the config might look like:

  jaeger_query:
    storage:
      traces: some_storage
      metrics: some_metrics_storage

  jaeger_storage:
    backends:
      some_storage:
        memory:
    metric_backends:
      some_metrics_storage:
        prometheus:
          endpoint: http://prometheus:9090

We can add a new section that would explicitly assign the roles to different backends:

  jaeger_storage:
    roles:
      traces: some_storage
      traces_archive: some_other_storage
      metrics: some_metrics_storage
    backends:
      ...

And we would remove the corresponding entries from jaeger_query and exporter. Currently query/exporter use the GetFactory(name) function from the extension, instead they would be asking for specific role like GetTracesStorageFactory, GetTraceArchiveStorageFactory, etc.

Benefits: