opencensus-shim OpenTelemetrySpanBuilderImpl.startSpan() doesn't set underlying parent when passed remote context

[arosien]

Describe the bug
When using the opencensus-shim, and thus the OpenCensus APIs, after injecting an incoming propagated context and creating a span, the created span doesn't have the same trace id as the propagated context.

val spanContext: SpanContext = ??? // parsed from incoming request with propagated trace info
val span = Tracing.getTracer
  .spanBuilderWithRemoteParent(spanName, spanContext)
  .startSpan()
// span.getContext.getTraceId != spanContext.getTraceid

This is because the shim's OpenTelemetrySpanBuilderImpl doesn't use the remote context to set the span's parent. (The only way to set the parent span is to invoke spanBuilderWithExplicitParent).

The underlying OpenTelemetry impl of a SpanBuilder only sets the parent if a parent span is provided.

Steps to reproduce
See above.

What did you expect to see?
I expect that a child span of a spanBuilderWithRemoteParent has the same trace id as that of the parent context.

The legacy implementation in OpenCensus properly sets the child's trace id to that of the parent context.

What did you see instead?
Child span doesn't have the same trace id as the parent when using the OpenCensus shim.

What version and what artifacts are you using?
Artifacts: opentelemetry-api, opentelemetry-opencensus-shim
Version: v1.25.0
How did you reference these artifacts?

Environment
Compiler: openjdk version "17.0.7" 2023-04-18
OS: MacOSX 13.3

Additional context

[jack-berg]

@jsuereth any thoughts on this?

[gavin-nia]

Just stumbled across something similar which may be the same issue (version 1.37.0).

We are trying to migrate from opencensus to opentelemetry and we use grpc for some microservices. As such, we will use the opencensus-shim to take advantage of existing grpc instrumentation.

One problem we've noticed is that the "sent" and "received" spans generated by the grpc client & server are not in the same trace. It sounds very much like the issue raised by @arosien.

This definitely makes the tracing much less useful (previously we could see the client & server calls in a single trace on jaeger).

Is it possible that this can be given some more consideration @jack-berg ?

[jkwatson]

@gavin-nia If you have time for a PR that addresses the issue, that would be fantastic. Jack is very over-burdened, and not an open-census user, so it would be great if someone from the OC community could submit a patch that addresses the issue.

[jack-berg]

Jack is very over-burdened, and not an open-census user

Its true but this happen to have caught my attention and I reviewed the implementation / fixed pretty quickly. Checkout PR #6415 for a fix. Would want some eyes from opencensus folks before merging.

[gavin-nia]

Thank you very much for the quick response and fix.

[gavin-nia]

I tried #6415 and can confirm that the client & server side had the expected parent-child relationship.

There does appear to be one other issue that needs addressed to get gRPC + shim working correctly and that is referenced here:
grpc/grpc-java#7732

[jack-berg]

@gavin-nia please review / approve #6415. We take approvals into consideration even if the person doesn't have a formal role in the repository!

With respect to the other gRPC issue, can you open a new issue in this repo for that?