Opencensus compatibility specification #1332

jsuereth · 2021-01-11T19:27:58Z

Fixes #766

Changes

Provide a specification of OpenCensus migration to open telemetry.

Specifies shims for TRACE that can be implemented per-langauage.
Specifies overarching goals for migration
ONLY accounts for specification<->specification issues and calls out possible incompatibility issues for unspecified behavior.
Does not specify Metric compatibility as that will wait for Metrics to be declared GA / stable.

A few notes for discussion:

This spec calls for semantic-name mapping so that users of an OpenCensus bridge will get OpenTelemetry-looking metrics even from OpenCensus instrumented libraries. The expectation is this eases the migration from OC => OTel, but that labels may "break" from OC-only implementation, and won't break as OC instrumented lirbaries are migrated to OTel.
This attempts to identify all known incompatibilites we've found between OpenCensus + OpenTelemetry APIs from the Java + Go implementations, as well as directly comparing the Specifications. . Unfortunately, a lot of incompatibilities are due to unspecified API, SDK or other behaviors, and cannot be (fully) addressed in this Specification. Instead a set of principles for how to make tradeoffs are proposed up-front for implementors of OpenCensus-bridge components.
The specification is expected to evolve with known incompatibilities as needed, in an attempt to keep language-specific design decisions around unspecified behavior as uniform as possible.

specification/compatibility/opencensus.md

github-actions · 2021-01-22T04:08:55Z

This PR was marked stale due to lack of activity. It will be closed in 7 days.

…OT complete, nor anywhere near done.

specification/compatibility/opencensus.md

jsuereth · 2021-01-29T13:15:25Z

@anuraaga Looks like i had a dropped commit when rebasing that lost all the committed suggestions from upstream, trying to recollect those now.

anuraaga

Just a few smaller points but this seems to be good, thanks!

specification/compatibility/opencensus.md

anuraaga · 2021-01-30T03:02:12Z

specification/compatibility/opencensus.md

+This adapter MUST use an OpenTelemetry `TextMapPropagator` to implement the
+OpenCensus `TextFormat`.
+
+This adapter SHOULD use configured OpenTelemetry `TextMapPropagator` on the


Just checking, is this something Java can't do but other languages may? For example, we have explicit extension points for B3 and W3C

https://github.com/open-telemetry/opentelemetry-java/blob/main/opencensus-shim/src/main/java/io/opentelemetry/opencensusshim/OpenTelemetryPropagationComponentImpl.java#L28

So it seems like it wouldn't ever make sense to use the OTel configured propagator instead of directly exposing the correct implementations for this sort of interface.

I can clarify this. I actually wanted to specify it so that OpenCensus shim will use the globallly configured propogator (see https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/context/api-propagators.md#get-global-propagator). Right now the implementation DOES NOT do this, but I'd like to fix it. Happy to hear thoughts/.copncerns.

My main reasoning for this shift from the Java impl is because I'd like to give OTel configuration the ability to control the impl of the propogator. One of the "principles" here is when adopting the shim, OTEL configuration should override OC, always.

Probably you may want to state this principal somewhere.

anuraaga · 2021-01-30T03:03:23Z

specification/compatibility/opencensus.md

+This adapter SHOULD use configured OpenTelemetry `TextMapPropagator` on the
+OpenTelemetry `TraceProvider` for text format propagation.
+
+This adapter MUST provide a default `W3CTraceContextPropagator` in lieu of a


Not sure what this sentence means. Is it the same as my above point, where we actually don't want to use the configured provider? More concretely, this line of code in Java?

https://github.com/open-telemetry/opentelemetry-java/blob/main/opencensus-shim/src/main/java/io/opentelemetry/opencensusshim/OpenTelemetryPropagationComponentImpl.java#L34

NO, this is related to selecting the W3C propagatoor off the global (see: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/context/api-propagators.md#get-global-propagator) and if there is none available, providing a default.

I'll take a stab at rewording this, need to think about a better phrasing.

Take look at the rewording, hopefully it's more clear.

Co-authored-by: Anuraag Agrawal <anuraaga@gmail.com>

bogdandrutu

Some minor comments, LGTM

specification/compatibility/opencensus.md

bogdandrutu · 2021-02-06T02:42:27Z

specification/compatibility/opencensus.md

+   Some OpenCensus APIs support "debug" and "defer" tracing flags in additon to
+   "sampled".  In this case, the OpenCensus bridge will do its best to support
+   and translate unspecified flags into the closest OpenTelemetry equivalent.


facepalm :)))

