Skip to content

Add otel trace instrumentation on gRPC calls #139

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

Closed
Fricounet opened this issue Jul 13, 2023 · 4 comments · Fixed by #140
Closed

Add otel trace instrumentation on gRPC calls #139

Fricounet opened this issue Jul 13, 2023 · 4 comments · Fixed by #140

Comments

@Fricounet
Copy link
Contributor

Fricounet commented Jul 13, 2023

I'm proposing to add some basic tracing instrumentation of the gRPC calls made by the CSI clients using the code in connection.go to connect to the CSI driver.
The feature simply consists in adding the otelgrpc.UnaryClientInterceptor() to the existing ChainUnaryInterceptor. This is enough to create traces for all gRPC calls and they can be easily exported by the client.

I think it can be done by creating a new ConnectWithGrpcInterceptor function that will call the connect function with the added interceptor as a DialOption. This way the feature will be opt-in for the users.

I can contribute it (and the implementation in the CSI components) if it is something the community wants to see. I already implemented something similar on the azuredisk-csi-driver and plan to do the same for aws and gcp drivers.

@jsafrane
Copy link
Contributor

/cc

@Fricounet
Copy link
Contributor Author

@jsafrane here is an example of the information gathered on a trace by the gRPC interceptor.
I collapsed the os and process fields because they are just some metadata that are not linked to the gRPC interceptor itself. Here, the relevant field for the interceptor are event which is the gRPC message itself and rpc which indicates which method was called.
I never noticed any confidential information leaking this way but if you have an example of a CSI components where you think it might happen, I'd be glad to test it on my side to check if there is any leakage.

image

@jsafrane
Copy link
Contributor

Thanks for the example, it looks good - it has just the method name and not secrets that are in method arguments.

@mauriciopoppe
Copy link
Member

/cc

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

Successfully merging a pull request may close this issue.

3 participants