cockroachdb / cockroachdb/cockroach
rpc,tracing: span metadata not propagated properly in local internal client optimization
- Dominant language
- Go
- Stars
- 32.5k
- Forks
- 4.1k
- PR merge metrics
- PR metrics pending
Description
See https://github.com/cockroachdb/cockroach/pull/135682.
When `DistSender.RangeFeed` wants to establish a rangefeed to a node-local replica, it uses the internal client adapter:
https://github.com/cockroachdb/cockroach/blob/e51e52eb33fcb0da96f2013483317b78d6aaee2a/pkg/kv/kvclient/kvcoord/dist_sender_mux_rangefeed.go#L395
The implementation is here:
https://github.com/cockroachdb/cockroach/blob/e51e52eb33fcb0da96f2013483317b78d6aaee2a/pkg/rpc/context.go#L1039-L1047
Note how it runs the request through the streaming server interceptor.
Notably, this includes the tracing interceptor:
https://github.com/cockroachdb/cockroach/blob/e51e52eb33fcb0da96f2013483317b78d6aaee2a/pkg/rpc/context.go#L179-L182
That interceptor expects the parent span information to be present in grpc-style context metadata (see `ExtractSpanMetaFromGRPCCtx`):
https://github.com/cockroachdb/cockroach/blob/e51e52eb33fcb0da96f2013483317b78d6aaee2a/pkg/util/tracing/grpcinterceptor/grpc_interceptor.go#L139-L183
but it is not present there for requests that come in from the local internal server optimization.
As a result, the interceptor may up delegating directly to the handler (the `!SpanInclusionFunc(...)` case) unless tracing is turned on globally.
This has the effect of "passing through" the client span directly to the handler, which, on context cancellation cases, makes it likely that the server span will be used after the client span is finished. In tests, this may trigger the "span use after finish" assertion, but it would also have implications on production builds, which could be more prone to crashing as a result.
It turns out that on master at the time of writing, tracing _is_ globally enabled (the "active span registry" is on) and so we were always hitting the `StartSpanCtx` call in the snippet above, which would "bypass" the problem by making a new (and erroneously wholly unconnected span). This is more an accident than design, so once we fix that, we're going to have a problem.
Jira issue: CRDB-44677
Contributor guide
Assessment
This issue has not been assessed yet.