Implement Storage API v2 that operates on OTLP batches

[yurishkuro]

Summary

In OTEL-based Jaeger-v2 we don't want to force OTLP trace data to go through unnecessary transformation into Jaeger's current data model. We want to define a V2 version of the storage API so that the OTLP data can be passed through the receiver-exporter pipeline without additional conversions, for better efficiency.

Problem

More discussion in the doc https://docs.google.com/document/d/1s4_6VgAS7qAVp6iEm5KYvpiGw3h2Ja5T5HpKo29iv00/edit#heading=h.hlhu8pegwdcj

Progress

• Define Storage V2 API [jaeger-v2] Define an internal interface of storage v2 spanstore #5399
• Write path
  • Change jaegerstorage extension to expose Storage V2 API [v2] Add v1 factory converter to v2 storage factory #5497
  • Change storageexporter to use V2 API exclusively [v2] Add v1 factory converter to v2 storage factory #5497
    • This is achieved by using an adapter that provides v2 API over v1 storage implementation, for writes only
• Read path (more complicated)
  • discussed in the Migration section of the doc
• Remove jaegerstorage.GetStorageFactory once not used

[tarhphilomina]

Can I take this issue ?

[yurishkuro]

you can try - this is a design issue

[mahadzaryab1]

@yurishkuro For the read path implementation, do we just want to expand the v2 adapter to implement spanstore.Reader (in storage_v2) that just wraps the v1 implementation?

[yurishkuro]

Not quite, please see the Google doc.

[mahadzaryab1]

@yurishkuro Here are the remaining usages of GetStorageFactory that cannot be removed right now because they do runtime casts to other factories which the factoryadapter does not implement currently:

• Remote Sampling Extension - requires a cast to storage.SamplingStoreFactory (https://github.com/jaegertracing/jaeger/blob/main/cmd/jaeger/internal/extension/remotesampling/extension.go#L184-L189)
• Storage Cleaner - requires a cast to storage.Purger (https://github.com/jaegertracing/jaeger/blob/main/cmd/jaeger/internal/integration/storagecleaner/extension.go#L48-L54)
• Archive Reader / Writer in query_service.go - requires a cast to storage.ArchiveFactory (https://github.com/jaegertracing/jaeger/blob/main/cmd/query/app/querysvc/query_service.go#L149)

Do you have any thoughts on how we should proceed with the above?

[yurishkuro]

Good question. We might be missing an abstraction layer. Let's think about what it means to define a cassandra backend. On one hand, there are some connection parameters that would be applicable regardless of what the backend is used for. On the other hand, there are some business-specific parameters, e.g. the Schema structure (heavily extended in #5922) that is mostly specific to trace storage, i.e. a dependency or sampling storage might have a different keyspace and schema, we could even implement metric storage on top of Cassandra. So what exactly does the backend section define - pure connectivity options, or also parameters specific to the type of factory we need? Right now it's both, except for the metric store. This is the root of the inconsistency.

Basically, we have two options:

1. a backend is a superset of parameters that are required for any role that the storage might take. This is currently the case except for metric store.
2. a backend is just the connectivity information, and we need a higher level abstraction that includes parameters needed for the role / factory type implementation, such as trace_store, dep_store, sampling_store (with distributed lock), metric_store, etc. Each of those stores would have to either refer to a "backend" that only defines connectivity, or just embed that connectivity config (which is what metric store is doing).

I think to avoid breaking 2.x config compatibility we should stick with option 1 and maybe in the future we can pull the roles apart similar to how metric_store has done already. That means that a backend has to implement all distinct storage factory interfaces, similar to have it was done in v1 API - a (trace store) Factory implementation could optionally implement other factory interfaces.

[mahadzaryab1]

@yurishkuro Got it - so does this mean that our factoryadapter needs to implement all the required interfaces?

[yurishkuro]

yes, I think so