[zhaoyboo]

Depends on #173.

Summary

Host-only counters for the waits and copies the ring already performs, read
without CUDA work, and a log line with the ring's footprint.

Implementation details

host_wait_counter.h, pipeline_telemetry.h; RingEngine.get_stats()
(prepare outcomes, sync and forced-drain waits, toggle pre-apply waits,
staging waits, ready-to-commit latency, D2H errors, pageable copies);
dmi.sync_stats readers (synchronization_stats, pipeline_stats,
gather_sync_stats over an integration's row_table_stats()); INFO log of
the payload ring, pinned staging and task entries at ring build.
docs/ring-telemetry.md.

Compatibility and scope

Read-only additions.

Validation

• test_ring_telemetry.py (5); all native ring suites pass.
• CPU tests: 2325 passed, 458 skipped.

[Samfisheryu]

Independent audit — non-GPU stage

Reviewed head 526e05f799df778c5cb09412b033a641427e8121 against parent 7f984280231ef21c51be4d58c8562b353a649e98. GPU validation and benchmarks remain deferred until explicitly authorized.

[P2] Return unavailable when the native engine has no snapshot API

Location: _native_counters, especially line 34.

The module promises {"available": False, ...} when a native build does not expose telemetry. A native RingEngine from before this PR has no get_stats method, but both synchronization_stats and pipeline_stats call it unconditionally before checking missing fields. Updating Python code before rebuilding the extension therefore raises AttributeError instead of returning the advertised unavailable/rebuild-required result.

This was reproduced by executing the real reader module with an object matching the older API shape. No torch import, native extension, CUDA API or GPU work is involved.

Copyable CPU reproduction from this PR checkout

Run from the repository root at the audited commit:

CUDA_VISIBLE_DEVICES='' python - <<'PY'
import importlib.util
from pathlib import Path

path = Path("src/dmi/sync_stats.py")
spec = importlib.util.spec_from_file_location("audited_sync_stats", path)
stats = importlib.util.module_from_spec(spec)
spec.loader.exec_module(stats)

class LegacyRingEngine:
    # API shape of a pre-#174 native RingEngine: no get_stats method.
    pass

for name in ("synchronization_stats", "pipeline_stats"):
    try:
        print(name, getattr(stats, name)(LegacyRingEngine()))
    except Exception as exc:
        print(name, type(exc).__name__, str(exc))
PY

Observed:

synchronization_stats AttributeError 'LegacyRingEngine' object has no attribute 'get_stats'
pipeline_stats AttributeError 'LegacyRingEngine' object has no attribute 'get_stats'

Expected: both calls return available=False with a reason that the native telemetry API is absent/rebuilding is required.

Suggested change: check for a callable get_stats before invoking it, as gather_sync_stats already does for its reader. Preserve propagation of genuine errors from an existing get_stats implementation; a broad exception swallow would hide real faults. Add the missing-method case alongside the existing incomplete-snapshot cases.

Performance/architecture observations, not measured regressions: snapshot reads are host-side in the inspected implementation. Instrumentation itself is still active on ordinary paths: clocks and atomic counters run even when no telemetry reader is used. That is shared-path overhead and needs the deferred default-off pipeline comparison; “host-only” does not establish zero overhead.

Validation context: both baseline and cumulative head built their CPU-only native targets. Python compile/package checks passed, and the cumulative head passed F821. The full CPU run plus the targeted C++ runtime-environment rerun yielded 2,781 passing unique test nodes, one GPU-dependent skip, and one unchanged hard-coded temporary-directory permission failure. The baseline had 2,700 passing unique nodes and the same two remaining outcomes. Replaying that directory-bound test's assertions with a private writable spool passed on both versions. These are merged run results, not a claim that a single full invocation was entirely green. None of those standard tests covered the missing-method case above.

[zhaoyboo]

@Samfisheryu thanks for the audit. Fixed in abaf9b1, rebased onto the revised #173.

[P2] A native engine without get_stats reports unavailable. synchronization_stats and pipeline_stats now look up a callable get_stats before calling it, as gather_sync_stats already does for its reader. A RingEngine built before this PR returns available=False with reason="native telemetry unavailable (RingEngine has no get_stats); rebuild required" and the full missing_fields list. Errors raised by an existing get_stats still propagate; there is no broad exception swallow. Your reproduction against this commit:

synchronization_stats {'available': False, 'reason': 'native telemetry unavailable (RingEngine has no get_stats); rebuild required', 'missing_fields': [...]}
pipeline_stats {'available': False, 'reason': 'native telemetry unavailable (RingEngine has no get_stats); rebuild required', 'missing_fields': [...]}

Tests in tests/test_ring_telemetry.py: test_readers_report_a_native_build_without_get_stats (both readers) and test_readers_propagate_errors_from_an_existing_get_stats. These sit alongside the existing incomplete-snapshot cases.

On the overhead note: agreed. Snapshot reads are host-only, but the clocks and atomic counters run on ordinary paths whether or not a reader is used. "Host-only" is not a zero-overhead claim, and the default-off pipeline comparison stays on the deferred list.

Verification for this revision (full stack), CPU only: the native extension builds, and the -m cpu suite passes (2388 passed, 459 skipped). The one failure, test_cpu_native_build.py::test_every_torch_including_compile_requests_cxx20[host], also fails on main. The PR-specific files pass too (test_ring_telemetry.py, test_ring_consumer_queues.py, test_native_ring_consumer_fixes.py, test_hook_row_features.py, test_node_toggle.py, test_toggle_registry_host.py, test_row_table_clamp.py); only the three CUDA-device cases in test_ring_telemetry.py are skipped. No GPU tests or benchmarks were run; those wait for the owner's go-ahead.

[Samfisheryu]

Independent re-review of abaf9b18edf2ebc06fb6f7839cc6a396dbbf7cb8 (non-GPU stage).

C174-1 is resolved. I independently ran the actual readers against both a legacy API-shaped object and a working API that raises a sentinel exception:

synchronization_stats(legacy): available=False, full missing_fields list
pipeline_stats(legacy):        available=False, full missing_fields list
existing get_stats raising RuntimeError: propagated unchanged, both readers

The absence/non-callability check is correctly placed before invocation; existing getter failures are not swallowed. The six CPU telemetry cases pass; the three GPU cases were deselected, not counted as passes.

Full-stack local checks: CPU-only native targets built from this exact checkout; Python compile, package-layout and F821 checks passed. The full CPU suite reported 2,846 passed, 1 failed, 1 skipped, 513 deselected. The failure remains the unchanged test_selection_shares_the_wire_layout hard-coded temporary-directory permission conflict seen on baseline; the same assertions pass with a private writable spool. The skip requires a CUDA device. These results do not close the independently reproduced upstream issues.

This stack is not ready for merge yet. The remaining #171–#173 findings are reported on the introducing PRs. In particular the reviewed commit's CI also has concrete upstream failures:

• Native ring-test job: undefined reference to ring_producer_launch_count() in the standalone test_ring_engine target (introduced in [Core] Add CUDA-graph node toggling and device-side toggle support #171).
• CPU skip-gate job: the new [Core] Add whole-hook BatchQueue/SliceQueue handoff, pageable staging pool, and capture leases #173 CPU harness skips because the runner lacks CUDA headers, a non-hardware skip rejected by the gate.

Telemetry's ordinary-path clock/atomic cost remains a measurement item; the missing-API fix itself adds no work to producer hot paths. No GPU validation or benchmark was performed.

[zhaoyboo]

@Samfisheryu thanks for confirming C174-1. This branch is now c60d511, rebased cleanly onto the revised #173 (21f58b1). The telemetry commit is unchanged by the rebase. Both CI failures you traced to #171 and #173 are fixed there (details on those PRs).

Checks on this head, CPU only:

• ruff check --select F821 (0.16.5) and compileall both pass.
• make -C native cpu-goals builds. A clean make -C native all builds, the backend loads, and the sink binds the backend's RecordSink through both import orders; test_the_sink_derives_from_the_engines_record_sink passes.
• The previously failing native step: make -C tests/native/ring clean all CXX_STD=c++20 links every target. test_record_consumer reports 151 passed and test_clickhouse_record_sink 21 passed, with CUDA_VISIBLE_DEVICES empty.
• The previously failing gate: pytest -m cpu plus the workflow's skip gate reports 2833 collected, 1 skipped for absent hardware, 0 unexplained. The six CPU telemetry tests pass.
• Your five reproductions pass as described on [Core] Add CUDA-graph node toggling and device-side toggle support #171, [Core] Add per-hook request-row gather, PackedDemandPlanner, and fused inline projection heads #172 and [Core] Add whole-hook BatchQueue/SliceQueue handoff, pageable staging pool, and capture leases #173. That includes pool_failure under ASan at every injection point, with zero ASan errors.
• DMI-vLLM-Integration against this head: the PR's own CPU tests pass (64). The rest of its CPU suite gives the same result as against the previous stack build (629 passed). Its remaining failures are vLLM 0.29.0 version checks, not run under the local 0.27.1.

No GPU tests, CUDA initialization or benchmarks were run. GPU validation and the telemetry overhead measurement stay deferred until the owner gives the go-ahead.

[Samfisheryu]

Independent third review of c60d5118daafa388a35907f9667c5195277abe64.

This revision carries the updated dependency stack; its telemetry implementation is unchanged from the previously reviewed fix. The missing/non-callable getter returns unavailable, while errors from an existing getter still propagate. The 6 CPU telemetry cases pass; 3 device cases were not executed.

All 7 remaining round-two items in #171–#173 have now been independently checked with corrected expectations and previous-version controls. No confirmed non-GPU item remains open at these fixed revisions. The latest full-stack workflow passes CPU/package checks, live catalog checks, native compilation and standalone host test linking.

Local full-stack CPU result: 2,899 passed, 1 failed, 1 skipped (513 deselected). The failure is the unchanged fixed-temporary-directory permission case. Repeating its layout assertions with a private writable spool passes on this revision, as on the baseline; the existing shared temporary directory was left untouched. The skip requires a device. The independent allocation, queue-reference, counter and shape/capacity evidence is summarized on the introducing PRs.

This closes the previous non-GPU findings; it is not a final GPU or performance approval. Actual kernels, graph replay, device memory behavior, staging-saturated deadline timing and default-off overhead still await the owner's explicit instruction. No GPU tests or benchmarks were started.

[Samfisheryu]

Approved at c60d5118daafa388a35907f9667c5195277abe64 following the completed independent code and CPU/host review. The issues raised for this PR have been addressed and independently rechecked. The reviewed commit's CI is successful.

Review record: #174 (comment)

GPU execution and end-to-end performance benchmarks remain a separate follow-up, pending the owner's explicit instruction as agreed. This approval records the completed code/CPU review and does not claim those measurements.