Skip to content

chore: improve gRPC spans#1206

Merged
bobbinth merged 2 commits intonextfrom
santiagopittella-improve-grpc-otel-spans
Sep 4, 2025
Merged

chore: improve gRPC spans#1206
bobbinth merged 2 commits intonextfrom
santiagopittella-improve-grpc-otel-spans

Conversation

@SantiagoPittella
Copy link
Collaborator

closes #1198

@SantiagoPittella SantiagoPittella added the no changelog This PR does not require an entry in the `CHANGELOG.md` file label Sep 3, 2025

// Extract the method name from the HTTP path (e.g., "/rpc.rpc/CheckNullifiers" ->
// "CheckNullifiers")
let method_name = request.uri().path().rsplit('/').next().unwrap_or("Unknown");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that we are not doing an exhaustive match on the method, maybe we can just unwrap_or_default() and keep it empty?


let span = add_otel_span_attributes(span, request);
add_network_attributes(span, request)
dynamic_trace_fn(request, TracedComponent::RemoteProverProxy)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we just call the dynamic fn in the services and get them to pass in the enum?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can simplify even further because the path also contains the service name, https://github.com/0xMiden/miden-node/pull/1206/files#r2320988255


let span = add_otel_span_attributes(span, request);
add_network_attributes(span, request)
dynamic_trace_fn(request, TracedComponent::RemoteProverProxy)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can simplify even further because the path also contains the service name, https://github.com/0xMiden/miden-node/pull/1206/files#r2320988255

Copy link
Collaborator

@Mirko-von-Leipzig Mirko-von-Leipzig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should simplify further as per #1206 (comment)

Copy link
Collaborator

@Mirko-von-Leipzig Mirko-von-Leipzig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🥳

Copy link
Contributor

@bobbinth bobbinth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a very in-depth review from me - but looks good! Thank you!

@bobbinth bobbinth merged commit 1ae1965 into next Sep 4, 2025
6 checks passed
@bobbinth bobbinth deleted the santiagopittella-improve-grpc-otel-spans branch September 4, 2025 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no changelog This PR does not require an entry in the `CHANGELOG.md` file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve gRPC spans for OTel integration

4 participants