-
Notifications
You must be signed in to change notification settings - Fork 21.7k
rpc: add generic OpenTelemetry tracing for JSON-RPC calls #33452
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
base: master
Are you sure you want to change the base?
Conversation
f99f204 to
cb45359
Compare
7c245f1 to
c35e582
Compare
|
@fjl I am getting this error when the lint step is run: https://github.com/ethereum/go-ethereum/actions/runs/20384597266/job/58582793251?pr=33452 looks like the otel dependencies I added to geth changed some transitive dependencies and now they don't match what is in cmd/keeper. I can get it to pass by running |
Scfastermind
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Terminado breackingfast hoy
bc7b9b3 to
9cb042d
Compare
04615b8 to
fa8860d
Compare
|
|
||
| // Start tracing span before invoking the subscription method. | ||
| ctx, span := h.startSpan(cp.ctx, msg, cp.isBatch) | ||
| defer span.End() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fjl do we want to trace subscriptions?
Summary
This PR adds generic OpenTelemetry tracing to the JSON-RPC server in
package rpc.Tracing is disabled by default by relying on the global OpenTelemetry tracer provider, which is a noop unless configured elsewhere. Server.SetTracerProvider is supported for tests, but we might want to ditch this and just rely on helpers accessing the global provider.
Follow-ups