fix(aleo): normalize method labels in metrics to prevent high cardinality#7989
fix(aleo): normalize method labels in metrics to prevent high cardinality#7989nambrot-agent wants to merge 1 commit intomainfrom
Conversation
…lity The Aleo adapter was using full API paths (including dynamic values like block numbers) as the method label in Prometheus metrics. This created thousands of unique metric series (e.g., method="block/15544045"). This fix adds a normalize_method() function that maps dynamic paths to static method names, reducing unique method values from ~3,359 to ~14.
📝 WalkthroughWalkthroughThis change introduces a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@rust/main/chains/hyperlane-aleo/src/provider/metric.rs`:
- Around line 56-61: The metric path matcher is too permissive: the branch using
path.starts_with("statePaths") can misclassify endpoints like "statePathsFoo";
update the condition in the same matcher where path.starts_with("statePath/")
returns "get_state_path" and the branch that returns "get_state_paths" so it
only matches exactly "statePaths" or "statePaths/" (or otherwise checks for a
trailing slash) instead of a bare starts_with; modify the
path.starts_with("statePaths") check to require equality or a slash-aware check
so only the intended "get_state_paths" endpoint is labeled.
| if path.starts_with("statePath/") { | ||
| return "get_state_path"; | ||
| } | ||
| if path.starts_with("statePaths") { | ||
| return "get_state_paths"; | ||
| } |
There was a problem hiding this comment.
Tighten the statePaths match to avoid accidental collisions.
starts_with("statePaths") will also match any future endpoints like statePathsFoo, which would mislabel metrics. Safer to require an exact match or a slash.
Suggested tweak
- if path.starts_with("statePaths") {
+ if path == "statePaths" || path.starts_with("statePaths/") {
return "get_state_paths";
}📝 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.
| if path.starts_with("statePath/") { | |
| return "get_state_path"; | |
| } | |
| if path.starts_with("statePaths") { | |
| return "get_state_paths"; | |
| } | |
| if path.starts_with("statePath/") { | |
| return "get_state_path"; | |
| } | |
| if path == "statePaths" || path.starts_with("statePaths/") { | |
| return "get_state_paths"; | |
| } |
🤖 Prompt for AI Agents
In `@rust/main/chains/hyperlane-aleo/src/provider/metric.rs` around lines 56 - 61,
The metric path matcher is too permissive: the branch using
path.starts_with("statePaths") can misclassify endpoints like "statePathsFoo";
update the condition in the same matcher where path.starts_with("statePath/")
returns "get_state_path" and the branch that returns "get_state_paths" so it
only matches exactly "statePaths" or "statePaths/" (or otherwise checks for a
trailing slash) instead of a bare starts_with; modify the
path.starts_with("statePaths") check to require equality or a slash-aware check
so only the intended "get_state_paths" endpoint is labeled.
🦀 Rust Agent Docker Image Built Successfully
Full image paths |
|
@claude review |
|
Claude finished @paulbalaji's task in 1m 4s —— View job Code Review: Aleo Metrics Method Label Normalization
Overall: Good fix that correctly addresses the high-cardinality metrics issue. The approach is sound and matches the problem statement. Issues to Address1. The current check // Current (problematic)
if path.starts_with("statePaths") {
// Better - handles the query string case
if path == "statePaths" || path.starts_with("statePaths?") {Suggestions (Non-blocking)2. Consider adding unit tests for The function has multiple branches with order-dependent logic. Unit tests would:
Example test cases: #[test]
fn test_normalize_method() {
assert_eq!(normalize_method("block/15544045"), "get_block");
assert_eq!(normalize_method("block/latest"), "block/latest");
assert_eq!(normalize_method("block/123/transactions"), "get_block_transactions");
assert_eq!(normalize_method("statePaths?commitments=abc"), "get_state_paths");
assert_eq!(normalize_method("unknown/endpoint"), "unknown");
}3. Minor inconsistency in naming convention Static paths keep their original names ( What Looks Good
|
Review SummaryAlready Caught (ACK)
Additional Findings
TL;DRGood PR, fix rustfmt, use |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7989 +/- ##
=======================================
Coverage 77.02% 77.02%
=======================================
Files 117 117
Lines 2651 2651
Branches 244 244
=======================================
Hits 2042 2042
Misses 593 593
Partials 16 16
🚀 New features to boost your workflow:
|
3 participants
Summary
Fixes a bug where the Aleo chain adapter was putting dynamic values (like block numbers) in the Prometheus
methodlabel, creating a new metric series for every unique API call.Before:
→ ~3,359 unique method values for Aleo (vs ~14 expected)
After:
→ ~14 unique method values (matching other chains like Ethereum)
Changes
normalize_method()function inrust/main/chains/hyperlane-aleo/src/provider/metric.rsthat maps dynamic API paths to static method namestrack_request()to use the normalized method name instead of the raw pathImpact
This fix eliminates ~6,700 unnecessary metric series caused by high-cardinality labels from the Aleo adapter.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.