fix(http-invocation): remove account ID from request latency metric - #771
Conversation
Signed-off-by: Bora Oztekin <boztekin@nvidia.com>
📝 WalkthroughWalkthroughThe invocation latency metric no longer includes the ChangesInvocation latency metrics
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
🛡️ CodeQL Analysis🚨 Found 2 issue(s) Severity Breakdown:
📋 Top Issues🔗 View full details in Security tab 🕐 Last updated: 2026-08-11 18:15:52 UTC | Commit: 7dff4a8 |
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
`@src/invocation-plane-services/http-invocation/crates/server/src/metrics/mod.rs`:
- Around line 379-386: Update the assertions for the metric key in the relevant
test to verify the complete label set: assert that exactly two labels exist and
that they are function_id and function_version_id with the expected values.
Remove the separate absence-only check for nca_id, while preserving the existing
expected label values.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 3425c7b6-64a5-447e-aa82-e842cca4bb37
📒 Files selected for processing (2)
src/invocation-plane-services/http-invocation/crates/server/src/metrics/mod.rssrc/invocation-plane-services/http-invocation/crates/server/src/routes/post_pexec.rs
💤 Files with no reviewable changes (1)
- src/invocation-plane-services/http-invocation/crates/server/src/routes/post_pexec.rs
|
🎉 This PR is included in version nvcf-invocation-service-v0.10.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
TL;DR
Remove the
nca_idlabel fromfunction_request_latencywhile retainingfunction_idandfunction_version_id.This reduces histogram series cardinality without changing request handling.
Additional Details
This change:
nca_idfrom therecord_invocation_endmetric API and histogram labelsnca_idis omittedRollout order: deploy the autoscaler query change in #769 before deploying this change. The updated autoscaler query supports both the current and new metric label schemas.
For the Reviewer
Please focus on the emitted
function_request_latencylabel set and the rollout relationship with #769.For QA
Automated validation completed:
cargo test --package nvcf-invocation-service --libChecklist
Summary by CodeRabbit