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

Formalize the translation of OpenTracing references to span parent and links #562

Open
william-tran opened this issue Apr 10, 2020 · 5 comments
Assignees
Labels
area:sdk Related to the SDK priority:p2 Medium priority level release:allowed-for-ga Editorial changes that can still be added before GA since they don't require action by SIGs spec:trace Related to the specification/trace directory

Comments

@william-tran
Copy link

william-tran commented Apr 10, 2020

In the Java shim, the first reference is used as the parent regardless of the reference type: https://github.com/open-telemetry/opentelemetry-java/blob/v0.3.0/opentracing_shim/src/main/java/io/opentelemetry/opentracingshim/SpanBuilderShim.java#L74-L89

In Golang, its the first CHILD_OF: https://github.com/open-telemetry/opentelemetry-go/blob/v0.4.2/bridge/opentracing/bridge.go#L540-L548

In the Jaeger Java client, when spans are converted to thrift, the parent is set if and only if there is a single child reference: https://github.com/jaegertracing/jaeger-client-java/blob/v1.2.0/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/reporters/protocols/JaegerThriftSpanConverter.java#L50. I'm currently hitting this using the thrift HTTP receiver in the opentelemetry-collector. See jaegertracing/jaeger-client-java#705 for tracking this behaviour.

The spec should explicitly call out what the translation should be to avoid inconsistencies when translating to OTel.

@william-tran
Copy link
Author

@dmitryax Does this issue reflect some of your concern in open-telemetry/opentelemetry-collector#844?

@andrewhsu
Copy link
Member

Talked to @carlosalberto, looks like this is a sub-issue of #114

@Oberon00
Copy link
Member

Oberon00 commented Aug 3, 2020

And #65 is a sub-issue of this. EDIT: Or is this issue a duplicate of #65?

@mtwo
Copy link
Member

mtwo commented Aug 31, 2020

Note from the maintainers meeting, @carlosalberto committed to completing this by the end of the week, which we need to do to hit our tracing spec RC deadlines.

@andrewhsu andrewhsu added priority:p2 Medium priority level and removed priority:p1 Highest priority level labels Sep 1, 2020
@andrewhsu andrewhsu added release:allowed-for-ga Editorial changes that can still be added before GA since they don't require action by SIGs and removed release:required-for-ga Must be resolved before GA release, or nice to have before GA labels Sep 25, 2020
@andrewhsu
Copy link
Member

from the issue triage meeting today with TC, allowed for GA for editorial change only

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:sdk Related to the SDK priority:p2 Medium priority level release:allowed-for-ga Editorial changes that can still be added before GA since they don't require action by SIGs spec:trace Related to the specification/trace directory
Projects
None yet
Development

No branches or pull requests

7 participants