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

Where to contribute a propagator for the gRPC plugin? #1071

Closed
1 task done
nkelly75 opened this issue May 18, 2020 · 6 comments
Closed
1 task done

Where to contribute a propagator for the gRPC plugin? #1071

nkelly75 opened this issue May 18, 2020 · 6 comments
Labels
Discussion Issue or PR that needs/is extended discussion.

Comments

@nkelly75
Copy link

  • This only affects the JavaScript OpenTelemetry library

While using the grpc plugin in a node service I was able to successfully export spans (to two tracing back-ends I was testing). However, the spans didn’t take their context from incoming gRPC calls as expected (leading to new "shallow" traces being created). Having discussed with @austinlparker we realized a propagator was required that would use the grpc-trace-bin gRPC header. I have an initial working implementation (inspired by previous OpenCensus work) and would like to contribute to the appropriate OpenTelemetry repo.

Looks like options include:

Please advise on the best location for the propagator and I will work to get a pull request in place.

@mayurkale22
Copy link
Member

We had implemented binary propagator, but removed it with respect to open-telemetry/opentelemetry-specification#426 in #762 and #804.

I am not sure current status of specs, @dyladan any idea?

@mayurkale22 mayurkale22 added the Discussion Issue or PR that needs/is extended discussion. label May 18, 2020
@dyladan
Copy link
Member

dyladan commented May 18, 2020

Binary propagator comes up periodically on spec calls. Afaik it is something they consider a requirement before GA but isn't the highest priority thing right now

@nkelly75
Copy link
Author

Some extra context. While learning more about OpenTelemetry following observe2020 I had started with a fork of the microservices-demo (originally instrumented using OpenCensus) where spans were exported to a local OpenTelemetry Collector which then exported to the two backends. Then I wanted to learn more about direct OTel instrumentation in code and chose to modify the PaymentService (node) so it was instrumented using OpenTelemetry (rather than OpenCensus). I added OTel “attributes” on spans to generate some currency tags for demo purposes. The incoming gRPC calls (from CheckoutService, still instrumented with OpenCensus) had the grpc-trace-bin headers. Without the binary propagator the end-to-end demo wasn’t effective.

I wasn’t aware that a binary propagator had existed previously. Just learned today that it was removed as part of #804. I now see also that the binary format was removed from the Propagators section of the OTel Spec (as part of #426).

Looks like there are good reasons for these removals in the long term. However, in order to have interoperability with existing OpenCensus-instrumented services, is there an argument for having a binary “CensusPropagator” (working off the grpc-trace-bin header) in the contrib repo?

@dyladan
Copy link
Member

dyladan commented May 18, 2020

is there an argument for having a binary “CensusPropagator” (working off the grpc-trace-bin header) in the contrib repo

That sounds like a great idea 👍

@mayurkale22
Copy link
Member

I think we agree on this one, can we close issue?

@dyladan
Copy link
Member

dyladan commented May 19, 2020

Created a new issue in the contrib repo. @nkelly75 you have to comment on the new issue if you want me to assign it to you.

@dyladan dyladan closed this as completed May 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion Issue or PR that needs/is extended discussion.
Projects
None yet
Development

No branches or pull requests

3 participants