query-frontend: also extract Trace from TraceByIDResponse when returning protobuf

[yvrhdn]

What this PR does:
Fixes a bug introduced by #1007 and reported in #1024.

When querying Tempo with Accept: application/protobuf (as Grafana does) the query-frontend will not unmarshal/marshal the response from other middleware but passes it directly to the caller. #1007 changes TraceByIDResponse to add metrics. To avoid passing this to the caller, we should also unmarshal and marshal protobuf responses.

Which issue(s) this PR fixes:
Fixes #1024

Checklist

• Tests updated
• Documentation added
• CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

[yvrhdn]

I dislike that we unmarshal and marshal TraceByIDResponse 2 times in the query-frontend. I.e.:

• query-sharding combines responses and creates a TraceByIDResponse:

tempo/modules/frontend/querysharding.go

Lines 169 to 174 in e7a3731

	buff, err := proto.Marshal(&tempopb.TraceByIDResponse{
	Trace: overallTrace,
	Metrics: &tempopb.TraceByIDMetrics{
	FailedBlocks: totalFailedBlocks,
	},
	})

• frontend.go has to unmarshal this TraceByIDResponse again because we have to return TraceByIDResponse.Trace:

https://github.com/kvrhdn/tempo/blob/3b9d783f17cd4bc3420caae20ccf415ce86505ca/modules/frontend/frontend.go#L158-L182

I guess this is a limitation of building a processing pipeline with http.RoundTripper (and not something we want to fix in this PR)

[joe-elliott]

I guess this is a limitation of building a processing pipeline with http.RoundTripper (and not something we want to fix in this PR)

Agree. For larger traces repeatedly marshalling/unmarshalling is very inefficient. However, for most queries, this won't even be noticeable so I'm fine with keeping as is for now.