Skip to content

Avoid infinite Cloud Trace export loop #14611

@dbolduc

Description

@dbolduc

I have seen the Cloud Trace exporter get into an export loop when GOOGLE_CLOUD_CPP_OPENTELEMETRY_TRACING is set.

Past design decision: #11897 (comment)

The exporter is implemented by a trace client. The trace client is fully generated. Also, env vars take precedence over options, so we do not have a good way to override this behavior. Note that the tracing stub / connections are included when the connection is created. So we cannot override the behavior with a client option or a call option.

Proposed Fix

I think we should just explicitly set OpenTelemetryTracingOption == false in the options defaulter iff it is the cloud trace client.

We would verify this with a simple unit test which I would put in google/cloud/opentelemetry. (I don't want to put it in google/cloud/trace because it may complicate the build scripts. google/cloud/opentelemetry already has custom builds).

Alternatives Considered

A. Add some internal::DisableTracingOption, which only gets set in the Cloud Trace exporter.

internal options are a bit of a smell.

B. Decide that the OpenTelemetryTracingOption takes precedence over the environment variable.

Veto. We already released these options/env vars and can't override existing behavior.

C. Have the exporter drop spans named "trace_v2::TraceClient::BatchWriteSpans" or "google.devtools.cloudtrace.TraceService/BatchWriteSpans"

Pro:

  • No changes to common
    Cons:
  • Magic strings. If we update the naming conventions of the span, we need to remember to also update the exporter. If we do remember to use this, we are safe. (It is not like there will be a version mismatch between the exporter and the underlying trace client. We have one version for all of google-cloud-cpp.)
  • There is no way to trace the exports, even if you wanted to. They happen in band. It could lead to blocks in the application that we would like to explain?
  • If gRPC is instrumented, and applications enable tracing, now we have the versioning problem.
    • Hm... This seems like an unavoidable problem.

D. Add some sort of short-circuit in the exporter?

E. Override the environment before creating trace client in exporter

No, we should avoid setenv in library code.

F. Tracing layers could check for OpenTelemetryTracingOption == false before tracing.

This one isn't a bad option.

Metadata

Metadata

Assignees

Labels

priority: p2Moderately-important priority. Fix may not be included in next release.type: bugError or flaw in code with unintended results or allowing sub-optimal usage patterns.

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions