You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
metrics: add requested vCPU and memory distributions - #1505
Add sandbox.api.requested.vcpu and sandbox.api.requested.memory_bytes distributions for effective requested sandbox sizes after template and warm-pool defaults. This adds the emitter and tag vocabulary only; no production recording call sites are added.
Both distributions carry kind, surface, admission, template, and the request-scoped client_id, with an optional defaulted tag for creates. Reuse AdmissionDecision for accepted/rejected labels, and convert memory from MiB to bytes using a u64 intermediate.
Validation
For the review fixes: git diff --cached --check and uv tool run pre-commit run passed, including the repository's gitleaks check. Verified the published file matches the locally reviewed patch and the telemetry test module and file are removed. Rust tests were not rerun for these documentation/test-removal changes; this sandbox has no Rust toolchain. Runtime emission and backend ingestion have not been validated.
Severity and risk
Severity does not apply: this is additive observability infrastructure, not a bug fix. Risk is low because no production call sites emit these metrics yet; review should focus on metric names, tag vocabulary, and unit conversion.
Rollback: revert this PR.
Stack
Requested-size metrics stack; merge from bottom to top:
This PR: metric definitions and emitter, based on main.
No actionable comments were generated in the recent review.
Walkthrough
The sandbox API now exports RequestedKind, RequestedSize, and requested. The function emits vCPU and memory distributions with request tags, converts memory from MiB to bytes, and includes the defaulted tag when provided.
Possibly related PRs
perplexityai/SPACE#1510: Adds call sites that construct RequestedSize values and use the metrics API introduced here.
Merge Risk: Minimal · up to 820d1
This PR adds the requested-size metric API without changing production recording behavior. It is mergeable after normal checks.
Security architecture risk: Minimal · up to 18be6
The change adds an unused telemetry-recording API without changing request handling, access controls, or sandbox lifecycle behavior. No material security risk introduced by this PR was identified.
Retained concerns
No architecture-level concerns identified.
Security review details
Security Blast Radius
inferred — The new exposure is a library API available to dependent Rust code, not a newly registered network endpoint. With only a definition and re-export added, this PR introduces no production path carrying attacker-controlled input into the two new distributions.
Trust Boundaries and Controls
observed — RequestedSize has no client identity override. The emitter reads the existing task-local ClientIdTag vocabulary, which represents an allowlisted identifier, overflow, or unknown; calls outside the request scope return unknown. AdmissionDecision is only serialized as accepted or rejected and grants no admission authority.
Pre-merge checks: 5
Check name
Status
Explanation
Title check
The title "metrics: add requested vCPU and memory distributions" starts with a lowercase area and colon, followed by a lowercase summary verb. It names the main change: new requested vCPU and memory d…
Description Follows The Pr Template
The PR description includes all required sections with real content: ## Summary states what the change adds, ## Validation names checks and results, ## Severity and risk states that severity doe…
Bug Fixes Name A Regression Test
PASS: This pull request adds requested-size metrics definitions and re-exports under crates/metrics. The diff adds no application behavior fix or production recording call site, so the regression-te…
Weakened Safeguards Are Justified
PASS. The reviewed diff changes only crates/libs/metrics/src/sandbox/api.rs and adds crates/libs/metrics/src/sandbox/api/requested.rs. It removes no tests, assertions, lints, CI checks, or reposit…
Public-Safe Content
No public-safety violation is present. The reviewed diff adds only Rust metric API and documentation; it contains no credentials, tokens, private keys, internal URLs or hostnames, cloud IDs/ARNs, work…
Full details: Title check
Explanation
The title "metrics: add requested vCPU and memory distributions" starts with a lowercase area and colon, followed by a lowercase summary verb. It names the main change: new requested vCPU and memory distributions in the metrics crate. The title matches the stated objectives of PR #1505, which adds emitters and tag vocabulary for sandbox.api.requested.vcpu and sandbox.api.requested.memory_bytes. It has no trailing period and no ticket ID.
Full details: Description Follows The Pr Template
Explanation
The PR description includes all required sections with real content: ## Summary states what the change adds, ## Validation names checks and results, ## Severity and risk states that severity does not apply and describes low risk, and AI assistance: identifies the tool and the parts it drafted or changed.
Full details: Bug Fixes Name A Regression Test
Explanation
PASS: This pull request adds requested-size metrics definitions and re-exports under crates/metrics. The diff adds no application behavior fix or production recording call site, so the regression-test requirement does not apply.
Full details: Weakened Safeguards Are Justified
Explanation
PASS. The reviewed diff changes only crates/libs/metrics/src/sandbox/api.rs and adds crates/libs/metrics/src/sandbox/api/requested.rs. It removes no tests, assertions, lints, CI checks, or repository guards, and adds no allowlist or skip configuration.
Full details: Public-Safe Content
Explanation
No public-safety violation is present. The reviewed diff adds only Rust metric API and documentation; it contains no credentials, tokens, private keys, internal URLs or hostnames, cloud IDs/ARNs, workstation paths, logs, measurements, or undisclosed vulnerability details. The title and description contain only a public GitHub pull-request link and technical change details.
Tip
🎯 Customize your risk assessments
Guide how CodeRabbit assesses merge risk and security architecture risk. Highlight sensitive changes and explain your security boundaries using Merge Risk Instructions and Security Risk Instructions in your review settings. Security architecture risk assessments require the Advanced plan or above.
✅ Automatic approvals, with your rules
Let CodeRabbit approve pull requests once required threads are resolved, pre-merge checks pass, and your risk limits are met. Enable Automatic approvals in review settings, with overrides for teams or file paths.
Recent review info
Automatic approval policy
Commit: 820d184539b6.
Setting
Effective value
Source
Automatic approvals
Enabled
Global configuration
Merge risk
Low
Override 2
Security risk
Not set
Global configuration
Matching overrides (numbered as in the resolved configuration):
Any matching disabled override withholds approval, regardless of risk. Otherwise, the strictest explicitly set limit wins for each score. Limits omitted by all matching overrides inherit the global configuration.
Not set means no limit was configured after inheritance. Use @coderabbitai configuration to inspect the resolved settings and their sources.
Reviewing files that changed from the base of the PR and between 1c5a928 and 820d184.
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 10 remain after this review.
Add the sandbox.api.requested.vcpu and
sandbox.api.requested.memory_bytes distributions and their bounded tag
vocabulary: kind (create, activate, resume), surface, admission,
template, client_id and, on creates, defaulted. admission takes its
accepted and rejected values from the existing AdmissionDecision the
cluster admission limiter reports. memory_bytes is the guest memory in
MiB converted to bytes.
Nothing records them yet; later changes record creates, activations
and resumes. The module documentation defines each tag and value.
Clarify that paused runtime recovery can require admission and be rejected. Remove the telemetry test module declaration in accordance with the repository test policy; the test file is removed in the following commit.
Remove metric-value and tag assertion tests to comply with the repository test-economy policy. The test module declaration was removed in the preceding commit.
Keep the metric units, tags, recording boundaries and runtime-recovery caveat while removing redundant detail. Documentation only; emitter behavior is unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Summary
Add
sandbox.api.requested.vcpuandsandbox.api.requested.memory_bytesdistributions for effective requested sandbox sizes after template and warm-pool defaults. This adds the emitter and tag vocabulary only; no production recording call sites are added.Both distributions carry
kind,surface,admission,template, and the request-scopedclient_id, with an optionaldefaultedtag for creates. ReuseAdmissionDecisionforaccepted/rejectedlabels, and convert memory from MiB to bytes using au64intermediate.Validation
For the review fixes:
git diff --cached --checkanduv tool run pre-commit runpassed, including the repository's gitleaks check. Verified the published file matches the locally reviewed patch and the telemetry test module and file are removed. Rust tests were not rerun for these documentation/test-removal changes; this sandbox has no Rust toolchain. Runtime emission and backend ingestion have not been validated.Severity and risk
Severity does not apply: this is additive observability infrastructure, not a bug fix. Risk is low because no production call sites emit these metrics yet; review should focus on metric names, tag vocabulary, and unit conversion.
Rollback: revert this PR.
Stack
Requested-size metrics stack; merge from bottom to top:
main.gil/requested-size-1-metric.This PR does not wire the metrics into create or lifecycle requests.
AI assistance: Perplexity Computer drafted this description and applied the review fixes (resume documentation and removal of telemetry assertions).