Repository navigation
Feat/monitor - #1537
Merged
Merged
Feat/monitor#1537
Conversation
Remove legacy file-storage settings and prepare system monitoring. - Deleted SaveSettings and S3Settings (swanlab/sdk/internal/settings/save.py, .../s3.py) and removed Settings.Save and the nested Save config from swanlab/sdk/internal/settings/__init__.py. - Adjusted Settings.web_host default retrieval to coerce model default to str. - Removed SystemEnvironment.shim usage from swanlab/sdk/internal/run/system/__init__.py and dropped the shim field from the SystemEnvironment typing (swanlab/sdk/typings/run/system/__init__.py). - Added a new system monitor module stub at swanlab/sdk/internal/run/system/monitor/__init__.py with a new(shim: SystemShim) placeholder. - Minor doc/filename update in swanlab/sdk/internal/run/utils_fmt.py (file header and description). Migration notes: Update any code referencing Settings.Save, SaveSettings.S3, or SystemEnvironment.shim to the new configuration or provide alternative wiring for the removed storage/shim fields.
Rename Run.run_dir to Run.dir and add Run.name (experiment name). Update Run.url to build cloud run URL directly. Introduce hardware vendor module exports (CPU, Memory, Apple) and add factory classmethod new(run, shim) plus collect() stubs for platform-specific implementations. Update vendor protocol ABCs to include __init__, new and collect abstract methods and add TYPE_CHECKING Run imports and SystemShim usage in typings. Add a new system.monitor package and minor README/typing text tweaks.
Introduce a HardwareMonitor component and wire it into run/system lifecycle. system.new now constructs and returns an optional HardwareMonitor when monitoring is enabled. Run now holds a _hardware_monitor, starts it on run start, and stops/joins it during finish; the run state gains a "finishing" phase and an alive property (allows logging while finishing). The previous _cleanup logic is inlined into finish. Add an assertion when creating Timer threads. Update settings example key and tests to use class-level .get() for hardware vendor probes and expect the monitor to be returned. Remove the old monitor module and add the new hardware_monitor package.
Reorganize and annotate Run.__init__ with numbered section headers and more explicit inline comments (1. 基础状态准备, 2. 初始化运行时状态 with 2.1-2.4, and 3. 注册副作用). This is a non-functional refactor: only comments and minor whitespace were changed to clarify initialization steps (emitter/core/builder/consumer setup, startup event, system info, config binding, consumer start, and side-effect registration). No runtime logic was modified.
Rename the hardware monitoring API and update all usages for consistency. The hardware_monitor package was renamed to run/system/monitor, the class HardwareMonitor was renamed to Monitor, and exports (__all__) and imports were updated accordingly. The Run class attribute _hardware_monitor was renamed to _monitor and all start/cancel/join calls were adjusted. system.new() now returns a Monitor instance (renamed variable) and the Settings doc example was updated from Settings.HardwareMonitor to Settings.Monitor.
Introduce per-metric step tracking and unify system metric collection across CPU, Memory and Apple Silicon vendors. Key changes: - metrics: add _step and next() to ScalarMetric and MediaMetric; expose RunMetrics.next_metric_step() to advance per-metric steps. - monitor: complete rewrite of Monitor to gather CollectorProtocol implementations, register SystemScalar definitions, schedule periodic collection via ThreadPoolExecutor + Timer, emit MetricLogEvent with timestamps and per-metric steps, and provide fmt_system_key/is_system_key helpers. - hardware vendors: CPU, Memory and Apple modules now return (instance, SystemScalars), use psutil handlers to collect values, and register SystemScalar descriptors (color generation used). - typings: add SystemScalar/SystemScalars models and CollectorProtocol/CollectResult types; adapt vendor new()/collect signatures accordingly. - Run: update monitor.start invocation, change Run._state typing, simplify shutdown flow (use monitor.stop), and tighten is_alive logic. - transforms/text: remove legacy "__swanlab__" suffix from generated text filenames so front-end handles historical versions. - utils.generate_id: allow custom character set via new characters parameter and adjust docstring. These changes enable structured system metric definition/collection and safer async monitoring while adjusting related typings and run lifecycle callsites.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a hardware monitoring system for tracking CPU and memory metrics across various platforms, including specialized support for Apple Silicon. Key changes include the implementation of a Monitor class using periodic timers, refactoring the Run class to manage monitoring lifecycles, and renaming several public attributes such as run_dir to dir. Review feedback identifies a potential race condition during monitor initialization and recommends replacing assert statements with explicit exceptions in production code. Additionally, the reviewer suggests using non-blocking calls for CPU utilization metrics to prevent timing drift and notes a documentation error regarding duplicate property descriptions.
Call psutil.cpu_percent(interval=None) once to pre-warm and switch collector handlers to non-blocking cpu_percent(interval=None) to avoid blocking on metric collection. In Monitor, avoid creating a ThreadPoolExecutor when there are no collectors and log a debug message; keep the executor sizing logic unchanged. Add comprehensive unit tests for Apple.new, CPU.new and Memory.new collectors (including SystemShim usage and collect() behavior), and adjust small test imports/renames related to Apple.get.
Remove per-metric step tracking and next() from ScalarMetric and introduce a global system step in RunMetrics (_global_system_step) with next_system_step(). This ensures system/internal metrics use a single, monotonically increasing step. Update Monitor to emit one MetricLogEvent containing all system metrics (with keys formatted via fmt_system_key) and a single timestamp/step instead of emitting one event per metric. Also clarify next_step behavior to keep the global user step monotonic.
Use fresh psutil.Process() calls for per-call CPU/memory metrics instead of reusing a single Process instance to avoid stale or incorrect values. Add error handling and logging around collector handler invocations to prevent a single failing handler from breaking collection. Simplify monitor executor sizing to a fixed 2 workers (remove math import), make the monitor timer non-immediate to avoid producing useless initial data, and ensure executor.shutdown waits for tasks to finish. Minor comment/cleanup adjustments.
Ensure tests use the mocked Process instance when calling collector.collect by patching psutil.Process to return mock_proc. Added this patch to test_hardware_apple.py, test_hardware_cpu.py, and test_hardware_memory.py and grouped patches for clarity. This prevents accidental calls to the real psutil.Process during metric collection.
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.
搭建硬件监控大致框架，为 #1392 做准备，未来resume也可以继续监控硬件