feat: bridgecall metrics #600
Conversation
WalkthroughThe recent changes enhance telemetry capabilities across various components of the system by introducing structured metric tracking for bridge calls and outgoing transactions. New constants and functions improve consistency and observability in logging, fostering easier debugging. The modifications facilitate detailed monitoring by ensuring metrics are only recorded during relevant operations, thus promoting a more robust telemetry framework. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Keeper
participant Telemetry
User->>Keeper: Initiate Bridge Call
Keeper->>Telemetry: Check ctx.IsCheckTx()
alt Not CheckTx
Keeper->>Telemetry: Log Bridge Call Metrics
end
Telemetry-->>Keeper: Return Metric Status
Keeper-->>User: Complete Bridge Call
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 Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (6)
- telemetry/label.go (1 hunks)
- telemetry/metrics.go (1 hunks)
- x/crosschain/keeper/bridge_call_in.go (3 hunks)
- x/crosschain/keeper/bridge_call_out.go (2 hunks)
- x/crosschain/keeper/outgoing_pool.go (2 hunks)
- x/crosschain/types/metrics.go (1 hunks)
Files skipped from review due to trivial changes (1)
- telemetry/label.go
Additional comments not posted (15)
x/crosschain/types/metrics.go (2)
3-7: LGTM!The constants for metric keys are well-named and follow best practices.
9-13: LGTM!The constants for metric labels are well-named and follow best practices.
telemetry/metrics.go (2)
24-26: LGTM!The function is a straightforward wrapper around
SetGaugeLabelsWithTokenand appears correct.
28-31: LGTM!The function is a straightforward loop that calls
SetGaugeLabelsWithCoinfor each coin and appears correct.x/crosschain/keeper/bridge_call_in.go (5)
Line range hint
77-91: LGTM!The function correctly handles minting and unlocking coins. Ensure that the bank keeper and ERC20 keeper are correctly integrated.
Line range hint
117-139: LGTM!The function correctly handles token transfers and conversion. Ensure that the bank keeper and ERC20 keeper are correctly integrated.
Line range hint
141-160: LGTM!The function correctly converts coins to bridge call tokens. Ensure that the ERC20 keeper is correctly integrated.
39-52: LGTM! Verify telemetry integration.The telemetry integration appears correct. Ensure that the labels and metrics are correctly tracked.
Verification successful
The telemetry integration in the
BridgeCallHandlerfunction appears correct. The labels and metrics are properly tracked. Here are the relevant locations in the codebase:
x/crosschain/keeper/bridge_call_in.go: Implementation ofBridgeCallHandler.x/crosschain/keeper/attestation_handler.go: Invocation ofBridgeCallHandler.x/crosschain/keeper/bridge_call_in_test.go: Tests forBridgeCallHandler.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify telemetry integration for `BridgeCallHandler`. # Test: Search for the function usage. Expect: Correct telemetry labels and metrics. rg --type go -A 10 $'BridgeCallHandler'Length of output: 3418
94-100: LGTM! Verify telemetry integration.The telemetry integration appears correct. Ensure that the labels and metrics are correctly tracked.
Verification successful
Let's verify the telemetry integration by checking the definitions of
types.MetricsKeyBridgeCallInandtypes.MetricsLabelModule, and ensure they are correctly used in the code.
Telemetry integration verified.
The telemetry integration in the
BridgeCallTransferAndCallEvmfunction is correct. The labels and metrics are appropriately defined and used.
MetricsKeyBridgeCallInis defined as"bridge_call_in".MetricsLabelModuleis defined as"module".Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify telemetry integration for `BridgeCallTransferAndCallEvm`. # Test: Search for the function usage. Expect: Correct telemetry labels and metrics. rg --type go -A 10 $'BridgeCallTransferAndCallEvm'Length of output: 1913
Script:
#!/bin/bash # Description: Verify telemetry labels and metrics definitions. # Search for the definition of MetricsKeyBridgeCallIn rg --type go 'MetricsKeyBridgeCallIn' # Search for the definition of MetricsLabelModule rg --type go 'MetricsLabelModule'Length of output: 1016
x/crosschain/keeper/bridge_call_out.go (3)
64-64: Ensure context check is necessary.The context check
!ctx.IsCheckTx()ensures that telemetry tracking is only performed during actual transactions, not during checks. This is a good practice to avoid unnecessary telemetry during transaction validation phases.
65-73: Telemetry tracking for tokens.The code correctly sets gauge labels for each token involved in the outgoing bridge call. This helps in detailed monitoring and analytics.
74-83: Increment counter with labels.The code increments a counter with specific labels indicating the module, sender, refund address, recipient address, and token labels. This is a good practice for detailed telemetry.
x/crosschain/keeper/outgoing_pool.go (3)
56-56: Ensure context check is necessary.The context check
!ctx.IsCheckTx()ensures that telemetry tracking is only performed during actual transactions, not during checks. This is a good practice to avoid unnecessary telemetry during transaction validation phases.
57-61: Telemetry tracking for outgoing amount.The code correctly sets gauge labels for the outgoing amount. This helps in detailed monitoring and analytics.
63-72: Increment counter with labels.The code increments a counter with specific labels indicating the module, sender, receiver, and token denomination. This is a good practice for detailed telemetry.
Comments failed to post (1)
telemetry/metrics.go
15-22: Handle potential errors in float32 conversion.
The conversion to float32 should handle potential errors to avoid unexpected behavior.
- amountFloat32, _ := new(big.Float).SetInt(amount).Float32() + amountFloat32, accuracy := new(big.Float).SetInt(amount).Float32() + if accuracy != big.Exact { + return + }Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.func SetGaugeLabelsWithToken(keys []string, token string, amount *big.Int, labels ...metrics.Label) { if amount.Cmp(maxFloat32) == 1 { return } amountFloat32, accuracy := new(big.Float).SetInt(amount).Float32() if accuracy != big.Exact { return } telemetry.SetGaugeWithLabels(append(keys, token), amountFloat32, append(labels, telemetry.NewLabel(LabelToken, token))) }
Summary by CodeRabbit