Avoid crashes in tracing wrappers for streams

[dbolduc]

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

return EndSpan(*std::move(context_), *std::move(span_), std::move(status));

The gRPC team tells us in practice that when one of these streams is started (i.e. Start() has been called and returns true), the grpc::ClientContext will have metadata available.

google-cloud-cpp/google/cloud/internal/async_streaming_read_rpc_tracing.h

Line 45 in aa2d6c1

future<bool> Start() override {

Testing

For example, the following test will crash at HEAD, but should not.

TEST(AsyncStreamingReadRpcTracing, UnstartedStreamShouldNotExtractMetadata) {
  auto span_catcher = testing_util::InstallSpanCatcher();

  {
    auto mock = std::make_unique<MockStream>();
    auto span = MakeSpan("span");
    auto context = std::make_shared<grpc::ClientContext>();
    TestedStream stream(context, std::move(mock), span);
  }

  auto spans = span_catcher->GetSpans();
  EXPECT_THAT(spans, ElementsAre(SpanNamed("span")));
}

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 a grpc::ClientContext when the stream was not started successfully.

google-cloud-cpp/google/cloud/internal/opentelemetry.h

Lines 205 to 207 in aa2d6c1

	future<T> EndSpan(
	opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span> span,
	future<T> fut) {

Tasks

The following classes all have the same problem:

• AsyncStreamingReadRpcTracing
• AsyncStreamingWriteRpcTracing
• AsyncReadWriteStreamingRpcTracing

[coryan]

FWIW, this is stopping me from using tracing with the storage_experimental::AsyncClient. I think the thing also crashes if the request is cancelled too early.