feat(op-service): complete separation of op-node/op-service metrics #10545
Conversation
WalkthroughWalkthroughThe recent changes focus on consolidating and refactoring the metrics handling within the Changes
Recent Review DetailsConfiguration used: .coderabbit.yml Files selected for processing (7)
Files skipped from review due to trivial changes (1)
Additional comments not posted (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @hamdiallam and the rest of your teammates on |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Out of diff range and nitpick comments (30)
op-service/testutils/mock_rpc.go (6)
20-22: Ensure that theClosemethod is properly tested to verify that it correctly calls the mock'sCalledmethod.
28-31: Consider adding more detailed comments to explain the purpose of theCallContextmethod and how the mock'sCalledmethod is used to simulate the behavior.
37-40: Ensure that theBatchCallContextmethod is properly tested to verify that it correctly handles batch calls and returns the appropriate error.
62-65: Ensure that theEthSubscribemethod is properly tested to verify that it correctly handles subscriptions and returns the appropriate subscription and error.
33-35: Consider adding more detailed comments to explain the purpose of theExpectCallContextmethod and how the mock'sOnmethod is used to set expectations for theCallContextmethod.
42-60: Ensure that theExpectBatchCallContextmethod is properly tested to verify that it correctly sets expectations for batch calls and handles the mutation of arguments.op-service/metrics/rpc_metrics.go (8)
33-39: Ensure that theRPCClientMetricsstruct is properly tested to verify that it correctly tracks client RPC metrics using Prometheus counters and histograms.
41-44: Ensure that theRPCServerMetricsstruct is properly tested to verify that it correctly tracks server RPC metrics using Prometheus counters and histograms.
46-50: Ensure that theRPCMetricsstruct is properly tested to verify that it correctly combines both client and server metrics.
Line range hint
161-171: Ensure that theNoopRPCMetricsstruct is properly tested to verify that it correctly implements theRPCMetricerinterface with no-op methods.
52-58: Ensure that theMakeRPCMetricsfunction is properly tested to verify that it correctly creates and initializes anRPCMetricsinstance with the given namespace and factory.
Line range hint
60-93: Ensure that theMakeRPCClientMetricsfunction is properly tested to verify that it correctly creates and initializes anRPCClientMetricsinstance with the given namespace and factory.
128-149: Ensure that theMakeRPCServerMetricsfunction is properly tested to verify that it correctly creates and initializes anRPCServerMetricsinstance with the given namespace and factory.
Line range hint
110-127: Consider adding more detailed comments to explain the purpose of theRecordRPCClientResponsefunction and how it converts errors into metrics-friendly strings.op-service/client/rpc.go (7)
Line range hint
173-187: Ensure that theBaseRPCClientstruct is properly tested to verify that it correctly wraps anrpc.Clientinstance and sets appropriate timeouts for RPC calls.
188-191: Ensure that theInstrumentedRPCClientstruct is properly tested to verify that it correctly tracks Prometheus metrics for each RPC call.
Line range hint
25-28: Consider adding more detailed comments to explain the purpose of therpcConfigstruct and how its fields are used to configure RPC clients.
Line range hint
75-91: Ensure that theNewRPCfunction is properly tested to verify that it correctly creates and configures an RPC client with the given options.
Line range hint
94-99: Ensure that theNewRPCWithClientfunction is properly tested to verify that it correctly wraps an existing RPC client with additional functionality, such as polling.
Line range hint
102-122: Ensure that thedialRPCClientWithBackofffunction is properly tested to verify that it correctly handles dialing an RPC endpoint with exponential backoff.
Line range hint
224-237: Consider adding more detailed comments to explain the purpose of theinstrumentBatchfunction and how it tracks metrics for batch RPC calls.op-service/client/client.go (3)
57-69: Ensure that theInstrumentedClientstruct is properly tested to verify that it correctly tracks Prometheus metrics for each Ethereum client call.
Line range hint
17-53: Consider adding more detailed comments to explain the purpose of theClientinterface and how its methods are used for various blockchain-related operations.
65-69: Ensure that theNewInstrumentedClientfunction is properly tested to verify that it correctly creates and initializes anInstrumentedClientinstance with the given RPC client and metrics.op-service/testutils/mock_client.go (6)
22-24: Ensure that theClosemethod is properly tested to verify that it correctly calls the mock'sCalledmethod.
30-33: Consider adding more detailed comments to explain the purpose of theRPCmethod and how the mock'sCalledmethod is used to simulate the behavior.
39-42: Ensure that theChainIDmethod is properly tested to verify that it correctly returns the expected chain ID and error.
48-51: Ensure that theBlockByHashmethod is properly tested to verify that it correctly returns the expected block and error.
102-105: Ensure that theTransactionByHashmethod is properly tested to verify that it correctly returns the expected transaction, pending status, and error.
273-276: Ensure that theCallContractmethod is properly tested to verify that it correctly returns the expected result and error.
hamdiallam
changed the title
client.RPC&client.Clientare unusable outside of op-node as the instrumented versions contain dependencies on op-node metrics.Client#RPC()method allowing a caller to get a instrumented RPC instance from the instrumented eth client.