Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Alternative OpenCensus shim implementation proposal #2013

Open
bogdandrutu opened this issue Nov 5, 2020 · 6 comments
Open

Alternative OpenCensus shim implementation proposal #2013

bogdandrutu opened this issue Nov 5, 2020 · 6 comments
Labels
Feature Request Suggest an idea for this project

Comments

@bogdandrutu
Copy link
Member

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 bogdandrutu added the Feature Request Suggest an idea for this project label Nov 5, 2020
@bogdandrutu
Copy link
Member Author

/cc @james-bebbington @nilebox @zoercai

@bogdandrutu
Copy link
Member Author

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
Copy link
Member Author

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

@nilebox
Copy link
Member

nilebox commented Nov 5, 2020

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
Copy link
Member Author

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
Copy link
Contributor

@jsuereth do you think this is still relevant?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Request Suggest an idea for this project
Projects
None yet
Development

No branches or pull requests

3 participants