Skip to content

Add example for sleuth/brave integration #264

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

Open
asarkar opened this issue Aug 2, 2019 · 9 comments
Open

Add example for sleuth/brave integration #264

asarkar opened this issue Aug 2, 2019 · 9 comments
Labels
examples Everything related to examples help wanted Requesting external help
Milestone

Comments

@asarkar
Copy link

asarkar commented Aug 2, 2019

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.

@ST-DDT
Copy link
Collaborator

ST-DDT commented Aug 3, 2019

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
Copy link
Collaborator

ST-DDT commented Aug 3, 2019

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

@ST-DDT ST-DDT added the question A question about this library or its usage label Aug 3, 2019
@ST-DDT
Copy link
Collaborator

ST-DDT commented Aug 3, 2019

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

Ref: #239

@ST-DDT ST-DDT self-assigned this Aug 3, 2019
@asarkar
Copy link
Author

asarkar commented Aug 4, 2019

  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.

@ST-DDT
Copy link
Collaborator

ST-DDT commented Aug 4, 2019

  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

@asarkar
Copy link
Author

asarkar commented Aug 4, 2019

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

@ST-DDT ST-DDT changed the title Confusing grpc-spring-boot-starters Add example for sleuth/brave integration Aug 25, 2019
@ST-DDT ST-DDT added enhancement A feature request or improvement examples Everything related to examples and removed enhancement A feature request or improvement question A question about this library or its usage labels Aug 25, 2019
@ST-DDT ST-DDT removed their assignment Sep 7, 2019
@ST-DDT ST-DDT added the help wanted Requesting external help label Sep 7, 2019
@mattdkerr
Copy link
Contributor

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.

@ST-DDT
Copy link
Collaborator

ST-DDT commented Mar 23, 2020

It would be highly desirable to fix this issue first.

Could you describe the error in more detail please?

@tejabhgit
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
examples Everything related to examples help wanted Requesting external help
Projects
None yet
Development

No branches or pull requests

4 participants