search

getsentry / sentry-java

A Sentry SDK for Java, Android and other JVM languages.
https://docs.sentry.io/
MIT License
1.15k stars 434 forks source link

Support of gRPC for performance monitoring #1512

Open ahmadshabib opened 3 years ago

ahmadshabib commented 3 years ago

Hey guys, Afaik the current support of the performance monitoring and transaction is applicable to spring, spring-boot, and servlet. To give better support to performance monitoring of microservices that only expose RPC endpoints it would be nice to do it out of the box for such applications. Is it part of any future plan, What do you guys think?

marandaneto commented 3 years ago

@ahmadshabib thanks for raising this. so far we've no plans to add an integration for it but it does make sense, let's keep it open and see if people upvote it, we could prioritize if enough people ask for the same integration. for now, you can use the custom instrumentation, where you start and finish your own transactions/spans manually.

ahmadshabib commented 3 years ago

@marandaneto thanks for your response. I can work on an MVP for gRPC only as integration till we reach the point of full integration of other implementation of RPC, what do you think?

marandaneto commented 3 years ago

@ahmadshabib nice, you could do an MVP and share as a GH Gist initially maybe? would it be on top of which gRPC impl.? maybe https://github.com/grpc/grpc-java you could see how its done on OkHttp https://github.com/getsentry/sentry-java/blob/main/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt which should not be that different

marandaneto commented 3 years ago

it's likely possible with io.grpc.ClientInterceptors

caseyduquettesc commented 3 years ago

Here's a highly opinionated interceptor implementation. I figure someone can use it as a starting point to unblock themselves.

https://gist.github.com/caseyduquettesc/a7a8733d048af455a36f6536654d4d2c

bruno-garcia commented 3 years ago

This looks promising. Thanks @caseyduquettesc for the gist. Would you be willing to send a PR adding a package? It would look like the OkHttp structure and tests for example.

caseyduquettesc commented 3 years ago

I'm not sure how it would be designed. My example adds breadcrumbs and spans with text specific to my use case. To add a package with the same behavior that can be generic enough for everyone, we would need figure out

I don't mind submitting the PR if I knew how to solve those problems, which could be solved by subclassing, but the subclass would be about the same size as my example. At that point, maybe it's better served as an example and not a utility?

marandaneto commented 3 years ago

@caseyduquettesc thanks for your reply.

Should it create breadcrumbs and should the breadcrumbs be customizable (both their creation and content)?

we can create the breadcrumbs with the values we have as we do for OkHttp, the final user always can use the BeforeBreadcrumbCallback if necessary to customize them.

What should the span descriptions be? Streaming grpc spans would be highly dependent on the context unless they simply said "message" or something generic.

makes sense, similar to the case above, in case we cannot figure out the description, we can add a generic one, we can add a beforeSpanCallback so the user can customize the span after its creation too, See https://github.com/getsentry/sentry-java/blob/main/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt#L18

Should which events get spans be customizable? In our case, we chose not to create spans for every message because it became a little spammy looking at a trace.

similar to the case above, the beforeSpanCallback can be used to drop the span.

The idea is to offer best-effort support, but the user is always able to customize them.

happy to guide you in case you wanna submit a PR after clarifying the cases above.

caseyduquettesc commented 3 years ago

Sounds good. I think this is doable through callbacks

adinauer commented 1 year ago

In case you're still waiting for this feature, you could give our OpenTelemetry Integration a try.

adinauer commented 3 months ago

Hey everyone, we've just released 8.0.0-alpha.2 of the Java SDK, which has an improved version of our OpenTelemetry integration that supports gRPC amongst many other libraries and frameworks. There's instructions how to upgrade in the changelog. If you decide to give it a try, any feedback is welcome 🙏 .