chore: adding eth client tracing

[chatton]

Overview

[github-actions]

The latest Buf updates on your PR. Results from workflow CI / buf-check (pull_request).

Build	Format	Lint	Breaking	Updated (UTC)
✅ passed	⏩ skipped	✅ passed	✅ passed	Jan 8, 2026, 3:54 PM

[claude]

Claude finished @chatton's task in 2m 10s —— View job

---

Code Review Complete

I've completed a comprehensive review of this PR which adds OpenTelemetry tracing for Ethereum RPC client interactions. Overall, the implementation is clean, follows the existing decorator pattern established in the codebase, and includes thorough test coverage. Below is my detailed feedback.

---

Summary

This PR successfully introduces observability for Ethereum JSON-RPC calls by:

• Creating an EthRPCClient interface to abstract ethclient calls
• Implementing a tracing decorator using OpenTelemetry
• Adding comprehensive unit tests with span verification
• Integrating with the existing tracing infrastructure

The changes are well-structured and consistent with the existing EngineRPCClient tracing pattern.

---

Code Quality & Architecture

Strengths:

• Clean abstraction: The EthRPCClient interface (execution.go:145-152) properly abstracts the two Ethereum RPC methods needed, enabling testability and decoration
• Consistent pattern: Follows the same decorator pattern as EngineRPCClient, maintaining architectural consistency across the codebase
• Proper separation: Three-file structure (client implementation, tracing decorator, tests) provides clear separation of concerns
• Interface compliance: Mock implementation in tests properly implements the interface

Observations:

• The EthRPCClient interface only exposes two methods (HeaderByNumber and GetTxs). This is appropriate for current needs, but if more ethclient methods are needed in the future, they should be added to this interface rather than bypassing it
• The tracer name "ev-node/execution/eth-rpc" (eth_rpc_tracing.go:24) is consistent with the engine client's "ev-node/execution/engine-rpc" naming convention

---

Error Handling & Resilience

Strengths:

• Proper error propagation: Both traced methods correctly propagate errors after recording them in spans (eth_rpc_tracing.go:46-48, 72-74)
• Correct OpenTelemetry usage: Uses both span.RecordError(err) and span.SetStatus(codes.Error, err.Error()) as recommended
• No error swallowing: The decorator doesn't introduce any error handling that could mask issues

Observations:

• Error handling is straightforward and appropriate for a tracing decorator
• The implementation correctly doesn't add retry logic or resilience patterns at this layer (those belong in the calling code)

---

Security

No security concerns identified. The tracing code:

• Doesn't introduce authentication/authorization logic
• Doesn't handle secrets or credentials
• Properly sanitizes logged data (block hashes, state roots, gas metrics are safe to trace)
• Doesn't expose sensitive transaction content beyond counts

---

Performance & Resource Efficiency

Strengths:

• Minimal overhead: Tracing decorator adds negligible overhead (span creation/attributes)
• Conditional application: Tracing is only applied when tracingEnabled=true (execution.go:240-242), avoiding overhead when not needed
• No blocking operations: All span operations are non-blocking

Observations:

• The timestamp attribute (eth_rpc_tracing.go:56) converts uint64 to int64. While this works, consider that block timestamps far in the future could theoretically overflow. This is an edge case and likely not a practical concern.

---

Testing & Reliability

Strengths:

• Comprehensive test coverage: Tests cover success cases, error cases, and edge cases (empty pool, nil block number for "latest")
• Proper test isolation: Each test uses a fresh tracer provider with cleanup (eth_rpc_tracing_test.go:27-29)
• Span verification: Tests verify span names, status codes, attributes, and error events
• Helper functions: Good use of test helpers (setupTestEthRPCTracing, requireAttribute)

Issues to Address:

1. Missing attribute check in error test (eth_rpc_tracing_test.go:170-178)

   The test verifies that success-only attributes are NOT set on error, but it misses the timestamp attribute. Gemini's suggestion is valid - the verification loop should be more explicit:

   // Current code only checks 4 of 5 success attributes
   for _, attr := range attrs {
       key := string(attr.Key)
       require.NotEqual(t, "block_hash", key)
       require.NotEqual(t, "state_root", key)
       require.NotEqual(t, "gas_limit", key)
       require.NotEqual(t, "gas_used", key)
       // Missing: "timestamp" check
   }

   Recommendation: Use a more explicit approach as suggested by Gemini Code Assist:

   for _, attr := range attrs {
       switch string(attr.Key) {
       case "block_hash", "state_root", "gas_limit", "gas_used", "timestamp":
           t.Fatalf("unexpected attribute %q found on error span", attr.Key)
       }
   }

   Fix this →

2. Verbose attribute check in error test (eth_rpc_tracing_test.go:273-277)

   The loop checking for forbidden tx_count attribute can be simplified as Gemini suggested:

   for _, attr := range attrs {
       if string(attr.Key) == "tx_count" {
           t.Fatalf("unexpected attribute 'tx_count' found on error span")
       }
   }

   This is more readable and clearer in intent.
   Fix this →

---

Documentation & Developer Experience

Strengths:

• Clear interface documentation: EthRPCClient interface methods have doc comments explaining their purpose (execution.go:147-151)
• Function comments: Decorator function has explanatory comment (eth_rpc_tracing.go:20-21)
• Consistent naming: Span names (Eth.GetBlockByNumber, TxPool.GetTxs) follow a clear convention

Areas for Improvement:

1. Missing package documentation: The new files (eth_rpc_client.go, eth_rpc_tracing.go) don't have package-level comments. While not critical, adding brief explanations would help developers understand the purpose of these files at a glance.

2. Test helper documentation: The mockEthRPCClient struct (eth_rpc_tracing_test.go:40-44) could benefit from a comment explaining its purpose, especially since it uses function fields.

---

Additional Observations

Integration Quality:

• The integration into NewEngineExecutionClient (execution.go:236-242) is clean and follows the same pattern as the engine client wrapping
• The conditional tracing based on tracingEnabled flag is implemented correctly
• Proper handling of both clients (engine and eth) with the same tracing configuration

go.mod changes:

• The SDK version update (go.mod:23) go.opentelemetry.io/otel/sdk v1.38.0 appears to be a necessary dependency update for the tracing functionality

---

Recommendations Summary

Must Fix:

1. Add timestamp attribute check in error test (eth_rpc_tracing_test.go:170-178)

Should Fix:
2. Simplify tx_count attribute check for readability (eth_rpc_tracing_test.go:273-277)

Consider:
3. Add package-level documentation to new files
4. Add documentation to test helper structs

---

Conclusion

This is a well-implemented PR that successfully adds Ethereum RPC tracing following established patterns in the codebase. The main issues are minor test improvements suggested by both myself and Gemini Code Assist. Once the test assertion gap is addressed, this will be ready to merge.

Approval Status: Recommend approval after addressing the missing timestamp attribute check in tests.

---

[gemini-code-assist]

Summary of Changes

Hello @chatton, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the observability of Ethereum RPC interactions within the EVM execution engine by integrating OpenTelemetry tracing. It refactors the existing EngineClient to utilize a new EthRPCClient interface, allowing for a clean separation of concerns and the introduction of a tracing decorator. This change ensures that critical Ethereum API calls, such as fetching block headers and transaction pool content, are automatically instrumented with detailed tracing information, providing better insights into performance and potential issues.

Highlights

• New EthRPCClient Interface: Introduced an EthRPCClient interface in execution.go to abstract Ethereum JSON-RPC calls, enabling easier mocking and decoration (e.g., for tracing).
• Concrete EthRPCClient Implementation: Added eth_rpc_client.go which provides a concrete implementation of the EthRPCClient interface, wrapping the standard go-ethereum/ethclient.
• OpenTelemetry Tracing for EthRPCClient: Implemented eth_rpc_tracing.go which contains tracedEthRPCClient, a decorator that wraps an EthRPCClient and adds OpenTelemetry spans for HeaderByNumber and GetTxs methods, including relevant attributes and error handling.
• Integration into EngineClient: Modified EngineClient in execution.go to use the new EthRPCClient interface. The NewEngineExecutionClient constructor now conditionally wraps the ethclient with the tracing decorator if tracing is enabled.
• Unit Tests for Tracing: Added eth_rpc_tracing_test.go with comprehensive unit tests for the tracedEthRPCClient, verifying span creation, attribute setting, and error reporting for both successful and failed RPC calls.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request introduces OpenTelemetry tracing for Ethereum RPC client interactions. The changes are well-structured, using a decorator pattern to add tracing capabilities without modifying the core client logic. A new EthRPCClient interface is introduced, which improves modularity and testability. The accompanying tests are thorough and cover success and error cases for the new tracing functionality. My feedback includes a couple of suggestions to improve the clarity and completeness of the test assertions.

[gemini-code-assist]

This loop to verify that block header attributes are not set on error is confusing and incomplete. While it correctly fails if an unexpected attribute is present, its logic is hard to follow. Additionally, it's missing a check for the timestamp attribute, which is set on success.

A clearer and more complete approach is to iterate through the attributes and explicitly fail if any of the success-only attributes are found.

for _, attr := range attrs {
		switch string(attr.Key) {
		case "block_hash", "state_root", "gas_limit", "gas_used", "timestamp":
			t.Fatalf("unexpected attribute %q found on error span", attr.Key)
		}
	}

[gemini-code-assist]

This loop to verify that the tx_count attribute is not set on error works, but it can be written more clearly to express the intent of checking for a specific forbidden attribute.

for _, attr := range attrs {
		if string(attr.Key) == "tx_count" {
			t.Fatalf("unexpected attribute 'tx_count' found on error span")
		}
	}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.67%. Comparing base (931d2ac) to head (8d7fc84).

Additional details and impacted files

@@                   Coverage Diff                    @@
##           cian/add-tracing-part-3    #2960   +/-   ##
========================================================
  Coverage                    58.67%   58.67%           
========================================================
  Files                           93       93           
  Lines                         8863     8863           
========================================================
  Hits                          5200     5200           
  Misses                        3074     3074           
  Partials                       589      589           

Flag	Coverage Δ
combined	58.67% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.