open-telemetry/opentelemetry-go#1515

bogdandrutu · 2021-02-06T02:43:48Z

specification/compatibility/opencensus.md

+This adapter MUST use an OpenTelemetry `TextMapPropagator` to implement the
+OpenCensus `TextFormat`.
+
+This adapter SHOULD use configured OpenTelemetry `TextMapPropagator` on the


Probably you may want to state this principal somewhere.

bogdandrutu

Please add a status

jsuereth · 2021-02-11T15:18:45Z

@bogdandrutu for "status" do you mean the stable, experimental etc? I'll go look up the rules and annotate sections accordingly.

bogdandrutu · 2021-02-11T16:23:33Z

@jsuereth yes what we call the document status (e.g. experimental, stable, etc.)

bogdandrutu · 2021-02-12T05:17:20Z

@jsuereth ping :)

bogdandrutu · 2021-02-16T22:05:56Z

@jsuereth ping :)

jsuereth · 2021-02-17T18:02:10Z

@bogdandrutu Will get to this later today/tomorrow. Sorry for delays, had a lot of urgent things / Scala.Love conference prep to do last week.

jsuereth · 2021-02-17T20:34:11Z

@bogdandrutu Ok, so I updated stability guarantees. Here's the plan:

This document declares "Trace" as Experimental, but "feature frozen". We won't do any additional mapping beyond what's outlined in this document.
If we discover more issues in "Trace" as we implement (or adapt) the compatibility shims in languages, we have room to fix the specification.
Once we have N languages implemented against the specification we mark "Trace" as "Stable, Feature Freeze".
- At least ONE dynamic language (Python, Node, etc. missing today)
- At least ONE statically typed language (Java, Go, C#)
- Note: Today we have shim prototypes in Go + Java
We follow with Metric definition in similar fashion once metrics are stable in the OTEL spec.

Sound reasonable?

reyang · 2021-02-18T02:44:35Z

specification/compatibility/opencensus.md

+#### Known Incompatibilities
+
+Below are listed known incompatibilities between OpenTelemetry and OpenCensus
+specifications.   Applications leveraging unspecified behavior from OpenCensus


Suggested change

specifications. Applications leveraging unspecified behavior from OpenCensus

specifications. Applications leveraging unspecified behavior from OpenCensus

There are other places in the doc that also have 3 SPCs.

reyang · 2021-02-18T02:53:55Z

specification/compatibility/opencensus.md

+details of the OpenTelemetry SDK.
+
+More specifically, the intention is to allow OpenCensus instrumentation to be
+recorded using OpenTelemetry. This Shim Layer MUST NOT publicly expose any


Might need to clarify where the shim should go - in OpenTelemetry project or OpenCensus project.

https://github.com/open-telemetry/opentelemetry-specification/blob/9047c91412d3d4b7f28b0f7346d8c5034b509849/specification/versioning-and-stability.md#a-note-on-replacing-signals

OpenTelemetry already supports two tracing APIs: OpenTelemetry and OpenTracing. We invented a new tracing API, but continue to support the old one.

I can do. In practice, we've actually released new OpenCensus versions that expose hooks for the shim, and the shim lives on the OTEL side (as that's where you configure your pipeline).

As an FYI for you, I expect we might have to do something similar in Python/JavaScript if we run into similar issues as what we found in Go/Java.

reyang

LGTM as "Experimental".

bogdandrutu · 2021-02-22T16:50:48Z

@jsuereth please fix the build 👯

jsuereth · 2021-02-22T19:43:59Z

@bogdandrutu fixed :) Sorry, for some reason can't run that check locally.

jmacd

LGTM

jsuereth requested review from a team January 11, 2021 19:27

github-actions bot assigned jmacd Jan 11, 2021

anuraaga reviewed Jan 12, 2021

View reviewed changes

anuraaga reviewed Jan 15, 2021

View reviewed changes

specification/compatibility/opencensus.md Show resolved Hide resolved

specification/compatibility/opencensus.md Outdated Show resolved Hide resolved

github-actions bot added the Stale label Jan 22, 2021

Base automatically changed from master to main January 27, 2021 21:16

jsuereth added 3 commits January 28, 2021 15:53

DRAFT - preliminary OpenCensus compatibility specification. This is N…

4f36bf2

…OT complete, nor anywhere near done.

Added known incompatibility section.

8bea30c

Updates from comments, and semantic convention mappings.

debc173

jsuereth force-pushed the wip-opencensus-compatibility branch from 7dbd03b to debc173 Compare January 28, 2021 20:54

Fixes from review.

bbb438c

jsuereth changed the title ~~Wip opencensus compatibility~~ Opencensus compatibility specification Jan 28, 2021

anuraaga reviewed Jan 29, 2021

View reviewed changes

jsuereth added 3 commits January 29, 2021 08:59

Reapply missing commits from local rebase.

33d601c

More typo fixes and review changes.

7abfbce

Fixes from linter/spellchecker.

17f33b0

jsuereth mentioned this pull request Jan 29, 2021

Release v1.0.0 #1372

Merged

anuraaga approved these changes Jan 30, 2021

View reviewed changes

github-actions bot removed the Stale label Jan 30, 2021

Bogdan Drutu and others added 3 commits February 5, 2021 18:36

Update specification/compatibility/opencensus.md

c88c057

Co-authored-by: Anuraag Agrawal <anuraaga@gmail.com>

Update specification/compatibility/opencensus.md

15a7bca

Co-authored-by: Anuraag Agrawal <anuraaga@gmail.com>

Merge branch 'main' into wip-opencensus-compatibility

68d0251

bogdandrutu approved these changes Feb 6, 2021

View reviewed changes

Fixes from reivew.

722b6b1

bogdandrutu approved these changes Feb 11, 2021

View reviewed changes

skonto mentioned this pull request Feb 17, 2021

[Spike] Migrate API Server Source to the Open Telemetry lib knative/eventing#4769

Closed

Update document status.

f561b16

reyang reviewed Feb 18, 2021

View reviewed changes

reyang approved these changes Feb 18, 2021

View reviewed changes

Merge branch 'main' into wip-opencensus-compatibility

6d0208d

Fix broken api link.

456bd6b

bogdandrutu approved these changes Feb 22, 2021

View reviewed changes

bogdandrutu merged commit 0a0ef44 into open-telemetry:main Feb 22, 2021

jsuereth deleted the wip-opencensus-compatibility branch February 22, 2021 22:38

jmacd reviewed Feb 23, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Opencensus compatibility specification #1332

Opencensus compatibility specification #1332

jsuereth commented Jan 11, 2021

github-actions bot commented Jan 22, 2021

jsuereth commented Jan 29, 2021

anuraaga left a comment

anuraaga Jan 30, 2021 •

edited

Loading

jsuereth Jan 30, 2021

bogdandrutu Feb 6, 2021

anuraaga Jan 30, 2021

jsuereth Jan 30, 2021

jsuereth Feb 9, 2021

bogdandrutu left a comment

bogdandrutu Feb 6, 2021

bogdandrutu Feb 6, 2021

bogdandrutu Feb 6, 2021

bogdandrutu left a comment

jsuereth commented Feb 11, 2021

bogdandrutu commented Feb 11, 2021

bogdandrutu commented Feb 12, 2021

bogdandrutu commented Feb 16, 2021

jsuereth commented Feb 17, 2021

jsuereth commented Feb 17, 2021

reyang Feb 18, 2021

reyang Feb 18, 2021

reyang Feb 18, 2021

jsuereth Feb 18, 2021

reyang left a comment

bogdandrutu commented Feb 22, 2021

jsuereth commented Feb 22, 2021

jmacd left a comment

	specifications. Applications leveraging unspecified behavior from OpenCensus
	specifications. Applications leveraging unspecified behavior from OpenCensus

Opencensus compatibility specification #1332

Opencensus compatibility specification #1332

Conversation

jsuereth commented Jan 11, 2021

Changes

github-actions bot commented Jan 22, 2021

jsuereth commented Jan 29, 2021

anuraaga left a comment

Choose a reason for hiding this comment

anuraaga Jan 30, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bogdandrutu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bogdandrutu left a comment

Choose a reason for hiding this comment

jsuereth commented Feb 11, 2021

bogdandrutu commented Feb 11, 2021

bogdandrutu commented Feb 12, 2021

bogdandrutu commented Feb 16, 2021

jsuereth commented Feb 17, 2021

jsuereth commented Feb 17, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

reyang left a comment

Choose a reason for hiding this comment

bogdandrutu commented Feb 22, 2021

jsuereth commented Feb 22, 2021

jmacd left a comment

Choose a reason for hiding this comment

anuraaga Jan 30, 2021 •

edited

Loading