Support for query-path OTLP

[yurishkuro]

Is your feature request related to a problem? Please describe.

Jaeger-v2 is being reengineered to run as a custom build of OTel-collector (jaegertracing/jaeger#4843). The existing implementations of Jaeger's "exporter"-equivalents (aka Storage implementations) are using Jaeger's internal data model, which requires transformation from incoming OTLP from the receivers. We want to avoid this and change storage implementations to work directly with OTLP.

By being tightly integrated with OTEL pipeline, it means that the write path in our storage implementations will use pdata/ptrace. No particular issues here.

However, Jaeger also implements a query / read path, for example:

message SpansResponseChunk {
  repeated opentelemetry.proto.trace.v1.ResourceSpans resource_spans = 1;
}

service QueryService {
  rpc GetTrace(GetTraceRequest) returns (stream SpansResponseChunk) {}
  rpc FindTraces(FindTracesRequest) returns (stream SpansResponseChunk) {}
}

Ideally, the responses from these RPCs should also return OTLP. However, this requires integration at the level of generated protobuf types, which are hidden in OTel-collector. The only way we were able to do this today is to generate our own otlp types, and then perform multiple transforms: otel-pdata -> binary proto -> jaeger-otlp -> final output.

Describe the solution you'd like

Ideally, I would like to have our internal Storage API (read side) to use pdata, e.g.

type SpanReader interface {
  GetTrace(traceID string) ptrace.Traces
}

and the gRPC definition to remain the same as shown above, and use the most efficient way of translating ptrace.Traces -> SpansResponseChunk protobuf messages. This requires some support from OTEL-collector to make this possible. I don't know what that is exactly, e.g.:

1. open up internal proto types (I think this is unlikely)
2. perhaps extend ptrace/ptraceotlp with additional reusable types, similar to how ExportTraceServiceResponse is supported, but a new type that actually contains traces (essentially an equivalent of SpansResponseChunk above)

Describe alternatives you've considered

I've explored a number of alternatives, documented here.

My two possible, but not ideal, solutions right now are:

• do not use pdata at all in the read path of Storage API, use some internal version of OTLP
• fork/clone pdata into Jaeger with our own proto types, so that at least at the API level the read and write paths are similar, although with distinct import paths (cloning would need to be automated to stay in sync)

These are both pretty bad as they force us to create a whole parallel machinery that is nearly identical to pdata but has one extra public type wrapper.

I would like to get suggestions from the collector maintainers how best to approach this. Happy to prototype.

Proposal

1. Similar to opentelemetry-proto/blob/main/opentelemetry/proto/collector/trace/v1/trace_service.proto, define query/trace/v1/trace_service.proto with a single message (for now - in the future this could be extended to a full service definition if Query SIG comes up with a proposal)

message QueryServiceTraceResponse {
  repeated opentelemetry.proto.trace.v1.ResourceSpans resource_spans = 1;
}

2. Implement a corresponding pdata wrapper for that message similar to ExportResponse:

   opentelemetry-collector/pdata/ptrace/ptraceotlp/response.go

   Lines 16 to 20 in 2c96088

   	// ExportResponse represents the response for gRPC/HTTP client/server.
   	type ExportResponse struct {
   	orig *otlpcollectortrace.ExportTraceServiceResponse
   	state *internal.State
   	}

3. Repeat as needed for logs/metrics (not needed for Jaeger right now)

[atoulme]

Can 2. be done in a separate package from pdata?

[yurishkuro]

Can 2. be done in a separate package from pdata?

No, because it needs to reference opentelemetry.proto.trace.v1.ResourceSpans, which is internal to pdata.

[yurishkuro]

added concrete proposal

[mx-psi]

Can 2. be done in a separate package from pdata?

No, because it needs to reference opentelemetry.proto.trace.v1.ResourceSpans, which is internal to pdata.

Can it be done on a separate module inside the pdata folder then?

[yurishkuro]

Can it be done on a separate module inside the pdata folder then?

I suppose, as long as it has access to pdata/internal/data/protogen

[mx-psi]

@open-telemetry/collector-maintainers what do you think about having this on a separate module? Could be something like go.opentelemetry.io/pdata/ptrace/ptracequery work?

[bogdandrutu]

First proposal

Here is a path that I know about, it may be a bit ugly but tell me if that satisfy your requirement.

• In the opentelemetry-proto we have a proto message that is exactly your SpansResponseChunk (https://github.com/open-telemetry/opentelemetry-proto/blob/main/opentelemetry/proto/trace/v1/trace.proto#L38) can you use that?
• To convert a ptrace.Traces to TracesData you can use https://github.com/open-telemetry/opentelemetry-collector/blob/main/pdata/ptrace/pb.go.
• If you DO NOT use the autogenerated grpc Server (because that requires you to have the proto generate object) stub you can actually implement the equivalent stub manually using the grpc primitives directly.

Second proposal

Another option is to change a bit your public gRPC definition to be:

message SpansResponseChunk {
  opentelemetry.proto.trace.v1.TracesData traces = 1;
  ... // may have later other properties.
}

Then only in the server that uses the pdata, change the definition to an wire equivalent that allows you to directly use the bytes:

message SpansResponseChunk {
  bytes traces = 1;  // populate using https://github.com/open-telemetry/opentelemetry-collector/blob/main/pdata/ptrace/pb.go
  ... // may have later other properties.
}

[yurishkuro]

@bogdandrutu proposal 1 did the trick, thanks!