Skip to content
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

Support of gRPC for performance monitoring #1512

Closed
ahmadshabib opened this issue Jun 6, 2021 · 12 comments
Closed

Support of gRPC for performance monitoring #1512

ahmadshabib opened this issue Jun 6, 2021 · 12 comments
Labels
feature request OpenTelemetryDoesIt Redirect to OpenTelemetry as it already provides the required functionality performance Performance API issues Platform: Java

Comments

@ahmadshabib
Copy link

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 marandaneto added feature request performance Performance API issues labels Jun 7, 2021
@marandaneto
Copy link
Contributor

@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
Copy link
Author

@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
Copy link
Contributor

@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
Copy link
Contributor

it's likely possible with io.grpc.ClientInterceptors

@caseyduquettesc
Copy link

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

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

caseyduquettesc commented Jul 27, 2021

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

  • Should it create breadcrumbs and should the breadcrumbs be customizable (both their creation and content)?
  • What should the span descriptions be? Streaming grpc spans would be highly dependent on the context unless they simply said "message" or something generic.
  • 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.

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

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

Sounds good. I think this is doable through callbacks

@adinauer
Copy link
Member

adinauer commented Feb 7, 2023

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

@adinauer adinauer added the OpenTelemetryDoesIt Redirect to OpenTelemetry as it already provides the required functionality label Feb 7, 2023
@adinauer
Copy link
Member

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 🙏 .

@stephanie-anderson
Copy link

Closing this issue, as work is being tracked in our project issue for v8: #3752

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request OpenTelemetryDoesIt Redirect to OpenTelemetry as it already provides the required functionality performance Performance API issues Platform: Java
Projects
Archived in project
Development

No branches or pull requests

7 participants