Skip to content
This repository has been archived by the owner on Jul 31, 2023. It is now read-only.

plugin/ocgrpc: add B3 trace propagation #666

Open
terinjokes opened this issue Apr 3, 2018 · 23 comments
Open

plugin/ocgrpc: add B3 trace propagation #666

terinjokes opened this issue Apr 3, 2018 · 23 comments

Comments

@terinjokes
Copy link

terinjokes commented Apr 3, 2018

In February, support for B3 propagation was added for HTTP clients and servers in #380. It is common in B3 environments to also use these same headers in GRPC metadata, as described in b3-propagation.

I'm keen to try using OpenCensus in my project, but I need to maintain tracing compatibility with other services over both HTTP and GRPC. At this time, the ocgrpc plugin does now allow for alternative propagation formats.

@rakyll
Copy link
Contributor

rakyll commented Apr 3, 2018

/cc @bogdandrutu

@rakyll
Copy link
Contributor

rakyll commented Apr 3, 2018

gRPC uses the propagation format we implement in OpenCensus and even though tracing is configurable in some language implementations of gRPC, it is not in some languages like Java.
If we allow the Go implementation to use B3, it won't be compatible with gRPC Java.

This might be a problem if you are depending on a sidecar or load balancer that only understands gRPC B3 but shouldn't be a problem otherwise.

You can always propagate traces in the binary format among gRPC servers and use our HTTP propagation formats when propagating on HTTP. Inside the process, we convert the headers/metadata to trace.SpanContext anyways. How you propagate on wire is not that important unless you are intercepting network and trying to interact with tracing headers/metadata at that level.

@terinjokes
Copy link
Author

This might be a problem if you are depending on a sidecar or load balancer that only understands gRPC B3 but shouldn't be a problem otherwise.

We do this, as well as communicating over GRPC to services that have no knowledge of OpenCensus, but expect B3 propagation over GRPC.

@rakyll
Copy link
Contributor

rakyll commented Apr 3, 2018

I see. Let's wait for @bogdandrutu's opinions on this.

@bogdandrutu
Copy link
Contributor

@terinjokes I have some questions:

  • what languages do you need this?
  • Do you expect to always send the B3 propagation format to the next service or do you want to have this as a configuration?

@terinjokes
Copy link
Author

terinjokes commented Apr 4, 2018

I'm primarily writing Go. Many of the services I interact with (which our likely using OpenZipkin instead of OpenCensus) are also written in Go, though some are not.

To prevent fragmentation of distributed tracing data, we've specified using B3 propagation between all services, where this is a reasonable ask. As linked above, this is defined for HTTP (headers) and GRPC (metadata), and we use similarly named fields when enqueing in a message queue.

B3 was chosen for its broad support between applications, languages, and frameworks.

@rakyll
Copy link
Contributor

rakyll commented Apr 6, 2018

Talked to @bogdandrutu offline and we think providing an option for B3 propagation is fair and required to support legacy systems. We will keep using the binary format and add B3 headers if users wanted them.

@rakyll rakyll self-assigned this Apr 6, 2018
@bogdandrutu
Copy link
Contributor

@terinjokes to give you some reasons why, grpc-java is instrumented with OpenCensus (soon will have Go and the rest of the languages) and we use the binary format there because it is lower overhead (sending multiple grpc metadata is expensive) also sending binary vs hex is faster.

Long term we want all gRPC users to use the binary format but we are aware of the legacy instrumentation using b3 used by OpenZipkin and will try to work with them to switch to the new format as well.

@codefromthecrypt
Copy link

codefromthecrypt commented Apr 7, 2018 via email

@codefromthecrypt
Copy link

codefromthecrypt commented Apr 7, 2018 via email

@rakyll
Copy link
Contributor

rakyll commented Apr 12, 2018

For example if the binary format is somehow related to trace context or
something such that would be a decent sell as the folks who are less likely
to move may be on account of not being quite concerned about the cost to
marshal hex which to many is negligible as it is far under a microsecond
and http header compression should take care of the repeated B3 keys.

I asked @bogdandrutu about this a year ago and he should be the one to figure it out.

That way using the format internal to grpc doesnt bleed
into other systems like http.

The gRPC format is not leaking into other systems because we immediately convert the header to a SpanContext and propagate it in a context.Context object inside the project. Are you talking about something else?

@codefromthecrypt
Copy link

codefromthecrypt commented Apr 12, 2018 via email

@terinjokes
Copy link
Author

terinjokes commented Apr 18, 2018

@rakyll I think sending tracing metadata in both formats is a reasonable alternative.

I'll likely need to coordinate with the middleware (in this case, Envoy) to ensure when it creates child spans, it also updates the binary format.

@rakyll
Copy link
Contributor

rakyll commented Apr 23, 2018

Maybe @bogdandrutu should update Envoy to use the binary format if it is the format gRPC blesses.

@semistrict
Copy link
Contributor

I need this for Istio trace support. I will probably start working on it in the next day or so.

@semistrict semistrict assigned semistrict and unassigned rakyll May 30, 2018
@vaijab
Copy link

vaijab commented May 30, 2018

I need this for Istio trace support.

I had an impression that Istio will have OpenCensus native support?

@semistrict
Copy link
Contributor

For now, it uses B3 as the propagation format. I might try to support other propagation formats as a follow up but it requires changes in a number of different Istio components so want to just go with B3 for now.

@rakyll
Copy link
Contributor

rakyll commented Jul 13, 2018

Duplicate of census-instrumentation/opencensus-specs#136.

@rakyll rakyll closed this as completed Jul 13, 2018
@semistrict
Copy link
Contributor

We can use this to track implementation once the spec issue is accepted.

@semistrict semistrict reopened this Jul 16, 2018
@codefromthecrypt
Copy link

codefromthecrypt commented Jul 17, 2018 via email

@odeke-em odeke-em changed the title Add B3 for ocgrpc plugin/ocgrpc: Add B3 trace propagation Aug 20, 2018
@odeke-em odeke-em changed the title plugin/ocgrpc: Add B3 trace propagation plugin/ocgrpc: add B3 trace propagation Aug 20, 2018
@fho
Copy link

fho commented Aug 13, 2019

We also require support for B3 propagation especially on the GRPC server-side.
We have GRPC-Clients in different languages (e.g. python, php, nodejs) which propagate the tracing information in B3 format to the GRPC-Servers.
We can not change all applications at once to use the binary format for GRPC propagation.
Support for B3 format would allow us to migrate to opencensus in the Go-GRPC-Servers and then migrate step by step the other apps to use opencensus.

@jcxplorer
Copy link

I've made a small proof of concept for this and tested it with Istio. It's a small change, so if there's interest in merging it, I'd be happy to clean it up, add tests, and make it optional in ocgrpc.ClientHandler. I don't think we need support in ServerHandler, because we still pass the current grpc-trace-bin. At least for distributed tracing with Istio, this is enough.

Any of the current maintainers willing to accept a PR?

@tmc
Copy link

tmc commented Sep 12, 2020

Is there an opentelemetry equivalent for this issue?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

10 participants