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

Add opencensus tracing bridge #1305

Merged
merged 1 commit into from
Nov 12, 2020

Conversation

dashpole
Copy link
Contributor

@dashpole dashpole commented Nov 4, 2020

Issue: #93

This adds an opencensus bridge for tracing. Tracing is particularly important to have a bridge for, as different libraries used in a single go binary may use either opencensus or opentelemetry. In that case, opentelemetry and opencensus spans would not properly share parent-child relationships. The same problem doesn't exist for metrics, as a single go binary can use both opencensus and opentelemetry libraries without a loss of functionality (assuming appropriate exporters are registered for each)

Using the bridge requires overriding the opencensus default tracer with the bridge.:

octrace.DefaultTracer = bridge.NewTracer(global.Tracer("mytracer"))

This will cause all calls to the OpenCensus global functions (such as StartSpan) to call the relevant OpenTelemetry function. OpenCensus spans recorded while using this bridge will not export to OpenCensus exporters anymore. Instead, they will export to the registered opentelemetry exporter(s).

Note: This adds new go.mod/go.sum so "regular" users of opentelemetry don't have to pull in opencensus imports.
Note: This uses an unreleased commit for opencensus. I can see if I can get a release cut if that is a blocker for this PR.

@codecov
Copy link

codecov bot commented Nov 4, 2020

Codecov Report

Merging #1305 (863d936) into master (27aa1f6) will increase coverage by 0.0%.
The diff coverage is 80.8%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #1305   +/-   ##
======================================
  Coverage    77.3%   77.3%           
======================================
  Files         122     123    +1     
  Lines        5978    6067   +89     
======================================
+ Hits         4622    4694   +72     
- Misses       1107    1120   +13     
- Partials      249     253    +4     
Impacted Files Coverage Δ
bridge/opencensus/bridge.go 80.8% <80.8%> (ø)

@dashpole dashpole force-pushed the opencensus_bridge branch 2 times, most recently from 93d25a9 to 1a915b8 Compare November 4, 2020 17:01
@MrAlias MrAlias added enhancement New feature or request pkg:bridges Related to a bridge package priority:p2 labels Nov 4, 2020
@MrAlias MrAlias added this to the RC1 milestone Nov 4, 2020
Copy link
Contributor

@MrAlias MrAlias left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Only comments are not blocking.

I think the use of the error handler to address incompatibilities with the APIs is the correct way to proceed. Thanks for the contribution.

bridge/opencensus/bridge.go Outdated Show resolved Hide resolved
bridge/opencensus/bridge.go Outdated Show resolved Hide resolved
bridge/opencensus/bridge.go Outdated Show resolved Hide resolved
bridge/opencensus/bridge_test.go Outdated Show resolved Hide resolved
Copy link
Member

@krnowak krnowak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One nit, otherwise looks good.

bridge/opencensus/bridge.go Show resolved Hide resolved
@MrAlias
Copy link
Contributor

MrAlias commented Nov 12, 2020

@dashpole the upstream move of packages has caused conflicts here. Let me know if you would like help resolving these, I'm happy to push a fix if you don't have the time.

@dashpole
Copy link
Contributor Author

Updated!

@MrAlias MrAlias merged commit f6df5df into open-telemetry:master Nov 12, 2020
@MrAlias MrAlias mentioned this pull request Nov 20, 2020
AzfaarQureshi pushed a commit to open-o11y/opentelemetry-go that referenced this pull request Dec 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request pkg:bridges Related to a bridge package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants