Skip to content

Commit 942df79

Browse files
saltas888claude
andcommitted
refactor(telemetry): give the failed-reading contract and key layout one owner each [INFP-589]
The producer half of "what a failed reading looks like" (the unknown-host sentinel) lived in the component service while the consumer half (the all-None predicate) lived in the resources module, tied together only by prose. Both now live on WorkerResourceReading itself — a failed() constructor and an is_failed property derived from the field set, so a new figure is covered automatically. The resource cache-key layout was restated three times in component.py (writer f-string, list-keys glob, parse regex); all three now derive from a single RESOURCE_KEY_PREFIX so they cannot drift apart silently. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 98831da commit 942df79

2 files changed

Lines changed: 30 additions & 25 deletions

File tree

backend/infrahub/services/component.py

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,18 +21,17 @@
2121

2222
PRIMARY_API_SERVER = "workers:primary:api_server"
2323
WORKER_MATCH = re.compile(r":worker:([^:]+)")
24-
RESOURCE_COMPONENT_MATCH = re.compile(r"workers:resources:([^:]+):worker:")
24+
25+
# Single owner of the resource-key layout: the writer f-string, the list-keys glob
26+
# and the parse regex are all derived from this prefix so they cannot drift apart.
27+
RESOURCE_KEY_PREFIX = "workers:resources:"
28+
RESOURCE_COMPONENT_MATCH = re.compile(re.escape(RESOURCE_KEY_PREFIX) + r"([^:]+):worker:")
2529

2630
# The per-process resource read can transiently fail (a psutil hiccup, a momentary
2731
# hostname-lookup failure); a few immediate retries cover that before the reading
2832
# is written as null and the failure logged for traceability.
2933
RESOURCE_READ_MAX_ATTEMPTS = 3
3034

31-
# Host stand-in written when the resource read fails outright; such a reading
32-
# carries no figures and is dropped from the aggregate, so the value is never
33-
# summed and only needs to be non-raising.
34-
_UNKNOWN_HOST = "unknown"
35-
3635
log = get_logger()
3736

3837

@@ -129,7 +128,7 @@ async def refresh_heartbeat(self) -> None:
129128
expires=KVTTL.FIFTEEN,
130129
)
131130
await self.cache.set(
132-
key=f"workers:resources:{component}:worker:{WORKER_IDENTITY}",
131+
key=f"{RESOURCE_KEY_PREFIX}{component}:worker:{WORKER_IDENTITY}",
133132
value=self._read_own_resources().model_dump_json(),
134133
expires=KVTTL.FIFTEEN,
135134
)
@@ -160,7 +159,7 @@ def _read_own_resources(self) -> WorkerResourceReading:
160159
worker_id=WORKER_IDENTITY,
161160
error=str(last_error),
162161
)
163-
return WorkerResourceReading(host=_UNKNOWN_HOST)
162+
return WorkerResourceReading.failed()
164163

165164
async def read_worker_resources(self) -> dict[str, dict[str, WorkerResourceReading]]:
166165
"""Return the latest worker resource readings grouped by component and host.
@@ -169,7 +168,7 @@ async def read_worker_resources(self) -> dict[str, dict[str, WorkerResourceReadi
169168
report identical values, so a later reading for a host simply overwrites
170169
the earlier one.
171170
"""
172-
keys = await self.cache.list_keys(filter_pattern="workers:resources:*")
171+
keys = await self.cache.list_keys(filter_pattern=f"{RESOURCE_KEY_PREFIX}*")
173172
values = await self.cache.get_values(keys=keys)
174173

175174
grouped: dict[str, dict[str, WorkerResourceReading]] = {}

backend/infrahub/telemetry/resources.py

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,27 @@ class WorkerResourceReading(BaseModel):
6060
memory_total: int | None = None
6161
memory_available: int | None = None
6262

63+
@classmethod
64+
def failed(cls) -> WorkerResourceReading:
65+
"""The reading written when a process's self-read failed outright.
66+
67+
It carries no figures and a host stand-in, so it is recognisable as
68+
failed (see ``is_failed``) and is dropped from aggregation rather than
69+
summed or used for host dedup.
70+
"""
71+
return cls(host="unknown")
72+
73+
@property
74+
def is_failed(self) -> bool:
75+
"""Whether this reading is a failed self-read rather than a contribution.
76+
77+
A healthy host always reports at least its logical CPU count and memory
78+
capacity; only a read that failed after its retries carries every figure
79+
as ``None``. Derived from the field set so a new figure is covered
80+
automatically.
81+
"""
82+
return all(value is None for value in self.model_dump(exclude={"host"}).values())
83+
6384

6485
@dataclass(frozen=True)
6586
class ResourceAggregate:
@@ -227,21 +248,6 @@ def _sum_over_hosts(values: list[int | None]) -> int | None:
227248
return sum(value for value in values if value is not None)
228249

229250

230-
def _is_failed_reading(reading: WorkerResourceReading) -> bool:
231-
"""A reading carrying a host but no figure at all is a failed self-read.
232-
233-
A healthy host always reports its logical CPU count and memory capacity; only a
234-
read that failed after its retries carries every figure as ``None``. Such a
235-
reading is not a real contribution and must not null the fleet.
236-
"""
237-
return (
238-
reading.processor_available is None
239-
and reading.processor_assigned is None
240-
and reading.memory_total is None
241-
and reading.memory_available is None
242-
)
243-
244-
245251
def aggregate(readings: Iterable[WorkerResourceReading]) -> ResourceAggregate:
246252
"""Collapse per-process readings into one figure per field for a component.
247253
@@ -253,7 +259,7 @@ def aggregate(readings: Iterable[WorkerResourceReading]) -> ResourceAggregate:
253259
"""
254260
by_host: dict[str, WorkerResourceReading] = {}
255261
for reading in readings:
256-
if _is_failed_reading(reading):
262+
if reading.is_failed:
257263
continue
258264
by_host.setdefault(reading.host, reading)
259265

0 commit comments

Comments
 (0)