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

grpc-trace-bin support for gRPC trace? #639

Closed
youngbupark opened this issue Jun 6, 2020 · 10 comments
Closed

grpc-trace-bin support for gRPC trace? #639

youngbupark opened this issue Jun 6, 2020 · 10 comments
Labels
spec:context Related to the specification/context directory spec:trace Related to the specification/trace directory triage:deciding:needs-info triage:deciding:tc-inbox

Comments

@youngbupark
Copy link

Looks like current spec describes only W3C traceparent header for span context propagation for now.

However, Opencensus gRPC spec uses grpc-trace-bin header for spancontext propagation.

Does open-telemetry have a plan to support it?

@Oberon00
Copy link
Member

Oberon00 commented Jun 7, 2020

This requires #437 "Update Binary format in the Specification" (since #426 removed it; it was there before but was "temporarily" removed due to a larger refactoring of both in-process and cross-process context propagation via open-telemetry/oteps#66). However, #437 depends on #577 "Small clean up for Propagators" which turned out to be not so small (has been in review for over a month now).

@bogdandrutu bogdandrutu added the spec:trace Related to the specification/trace directory label Jun 12, 2020
@carlosalberto carlosalberto added the spec:context Related to the specification/context directory label Jun 26, 2020
@carlosalberto
Copy link
Contributor

While Binary is expected to make it (back) into GA, I'm wondering whether we need this specific item too.

Opinion on this @bogdandrutu ?

@carlosalberto carlosalberto added the release:required-for-ga Must be resolved before GA release, or nice to have before GA label Jul 10, 2020
@andrewhsu andrewhsu added this to Required for GA, needs action. Add label:release:required-for-ga to issues and PRs when moving them to this column. in GA Spec Burndown Jul 13, 2020
@andrewhsu andrewhsu added the priority:p3 Lowest priority level label Jul 17, 2020
@JamesNK
Copy link

JamesNK commented Jul 17, 2020

How will grpc-trace-bin work?

  1. Must both traceparent and grpc-trace-bin headers be present on a gRPC HTTP request?
  2. Or is grpc-trace-bin an optional additional for backwards compatibility. Servers check for traceparent, and if it is not present then use grpc-trace-bin? (or the other way around)
  3. Or are gRPC HTTP requests unique that it uses grpc-trace-bin while other HTTP requests use traceparent?

I'd like to understand the motivation here because there is no detail or discussion.

@dyladan
Copy link
Member

dyladan commented Jul 20, 2020

Must both traceparent and grpc-trace-bin headers be present on a gRPC HTTP request?

grpc-trace-bin is the binary equivalent of the traceparent. They transmit the same data.

Or is grpc-trace-bin an optional additional for backwards compatibility. Servers check for traceparent, and if it is not present then use grpc-trace-bin? (or the other way around)

it is required for compatibility with opencensus (at least in js) because that is what OC used.

Or are gRPC HTTP requests unique that it uses grpc-trace-bin while other HTTP requests use traceparent?

the motivation is simply that gRPC is a binary protocol which can take advantage of the more tightly packed binary format to save some bytes on each request

@arminru arminru added release:after-ga Not required before GA release, and not going to work on before GA and removed priority:p3 Lowest priority level release:required-for-ga Must be resolved before GA release, or nice to have before GA labels Sep 25, 2020
@andrewhsu andrewhsu removed this from Required for GA, needs action. Add label:release:required-for-ga to issues and PRs when moving them to this column. in GA Spec Burndown Sep 26, 2020
@rakyll
Copy link
Contributor

rakyll commented Mar 10, 2021

I think we also need to have a separate discussion with gRPC to deprecate grpc-trace-bin and switch them to the TraceContext header.

@JosemyDuarte
Copy link

Is this still in development? I'm trying to enable distributed tracing from a JS service to a Golang service which communicate via gRPC but it doesn't work as described on @opentelemetry/instrumentation-grpc, could grpc-trace-bin be the reason? If it so, then how it's @opentelemetry/instrumentation-grpc supposed to work if is not sending grpc-trace-bin?

@hdost
Copy link

hdost commented Feb 21, 2024

Also an interested party here

@trask
Copy link
Member

trask commented Feb 21, 2024

In Java gRPC Instrumentation, we don't have any special handling for this, which means we (only) use the traceparent header when W3C propagator is used.

@austinlparker austinlparker added triage:deciding:community-feedback and removed release:after-ga Not required before GA release, and not going to work on before GA labels Jun 18, 2024
@austinlparker
Copy link
Member

Is this related to #437?

@jack-berg
Copy link
Member

I believe so. Closing since this is dependent on #437, which was closed for the reasons listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spec:context Related to the specification/context directory spec:trace Related to the specification/trace directory triage:deciding:needs-info triage:deciding:tc-inbox
Projects
None yet
Development

No branches or pull requests