Fix opencensus shim spanBuilderWithRemoteParent behavior #6415
Conversation
| SpanConverter.mapSpanContext(ocRemoteParentSpanContext, /* isRemoteParent= */ true); | ||
| otelSpanBuilder.setParent( | ||
| Context.current().with(io.opentelemetry.api.trace.Span.wrap(spanContext))); | ||
| otelSpanBuilder.addLink(spanContext); |
There was a problem hiding this comment.
Wasn't sure if anyone is dependent on the existing behavior.
There was a problem hiding this comment.
Do you think we'd remove the addLink before marking the opencensus shim stable?
There was a problem hiding this comment.
I'm not sure. I'd want the spec opencensus compatibility doc to say what to do here before marking stable. That doc is currently lax on details which we see in the opentracing compatibility doc, so I don't see this happening any time soon.
Thanks @gavin-nia, so we can tag this PR as Fixes #6416? |
Yes, I believe so. |
Fixes #5475.
The behavior of
spanBuilderWithRemoteParentseems wrong right now. It adds a span link instead but does not set the span represented by the context argument to be the parent. Not sure why this is, but I can't find anything in the opencensus compatibility spec which states that this is intended, so I'm going to assume its just a bug in the implementation.