Repository navigation
feat(sandbox): lifecycle telemetry: durations, retries, sandbox ids and a stop span - #3633
Merged
Merged
Conversation
iguehring-nvidia
force-pushed
the
iguehring/sandbox-telemetry
branch
from
September 22, 2026 22:15
8bbcaf2 to
1fd06b3
Compare
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
iguehring-nvidia
changed the base branch from
iguehring/sandbox-active-counter
to
main
September 22, 2026 22:15
iguehring-nvidia
force-pushed
the
iguehring/sandbox-telemetry
branch
from
September 22, 2026 22:17
1fd06b3 to
28b9946
Compare
iguehring-nvidia
marked this pull request as ready for review
September 22, 2026 22:26
iguehring-nvidia
enabled auto-merge
September 22, 2026 22:49
Contributor
|
/ok to test 28b9946 |
iguehring-nvidia
force-pushed
the
iguehring/sandbox-telemetry
branch
from
September 23, 2026 08:13
878fa72 to
ae7d23d
Compare
iguehring-nvidia
force-pushed
the
iguehring/sandbox-telemetry
branch
from
September 23, 2026 09:00
ae7d23d to
950e42b
Compare
This was referenced Sep 23, 2026
Closed
ko3n1g
added a commit
that referenced
this pull request
Sep 23, 2026
## Background / motivation - Adopt released [FW-CI-templates v1.11.0](https://github.com/NVIDIA-NeMo/FW-CI-templates/releases/tag/v1.11.0), containing NVIDIA-NeMo/FW-CI-templates#582's SSO asset discovery/retry fix. - Replaces this PR's temporary candidate override after successful integration testing. Gym #3633 previously failed on deleted SSO asset `583229410`. ## What changed - Pin both `_cicd_preflight.yml` callers (`cicd-main.yml` and `release.yaml`) to release commit **`037094bcc9138c72bc125418e6e3653f9e1c4160`**. - Pin each caller's `workflow_ref` to the same release commit, so the inner action checkout cannot silently follow `main`. - Leave unrelated workflow versions, runner configuration, credentials, membership policy, and test jobs unchanged. ## Details - Public SHA literals carry the standard secrets-scanner false-positive annotation. - CI scope: **full** (workflow configuration). Model rollouts: **N/A**; no model, environment, agent, verifier, or benchmark behavior changes. - No user-facing API or migration impact. No merge or deployment performed. ```mermaid flowchart LR Gym[Gym CI or release] --> Workflow[Preflight at v1.11.0 SHA] Workflow --> Checkout[Inner checkout at same release SHA] Checkout --> SSO[Current SSO asset discovery and retry] SSO --> Gate[Existing membership and runner selection] ``` ## Tested - Release tag resolves to `037094bcc9138c72bc125418e6e3653f9e1c4160`; released downloader exists at that commit. - `pre-commit run --all-files`: passed. - `actionlint -shellcheck=''` on both changed workflows: passed. Full actionlint's pre-existing SC2129 warning in the unrelated container-build block was reproduced on the base. - Scoped `detect-secrets-hook` and `git diff --check`: passed. - Current Gym head: `c97b6e0398f8fff05df4f14ee852a060d5b2d671`; release-pin CI pending. - Historical proof of the same SSO fix before release: https://github.com/NVIDIA-NeMo/Gym/actions/runs/35845632625/job/107131012058 — candidate checkout verified, **8,177-user roster downloaded**, membership and runner selection passed. This is not a claim that current-head or broader Gym CI has passed. --------- Signed-off-by: oliver könig <okoenig@nvidia.com>
…n counter Gym had no "sandboxes running now" signal: the sandbox span group emits gym.sandbox.start and gym.sandbox.exec spans but no stop span and no gauge, so dashboards had to approximate the count from in-flight /run requests on the agent server. AsyncSandbox now adds +1 to gym.sandbox.active when a provider hands back a handle and -1 when Gym releases it (also when the provider's close fails or the initial file upload fails), dimensioned by nemo.gym.sandbox.provider. An up-down counter rather than a gauge so a backend sums correctly across the agent-server processes that each hold a share of the sandboxes. A connect()ed sandbox is not subtracted, since the starting process counted it. The instrument lives on the lens meter but is Gym-owned; it follows the existing sandbox span-group gate and is a no-op without telemetry. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Ingo Guehring <iguehring@nvidia.com> (cherry picked from commit 2e188aa)
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Ingo Guehring <iguehring@nvidia.com>
…rs with real buckets nemo_gym/telemetry/metrics.py can only reach nemo-lens's five fixed, undimensioned instruments. Anything that needs an attribute (a sandbox provider, a site) has to be created directly on the lens meter, so this adds gym_metrics.py: a per-meter instrument cache and histogram / counter / up-down-counter recorders that are no-ops without telemetry and never raise. The sandbox lifecycle set lives here: gym.sandbox.active moves in from metrics.py unchanged in behaviour; gym.sandbox.startup_duration_ms, gym.sandbox.exec_duration_ms and gym.sandbox.create_retry_total are added for the next commits to record into. The two histograms declare explicit bucket boundaries up to thirty minutes: the SDK default ends at ten seconds, and a remote sandbox start routinely takes a minute, so every sample would otherwise land in the overflow bucket and a p95 would read "10 s" whatever the truth. The registry helpers and the three duration/retry recorders are taken from Onur Yilmaz's #2935, which this series splits out. Co-authored-by: Onur Yilmaz <oyilmaz@nvidia.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Ingo Guehring <iguehring@nvidia.com>
…ans, a stop span AsyncSandbox records gym.sandbox.startup_duration_ms around provider.create and gym.sandbox.exec_duration_ms around each command, per provider, with the explicit bucket boundaries from gym_metrics. The start and exec spans carry nemo.gym.sandbox.id so a trace can be joined to the provider's own logs, and stop() now opens a gym.sandbox.stop span around provider.close, so a trace pairs a sandbox's start with its end. All of it sits under the existing sandbox span group; nothing is recorded with the group off. The duration histograms and the id attribute are taken from Onur Yilmaz's #2935. Co-authored-by: Onur Yilmaz <oyilmaz@nvidia.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Ingo Guehring <iguehring@nvidia.com>
Retrying a create is provider-internal, so the facade cannot see it; the two providers that retry record gym.sandbox.create_retry_total from their tenacity before_sleep callback, next to the warning they already log. On a full Terminal-Bench 4 run against a degraded OpenSandbox this is the difference between "39 rollouts dropped" and "101 retries, then 39 dropped" being visible on a board rather than only in the driver log. The Daytona hook is from Onur Yilmaz's #2935. Co-authored-by: Onur Yilmaz <oyilmaz@nvidia.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Ingo Guehring <iguehring@nvidia.com>
The counter change had moved `_stopped = True` into a finally, so a sandbox whose provider close() raised was marked stopped and a later stop() no longer retried the close. The compose group relies on that retry (test_docker_compose::test_failed_cleanup_can_be_retried). A rejected close has released nothing: the sandbox stays started, stop() stays retryable, and gym.sandbox.active drops on the attempt that succeeds, once. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Ingo Guehring <iguehring@nvidia.com>
iguehring-nvidia
force-pushed
the
iguehring/sandbox-telemetry
branch
from
September 23, 2026 10:15
950e42b to
22351ab
Compare
Contributor
|
🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA-NeMo/Gym/actions/runs/35848865661 |
Glorf
added a commit
that referenced
this pull request
Sep 28, 2026
## Summary Add an opt-in `sink_window` history policy to the OSWorld Nemotron adapter: retain the earliest screenshots alongside a recent-image window, with intervening turns represented as text in chronological order. This branch includes upstream `main` at `d4c0f86c7` through a normal merge, including the NumPy metadata fix #3649, CI preflight update #3658, and sandbox lifecycle telemetry #3633. The OSWorld foundation from #2554/#3140 was consolidated and merged through #3592, so this PR no longer carries the old exact-trace/Core stack or depends on #2554. It does not require #3591. No separate issue is needed for this follow-up policy extension. ## Changes - Add configurable sink, low-water and high-water counts. Both watermarks include the sink; equal watermarks slide, while unequal watermarks provide hysteresis. - Represent non-contiguous image selections with intervals and render folded text in chronological order. - Distinguish context overflow, transport failure, truncated output, empty output and unparseable output; retain `agent_outcome_family=model_response_invalid` for compatibility. - On context overflow, reduce the recent-image window within the existing retry budget, preserve the sink/current observation, record actual shrink events, and stop when further reduction is impossible. - Preserve main's immediate model-timeout propagation. Report the terminal attempt's completion fact so an earlier malformed sample cannot hide a subsequent transport failure. The latest synchronization also annotates six deterministic SHA-256 prompt goldens that the previous secrets-detector run falsely identified as credentials. Only those literal lines are annotated; the digest values, assertions, repository scanner configuration, and baseline are unchanged. ## Design and compatibility History policy selects image/text turns; the adapter owns model protocol rendering and parsing. The runner owns horizon and runtime failure handling, the evaluator owns reward, and runtime admission owns masking. This PR does not change those ownership boundaries or add a training/evaluation switch. Fixed-window and hysteresis policy IDs and normal-path rendered prompts remain covered by deterministic golden tests, including fixed-3 and 3-10-3. Sink-window is opt-in, not a new default or a claim of improved benchmark score. Context-overflow recovery and more precise failure outcomes are intentional behavioral changes and may affect failed-run outcomes. The legacy scalar `image_window_start` accessor rejects non-contiguous plans; consumers should use `image_intervals` or per-turn decisions. Scope relative to current main remains limited to `responses_api_agents/osworld_agent/`: two implementation files, documentation, and tests. No Gym Core, dependency, deployment, sandbox-provider or model-server changes are included in the PR diff. ## Validation Re-run on the final main-synchronized tree: - **440 passed, zero skipped** across OSWorld agent, resource-server and benchmark tests; one non-failing Starlette deprecation warning. - `pre-commit run --all-files`: passed. - `uv lock --check --offline`: passed. - `git diff --check`: passed. - The same `detect-secrets==1.5.0` hook and checked-in baseline used by CI passed on all nine PR-scoped files after the inline false-positive annotations. - Existing model-timeout propagation, runner admission, prompt-parity, sink selection, overflow recovery and policy-identity regressions passed. The unchanged-overflow retry and mixed parse/transport completion bugs had been reproduced before their earlier fixes. Test invocation, using an existing compatible development environment from this checkout: ```bash export PYTHONPATH="$PWD" export NEMO_GYM_EXTRA_ROOTS="$PWD" python -m pytest \ responses_api_agents/osworld_agent/tests \ resources_servers/osworld/tests \ benchmarks/osworld/tests \ --import-mode=importlib -o addopts='' -o log_cli=false -q pre-commit run --all-files ``` The explicit source root prevents an older editable checkout from shadowing the branch under test; importlib mode avoids the shared `osworld` test-package name collision. These checks are local evidence, not a claim that the upstream CI run has completed. ## Score reproduction OSWorld 361 tasks, Nemotron-3-Nano-Omni v3 (`24e67ea0`), fixed-3 history, 100 steps, one A/B run per arm. Control = main `d4c0f86c7` + #3591 (needed for the E2B `endpoint()` on AgentEnv); treatment = control + this PR. Raw SR 44.61% → 46.62% (Δ +2.01 pp, paired 95% CI [−2.12, +6.07] pp); binary 154 → 159 of 361 (McNemar p = 0.59); 0 masked and 0 missing in both arms; identical history-policy and agent-contract IDs. No degradation. The context-overflow shrink path was not triggered in either run. --------- Signed-off-by: Jeff Peng <jepeng@nvidia.com> Co-authored-by: Michał Bień <mbien@nvidia.com>
This was referenced Oct 1, 2026
4 tasks
This branch was previously deployed
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Supersedes #3581 (its two commits are the first two here) and takes the sandbox part of #2935, with @oyilmaz-nvidia as co-author on the hunks from there. Agreed with Onur as the split of #2935 that this side owns.
What
One coherent "sandbox lifecycle telemetry" set in
AsyncSandboxand the providers, all under the existingsandboxspan group, all attributed bynemo.gym.sandbox.provider:gym.sandbox.activestart()/stop()gym.sandbox.startup_duration_msstart()gym.sandbox.exec_duration_msexec()gym.sandbox.create_retry_totalnemo.gym.sandbox.idgym.sandbox.stopprovider.closestop()Supporting change:
nemo_gym/telemetry/gym_metrics.py, a small registry for attributed instruments (per-meter cache, histogram / counter / up-down-counter recorders, no-op without telemetry, never raises), taken from #2935.record_sandbox_activelives there;telemetry/metrics.pykeeps forwarding to nemo-lens's five fixed instruments and points here for anything attributed.Why
Gym's sandbox API emitted
gym.sandbox.startandgym.sandbox.execspans and nothing else: no stop span, no metric. Three questions a dashboard could not answer: how many sandboxes exist right now (dashboards approximated it from in-flight/runrequests, which undercounts by more than two times on Terminal-Bench 4, where a session holds an agent sandbox, a shared-logs helper and later a verifier sandbox); how long provisioning and commands take as a metric rather than only as spans, which need a span-metrics connector somewhere to become one; and how often creation has to be retried. On a degraded provider the last one is the difference between "39 rollouts dropped" and "101 retries, then 39 dropped".Design points:
gym.sandbox.active: several server processes each hold a share of the sandboxes; a backend sums counters correctly across processes, while a gauge shows whichever process wrote last. −1 happens once the provider has actually released the sandbox: aclose()the provider rejected has released nothing, so the sandbox stays started and counted and a laterstop()retries it (the compose group depends on that retry). Aconnect()ed sandbox is neither counted nor subtracted, since the starting process counted it.create()once. The two providers that retry (OpenSandbox, Daytona) record from their tenacitybefore_sleepcallback, next to the warning they already log, so the counter can never disagree with the log.provider.close; a failing close yields an error-status stop span and leaves the count unchanged until the retry succeeds.Deliberately not in this PR (they belong to other splits of #2935): the CPU / memory / GPU sampling at span close, and the
TimedSemaphoreswap in the providers.Validation
pytest tests/unit_tests/telemetry tests/unit_tests/test_sandbox.py tests/unit_tests/test_opensandbox_provider.py: pass locally (nemo-lens-gated span tests skip locally and run on the cluster venv). Tests, all against a real in-memory OTel reader:test_sandbox_active.py(10): start then stop moves 1 → 0 and a second stop stays 0; failed create counts nothing; failed initial upload releases the count; a failed close keeps the sandbox counted until a retried stop releases it; group off records nothing;connect()ed sandbox not subtracted; no-op without telemetry; errors never reach the caller; non-monotonic and dimensioned by provider; one instrument per meter.test_sandbox_lifecycle_metrics.py(4): one startup sample per start and one exec sample per command with the provider attribute; explicit bounds end at 1,800,000 ms; failed create records no startup sample; group off records nothing.test_sandbox_create_retry.py(3): both providers' callbacks count by provider when the group is on, silent when off.test_call_sites.py(+1 test, +3 assertions):nemo.gym.sandbox.idon start and exec spans; agym.sandbox.stopspan with provider and id.pre-commit run --files ...on the changed files: pass.Rollout evidence
Slurm cluster, Terminal-Bench 4 (mini-SWE), OpenSandbox provider,
sandboxspan group on, metrics exported through an OTLP collector that also derives span metrics, so every Gym-emitted number has an independent counterpart to check against.8-task run (concurrency 8, 24 min, all 8 rollouts scored, collector shut down cleanly):
gym.sandbox.create_retry_total{provider="opensandbox"}gym.sandbox.startup_duration_msgym.sandbox.startduration: 25 samples, mean 91.6 s, same p50 / p95gym.sandbox.exec_duration_msgym.sandbox.exec: 581 successful + 1 error span, p95 ≤ 5 sgym.sandbox.stopspangym.sandbox.activeWith the SDK's default buckets, the startup histogram (mean 92 s) would have placed every sample in the overflow bucket and reported "10 s" for any percentile; with the explicit boundaries the p50 / p95 land in the 60 s / 300 s buckets, matching the span-derived series bucket for bucket.
Note on the exec count: the duration histograms are recorded after the span closes, so an operation that raises never reaches the record and is absent from the histogram; it does appear in the span-derived series with
STATUS_CODE_ERROR. The one error exec accounts for the gap. This is deliberate: a failure's duration is not a latency, and the failure itself is counted where the status lives.66-task run at concurrency 16 (earlier, counter only):
gym.sandbox.activerose from 8 to a peak of 39 while the in-flight-rollouts proxy reported 16, and returned to 0 through 6SANDBOX_NOT_FOUNDrebuilds and 3 failed starts.Compatibility
Additive. Nothing is recorded unless telemetry is on and the
sandboxspan group is enabled. New moduletelemetry/gym_metrics.py; no public API changes.AsyncSandbox.stop()semantics are unchanged from main: a failedclose()leaves the sandbox started so the call can be retried (an earlier revision of this PR broke that and failedtest_docker_compose; fixed in the last commit).