Querier: streamline tracing spans

[bboreham]

What this PR does:

Remove internalQuerier trace handler: nethttp.MiddlewareFunc() starts a new top-level trace, but we already have tracing middleware from Server.HTTPServer.

One doubt: I'm not sure what happens in single-binary mode. I could not follow this comment:

cortex/pkg/cortex/modules.go

Lines 327 to 329 in ebef1e1

	// If queries are processed using the external HTTP Server, we need wrap the internal querier with
	// HTTP router with middleware to parse the tenant ID from the HTTP header and inject it into the
	// request context.

I cannot see how this code affects processed using the external HTTP Server, because it only impacts what happens to requests routed via the query-frontend worker.

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

Checklist

• NA Tests updated
• NA Documentation added
• CHANGELOG.md updated

[pracucci]

@jtlisi I think you've been the last one working on it. Could you review it, please?

[pracucci]

I just did a quick test with the single-binary and I confirm this PR breaks tracing there. I've run the single-binary with:

./development/tsdb-blocks-storage-s3-single-binary/compose-up.sh

What I get in master:

What I get with this PR:

[jtlisi]

@jtlisi I think you've been the last one working on it. Could you review it, please?

Sorry I took so long to get to this. It had been a while since I had worked on this and needed to re-familiarize myself with why this was needed.

1. We inject the retry parent trace span into the request as an HTTP header here in pkg/frontend/v1/frontend.go

   // RoundTripGRPC round trips a proto (instead of a HTTP request).
   func (f *Frontend) RoundTripGRPC(ctx context.Context, req *httpgrpc.HTTPRequest) (*httpgrpc.HTTPResponse, error) {
       // Propagate trace context in gRPC too - this will be ignored if using HTTP.
       tracer, span := opentracing.GlobalTracer(), opentracing.SpanFromContext(ctx)
       if tracer != nil && span != nil {
           carrier := (*grpcutil.HttpgrpcHeadersCarrier)(req)
           err := tracer.Inject(span.Context(), opentracing.HTTPHeaders, carrier)
           if err != nil {
               return nil, err
           }
       }

2. We then send the HTTPGRPC request, using a GRPC streaming service. This means there isn't a context scoped to the request because the httpgrpc.Request does not persist the context.
3. The only place the span now exists is the HTTP header of the request. In microservices mode, the request is passed to the weaveworks/common servers router and the tracing middleware will extract the Span Headers and inject them into the context. However, in single-binary mode, we use a separate router, specifically the one returned by the function edited in this PR. If tracing middleware is not applied to this router, then the span HTTP headers are never extracted. We could only apply the tracing middleware in single-binary mode. However, I don't think there is a downside in having an additional operation to keep traces between the two models consistent.

If that doesn't make sense let me know and I can try and rephrase or clarify things.

In general, one way we alleviate this is by moving the trace propagation to the frontend_worker instead of the HTTP router.

[bboreham]

The downside is described at #3913

[bboreham]

I re-introduced the tracing middleware in the single-binary path.
Still don't follow the comments there.

[pracucci]

I re-tested it and looks working fine both in microservices mode (with and without query-scheduler) and single-binary mode.