open-telemetry / opentelemetry-java

OpenTelemetry Java SDK
https://opentelemetry.io
Apache License 2.0
1.96k stars 806 forks source link

Alternative OpenCensus shim implementation proposal #2013

Open bogdandrutu opened 3 years ago

bogdandrutu commented 3 years ago

The current OpenCensus shim is implemented here: https://github.com/open-telemetry/opentelemetry-java/tree/master/opencensus-shim

This implementation has some disadvantages regarding the requirement to use a cache of Spans, spans need to be ended in a certain amount of time, etc. Right now the current approach is to rely on the opencensus-impl to do the work of accumulating events into Spans, export, etc.

First Proposal The proposal is to make the shim rely on the opentelemetry-sdk to do the heavy work of accumulating events into Spans, export them, etc.

This way you will benefit of the fact that this project is currently very active and maintained, and also I believe this will simplify the implementation, here are some thoughts:

  1. The OpenCensus.Tracer will be a simple wrapper of the OpenTelemetry.Tracer with a name "opencensus".
  2. The PropagationComponent can be implemented by wrapping the propagators from OpenTelemetry (w3c, b3). One small problem is that you will need to re-implement the BinaryFormat (this is most likely a duplicate code).
  3. The clock does not need to be re-implemented. just return the Millis clock.
  4. The ExporterComponent can be implemented by wrapping the BatchSpanProcessor, the OpenCensus.SpanExporter.Handler is equivalent with the OpenTelemetry.SpanExporter
  5. The TraceConfig is a simple wrapper of the config management in the OpenTelemetry.

Second Proposal Take a dependency on otel api in the opencensus API and redirect all calls. This may not be possible until GA.

bogdandrutu commented 3 years ago

/cc @james-bebbington @nilebox @zoercai

bogdandrutu commented 3 years ago

Also the ContextManager will redirect into the OpenTelemetry.Context which we will need to configure to be using as otelInGRPC, because OpenCensus users delegate the context propagation to gRPC Context.

bogdandrutu commented 3 years ago

Alternative to for example wrapping the ExporterComponent is to tell users to configure an OTel exporter. This can simplify the implementation.

nilebox commented 3 years ago

Alternative to for example wrapping the ExporterComponent is to tell users to configure an OTel exporter.

This is what we'd prefer I think, as this helps to avoid duplicate OC and OT spans being exported in case both OC and OT pipelines are configured in the application.

Current shim implementation configures OC Tracer with no-op exporter, so we can probably do the same.

bogdandrutu commented 3 years ago

We may still want to offer user an alternative to the RunningSpanStore option which can be extracted from the current OTel zpages and offer as a SpanProcessor.

jkwatson commented 3 years ago

@jsuereth do you think this is still relevant?