This repository was archived by the owner on Jul 31, 2023. It is now read-only.
Allow replacing trace SDK#1234
Merged
nilebox merged 1 commit intocensus-instrumentation:masterfrom Oct 22, 2020
Merged
Conversation
nilebox
approved these changes
Oct 14, 2020
|
@dashpole there is some |
c18a8c3 to
14793a8
Compare
14793a8 to
bca9ab0
Compare
Collaborator
Author
|
Tests were failing because the evaluation of nil changed on the interface. I had to make some updates. I added a section to the description on the change in behavior for evaluation against nil. |
Collaborator
Author
|
Is this good to merge? I don't have any power here |
|
@dashpole merged! I'd recommend trying with a pinned commit dependency first if that works well for the OpenTelemetry shim first before tagging a release. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does:
DefaultTracervariable that can be set by users, and defaults to the existing Tracer implementation.AttributesMotivation for the PR:
We would like to be able to assist users in their migration to OpenTelemetry. One way to do this is by writing an implementation of OpenCensus APIs which uses OpenTelemetry under the hood. This way, OpenCensus spans with an OpenTelemetry parent (or vice-versa) would be linked, and all telemetry would be exported through the same exporters (OpenTelemetry exporters in this case).
Rationale for the PR:
This adopts a similar model to OpenTelemetry, in which the SDK can be swapped out for a different implementation.
User Impact:
For users using the "typical" workflow, there should be no changes. For example,
still functions the same way. Even though the Span struct was changed to an interface, the same methods still exist. Only users that pass
Spans would need to change when updating:notice the
*Spanwas changed toSpanThis change also impacts the evaluation of Span against nil. For example,
The above will no longer function as expected. This is because
nil, when assigned to an interface, makes the interface non-nil: https://play.golang.org/p/BWutROXBt5y. Users should change to the following:Example Usage:
See https://github.com/dashpole/opencensus-migration-go/tree/replace_oc_sdk#opencensus-migration-go for an overview, and https://github.com/dashpole/opencensus-migration-go/tree/replace_oc_sdk/migration for an implementation of the new Tracer and Span APIs.
@nilebox @jsuereth @james-bebbington