feat(observability): drop the aiperf/tachometer scrape exclusion - #396
Merged
Merged
Conversation
Tachometer used to scrape the complement of AIPERF_SERVER_METRICS_URLS so a worker /metrics endpoint was never polled by both the benchmark client and the in-job scraper. Double-polling has since been validated as harmless, and the complement rule had a real cost: whichever endpoints the client claimed disappeared from the whole-window tachometer capture, so per-replica engine series were missing exactly when a benchmark ran. Remove the exclusion end to end: generate_tachometer_config loses its exclude_urls parameter, start_tachometer no longer derives the client set, and BenchmarkStageMixin._client_polled_metric_urls (whose only consumer was that exclusion) is deleted. Tachometer now scrapes every configured endpoint unconditionally. Signed-off-by: Yuewei Na <nv-yna@users.noreply.github.com>
nv-yna
requested review from
alec-flowers,
csahithi,
ishandhanani and
nlevin-ui
as code owners
September 6, 2026 12:28
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #396 +/- ##
=======================================
Coverage ? 73.44%
=======================================
Files ? 101
Lines ? 14032
Branches ? 0
=======================================
Hits ? 10306
Misses ? 3726
Partials ? 0 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
FrankD412
added a commit
to FrankD412/srt-slurm
that referenced
this pull request
Sep 10, 2026
Commit 7640839 ported this test from ajc/rust-cpu-exporter, but upstream dropped the aiperf/tachometer scrape exclusion in NVIDIA#396, so generate_tachometer_config no longer accepts exclude_urls and the test failed with a TypeError. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
FrankD412
added a commit
that referenced
this pull request
Sep 10, 2026
…-per-watt report (#410) * feat(telemetry): collect host CPU power * fix(sa-bench): publish power measurement windows * feat: record local wall-clock time in CPU power samples Adds a timestamp_local column (ISO 8601 with UTC offset) to the CPU power telemetry CSV, derived from timestamp_unix. Lets an offline consumer read a node's real UTC offset directly from the samples file instead of guessing the cluster's timezone when correlating against benchmark logs that only carry local time-of-day text. Bumps schema_version 1 -> 2; cpu_power_session.py's merge logic is updated for the shifted column indices. Signed-off-by: Frank Di Natale <3429989+FrankD412@users.noreply.github.com> * feat: add CPU/GPU trapezoidal energy-per-token report Adds srtctl.analysis.power_energy_report, a standalone module that joins a run's CPU (power/cpu/samples.csv) and GPU (power/samples.csv) power telemetry against each concurrency point's profiling window and token counts, integrating power into energy with numpy.trapezoid. Timestamps are read directly from existing wall-clock sources rather than reconstructed: aiperf's profile_export.jsonl already carries time.time_ns() per record, and sa-bench's result JSON already carries benchmark_start_time_unix/benchmark_end_time_unix (mtime-based derivation was tried and found unreliable once run directories get copied/archived). GPU energy is broken down per-device, per-node, and per-role using the topology already recorded in power/manifest.json. Wires this into run_postprocess (postprocess_stage.py) as a best-effort step alongside the perf dashboard build, writing <log_dir>/power_energy_report.json on every job where CPU/GPU telemetry and a supported benchmark type are present; quietly skipped otherwise. Signed-off-by: Frank Di Natale <3429989+FrankD412@users.noreply.github.com> * feat(telemetry): collect detailed hwmon power domains * fix(telemetry): map Grace hwmon power domains * feat(telemetry): support agentic/agentx/custom benchmark types telemetry validation previously required benchmark.type: sa-bench, rejecting agentic, agentx, and custom benchmark runs outright. * feat(cpu-power): port Rust cpu-power-exporter crate onto Kyle's branch Ships the statically-linked Rust binary (aarch64/x86_64 musl) that reads /sys/class/hwmon hwmon*/power*_average and serves cpu_power_acpi_watts on :9405/metrics for AIPerf --server-metrics scraping. Covers: - src/cpu-power-exporter/ Rust crate (tokio, tracing, clap, anyhow) - Workspace member added to Cargo.toml - docker/Dockerfile.cpu-power-exporter (musl static, two-arch) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Anthony Casagrande <acasagrande@nvidia.com> * feat(cpu-power): add two-arch CI matrix and make targets for cpu-power-exporter - Makefile: cpu-power-exporter / cpu-power-exporter-download targets, CPU_POWER_EXPORTER_RELEASE var, setup now depends on both downloads - release.yaml: unified change detection (single step, two outputs), build-cpu-power-exporter job (linux/amd64 + linux/arm64), graceful carry-forward for both binaries from the previous release, nullglob-safe gh release create Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Anthony Casagrande <acasagrande@nvidia.com> * feat(cpu-power): wire Rust exporter into telemetry launch and AIPerf metrics - telemetry_stage: add _resolve_bundled_binary (generalized from _resolve_tachometer_binary), add _start_cpu_power_prometheus_exporters which launches bin/cpu-power-exporter via srun on each worker node, call it from start_cpu_power_telemetry when prometheus_port > 0, fix dcgm_exporter guard (return None instead of raise when None) - benchmark_stage: inject cpu-power-exporter node URLs into AIPERF_SERVER_METRICS_URLS when cpu_power.prometheus_port > 0 - submit.py: validate_setup checks bin/cpu-power-exporter alongside tachometer - schema.py: add CpuPowerConfig.prometheus_port (default 9405), gate _validate_dcgm_power on dcgm_exporter is not None Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Anthony Casagrande <acasagrande@nvidia.com> * chore(cpu-power): add test recipe for lyris GB200 cpu-power-exporter e2e Uses trace-replay benchmark type so AIPerf actually consumes the AIPERF_SERVER_METRICS_URLS endpoints. Qwen3.5-27B BF16 to avoid the 8-minute FP4 JIT autotuning pass. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Anthony Casagrande <acasagrande@nvidia.com> * feat(telemetry): add stdlib Python Prometheus exporter for Grace CPU power Ports cpu_power_exporter.py from kylliang/power_study_20260901 (commit 2a13cb1). Default port updated to 9405 (consistent with the rest of the cpu-power stack). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Anthony Casagrande <acasagrande@nvidia.com> * feat(telemetry): fall back to Python exporter when Rust cpu-power-exporter is absent If the bundled Rust binary is not present or not executable (e.g. x86_64 nodes or a checkout without a pre-built bin/), _start_cpu_power_prometheus_exporters now falls back to `python3 -m srtctl.core.cpu_power_exporter` so Prometheus scraping still works everywhere. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Anthony Casagrande <acasagrande@nvidia.com> * feat(cpu-power-v2): port schema validation, IPv6 url_host, validate_setup, and tests from ajc/rust-cpu-exporter - Add url_host() to ip_utils for IPv6 URL bracket safety - Use url_host() in benchmark_stage.py CPU power URL construction - Add arch_from_binary() to dynamo_wheels.py - Rewrite CpuPowerConfig: drop 'dcgm' source, add storage_subdir, acpi_mandatory - Add _dynamo_system_ports(), _validate_collector_budget(), _validate_cpu_power(), _reject_inert_cpu_power_demand() to schema validation - Rewrite _validate_telemetry() to support CPU-only, DCGM-only, or combined modes - Fix _validate_observability() to not conflict CPU-only telemetry with Tachometer DCGM - Improve telemetry_stage.py: drop permitted_device_keys, use cpu_power.storage_subdir - Update Makefile: version marker, arch check, warn-on-failure for latest release - Add cpu-power-exporter-setup target; switch setup to use it - Update validate_setup() to conditionally require cpu-power-exporter - Add _cpu_power_exporter_problem() with executability and arch mismatch detection - Port IPv6 tachometer tests and cpu-only telemetry stage test from rust-cpu-exporter - Port validate_setup tests: disabled/enabled/arch-mismatch/non-executable cases Signed-off-by: Anthony Casagrande <acasagrande@nvidia.com> * ci: add cpu-power-exporter.yaml workflow for PR and push validation Mirrors tachometer.yaml: runs cargo fmt/test on source changes, and a two-arch Docker cross-compile to validate the release Dockerfile. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Anthony Casagrande <acasagrande@nvidia.com> * fix(tests): repair test function name mangled by cherry-pick auto-merge git cherry-pick of e11d928 concatenated the new test's def line with the pre-existing one during auto-merge, breaking the file's syntax. * fix(cpu-power-exporter): declare tracing workspace deps Cargo.toml already referenced tracing/tracing-subscriber via workspace = true, but the workspace root never declared them. * feat(cpu-power): add cpu/samples.csv wire-format constants Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * feat(cpu-power): add cpu/samples.csv row, writer, and reader * feat(cpu-power): parse dcgm/acpi cpu-power-exporter scrapes Implement CpuReading, ParsedCpuScrape, and parse_cpu_scrape to parse Prometheus-format /metrics text from the cpu-power-exporter Rust binary. Supports both DCGM (one aggregated value per socket) and ACPI (per-channel detail: cpu/sysio/grace) modes, with ACPI preferred when both present. In ACPI mode, total_power_w sums only grace-kind channels, reflecting the hardware's actual power boundary measurement for Grace SoC. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * feat(cpu-power): add best-effort CpuPowerCollector * refactor(power): replace host-side CPU power system with best-effort head-node scraper This branch had two CPU power telemetry implementations: an older host-side collector (src/srtctl/core/cpu_power.py, cpu_power_session.py, TelemetryConfig.cpu_power) that runs on every worker node and reads ACPI/DCGM sysfs directly, and a newer head-node scraper (src/srtctl/core/power/cpu_session.py, CpuPowerCollector) that polls each worker's cpu-power-exporter /metrics endpoint from the head node, matching the DCGM power leg's architecture. Keep the newer collector, delete the older system, and repoint the two things worth keeping from it: - AIPerf's AIPERF_SERVER_METRICS_URLS injection (benchmark_stage.py) - the submit-time preflight check for the bundled cpu-power-exporter binary (submit.py) both now read the new, much smaller TelemetryConfig.cpu_power_exporter (presence-gated, not a separate enabled flag) instead of the removed CpuPowerConfig. telemetry_stage.py gains start_cpu_power_telemetry / finalize_cpu_power_telemetry methods that launch the exporter with the same bundled-binary-with-Python-fallback pattern the old code used, but on the bare host (no container) and fully best-effort: launch failures are absorbed and CPU power never mutates the job's exit code. Also fixes a MagicMock-config test in test_power_collector.py that started reaching real endpoint-allocation code once start_cpu_power_telemetry stopped early-returning on a mocked telemetry.cpu_power.enabled. * test(dry-run): show cpu_power_exporter port in submit dry-run details Mirrors the existing dcgm_exporter row; cpu_power_exporter has no container_image so the row is just the port. * fix(power): restore standalone cpu-power-only telemetry mode; fix submit.py preflight gate - schema.py: _validate_telemetry now only raises when both dcgm_exporter and cpu_power_exporter are None, restoring standalone CPU-power-only telemetry. _validate_cpu_power_exporter() runs unconditionally when telemetry is enabled. - submit.py: preflight cpu_power_enabled gate now also requires config.telemetry.enabled, not just cpu_power_exporter being set. - telemetry_stage.py: start_tachometer's dcgm exporter sharing check now requires telemetry.dcgm_exporter to actually be configured, not just telemetry.enabled -- otherwise a CPU-power-only telemetry config silently suppressed the tachometer's own dcgm_exporter with no exporter running at all. - tests: add coverage for CPU-power-only telemetry (schema + validate_setup), and restore test_cpu_only_telemetry_leaves_tachometers_dcgm_exporter_running adapted to the new CpuPowerExporterConfig field. * fix(power): restore CPU power exporter port-collision validation; fix blank-total crash - schema.py: _validate_cpu_power_exporter now checks source is one of auto/acpi/dcgm, and rejects a port colliding with telemetry.dcgm_exporter, observability.tachometer.{dcgm,node}_exporter, or a Dynamo system port (via the previously-unused _dynamo_system_ports helper). Ported from the deleted _validate_cpu_power validator. - power_energy_report.py: load_cpu_samples now skips rows with a blank total_power_w instead of crashing with ValueError(float("")) -- blank is the documented normal case for an ACPI scrape with no grace channel. * fix(power): add source passthrough, fix stale recipe, and dry-run/het-launch bugs - schema.py: add CpuPowerExporterConfig.source (auto|acpi|dcgm), mirroring the Rust cpu-power-exporter binary's own --source flag; validated in _validate_cpu_power_exporter. - telemetry_stage.py: pass --source through to the bundled Rust binary; warn (not silently drop) when a non-auto source is requested but the ACPI-only Python fallback exporter is used instead. Also move the het-group resolution block inside the try/except so an unresolvable node is absorbed like any other launch failure, per the method's own best-effort docstring contract, instead of aborting the sweep. - configs/cpu-power-test.yaml: fix stale telemetry.cpu_power block (no longer loads -- "Unknown field") to the current telemetry.cpu_power_exporter shape, dropping the removed `required` flag and preserving `source: acpi`. - submit.py: move the cpu_power_exporter dry-run row inside the `telemetry.enabled` guard so it doesn't display a port that will never actually launch. - Tests: new TestCpuPowerExporterConfig source/collision cases, new TestCpuPowerExporterLaunch source-passthrough/fallback-warning/het-absorb cases, an explicit configs/cpu-power-test.yaml load test (configs/ isn't covered by the recipes/**/*.yaml glob and holds non-recipe files, so a narrow explicit test is safer than widening that glob), and a dry-run test confirming the row is hidden when telemetry is disabled. * docs(power): rewrite CPU power docs for the current cpu_power_exporter system - config-reference.md: replace the deleted telemetry.cpu_power block (BTK ordering, per-rail sensor names, sum-based total) with telemetry.cpu_power_exporter (port, source), the grace-rail-is-the-total convention, and best-effort/no-required semantics. - cpu-power-telemetry.md: full rewrite describing the current CpuPowerCollector head-node HTTP-scrape design, cpu/samples.csv format (including the blank-total-without-grace caveat), and the bundled Rust-binary-with-Python-fallback launch model. The old doc described a fully deleted system (srtctl.core.cpu_power, CpuPowerConfig, wait_for_readiness/.ready.json, per-node CSV merging, embedded-mode DCGM bindings) that no longer exists. - dcgm-4.7-runtime-support.md: fix the stale telemetry.cpu_power reference to telemetry.cpu_power_exporter. * style(power): apply ruff format to submit.py dry-run row Reformats the cpu_power_exporter dry-run row (added in the previous commit) to fit the 120-char line limit per ruff format. * style(power): fix ruff lint (import order, RUF012 ClassVar) * refactor(power): split sample loaders into path and handle variants * feat(power): add tolerant CSV reader for live samples.csv Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(power): add IncrementalPowerEmitter for per-case energy emission * feat(power): add IncrementalPowerWatcher daemon thread * feat(power): wire incremental power report into the postprocess mixin * feat(power): wire incremental power report into the sweep lifecycle * test(power): exercise multi-case emit loop and deterministic final-poll assertion _make_sa_bench_log_dir only ever built a single concurrency point, so the per-case emit loop in IncrementalPowerEmitter.poll() was never driven with more than one pending case -- a bug that dropped or over-added an unready case would have passed the whole suite. Generalize the fixture to accept multiple (concurrency, start, end) cases and add a test where samples bracket only the first of two. test_watcher_runs_a_final_poll_on_stop's ">= 1" assertion is a tautology: the loop's own first poll already satisfies it even if stop_and_finalize's final pass were deleted. Add a deterministic, timing-independent assertion in test_stop_without_start_is_safe (where self._thread is None, so the final pass is the ONLY possible poll) and note why in the timing-based test. * fix(power): mark permanently-uncovered cases dead instead of re-scanning forever A case that can never become ready (e.g. the collector died before the window closed, so the end-gap never shrinks) previously stayed in pending forever: every tick paid a full benchmark.out scan, three rglobs, and a read+parse+numpy build of samples.csv -- possibly O(100MB) -- in the daemon thread, at DEBUG-only visibility. Since samples only grow forward in time, "permanently unready" is decidable: once the newest sample seen is already past window.end_unix + MAX_SAMPLE_GAP_SECONDS, no future sample can shrink that gap. Track such cases like _emitted so later polls skip them, and log the reason once at WARNING. Also key _emitted (and the new _dead set) on source path rather than concurrency alone: discover_run's sources come from an rglob, so two sources could in principle share a concurrency (nested run dirs, an archived run copied under log_dir, a future multi-ISL layout), and the second would previously have been silently dropped. * fix(power): finalize incremental power report after node teardown, not before finalize_incremental_power_report() previously ran before stop_event.set() / registry.cleanup() / _run_host_teardown() in do_sweep.py's cleanup block. Its final pass plus up to a DEFAULT_JOIN_TIMEOUT_SECONDS thread join therefore delayed node-state reversion -- bad on the SLURM walltime-kill path this feature exists to survive, where a fixed grace clock is running. Move the call to immediately before run_postprocess(), after _run_host_teardown(). The ordering requirement that it must still run after finalize_power_telemetry/finalize_cpu_power_telemetry (so it reads closed, durable CSVs) continues to hold since both already ran earlier in the same block. Also tighten DEFAULT_JOIN_TIMEOUT_SECONDS from 30.0 to 10.0 so the worst case added to teardown is smaller. * docs(power): note the incremental index is idempotent per-process only The module docstring said "idempotent" without qualifying that the guarantee only holds within a single process's lifetime: the co-located power_energy_c<N>.json file is overwrite-idempotent, but power_energy_report.jsonl is append-only against an in-memory set, so a job requeued into the same log dir appends duplicate concurrency rows. Document that consumers should dedupe by concurrency, last-wins by emitted_at_unix (index_path's own docstring already says this, added alongside the dead-case-detection commit). * feat(power): add utilization metric constants and v2 samples header Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * feat(power): parse optional GPU utilization alongside power * feat(power): persist optional utilization columns; read v1 and v2 samples * feat(power): forward parsed utilization into persisted sample rows Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * feat(power): advertise samples schema version and utilization columns in manifest Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * chore(power): document UTILIZATION_METRICS coupling; assert validity with utilization present Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(power): finish the collect_interval_ms migration for the CPU power leg Upstream #401 retired telemetry.default_frequency in favour of collect_interval_ms. The DCGM leg picked that up in the rebase; the CPU power collector still read the removed field and raised AttributeError at launch. Derive its sample interval the same way the DCGM leg does, move the dry-run and telemetry fixtures to the new knob, expect the built-in node exporter that #358 now launches by default, and drop the retired name from the CPU power docs. * fix(power): check CPU exporter port collisions against the resolved tachometer exporters Upstream #358 makes the tachometer launch built-in DCGM (9401) and node (9101) exporters when no explicit block is configured. The CPU power exporter's collision check compared against the raw fields, which are None in that case, so a cpu_power_exporter on 9401 passed validation and collided at runtime. Compare against resolved_dcgm_exporter / resolved_node_exporter and cover both defaults. * feat(power): restore the host-side Python CPU power collector as an independent leg Commit 0975241 replaced the in-job host collector (srtctl.core.cpu_power, cpu_power_session, TelemetryConfig.cpu_power) with the head-node scraper over cpu-power-exporter. The scraper stays, but the host collector is needed as an alternative: it has fail-closed `required` semantics, needs no exporter binary or reserved port, and reads ACPI/DCGM directly on each node. Bring back cpu_power.py, cpu_power_session.py, and tests/test_cpu_power.py from the tree just before the cleanup (the newest version, with the Grace socket-total domain mapping and timestamp_local). Reinstate `telemetry.cpu_power` in Kyle Liang's original shape -- enabled, source (auto|acpi|dcgm), sample_interval_seconds, startup_timeout_seconds, required, storage_subdir -- without the `prometheus_port` field that was later bolted on: the exporter and its AIPerf URL injection now belong to `telemetry.cpu_power_exporter`, so the two blocks share no ports and no output directories and may be enabled independently or together. The stage gains start_cpu_power_host_telemetry / finalize_cpu_power_host_ telemetry alongside the scraper's methods, power_telemetry_blocks_benchmark honours cpu_power.required again, do_sweep calls both legs, and dry-run shows the block. power_energy_report is deliberately untouched: aligning this leg's CSV header and location with the scraper (and adding utilization) is follow-on work, noted in docs/cpu-power-telemetry.md. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * feat(power): collect CPU utilization in the host collector and report utilization per window The host-side collector (telemetry.cpu_power) now watches DCGM CPU entity fields 1100-1104 (total/user/nice/sys/irq) in the same field group as power field 1130 and reads them in the same latest-values call. Five trailing columns join every per-socket sample row (cpu_util_total, cpu_util_user, cpu_util_nice, cpu_util_sys, cpu_util_irq); the samples schema moves from v2 to v3. ACPI has no utilization, so those cells stay blank there, and the per-node metadata lists the field ids and the DCGM unit (fraction of socket CPU time). The GPU samples.csv is untouched: gpu_util_pct and sm_active already live there. The energy report gains utilization next to the joules. It reads the CPU and GPU utilization columns by name (absent columns and blank cells are skipped, so v1/v2 files still load), and for each concurrency window reports the mean and max of the samples inside the window per CPU socket and node, and per GPU, node, and role. Utilization is a gauge, so it is summarized rather than integrated, and a window with no utilization samples is a warning rather than an error. Discovery now also accepts the host collector's cpu_power/samples.csv (previously misclassified as a GPU file); when a run has both CPU legs, --cpu-samples picks one. _sorted_series becomes generic in its key type, which clears the long-standing invariance errors ty reported in this module. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * feat(power): report perf/W and a reported-vs-computed timing comparison per window Each concurrency window in the energy report now carries perf per watt: output and total tokens/s over the computed window, GPU average watts and CPU+GPU combined average watts (joules / duration), and the four tokens/s per watt ratios. These are the reciprocal of the existing J/token figures and are derived from the same window, so the two can never disagree. The combined variants are None, with a warning, unless both a CPU and a GPU leg produced samples, so they never silently equal the GPU-only number. Alongside, three timelines for comparison (never validation): computed -- the window the trapezoid integrates over, as before reported -- the benchmark's own account: sa-bench's `duration` and wall-clock start/end; aiperf's aggregate benchmark_duration and start_time/end_time; failing that, the profiling-phase NOTICE lines in benchmark.out when exactly one phase ran (time-of-day only, so duration without absolute start/end) coverage -- the first/last power samples actually spanned, and every breakdown row records its own sample_start/end and count Text output gains `timing:` and `perf/W:` lines; JSON gains `timing` and `perf_per_watt` objects and per-breakdown sample fields. The incremental during-job report inherits all of it through report_to_dict. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * feat(power): add sample-based power percentiles to every energy breakdown row Each breakdown row (socket, GPU, node, role) now carries a distribution of the power samples the trapezoid actually spanned: mean_w, min_w, p5_w, p50_w, p95_w, p99_w, max_w. The existing avg_power_w stays as the time-weighted average (joules / duration), and mean_w is kept next to it so any gap between the two under uneven sampling is visible rather than hidden. Node and role rows are computed over their summed series, so a node p99 is the 99th percentile of the node's total power, not a sum of per-device percentiles. The text table appends p50/p95/p99/max to each breakdown line; JSON gets all seven fields, and the incremental report inherits them. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(power): align Rust CPU power exporter domain naming with Python collector The ACPI OEM-string naming for CPU power rails varies by platform (e.g. "Grace Power Socket N" vs. a generic "Total Power socket N", some suffixed with "in uW"). Kyle's host-side Python collector already accounted for all these variants; the Rust exporter only recognized 3 literal substrings and was missing the "Total Power" spelling, the "in uW" suffix, and the CPU-rail/SoC-rail/DRAM domains entirely, silently dropping total_power_w on platforms that don't report a "Grace" rail. Renames the exporter's type= taxonomy to match Python's semantic kinds (total/cpu_rail/soc/dram) and updates the downstream Prometheus scrape parser to sum on kind=="total" instead of kind=="grace". Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * style(cpu-power): apply cargo fmt to the exporter crate Two commits landed without running rustfmt, so both Rust workflows fail at `cargo fmt --all --check` on six over-width lines in dcgm.rs and main.rs. No behaviour change. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * test(cpu-power): drop the collector-thread test the ported crate no longer has The wedged-collector test and its serve() helper reference Collector, CollectRequest, mpsc and thread, none of which exist in this crate: the port replaced the request/reply collector thread with a background poller that fills an RwLock cache, so a scrape never touches sysfs and the 503 timeout path the test asserted on is gone. The test binary therefore failed to compile, which CI never reached because rustfmt failed first. Also removes the now-unused COLLECT_TIMEOUT constant and oneshot import. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * build(cpu-power): install libc6-dev-arm64-cross for the arm64 static link The arm64 release build fails with `ld: cannot find -ldl`. libloading emits `-ldl` on every linux target, and a static link needs libdl.a, but `--no-install-recommends gcc-aarch64-linux-gnu` only pulls the glibc runtime cross package (libc6-arm64-cross); the dev package that ships /usr/aarch64-linux-gnu/lib/libdl.a is a Recommends. The amd64 build was unaffected because the base image already carries the host libc6-dev. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * test(telemetry): drop the exclude_urls test for a feature main removed Commit 7640839 ported this test from ajc/rust-cpu-exporter, but upstream dropped the aiperf/tachometer scrape exclusion in #396, so generate_tachometer_config no longer accepts exclude_urls and the test failed with a TypeError. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(telemetry): bracket IPv6 worker and frontend hosts in tachometer URLs get_hostname_ip can return an IPv6 literal, and the backend and frontend scrape targets interpolated it bare, so the colons inside the address were read as the port separator. Route both through url_host(), which the benchmark stage already uses for the cpu-power-exporter targets. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> --------- Signed-off-by: Frank Di Natale <3429989+FrankD412@users.noreply.github.com> Signed-off-by: Anthony Casagrande <acasagrande@nvidia.com> Co-authored-by: Kyle Liang <kylliang@nvidia.com> Co-authored-by: Anthony Casagrande <acasagrande@nvidia.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
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
2 participants
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.
What
Removes the complementary-scrape rule between the benchmark client and tachometer: tachometer now scrapes every configured endpoint, even when
AIPERF_SERVER_METRICS_URLSpoints the client at the same URL.generate_tachometer_configloses itsexclude_urlsparameter and the skip inside the backend-endpoint loopTelemetryStageMixin.start_tachometerno longer derives the client-polled setBenchmarkStageMixin._client_polled_metric_urlsdeleted (its only consumer was the exclusion)test_client_polled_urls_are_still_scrapedWhy
Double-polling a worker
/metricsendpoint has been validated as harmless, and the complement rule had a real cost: whichever endpoints the client claimed disappeared from the whole-window tachometer capture, so per-replica engine series went missing exactly when a benchmark was running — the one window the capture exists for.This also simplifies the ground for a follow-up PR that adds trtllm-serve-mode scrape targets.
Testing
make check— 1686 passed, 2 skipped.🤖 Generated with Claude Code