Remove dependency on Jaeger-Query

[joe-elliott]

Is your feature request related to a problem? Please describe.
Currently Tempo requires Jaeger-Query as a translation layer between Tempo and Grafana. This was a nice way to bootstrap the system, but we should drop the requirement to run this. It is currently a performance issue that is seen particularly with large traces (#237).

Describe the solution you'd like
Add an endpoint to the querier that returns a trace in the Jaeger format for Grafana to consume.

[annanay25]

Will essentially fix #275 as well

[morlay]

We could release multi arch for tempo without waiting jaeger too 🤣

[dgzlopes]

Didn't know a lot about the query path... So, I researched this one a bit!

I added a new handler:

// tempo/modules/querier/http.go
import (
       ...
       jaeger "github.com/jaegertracing/jaeger/model"
       jaegertranslator "go.opentelemetry.io/collector/translator/trace/jaeger"
       "go.opentelemetry.io/collector/consumer/pdata"
       ...
)
func (q *Querier) HandlerJaeger(...){
	...
	// Validate request
	...
	// Find trace
        resp, err := q.FindTraceByID(...)
	...
	// Internal trace representation from OTPL
	td := pdata.TracesFromOtlp(resp.Trace.Batches)
        // From internal representation to Jaeger Proto
	jaegerBatches, err := jaegertranslator.InternalTracesToJaegerProto(td)
	if err != nil {
		fmt.Errorf("error translating to jaegerBatches %v: %w", traceID, err)
	}

	jaegerTrace := &jaeger.Trace{
		Spans:      []*jaeger.Span{},
		ProcessMap: []jaeger.Trace_ProcessMapping{},
	}

	// otel proto conversion doesn't set jaeger processes
	for _, batch := range jaegerBatches {
		for _, s := range batch.Spans {
			s.Process = batch.Process
		}

		jaegerTrace.Spans = append(jaegerTrace.Spans, batch.Spans...)
		jaegerTrace.ProcessMap = append(jaegerTrace.ProcessMap, jaeger.Trace_ProcessMapping{
			Process:   *batch.Process,
			ProcessID: batch.Process.ServiceName,
		})
	}
        // Now that we have the jaegerTrace struct.. we have to transform it to UI friendly JSON
        // And write the response
}

And well, from my pov it makes more-or-less sense (after reading some Jaeger/Querier/OTel Collector code).

Sounds "reasonable"? Or do you have some other approach in mind? Probably the code above is a bit rough, but I added it for demonstration purposes.

[joe-elliott]

Yeah, it might be as easy as the above. I'm unsure if Jaeger-query does any manipulation of this object before it passes it on to Grafana that we need to mimic, but that would need to be looked into.

We also need to decide what path to put this on. Grafana expects to query /api/traces/{traceid} which is the same path we are already returning OTel json/proto on. I'm thinking we do a breaking change and return Jaeger json by default but allow (probably through headers?) the ability to request OTel.

This is also impacted by Annanay's work with the query frontend: #400

[dgzlopes]

Nice! Well, I couldn't make it work b/c on convertModelToUI (where the model is converted to JSON) I had some problems... But yeah, this needs more research (Grafana requirements).

While adding the new handler I thought the same! I like the header idea, but in case we do this, we should change TempoCLI too.

I suppose it's better to wait until #400 is merged to continue with this. Right?