open-telemetry / opentelemetry-specification

Specifications for OpenTelemetry
https://opentelemetry.io
Apache License 2.0
3.76k stars 891 forks source link

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

Open william-tran opened 4 years ago

william-tran commented 4 years ago

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 https://github.com/jaegertracing/jaeger-client-java/issues/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 commented 4 years ago

@dmitryax Does this issue reflect some of your concern in https://github.com/open-telemetry/opentelemetry-collector/pull/844?

andrewhsu commented 4 years ago

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

Oberon00 commented 4 years ago

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

mtwo commented 4 years ago

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 commented 4 years ago

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