[PR #428] [CLOSED] fix(observability): retain lifecycle metrics after caller cancellation #428

New issue

Closed

opened 2026-10-29 14:23:20 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-29 14:23:20 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/kvcache-ai/AgentENV/pull/428
Author: @guozy18
Created: 10/23/2026
Status: ❌ Closed

Base: main ← Head: sync/cancellation-safe-stage-metrics

📝 Commits (1)

a425938 fix: record sandbox stage metrics inside cancellation-safe orchestrator tasks

📊 Changes

5 files changed (+151 additions, -39 deletions)

View changed files

📝 Cargo.lock (+1 -0)
📝 Cargo.toml (+1 -0)
📝 src/api/impls/sandbox.rs (+10 -30)
📝 src/orchestrator/service.rs (+48 -9)
📝 src/orchestrator/tests.rs (+91 -0)

📄 Description

What

Record sandbox operation duration and in-flight metrics for the full lifetime of the cancellation-safe orchestrator task, including when an HTTP client disconnects before the operation finishes.

Why

The operation already survives request cancellation, but its timer lived in the HTTP handler. Dropping that handler prematurely decremented in-flight and lost the duration sample.

No linked issue; focused lifecycle observability fix.

Scope and non-goals

Move create, fork, pause, resume and capture timers into the existing task boundary. Add delete, network-update and extension-parameter-patch stage metrics. Image resolution and snapshot publication timers stay with their existing request-owned work.

Design and behavior changes

The timer is polled inside the task owned by run_cancellation_safe. Dropping the API handler or its result receiver does not drop that timer. Completion records the task's actual ok/error result and releases its in-flight guard.
Preserve the existing metric names (agentenv_sandbox_stage_duration_seconds and agentenv_sandbox_stage_inflight) and existing operation/stage labels. Infer cold/warm create from the launch source. Remove the corresponding API timers so requests are not counted twice.
Preserve upstream fork-with-specs, volume handling and the separate template-builder path. In upstream's per-child fork response, a successfully executed batch can contain child errors: the batch stage remains ok, while individual failures stay in the response.
Image resolution and repository publication retain their existing request-scoped timers; this change does not make those stages cancellation-safe.

Compatibility and operations

Public API or generated protocol: unchanged.
Configuration or defaults: unchanged.
Snapshot manifest, artifact layout, or storage format: unchanged.
Upgrade and rollback: no migration; dashboards can now see completed work after callers stop waiting, plus the three added operation series.
Host requirements, permissions, ports, or dependencies: unchanged; tests reuse the workspace's existing metrics debugging dependency.

Validation

cargo fmt --all -- --check
cargo clippy --locked --workspace --all-targets --all-features -- -D warnings

cargo test --locked -p agentenv --lib: 1017 passed; 1 P2P loopback timeout; 4 ignored
Isolated retry of transport_looks_up_and_fetches_from_peer_over_loopback: passed
Focused pause_stage_metrics_survive_caller_cancellation: passed (success + error cases)
Additional HTTP/KVM check on this standalone PR: passed
- disconnect a client while start-fresh is blocked, for success and rejection
- in-flight stays 1; no completed sample until release; exactly one full-duration sample afterwards
- one of two fork children rejected: batch metric stays ok and the other child succeeds
- warm create, capture, pause, resume, delete, network-update and patch stage labels/counts

The regression cancels a caller after pause begins, checks in-flight remains 1, and then checks one full-duration sample, the actual success/error label, in-flight 0, and final sandbox state. It runs against both successful and failed pauses with a thread-local recorder. The additional check used a separately built #428 server and daemon on upstream 9f62cbe, without #427 or fork/main. All 829 tracked source files were checked against the PR commit. Unrelated service and full OSS/P2P suites were not run.

GitHub unit-tests is currently blocked before running tests by tikv-jemalloc-sys: inherited MAKEFLAGS=" -- PROFILE=debug -j ..." makes its nested make treat -j as a target. The identical failure is present on the unchanged upstream baseline and in this PR's job. This PR changes neither the Makefile nor that dependency version. The direct Cargo and isolated runtime results above are separate evidence, not a claim that all GitHub checks pass.

Risks and reviewer notes

Metrics now include direct orchestrator callers and operations that outlive API handlers. Existing operation behavior and cancellation ownership are unchanged.

Checklist

One coherent change; no dependency on another open PR.
Focused cancellation regression covers success and error.
No credentials or private registry information in the diff.
No generated code changes.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/kvcache-ai/AgentENV/pull/428 **Author:** [@guozy18](https://github.com/guozy18) **Created:** 10/23/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `sync/cancellation-safe-stage-metrics` --- ### 📝 Commits (1) - [`a425938`](https://github.com/kvcache-ai/AgentENV/commit/a4259382525974a1e127f6b40ed8b897385612cd) fix: record sandbox stage metrics inside cancellation-safe orchestrator tasks ### 📊 Changes **5 files changed** (+151 additions, -39 deletions) <details> <summary>View changed files</summary> 📝 `Cargo.lock` (+1 -0) 📝 `Cargo.toml` (+1 -0) 📝 `src/api/impls/sandbox.rs` (+10 -30) 📝 `src/orchestrator/service.rs` (+48 -9) 📝 `src/orchestrator/tests.rs` (+91 -0) </details> ### 📄 Description ## What Record sandbox operation duration and in-flight metrics for the full lifetime of the cancellation-safe orchestrator task, including when an HTTP client disconnects before the operation finishes. ## Why The operation already survives request cancellation, but its timer lived in the HTTP handler. Dropping that handler prematurely decremented in-flight and lost the duration sample. ## Related issue No linked issue; focused lifecycle observability fix. ## Scope and non-goals Move create, fork, pause, resume and capture timers into the existing task boundary. Add delete, network-update and extension-parameter-patch stage metrics. Image resolution and snapshot publication timers stay with their existing request-owned work. ## Design and behavior changes - The timer is polled inside the task owned by `run_cancellation_safe`. Dropping the API handler or its result receiver does not drop that timer. Completion records the task's actual `ok`/`error` result and releases its in-flight guard. - Preserve the existing metric names (`agentenv_sandbox_stage_duration_seconds` and `agentenv_sandbox_stage_inflight`) and existing operation/stage labels. Infer cold/warm create from the launch source. Remove the corresponding API timers so requests are not counted twice. - Preserve upstream fork-with-specs, volume handling and the separate template-builder path. In upstream's per-child fork response, a successfully executed batch can contain child errors: the batch stage remains `ok`, while individual failures stay in the response. - Image resolution and repository publication retain their existing request-scoped timers; this change does not make those stages cancellation-safe. ## Compatibility and operations - Public API or generated protocol: unchanged. - Configuration or defaults: unchanged. - Snapshot manifest, artifact layout, or storage format: unchanged. - Upgrade and rollback: no migration; dashboards can now see completed work after callers stop waiting, plus the three added operation series. - Host requirements, permissions, ports, or dependencies: unchanged; tests reuse the workspace's existing metrics debugging dependency. ## Validation - [x] `cargo fmt --all -- --check` - [x] `cargo clippy --locked --workspace --all-targets --all-features -- -D warnings` ```text cargo test --locked -p agentenv --lib: 1017 passed; 1 P2P loopback timeout; 4 ignored Isolated retry of transport_looks_up_and_fetches_from_peer_over_loopback: passed Focused pause_stage_metrics_survive_caller_cancellation: passed (success + error cases) Additional HTTP/KVM check on this standalone PR: passed - disconnect a client while start-fresh is blocked, for success and rejection - in-flight stays 1; no completed sample until release; exactly one full-duration sample afterwards - one of two fork children rejected: batch metric stays ok and the other child succeeds - warm create, capture, pause, resume, delete, network-update and patch stage labels/counts ``` The regression cancels a caller after pause begins, checks in-flight remains 1, and then checks one full-duration sample, the actual success/error label, in-flight 0, and final sandbox state. It runs against both successful and failed pauses with a thread-local recorder. The additional check used a separately built #428 server and daemon on upstream `9f62cbe`, without #427 or fork/main. All 829 tracked source files were checked against the PR commit. Unrelated service and full OSS/P2P suites were not run. GitHub `unit-tests` is currently blocked before running tests by `tikv-jemalloc-sys`: inherited `MAKEFLAGS=" -- PROFILE=debug -j ..."` makes its nested make treat `-j` as a target. The identical failure is present on the unchanged [upstream baseline](https://github.com/kvcache-ai/AgentENV/actions/runs/39892803013/job/120216239840) and in [this PR's job](https://github.com/kvcache-ai/AgentENV/actions/runs/39910493540/job/120272109364). This PR changes neither the Makefile nor that dependency version. The direct Cargo and isolated runtime results above are separate evidence, not a claim that all GitHub checks pass. ## Risks and reviewer notes Metrics now include direct orchestrator callers and operations that outlive API handlers. Existing operation behavior and cancellation ownership are unchanged. ## Checklist - [x] One coherent change; no dependency on another open PR. - [x] Focused cancellation regression covers success and error. - [x] No credentials or private registry information in the diff. - [x] No generated code changes. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi added the

pull-request

label

2026-10-29 14:23:20 +00:00

bsakaguchi referenced this issue

2026-10-29 14:23:20 +00:00

[PR #427] [CLOSED] feat(custom-extension): include sandbox metadata in lifecycle hooks #429

bsakaguchi closed this issue

2026-10-29 14:23:20 +00:00