Avoid crashes in tracing wrappers for streams #14458
Labels
good first issue
This issue is a good place to started contributing to this repository.
priority: p2
Moderately-important priority. Fix may not be included in next release.
type: bug
Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Background
This is a similar issue to #13724.
Some of our tracing wrappers can crash. They crash when we try to access server initial metadata (aka "headers"), but the
grpc::ClientContext
has not been used.google-cloud-cpp/google/cloud/internal/async_streaming_read_rpc_tracing.h
Line 92 in aa2d6c1
The gRPC team tells us in practice that when one of these streams is started (i.e.
Start()
has been called and returnstrue
), thegrpc::ClientContext
will have metadata available.google-cloud-cpp/google/cloud/internal/async_streaming_read_rpc_tracing.h
Line 45 in aa2d6c1
Testing
For example, the following test will crash at HEAD, but should not.
We will also want a test like:
TEST(AsyncStreamingReadRpcTracing, StartedStreamShouldExtractMetadata) { ... }
To run the unit tests for
AsyncStreamingReadRpcTracing
with Bazel, we can say:bazel test --test_output=all //google/cloud:internal_async_streaming_read_rpc_tracing_test
Solution?
One fix is to keep using the
EndSpan(grpc::ClientContext, ...)
when the stream has been started successfully...But use the
EndSpan()
overload without agrpc::ClientContext
when the stream was not started successfully.google-cloud-cpp/google/cloud/internal/opentelemetry.h
Lines 205 to 207 in aa2d6c1
Tasks
The following classes all have the same problem:
AsyncStreamingReadRpcTracing
AsyncStreamingWriteRpcTracing
AsyncReadWriteStreamingRpcTracing
The text was updated successfully, but these errors were encountered: