grpc-spring icon indicating copy to clipboard operation
grpc-spring copied to clipboard

Add example for sleuth/brave integration

Open asarkar opened this issue 6 years ago • 9 comments

This is a usage question.

In the Sleuth docs for gRPC, they mention two variants. First one uses io.github.lognet:grpc-spring-boot-starter and io.zipkin.brave:brave-instrumentation-grpc and clients must use SpringAwareManagedChannelBuilder. In the second variant, nothing is said other than "Grpc Spring Boot Starter automatically detects the presence of Spring Cloud Sleuth and brave’s instrumentation for gRPC and registers the necessary client and/or server tooling."

There doesn't seem to be any relation between these projects, and are maintained by different people. yidongnan/grpc-spring-boot-starter doesn't seem to use SpringAwareManagedChannelBuilder anywhere. Should it? Also, does it need io.zipkin.brave:brave-instrumentation-grpc to work?

Perhaps the owners of the two projects should discuss merging them. It seems like a waste of time to develop two parallel projects for the same purpose.

asarkar avatar Aug 02 '19 22:08 asarkar

Related issue: #164

In the Sleuth docs for gRPC, they mention two variants. First one uses io.github.lognet:grpc-spring-boot-starter and io.zipkin.brave:brave-instrumentation-grpc and clients must use SpringAwareManagedChannelBuilder. In the second variant, nothing is said other than "Grpc Spring Boot Starter automatically detects the presence of Spring Cloud Sleuth and brave’s instrumentation for gRPC and registers the necessary client and/or server tooling."

We are aware of this, unfortunately there is nothing we can do about it. Our requests for adding better documentation have been refused.

yidongnan/grpc-spring-boot-starter doesn't seem to use SpringAwareManagedChannelBuilder anywhere. Should it?

No, I will explain it in detail below.

Also, does it need io.zipkin.brave:brave-instrumentation-grpc to work?

Yes, the dependency is required for sleuth to work in this context.

There doesn't seem to be any relation between these projects, and are maintained by different people. [...] Perhaps the owners of the two projects should discuss merging them. It seems like a waste of time to develop two parallel projects for the same purpose.

There are two major problems here:

  1. IMO it's impolite to ask others to drop their work.
  2. Both projects try to achieve different goals.
    • The LogNet one tries to be especially lightweight (server only) and maintain support for Spring-Boot 1.x
    • This one tries to cover the important parts while being extensible/customizable (server + client)

LogNet's goal trying to be lightweight/server only resulted in the controversial state where sleuth added a support class to their code, that is the part of the core of this library. Sleuth somewhat agrees with us that it shouldn't be (necessary to be a) part of Sleuth. See also this javadoc in the sleuth repo.

Sleuth's SpringAwareManagedChannelBuilder is roughly equivalent to our GrpcChannelFactory.

ST-DDT avatar Aug 03 '19 09:08 ST-DDT

Maybe I should drop a few words in the docs about sleuth/brave-instrumentation-grpc.

ST-DDT avatar Aug 03 '19 09:08 ST-DDT

Added a few lines in the upcoming documentation. Does this help you?

Ref: #239

ST-DDT avatar Aug 03 '19 11:08 ST-DDT

  1. "You need a Tracing bean in your application context." A code example would be great.
  2. Under additional notes, "those class solely exists" is a grammatical error; should be "those classes solely exist".

Other than the above, looks good to me.

asarkar avatar Aug 04 '19 05:08 asarkar

  1. Unfortunately I don't have one. All I could do is link to sleuth's TraceAutoConfiguration. Would that be sufficient? Unfortunately I'm not that familiar with sleuth/brave.
  2. Fixed

ST-DDT avatar Aug 04 '19 09:08 ST-DDT

Short of a working example, I think what you’ve is an excellent start.

asarkar avatar Aug 04 '19 11:08 asarkar

At this point in time it would make more sense to do tracing with OpenTelemetry, however using the intercepters caused issues for gRPC calls outbound on a @GrpcClient after receiving them from a @GrpcService annotated API. We had to use our own classes to read in the same @ConfigurationProperties for the client and add the interceptors for tracing in a @Configuration. It would be highly desirable to fix this issue first.

mattdkerr avatar Mar 23 '20 22:03 mattdkerr

It would be highly desirable to fix this issue first.

Could you describe the error in more detail please?

ST-DDT avatar Mar 23 '20 23:03 ST-DDT

At this point in time it would make more sense to do tracing with OpenTelemetry, however using the intercepters caused issues for gRPC calls outbound on a @GrpcClient after receiving them from a @GrpcService annotated API. We had to use our own classes to read in the same @ConfigurationProperties for the client and add the interceptors for tracing in a @Configuration. It would be highly desirable to fix this issue first.

Could you please provide the github example of your openTelemetry implementation with grpc, jaeger in springboot (client and server). I have tried many examples and nothing seems to work. Its a show stopper now in my project.

tejabhgit avatar Nov 15 '21 21:11 tejabhgit