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

Add otel trace instrumentation on gRPC calls #1299

Closed
Fricounet opened this issue Jul 19, 2023 · 3 comments · Fixed by #1403
Closed

Add otel trace instrumentation on gRPC calls #1299

Fricounet opened this issue Jul 19, 2023 · 3 comments · Fixed by #1403

Comments

@Fricounet
Copy link
Contributor

Fricounet commented Jul 19, 2023

Is your feature request related to a problem? Please describe.
Add some more observability to the csi driver. In particular, add gRPC tracing using opentelemetry.

Describe the solution you'd like in detail
We implemented it in our fork. So I can submit a similar PR here.
The feature is working as expected and we are able to get the gRPC traces.

Describe alternatives you've considered
None

Additional context
We did a similar instrumentation in aws-ebs-csi-driver and azuredisk-csi-driver

@mattcary
Copy link
Contributor

mattcary commented Sep 6, 2023

Hi, sorry for the delay in replying to this issue.

I'm fine with this in principle although I'm not familiar with open telementry. Note that we would probably not enable this for the managed GKE driver, so it would be available to self-deployed drivers only. Does that change your opinion about it?

@mattcary
Copy link
Contributor

mattcary commented Sep 6, 2023

I asked around a bit, and learned that we're in process of adding it to the csi sidecars (it's complicated because of csi-lib-utils dependencies).

Go ahead and make a PR against this repo like you did in your fork and I'll take a look at it. It looks like there will be a lot of dependency changes so we'll bump the minor version to release it.

Thanks for your contribution!

@Fricounet
Copy link
Contributor Author

Hi Matt, thanks for your reply. I do not expect this to be enabled by default on managed installation so that is fine for me.
I'll submit a PR soon then 🙇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants