Skip to content

spaced: measure payload admission pressure - #833

Open
ravi-pplx wants to merge 6 commits into
ravi/guest-execution-drainfrom
ravi/guest-execution-admission-metrics
Open

ravi-pplx wants to merge 6 commits into
ravi/guest-execution-drainfrom
ravi/guest-execution-admission-metrics

Conversation

@ravi-pplx

@ravi-pplx ravi-pplx commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Count payload admission decisions under spaced.guest.payload.* and measure weighted slots retained from observed cancellation until final release. Observers share one timestamp without retaining capacity. Tests retain admission, cancellation ownership and capacity-recovery coverage without asserting metric emissions. Mark RequestLifetime as must_use and add Debug. Admission limits, retry statuses and shutdown behavior are unchanged. Use the extracted gRPC metrics crate and add a payload re-export from the aggregate metrics crate.

Document that cancellation observations include server-initiated aborts and that transport-only retention can produce near-zero samples. Explain why normal native acquisition cannot identify adapter permit loss; the existing daemon regression guards that contract. Stacked on #827; follows the embedding RFC.

Validation

418 affected tests passed on the rebased stack, including five privileged probes. Formatting, affected all-feature/no-default-feature Clippy and doctests passed on Linux. Native macOS and Kind/Firecracker were not rerun locally; CI is pending on this head.

Local benchmarks on Rust 1.98 compared stack-base main (da5bfd2c) with the earlier implementation, before the stream fix: gRPC p50 −4.1% to +3.4%; native Direct stat +13 µs and identity listing +42 µs. Metrics alone measured −4.7% to +2.7%. Five alternating optimized runs, fixed CPUs, native/UDS gRPC and matching workers; native baseline used a benchmark-only getter. No exporter or production-load claim; these measurements were not rerun on the current head or Rust 1.99.1.

Severity and risk

Severity does not apply: observability improvement. Risk is telemetry overhead or misinterpreting decision counts as unique requests. Revert this PR to remove instrumentation.

AI assistance: Codex contributed code, tests and this description.

@cla-assistant

cla-assistant Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@ravi-pplx
ravi-pplx force-pushed the ravi/guest-execution-admission-metrics branch from 1778e2a to 21edb3d Compare October 23, 2026 03:45
@ravi-pplx
ravi-pplx force-pushed the ravi/guest-execution-admission-metrics branch from 21edb3d to eccde05 Compare October 23, 2026 04:26
@zibi-pplx

Copy link
Copy Markdown
Collaborator

Follow-up review at eccde0575192, including the full stack.

I found no additional admission-capacity or retry-semantics regression in the instrumentation. Three API/test-conformance points remain:

  • Mark RequestLifetime as #[must_use]. This is a side-effecting RAII guard: permit.request_lifetime(); silently drops it and records cancellation immediately, even when the request is continuing. The API guidelines specifically call for targeted must_use on guards whose drop has a meaningful side effect.
  • Add Debug for the new public RequestLifetime type. Its declaration currently lacks the public-type trait required by our API guidelines.
  • Remove the new telemetry-emission assertions while retaining behavioral coverage. The added assertions pin emitted names, tags, counters, and distributions. The code-quality skill explicitly says not to write unit tests that assert on telemetry. Keep the useful assertions that overload avoids decoding, cancellation retains capacity until the final owner releases it, and capacity then recovers.

For metric interpretation, a short documentation clarification would help: observed cancellation includes server-initiated upload aborts/shutdown, and brief transport-only permit retention can produce near-zero samples. I would not add cause tags or change ownership timing without a concrete consumer need; these are slot-retention metrics, not exclusively caller-cancellation or physical-cleanup measurements.

The generic metrics crate placement has an existing dependency-layering rationale, so I am not requesting another abstraction or crate for it. Also, the payload re-export is newly added by this stack; the PR description's wording that it is “existing” should be corrected.

Codex-assisted follow-up review. The six targeted payload-admission tests passed during review; lifecycle/stream findings are posted on #790 and #827.

@ravi-pplx
ravi-pplx force-pushed the ravi/guest-execution-admission-metrics branch from eccde05 to 344813e Compare October 23, 2026 16:47
Count payload admission decisions at transport and handler boundaries, and
measure weighted slots retained after cancellation until final release.
Share observations across permit clones without retaining capacity or
changing admission, retry, or shutdown behavior.

Document the metric boundaries: native acquisition is expected and cannot
identify an adapter that discarded its request extensions. Keep the daemon
permit-handoff regression as the guard for that contract.
Use spaced.guest.payload for guest admission and cancellation-retention
metrics. Keep the owning catalog in sync; labels and admission behavior
are unchanged.
Extend the admission tests with capture assertions for transport acquisition,
handler reuse and reacquisition, capacity refusal, and weighted cancellation
and release. Run async capture on the current thread and verify duplicate
cancellation observations emit one count while capacity remains owned.

Mutating the reused label or emitting an extra cancellation count makes the
respective regression fail. Production instrumentation is unchanged.
Assert request drop records cancellation independently of the supervisor.
Check retention timing and slot weighting without sleeping. Clarify that
closed admission semaphores and lifecycle refusals are distinct.
Keep payload admission metrics with the gRPC metrics crate and reuse the
shared emit capture. Preserve the public metrics re-export and all metric
names without restoring the common gRPC dependency on aggregate metrics.
@ravi-pplx
ravi-pplx force-pushed the ravi/guest-execution-admission-metrics branch from 344813e to 4c6155c Compare October 23, 2026 18:54

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants