go-grpc-middleware icon indicating copy to clipboard operation
go-grpc-middleware copied to clipboard

Published 20 hours ago • verified publisher icon grpc-ecosystem

Question: Opentracing here vs github.com/grpc-ecosystem/grpc-opentracing

Open feliksik opened this issue 8 years ago • 8 comments

How does the opentracing part of this repo relate to https://github.com/grpc-ecosystem/grpc-opentracing/tree/master/go/otgrpc (otgrpc)?

It look like both package build middleware, using https://github.com/opentracing/opentracing-go

Are they alternative implementations? Are you in touch with the authors of otgrpc?

feliksik avatar May 08 '17 11:05 feliksik

So, the otgrpc implements only a Unary handler, and this implements Streaming as well, both for client and server-side.

Both packages are indeed using opentracing-go since that is the official opentracing library for Go, and a lot of Tracer implementations are using it. As such, both pieces of middleware are agnostic as to what Tracer is used by the user: Zipkin, Jaeger, Stackdriver or whatever else.

The reason why this package is separate here is because it integrates with grpc_ctx and thus allows you to have traces tagged with, for example, fields extracted from the request.

@bhs, author of otgrpc for discussion context.

mwitkow avatar May 13 '17 10:05 mwitkow

Would be nice to have a single place for such middleware, especially within the same organization.

sagikazarmark avatar May 30 '17 00:05 sagikazarmark

@mwitkow just noticed this, sorry!

I believe that there's a PR to support Streaming with the otgrpc package... in any case, we (obviously) should not duplicate this effort in two places. Is there a reason that grpc_ctx support should/could not just be added to the otgrpc package?

bhs avatar May 30 '17 02:05 bhs

@bhs well grpc_ctx is an internal package of the go-grpc-middlewares. That's the only reason.

Can you link me to the Streaming PR for otgrpc?

mwitkow avatar Jun 01 '17 16:06 mwitkow

@mwitkow sure: https://github.com/grpc-ecosystem/grpc-opentracing/pull/25

bhs avatar Jun 02 '17 05:06 bhs

@bhs @mwitkow Are there any plans to join forces and merge the 2 middlewares?

F21 avatar Sep 29 '17 07:09 F21

@F21

  1. Apologies for my delay, I missed this @bhs :-/
  2. There are no plans that I'm aware of; that doesn't mean it's a bad idea, just that people have other priorities. I generally like the idea of minimizing the scope of packages, and for that reason have some natural bias towards the tracing-specific grpc-opentracing repo's charter; that said, everything is a tradeoff and if there's a compelling reason to merge the two middlewares I'm open to hearing about it.

bhs avatar Oct 05 '17 09:10 bhs

Is there a method to get the span in grpc handler? the middleware creates a span, and I cannot log into it.

dinfer avatar Jan 09 '18 03:01 dinfer

Seems not relevant, given tracing is no longer part of this repo (in v2), given OpenTelemetry maintains great interceptor.

bwplotka avatar Mar 19 '23 01:03 bwplotka