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
api: record requested size for activations and resumes - #1577
Extend requested vCPU and memory recording to lifecycle activations and resumes, using the sandbox row’s effective size. Record successful actions and explicit admission rejections; exclude callers joining another activation, unrelated failures, and unresolved or invalid sizes.
Document admission during resume recovery, eligible implicit wakes, and distribution counts representing eligible recorded calls rather than total incoming requests. Reuse the existing metric definitions and admission classification without adding telemetry-assertion tests.
Also has some style updates. Renamed requested_metrics to requested_size_metrics, and removed requested.rs since that code can just live inside api.rs with the rest of the metrics.
Validation
cargo test -p pplx-space-metrics -p pplx-space-api --all-features: passed locally.
cargo fmt --all -- --check: passed locally.
pre-commit run --all-files: passed.
git diff --check: passed.
Verified the published tree matches the reviewed Cloud commit and includes the updated PR3 as an ancestor.
Runtime metric ingestion has not been validated.
Severity and risk
Severity does not apply: this adds observability rather than fixing application behavior. The main risk is incorrect sample counting or classification; admission decisions and lifecycle behavior are intended to remain unchanged.
Rollback: revert this PR’s lifecycle recording changes. The parent metric definitions, admission classification, and create recording can remain.
Stack
Requested-size metrics stack; merge from bottom to top:
Move RequestedKind, RequestedSize and requested() from their own
sandbox/api/requested.rs into sandbox/api.rs, next to the other
sandbox API metrics, and delete the file. The file's module
documentation becomes the doc comment on requested(). The public
paths under pplx_space_metrics::sandbox::api are unchanged.
Move the entry-lifecycle classification into
SandboxAction::requested_kind, next to activation_class, and let
record_lifecycle_action decide whether a result counts: it now takes
the optional kind and the action's result and records only an
eligible kind that succeeded or was refused by cluster admission.
action() keeps the async block and the three exits that clear the
kind for a joined activation. Recording behavior is unchanged.
No actionable comments were generated in the recent review.
Walkthrough
The requested-size metrics API now records vCPU and memory distributions for eligible creates, activations, and resumes. The lifecycle controller classifies actions and records samples based on their final results. The observability documentation describes recording rules and exclusions.
Possibly related PRs
perplexityai/SPACE#1068: Changes restore-claim recovery charges and admission policy, which connect to this PR’s requested-size metrics for admission rejections.
Merge Risk: Low · up to f7344
This adds metrics for sandbox activation and resume. Lifecycle behavior is intended to stay the same, but the action flow was restructured and the new samples have no telemetry tests, so owners should check the new metrics after rollout.
Security architecture risk: Low · up to f7344
The change expands observability without an identified authorization bypass or change to sandbox ownership and admission behavior. Remaining uncertainty concerns production metric ingestion and operational coverage, rather than a demonstrated security defect.
Retained concerns
No architecture-level concerns identified.
Security review details
Security Blast Radius
inferred — The demonstrated exposure increase is additional requested-size telemetry from eligible controller-mediated lifecycle calls, including implicit wakes. The inspected changes do not grant callers additional sandbox, tenant, credential, or infrastructure authority. Data-plane paths bypassing this controller remain outside the documented metric population.
Trust Boundaries and Controls
observed — Inspected E2B callers retain scoped sandbox lookup before dispatch. Ingress implicit wake retains access and lifecycle-policy validation. The controller retains drain checks, activation validation, admission enforcement, and recovery charging; the new metric classification does not replace these controls.
Resilience and Maintainability Implications
observed — These distributions are operational observations, not a complete admission or security audit ledger. They intentionally exclude unrelated failures, joined activations, unresolved sizes, restoring-state actions, and paths outside the controller; retry samples are not deduplicated.
Pre-merge checks: 5
Check name
Status
Explanation
Title check
The title identifies the API area and summarizes the main change. It meets the specified formatting requirements.
Description Follows The Pr Template
The description contains all four required items: Summary, Validation, Severity and risk, and AI assistance.
Bug Fixes Name A Regression Test
This PR adds lifecycle metrics and documentation. The reviewed changes do not fix a defect in application behavior, so the regression-test requirement does not apply.
Weakened Safeguards Are Justified
The diff does not weaken a test, assertion, lint, CI check, or repository guard. The renamed metrics test module remains connected, and the lifecycle change adds a test.
Public-Safe Content
The diff, title, and description contain no identified content covered by the failure conditions.
Recent review info
Automatic approval policy
Commit: f734438e890b.
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.
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.
An ECW connect reaches the lifecycle controller as a wake. From a
paused sandbox, which is still resident in memory, that wake resumes
it as a native resume would, yet only the native resume verb was
recorded as kind:resume, so ECW callers never produced one. Classify
a resume, wake or implicit wake from paused as kind:resume.
Rename SandboxAction::requested_kind to requested_size_kind, document
what it classifies, and pin its table in a unit test. The docs' kind
row now names the wake and the ECW connect, and a wake of a paused
sandbox leaves the not-recorded list.
The reason will be displayed to describe this comment to others. Learn more.
Reviewed the lifecycle refactor and requested-size recording at f734438. No blocking correctness findings.
The inner async block preserves the existing lifecycle/admission/event flow while allowing the outer recording point to observe early returns. Eligible activations and paused resumes record the stored effective CPU/memory on success or explicit admission rejection; joined activations clear the sample, and unrelated failures or invalid/unknown sizes are excluded. Metric names and tags remain unchanged by the emitter move.
Validation: inspected the diff and supporting call paths; GitHub CI reports Rust tests, clippy and formatting successful. Kind smoke was skipped. I did not rerun tests locally or validate runtime metric ingestion.
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}
nikhilbirmiwal-pplx
left a comment
Summary
Extend requested vCPU and memory recording to lifecycle activations and resumes, using the sandbox row’s effective size. Record successful actions and explicit admission rejections; exclude callers joining another activation, unrelated failures, and unresolved or invalid sizes.
Document admission during resume recovery, eligible implicit wakes, and distribution counts representing eligible recorded calls rather than total incoming requests. Reuse the existing metric definitions and admission classification without adding telemetry-assertion tests.
Also has some style updates. Renamed
requested_metricstorequested_size_metrics, and removedrequested.rssince that code can just live insideapi.rswith the rest of the metrics.Validation
cargo test -p pplx-space-metrics -p pplx-space-api --all-features: passed locally.cargo fmt --all -- --check: passed locally.pre-commit run --all-files: passed.git diff --check: passed.Runtime metric ingestion has not been validated.
Severity and risk
Severity does not apply: this adds observability rather than fixing application behavior. The main risk is incorrect sample counting or classification; admission decisions and lifecycle behavior are intended to remain unchanged.
Rollback: revert this PR’s lifecycle recording changes. The parent metric definitions, admission classification, and create recording can remain.
Stack
Requested-size metrics stack; merge from bottom to top:
gil/requested-size-3-create.AI assistance: Perplexity Computer reviewed and updated the implementation, restacked the branch, and drafted this description.