Skip to content

Commit b6be122

Browse files
committed
fix(system-services-monitor): address architectural review findings
- Makefile: set MODULE_NAME := $(PYTHON_PACKAGE_NAME). common.mk defaults MODULE_NAME to the hyphenated dir basename, so python.mk's `coverage run --source=$(MODULE_NAME)` never matched the importable package and reported 0% coverage. Matches preflight-checks/nccl-allreduce. - pyproject.toml: relax python floor ^3.13 -> ^3.10 to match siblings (nccl-allreduce, dcgm-diag). Package uses no 3.11+/3.13-only syntax or stdlib (verified: no tomllib, PEP 695 type params, StrEnum, datetime.UTC, ExceptionGroup, etc.). - Dockerfile: pin poetry==2.3.3 + poetry-plugin-export==1.10.0 per .versions.yaml (was poetry==1.8.2 with no export plugin while running `poetry export`). Export invocation already matches the poetry-2.x sibling Dockerfiles. - cli.py: rename `exit = Event()` -> `stop_event` to stop shadowing the builtin (CodeRabbit flagged this on the prior #891). - platform_connector/event_processor.py: guard the UDS dial with _is_platform_connector_socket_present() before/between retries, mirroring gpu-health-monitor; log-and-skip instead of noisy gRPC stack traces when the socket isn't present yet. Adds events_sent_skipped_pc_unavailable counter.
1 parent a8402b1 commit b6be122

6 files changed

Lines changed: 46 additions & 7 deletions

File tree

health-monitors/system-services-monitor/Dockerfile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ FROM public.ecr.aws/docker/library/python:3.13-bookworm AS build
1919
ARG VERSION
2020

2121
RUN --mount=type=cache,target=/root/.cache/pip \
22-
pip install poetry==1.8.2
22+
pip install poetry==2.3.3 poetry-plugin-export==1.10.0
2323

2424
ENV POETRY_NO_INTERACTION=1 \
2525
POETRY_VIRTUALENVS_IN_PROJECT=1 \

health-monitors/system-services-monitor/Makefile

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@ HAS_DOCKER := 1
2525

2626
# Python package name (with underscores, not hyphens)
2727
PYTHON_PACKAGE_NAME := system_services_monitor
28+
# Coverage in make/python.mk uses --source=$(MODULE_NAME); common.mk defaults
29+
# MODULE_NAME to the (hyphenated) directory basename, which does not match the
30+
# importable package, yielding 0% coverage. Pin it to the package name.
31+
MODULE_NAME := $(PYTHON_PACKAGE_NAME)
2832

2933
# system-services-monitor specific settings (Python module)
3034
CLEAN_EXTRA_FILES := system_services_monitor.egg-info

health-monitors/system-services-monitor/pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ authors = ["Community Contributors"]
66
readme = "README.md"
77

88
[tool.poetry.dependencies]
9-
python = "^3.13"
9+
python = "^3.10"
1010
click = "^8.3.1"
1111
grpcio = "^1.78.0"
1212
prometheus-client = "^0.24.1"

health-monitors/system-services-monitor/system_services_monitor/cli.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ def cli(
6666
processing_strategy,
6767
verbose,
6868
):
69-
exit = Event()
69+
stop_event = Event()
7070

7171
# Resolve node name from CLI or environment
7272
if node_name is None:
@@ -108,7 +108,7 @@ def cli(
108108
prom_server, t = start_http_server(port)
109109

110110
def process_exit_signal(signum, frame):
111-
exit.set()
111+
stop_event.set()
112112
prom_server.shutdown()
113113
t.join()
114114

@@ -126,7 +126,7 @@ def process_exit_signal(signum, frame):
126126
enable_fabric_check=enable_fabric_check,
127127
)
128128

129-
watcher.start(exit)
129+
watcher.start(stop_event)
130130

131131

132132
if __name__ == "__main__":

health-monitors/system-services-monitor/system_services_monitor/platform_connector/event_processor.py

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
import dataclasses
2222
import logging as log
23+
import os
2324
import threading
2425
from time import sleep
2526
from typing import List
@@ -162,15 +163,44 @@ def health_check_completed(self, results: List[CheckResult]) -> None:
162163
for key in pending_cache_updates:
163164
self.entity_cache.pop(key, None)
164165

166+
def _is_platform_connector_socket_present(self) -> bool:
167+
# platform-connector removes the socket file on shutdown and on
168+
# startup before binding, so file-presence is a faithful proxy
169+
# for "PC is up" on this node.
170+
return os.path.exists(self._socket_path)
171+
165172
def send_health_event_with_retries(self, health_events: list[platformconnector_pb2.HealthEvent]) -> bool:
166173
"""Send health events to the platform connector with retries.
167174
175+
If the platform-connector Unix socket is absent at send time the send
176+
is skipped immediately (no gRPC call, no buffering, no cache mutation)
177+
and ``False`` is returned so the caller re-emits on the next cycle.
178+
168179
Returns:
169-
True if the send was successful, False if all retries were exhausted.
180+
True if the send was successful, False if the socket was missing or
181+
all retries were exhausted.
170182
Cache updates should only be performed by the caller when this returns True.
171183
"""
184+
if not self._is_platform_connector_socket_present():
185+
metrics.events_sent_skipped_pc_unavailable.inc()
186+
log.warning(
187+
"Platform-connector socket %s is missing; skipping send.",
188+
self._socket_path,
189+
)
190+
return False
191+
172192
delay = INITIAL_DELAY
173-
for _ in range(MAX_RETRIES):
193+
for attempt in range(MAX_RETRIES):
194+
# Re-check between retries so a connector that disappears
195+
# mid-flight short-circuits instead of burning the budget.
196+
if attempt > 0 and not self._is_platform_connector_socket_present():
197+
metrics.events_sent_skipped_pc_unavailable.inc()
198+
log.warning(
199+
"Platform-connector socket %s disappeared mid-retry; aborting send.",
200+
self._socket_path,
201+
)
202+
return False
203+
174204
with grpc.insecure_channel(f"unix://{self._socket_path}") as chan:
175205
stub = platformconnector_pb2_grpc.PlatformConnectorStub(chan)
176206
try:

health-monitors/system-services-monitor/system_services_monitor/platform_connector/metrics.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,3 +31,8 @@
3131
"fabric_monitor_events_sent_error",
3232
"Total number of failed health event sends to platform-connector UDS",
3333
)
34+
35+
events_sent_skipped_pc_unavailable = Counter(
36+
"fabric_monitor_events_sent_skipped_pc_unavailable",
37+
"Total number of health event sends skipped because the platform-connector Unix socket was missing",
38+
)

0 commit comments

Comments
 (0)