perf(metrics): cache usage series handles - #857
Conversation
📝 WalkthroughWalkthroughLLM usage metrics now cache usage label series with bounded, collision-safe lookup and lazily initialized counters. Recording handles non-zero token fields and optional micro-USD spend, with tests covering reuse, bounds, collisions, and concurrency. ChangesLLM usage metrics
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Metrics as Metrics::record_llm_usage
participant Cache as UsageSeriesCache
participant Counters as UsageSeriesHandles
Metrics->>Cache: Lookup or insert usage label series
Cache-->>Metrics: Return cached handles
Metrics->>Counters: Initialize required counters
Metrics->>Counters: Increment non-zero usage dimensions
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/aisix-obs/src/metrics.rs`:
- Around line 1574-1579: Update spend_micro_usd to reject non-finite scaled
values and any rounded micro-USD value outside the representable u64 range
before casting, returning None instead. Add boundary tests covering scaled
infinity and values at or beyond the u64 maximum while preserving valid positive
conversions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 779fe5ef-45c4-45be-850e-b16ae1fad423
📒 Files selected for processing (1)
crates/aisix-obs/src/metrics.rs
| fn spend_micro_usd(self) -> Option<u64> { | ||
| if !self.spend_usd.is_finite() || self.spend_usd <= 0.0 { | ||
| return None; | ||
| } | ||
| let micro_usd = (self.spend_usd * 1_000_000.0).round(); | ||
| (micro_usd > 0.0).then_some(micro_usd as u64) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
According to the official Rust Reference, what is the result of casting an out-of-range or infinite f64tou64withas?
💡 Result:
According to the Rust Reference and established language behavior, casting an infinite f64 to u64 using the as keyword results in the maximum value of the target integer type, which is u64::MAX [1][2]. In Rust, float-to-integer casts using as are defined to be saturating [1]. The conversion process follows these specific rules: 1. The floating-point value is rounded toward zero [1]. 2. If the value is NaN, it is converted to 0 [1]. 3. If the value is positive infinity or exceeds the maximum representable value of the target integer type, it is saturated to the maximum value of that type (u64::MAX) [1][2]. 4. If the value is negative infinity or is less than the minimum representable value of the target integer type (which is 0 for u64), it is saturated to the minimum value of that type (0) [1].
Citations:
- 1: Define UB in float-to-int casts to saturate rust-lang/rust#71269
- 2: https://users.rust-lang.org/t/convert-an-f64-to-an-i32/50991
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## locate file and inspect relevant symbols"
wc -l crates/aisix-obs/src/metrics.rs
rg -n "struct LlmUsage|impl LlmUsage|fn spend_micro_usd|spend_usd|spend_micro_usd" crates/aisix-obs/src/metrics.rs
echo
echo "## inspect spend conversion and nearby usage"
sed -n '1520,1595p' crates/aisix-obs/src/metrics.rs
sed -n '1595,1680p' crates/aisix-obs/src/metrics.rs
echo
echo "## tests around spend_micro_usd if present"
rg -n "spend_micro_usd|use_.*|LlmUsage" crates/aisix-obs/src/metrics.rs | tail -n 80
echo
echo "## read-only Rust behavior probe for f64-to-u64 casts"
cat >/tmp/f64_u64_sat.rs <<'RS'
fn main() {
for (name, value) in [
("inf_infinity", f64::INFINITY),
("nan", f64::NAN),
("round_to_9007199254740993", (u64::MAX as f64).round()),
("round_to_9007199254740992", (u64::MAX as f64 - 1.0_f64).round()),
("round_to_9007199254740991", (u64::MAX as f64 - 2.0_f64).round()),
("exact_9007199254740991", (u64::MAX as f64 - 2.0_f64).round()),
] {
println!("{name}: input={value} saturates_to_as_u64={:?}", value as u64);
}
let vals = [u64::MAX as f64, (u64::MAX as f64) - 1.0, (u64::MAX as f64) - 2.0];
for v in vals {
println!("round({:.1?}) = {:.1?}; as u64 = {:?}", v, v.round(), v.round() as u64);
}
}
RS
rustc /tmp/f64_u64_sat.rs -o /tmp/f64_u64_sat 2>/tmp/scratch_rustc.out || { cat /tmp/scratch_rustc.out; exit 0; }
/tmp/f64_u64_satRepository: api7/aisix
Length of output: 6410
🌐 Web query:
Rust Reference unsafe_unchecked float to primitive integer conversion f64 u64 saturating
💡 Result:
In Rust, there is no direct equivalent of a "saturating unchecked" float-to-integer conversion method in the standard library. The existing tool for unchecked conversion, f64::to_int_unchecked, strictly requires that the input value is within the range of the target integer type [1][2]. The standard approach for unchecked conversion is: f64::to_int_unchecked This method rounds the float toward zero and performs a raw conversion to the target integer type [1]. Safety Contract: To use this method safely, the caller must guarantee that: 1. The value is not NaN [1]. 2. The value is not infinite [1]. 3. The value is representable in the target integer type after truncating its fractional part [1]. If these conditions are not met, the operation results in undefined behavior [3]. Regarding saturating behavior: The standard 'as' cast in Rust for floating-point to integer conversion is defined to be saturating [4][3]. If you require saturating semantics for performance or logic, you should use the standard 'as' cast (e.g., value as u64), which safely handles out-of-range values and NaN by clamping to the integer bounds (or zero for NaN) [5][4]. If you specifically require the performance of an unchecked cast but also need saturation, you must implement the range checks manually before calling to_int_unchecked, or accept the safety guarantees of the standard 'as' cast [6][3].
Citations:
- 1: https://doc.rust-lang.org/stable/std/primitive.f64.html
- 2: https://doc.rust-lang.org/stable/src/core/convert/num.rs.html
- 3: Tracking issue for
{f32,f64}::to_int_uncheckedmethods rust-lang/rust#67058 - 4: Define UB in float-to-int casts to saturate rust-lang/rust#71269
- 5: https://doc.rust-lang.org/nightly/unstable-book/compiler-flags/saturating-float-casts.html
- 6: https://stackoverflow.com/questions/71430186/how-to-safely-convert-float-to-int-in-rust
Reject overflowed micro-USD values before casting.
Scaling a valid spend_usd can produce an infinite value; u64 casts saturate, so this can increment spend by u64::MAX instead of returning None. Reject non-finite scaled values and values that would overflow u64; add boundary tests.
Proposed fix
let micro_usd = (self.spend_usd * 1_000_000.0).round();
- (micro_usd > 0.0).then_some(micro_usd as u64)
+ if !micro_usd.is_finite() || micro_usd <= 0.0 || micro_usd >= u64::MAX as f64 {
+ return None;
+ }
+ Some(micro_usd as u64)📝 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.
| fn spend_micro_usd(self) -> Option<u64> { | |
| if !self.spend_usd.is_finite() || self.spend_usd <= 0.0 { | |
| return None; | |
| } | |
| let micro_usd = (self.spend_usd * 1_000_000.0).round(); | |
| (micro_usd > 0.0).then_some(micro_usd as u64) | |
| fn spend_micro_usd(self) -> Option<u64> { | |
| if !self.spend_usd.is_finite() || self.spend_usd <= 0.0 { | |
| return None; | |
| } | |
| let micro_usd = (self.spend_usd * 1_000_000.0).round(); | |
| if !micro_usd.is_finite() || micro_usd <= 0.0 || micro_usd >= u64::MAX as f64 { | |
| return None; | |
| } | |
| Some(micro_usd as u64) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/aisix-obs/src/metrics.rs` around lines 1574 - 1579, Update
spend_micro_usd to reject non-finite scaled values and any rounded micro-USD
value outside the representable u64 range before casting, returning None
instead. Add boundary tests covering scaled infinity and values at or beyond the
u64 maximum while preserving valid positive conversions.
1 participant
Summary
aisix_llm_*usage countersRelated to api7/AISIX-Cloud#1199.
Behavior pinned
Verification
cargo test -p aisix-obs --lib: 156 passed, 5 real-cloud smoke tests ignored, 0 failedcargo clippy --workspace --all-targets -- -D warningscargo fmt --all -- --checkgit diff --checkControlled performance test
GetBusbar OTB binaries and native topology on one ARM64 VM: 8 vCPU, gateway
cores 0-3, load generator cores 4-5, mock upstream cores 6-7, OpenAI
chat-completions path, concurrency 64, 10 seconds per run, five interleaved
baseline/optimized pairs. Baseline is exact
e1da81d; both release binariesretain function symbols.
3715e6c952b3a25f99dfb6a7bf9ac7436a79cdf8e92d6bde7a8b3d45137b3a72511e34ecd3a6c2f78a703041b255a6b57a861e1a1948f6910eec3c3fab3ee8cc12b767335b396f365d4dba877cc47affcb3e330cResult: +1.76% median RPS, -1.44% median p99, zero failures in every
run. All five paired throughput comparisons improved.
Reproduction load command used by each rung:
Perf evidence
Both profiles used
perf record -F 99 -g --call-graph dwarf,16384for 24seconds under c64 load.
Metrics::record_llm_usagePrometheusRecorder::register_countermetrics::key::Key::eqThe optimized usage-cache lookup itself accounts for 0.12% inclusive CPU.
After normalizing for throughput,
record_llm_usageCPU cost per request fallsby about 92%.
Summary by CodeRabbit