feat(telemetry): licensing resource-allocation telemetry [INFP-589] - #10003
feat(telemetry): licensing resource-allocation telemetry [INFP-589]#10003saltas888 wants to merge 16 commits into
Conversation
…P-589] Speckit spec + plan for per-component CPU/RAM telemetry (database, server, workers) to audit deployments against their licensed tier. Includes spec, plan, research decisions, data model, payload contract, and quickstart. Design artifacts only; no code changes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
] Critique report (product + engineering lenses). Applied inline: - additive-only invariant; payload version bump gated on receiver (X1) - success criteria reworded to 'available' as today's audit basis (P2) - net-new-vs-existing note, static-read-once (D12), version-gate (D13), host-is-internal, worker-count regression test note Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…P-589] 19 tasks across setup, foundational plumbing, US1 (MVP block), US2 (air-gapped), US3 (degradation), and polish. Encodes the critique E1 fix (new reader method, list_workers untouched) and D13 (no version bump this phase). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ead logging [INFP-589] - Drop the parallel resources block: extend database.system_info (processor_assigned) and the workers block (processor_*/memory_*), add a new server block for api_server. - All new fields reuse the existing processor_*/memory_* names; memory as total+available (usage derived), matching the DB. - psutil for host cores+RAM (already a dependency); stdlib for cgroup. - Log a warning on resource self-read failure (FR-005 traceability). - No payload_format bump this phase (D13). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…NFP-589] Foundational chunk for per-component resource telemetry: - extend TelemetryDatabaseSystemInfoData (processor_assigned) and TelemetryWorkerData (processor_*/memory_*); add TelemetryServerData and the server field on TelemetryData (additive). - telemetry/resources.py: logical-core + cgroup (v2/v1) reader with a per-process cache, and a pure host-dedup aggregate() function. - component heartbeat self-reports resources; new read_worker_resources() leaves list_workers/worker counts untouched. - 27 unit tests (reader + aggregation), all passing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…signed [INFP-589] US1 (MVP): the daily gather now aggregates git_agent and api_server host readings (dedup-by-host) into the extended workers fields and the new server block, each via safe_metric; database.get_system_info reads server.cypher.parallel.worker_limit (parameter-bound) into processor_assigned (0/auto/absent -> None). Version unchanged. Component tests: aggregation + server dedup + backward-compat, run on the testcontainers stack (2 new; full telemetry component suite green). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…FP-589] US2: component test proving the resource fields are in the locally stored snapshot with telemetry_optout=true and the remote send is SKIPPED (never POSTs). gather() already builds the full payload before the store/opt-out branch, so no flow change was needed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…snapshot [INFP-589] US3: wrap the DB processor_assigned read in safe_metric so a driver-side failure (not just Neo4jError) nulls only that field. Adds component tests for per-field null on DB read failure, worker undercount with the count intact, unbounded host -> null aggregate, and a self-read failure logging a warning then writing null. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…metrics [INFP-589] Polish: regression test proving the workers:resources heartbeat keys leave workers.total/active unchanged; changelog fragment and FAQ entry describing the new per-component processor_*/memory_* fields (database, server, workers). Full telemetry unit + component suites green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Address code-review findings: - aggregate() drops a failed self-read (all-null reading) so one failed worker undercounts instead of nulling the whole fleet; unbounded (processor_assigned=None on a healthy host) still nulls that field. - database read: extract pure _worker_limit_from_value (now unit-tested for positive/zero/negative/non-numeric) and stop the silent internal Neo4jError catch so safe_metric logs the real failure. - reader tolerates decode errors per-field; failed-read fallback uses a host sentinel so it cannot itself break the heartbeat. - reconcile spec/research D9 to the shipped null semantics (no separate measured-empty 0). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
10 issues found across 22 files
Confidence score: 2/5
backend/infrahub/telemetry/resources.pyhas multiple high-impact accuracy gaps (non-root cgroup path handling, cpuset awareness, and stale limits after startup), so telemetry can report host-level CPU/memory instead of the process/container limits and skew licensing allocation decisions. Resolve cgroup paths from/proc/self/cgroup, derive CPU from process affinity with fallback, and refresh cgroup-backed values on each heartbeat before merging.backend/infrahub/telemetry/tasks.pycurrently mixes API-server + git-agent worker counts with git-agent-only resource totals, soworkersfields can describe different fleets in mixed deployments and produce misleading capacity/licensing data. Keep count and allocation fields scoped to the same component set before merging.dev/specs/infp-589-resource-telemetry/research.mdanddev/specs/infp-589-resource-telemetry/plan.mdconflict on whetherTELEMETRY_VERSIONis bumped, which creates implementation ambiguity and raises payload compatibility risk if teams follow different documents. Align D10/D13, constraints, and project structure to one explicit decision (this phase: no version bump) before merging.backend/infrahub/services/component.pyreads cgroup/psutil data on the async path, which can block the event loop if OS calls stall and delay API/worker responsiveness. RunProcessResources.read()in a thread (and make_read_own_process_resourcesasync-safe) to de-risk production latency before merging.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="dev/specs/infp-589-resource-telemetry/tasks.md">
<violation number="1" location="dev/specs/infp-589-resource-telemetry/tasks.md:89">
P2: T017's changelog description uses 'resources block' terminology that the entire design explicitly avoids. The spec (FR-001), research (D14), and contract all say there is NO standalone resources block — fields are in-place on database.system_info, workers, and a new server block. A changelog fragment calling it a 'resources block' would create the wrong mental model for users reading release notes. Use terminology consistent with the design (e.g., 'per-component CPU and memory resource fields').</violation>
</file>
<file name="dev/specs/infp-589-resource-telemetry/research.md">
<violation number="1" location="dev/specs/infp-589-resource-telemetry/research.md:65">
P1: D10 and D13 give contradictory instructions on whether the payload_format version should be bumped. D10 says to increment TELEMETRY_VERSION to the change date, while D13 says it is not incremented until the receiving service confirms tolerance (fields ship under the existing version). The PR description confirms 'no payload-format bump in this phase', making D10 the incorrect half of the contradiction. A developer implementing from this document could follow D10 and break ingestion.</violation>
</file>
<file name="backend/infrahub/telemetry/tasks.py">
<violation number="1" location="backend/infrahub/telemetry/tasks.py:190">
P2: `workers` now combines API-server and git-agent counts with git-agent-only resource totals, so its count and allocation fields describe different fleets in deployments running both components. Keep these fields scoped to the same component population (or expose separate component counts) so licensing telemetry is not misleading.</violation>
</file>
<file name="backend/infrahub/services/component.py">
<violation number="1" location="backend/infrahub/services/component.py:144">
P2: Heartbeat handling blocks the async event loop while reading cgroup files and psutil data, potentially delaying API/worker tasks when those OS calls stall. Run `ProcessResources.read()` in a thread and make `_read_own_resources`/its heartbeat call async.</violation>
</file>
<file name="backend/infrahub/telemetry/resources.py">
<violation number="1" location="backend/infrahub/telemetry/resources.py:32">
P1: Components running beneath a non-root cgroup report host memory and no assigned CPU limit, making licensing allocation telemetry incorrect. Resolve this process's cgroup path from `/proc/self/cgroup` and account for ancestor limits before reading controller files.</violation>
<violation number="2" location="backend/infrahub/telemetry/resources.py:185">
P2: Cpuset-restricted API or worker containers report the host vCPU count rather than CPUs available to the process. Derive this field from the process CPU affinity (with an appropriate platform fallback).</violation>
<violation number="3" location="backend/infrahub/telemetry/resources.py:201">
P2: Runtime CPU or memory limit changes never reach telemetry after process startup. Refresh cgroup-backed resource values on each heartbeat; cache only truly stable identity data.</violation>
</file>
<file name="dev/specs/infp-589-resource-telemetry/spec.md">
<violation number="1" location="dev/specs/infp-589-resource-telemetry/spec.md:21">
P2: The US1 Independent Test lists `memory used` as a field the snapshot should contain, but the actual payload emits `memory_total` and `memory_available` (free memory), with `used` derived as `total − available` by the consumer. A reader or QA engineer following this test will look for a `memory_used` field that doesn't exist. Either replace `memory used` with `memory total` to match the real fields, or clarify that usage is derived.</violation>
</file>
<file name="dev/specs/infp-589-resource-telemetry/plan.md">
<violation number="1" location="dev/specs/infp-589-resource-telemetry/plan.md:29">
P1: The Constraints section says payload changes ship 'with a version bump,' but every other document in this feature (research D13, contract, tasks.md Notes, spec FR-008) agrees there is no version bump this phase — the bump is gated on receiving-service confirmation. If an implementer follows the plan's constraint, they will bump `TELEMETRY_VERSION` before the gate is cleared, potentially breaking existing ingestion. Change this to match the resolved decision.</violation>
<violation number="2" location="dev/specs/infp-589-resource-telemetry/plan.md:71">
P1: The Project Structure lists `constants.py` with the comment 'bump TELEMETRY_VERSION', but the resolved decision (research D13, contract, spec FR-008, tasks.md Notes) is no version bump this phase. tasks.md explicitly says 'Nothing in these tasks edits constants.py.' This would lead a developer following the plan to make an edit that violates the receiving-service gating requirement. Remove or update this entry to reflect the settled approach.</violation>
</file>
Shadow auto-approve: would not auto-approve because issues were found.
Re-trigger cubic
| - For a field where any *contributing* host is `null` because it is genuinely unbounded (e.g. one worker host has no cgroup CPU quota) → the aggregate for that field is `null`: a fleet containing an unbounded node has no finite total. | ||
| - **Rationale**: keeps `null` meaning "unknown / unbounded" throughout (no separate measured-empty `0` the consumer would have to distinguish), and makes an undercount observable via the worker count rather than silently nulling the fleet. | ||
|
|
||
| ## D10 — Payload version bump + receiving-service coordination |
There was a problem hiding this comment.
P1: D10 and D13 give contradictory instructions on whether the payload_format version should be bumped. D10 says to increment TELEMETRY_VERSION to the change date, while D13 says it is not incremented until the receiving service confirms tolerance (fields ship under the existing version). The PR description confirms 'no payload-format bump in this phase', making D10 the incorrect half of the contradiction. A developer implementing from this document could follow D10 and break ingestion.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At dev/specs/infp-589-resource-telemetry/research.md, line 65:
<comment>D10 and D13 give contradictory instructions on whether the payload_format version should be bumped. D10 says to increment TELEMETRY_VERSION to the change date, while D13 says it is not incremented until the receiving service confirms tolerance (fields ship under the existing version). The PR description confirms 'no payload-format bump in this phase', making D10 the incorrect half of the contradiction. A developer implementing from this document could follow D10 and break ingestion.</comment>
<file context>
@@ -0,0 +1,90 @@
+ - For a field where any *contributing* host is `null` because it is genuinely unbounded (e.g. one worker host has no cgroup CPU quota) → the aggregate for that field is `null`: a fleet containing an unbounded node has no finite total.
+- **Rationale**: keeps `null` meaning "unknown / unbounded" throughout (no separate measured-empty `0` the consumer would have to distinguish), and makes an undercount observable via the worker count rather than silently nulling the fleet.
+
+## D10 — Payload version bump + receiving-service coordination
+
+- **Decision**: Increment `TELEMETRY_VERSION` (`payload_format`, currently `"20260628"`) to the change date. All new fields are additive; no existing field is renamed or removed.
</file context>
| if TYPE_CHECKING: | ||
| from collections.abc import Iterable | ||
|
|
||
| CGROUP_ROOT = Path("/sys/fs/cgroup") |
There was a problem hiding this comment.
P1: Components running beneath a non-root cgroup report host memory and no assigned CPU limit, making licensing allocation telemetry incorrect. Resolve this process's cgroup path from /proc/self/cgroup and account for ancestor limits before reading controller files.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/infrahub/telemetry/resources.py, line 32:
<comment>Components running beneath a non-root cgroup report host memory and no assigned CPU limit, making licensing allocation telemetry incorrect. Resolve this process's cgroup path from `/proc/self/cgroup` and account for ancestor limits before reading controller files.</comment>
<file context>
@@ -0,0 +1,260 @@
+if TYPE_CHECKING:
+ from collections.abc import Iterable
+
+CGROUP_ROOT = Path("/sys/fs/cgroup")
+
+# The default CPU period the kernel uses when a cgroup v2 ``cpu.max`` line omits it.
</file context>
| ├── models.py # extend TelemetryDatabaseSystemInfoData (processor_assigned) + | ||
| │ # TelemetryWorkerData (processor_*/memory_* fields); add | ||
| │ # TelemetryServerData + server field on TelemetryData | ||
| ├── constants.py # bump TELEMETRY_VERSION (payload_format) |
There was a problem hiding this comment.
P1: The Project Structure lists constants.py with the comment 'bump TELEMETRY_VERSION', but the resolved decision (research D13, contract, spec FR-008, tasks.md Notes) is no version bump this phase. tasks.md explicitly says 'Nothing in these tasks edits constants.py.' This would lead a developer following the plan to make an edit that violates the receiving-service gating requirement. Remove or update this entry to reflect the settled approach.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At dev/specs/infp-589-resource-telemetry/plan.md, line 71:
<comment>The Project Structure lists `constants.py` with the comment 'bump TELEMETRY_VERSION', but the resolved decision (research D13, contract, spec FR-008, tasks.md Notes) is no version bump this phase. tasks.md explicitly says 'Nothing in these tasks edits constants.py.' This would lead a developer following the plan to make an edit that violates the receiving-service gating requirement. Remove or update this entry to reflect the settled approach.</comment>
<file context>
@@ -0,0 +1,99 @@
+├── models.py # extend TelemetryDatabaseSystemInfoData (processor_assigned) +
+│ # TelemetryWorkerData (processor_*/memory_* fields); add
+│ # TelemetryServerData + server field on TelemetryData
+├── constants.py # bump TELEMETRY_VERSION (payload_format)
+├── resources.py # NEW: read logical cores + memory (psutil) and the cgroup limit
+│ # (stdlib); host identifier; per-process ComponentResources
</file context>
| ├── constants.py # bump TELEMETRY_VERSION (payload_format) | |
| ├── constants.py # left unchanged this phase (version bump gated on receiving-service confirmation; research D13) |
|
|
||
| **Performance Goals**: Cold daily path; cost is negligible. Reads are O(active workers) cache keys (already scanned today) plus a handful of local file reads per process at heartbeat time | ||
|
|
||
| **Constraints**: MUST NOT block or fail the snapshot; each metric degrades independently to `null`; payload changes are additive with a version bump; **no new third-party dependency**; cgroup v2 primary with a v1 fallback, `null` where neither is present |
There was a problem hiding this comment.
P1: The Constraints section says payload changes ship 'with a version bump,' but every other document in this feature (research D13, contract, tasks.md Notes, spec FR-008) agrees there is no version bump this phase — the bump is gated on receiving-service confirmation. If an implementer follows the plan's constraint, they will bump TELEMETRY_VERSION before the gate is cleared, potentially breaking existing ingestion. Change this to match the resolved decision.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At dev/specs/infp-589-resource-telemetry/plan.md, line 29:
<comment>The Constraints section says payload changes ship 'with a version bump,' but every other document in this feature (research D13, contract, tasks.md Notes, spec FR-008) agrees there is no version bump this phase — the bump is gated on receiving-service confirmation. If an implementer follows the plan's constraint, they will bump `TELEMETRY_VERSION` before the gate is cleared, potentially breaking existing ingestion. Change this to match the resolved decision.</comment>
<file context>
@@ -0,0 +1,99 @@
+
+**Performance Goals**: Cold daily path; cost is negligible. Reads are O(active workers) cache keys (already scanned today) plus a handful of local file reads per process at heartbeat time
+
+**Constraints**: MUST NOT block or fail the snapshot; each metric degrades independently to `null`; payload changes are additive with a version bump; **no new third-party dependency**; cgroup v2 primary with a v1 fallback, `null` where neither is present
+
+**Scale/Scope**: A few components and a small number of workers per deployment (default `replicas: 2`). Trivial scale
</file context>
| **Constraints**: MUST NOT block or fail the snapshot; each metric degrades independently to `null`; payload changes are additive with a version bump; **no new third-party dependency**; cgroup v2 primary with a v1 fallback, `null` where neither is present | |
| **Constraints**: MUST NOT block or fail the snapshot; each metric degrades independently to `null`; payload changes are additive (no version bump this phase — gated follow-up once receiving service confirms tolerance); **no new third-party dependency**; cgroup v2 primary with a v1 fallback, `null` where neither is present |
| ## Phase 6: Polish & Cross-Cutting Concerns | ||
|
|
||
| - [x] T016 [P] Regression test in `backend/tests/component/telemetry/test_resources.py`: adding the `workers:resources:*` heartbeat key leaves `workers.total` and `workers.active` unchanged versus a baseline without it (critique E1) | ||
| - [x] T017 [P] Add a Towncrier changelog fragment `changelog/+resource-telemetry.added.md` describing the new per-component `resources` block (Constitution: user-facing telemetry change) |
There was a problem hiding this comment.
P2: T017's changelog description uses 'resources block' terminology that the entire design explicitly avoids. The spec (FR-001), research (D14), and contract all say there is NO standalone resources block — fields are in-place on database.system_info, workers, and a new server block. A changelog fragment calling it a 'resources block' would create the wrong mental model for users reading release notes. Use terminology consistent with the design (e.g., 'per-component CPU and memory resource fields').
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At dev/specs/infp-589-resource-telemetry/tasks.md, line 89:
<comment>T017's changelog description uses 'resources block' terminology that the entire design explicitly avoids. The spec (FR-001), research (D14), and contract all say there is NO standalone resources block — fields are in-place on database.system_info, workers, and a new server block. A changelog fragment calling it a 'resources block' would create the wrong mental model for users reading release notes. Use terminology consistent with the design (e.g., 'per-component CPU and memory resource fields').</comment>
<file context>
@@ -0,0 +1,142 @@
+## Phase 6: Polish & Cross-Cutting Concerns
+
+- [x] T016 [P] Regression test in `backend/tests/component/telemetry/test_resources.py`: adding the `workers:resources:*` heartbeat key leaves `workers.total` and `workers.active` unchanged versus a baseline without it (critique E1)
+- [x] T017 [P] Add a Towncrier changelog fragment `changelog/+resource-telemetry.added.md` describing the new per-component `resources` block (Constitution: user-facing telemetry change)
+- [x] T018 [P] Update the telemetry FAQ in `docs/docs/faq/faq.mdx` to mention the per-component cores/RAM (`resources`) block
+- [x] T019 Run the quickstart validation (`uv run invoke backend.test-unit`; component tests via testcontainers) and `uv run invoke format` + `uv run invoke lint`; fix any failures
</file context>
| # over distinct containers, counting a shared container once. | ||
| resources_by_component = await safe_metric(self.component.read_worker_resources()) or {} | ||
| workers_resources = await safe_metric( | ||
| aggregate_component_resources(resources_by_component.get("git_agent", {})) |
There was a problem hiding this comment.
P2: workers now combines API-server and git-agent counts with git-agent-only resource totals, so its count and allocation fields describe different fleets in deployments running both components. Keep these fields scoped to the same component population (or expose separate component counts) so licensing telemetry is not misleading.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/infrahub/telemetry/tasks.py, line 190:
<comment>`workers` now combines API-server and git-agent counts with git-agent-only resource totals, so its count and allocation fields describe different fleets in deployments running both components. Keep these fields scoped to the same component population (or expose separate component counts) so licensing telemetry is not misleading.</comment>
<file context>
@@ -157,6 +182,17 @@ async def gather(self) -> TelemetryData:
+ # over distinct containers, counting a shared container once.
+ resources_by_component = await safe_metric(self.component.read_worker_resources()) or {}
+ workers_resources = await safe_metric(
+ aggregate_component_resources(resources_by_component.get("git_agent", {}))
+ )
+ server_resources = await safe_metric(
</file context>
| last_error: Exception | None = None | ||
| for _ in range(RESOURCE_READ_MAX_ATTEMPTS): | ||
| try: | ||
| return self.process_resources.read() |
There was a problem hiding this comment.
P2: Heartbeat handling blocks the async event loop while reading cgroup files and psutil data, potentially delaying API/worker tasks when those OS calls stall. Run ProcessResources.read() in a thread and make _read_own_resources/its heartbeat call async.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/infrahub/services/component.py, line 144:
<comment>Heartbeat handling blocks the async event loop while reading cgroup files and psutil data, potentially delaying API/worker tasks when those OS calls stall. Run `ProcessResources.read()` in a thread and make `_read_own_resources`/its heartbeat call async.</comment>
<file context>
@@ -106,12 +119,64 @@ async def refresh_heartbeat(self) -> None:
+ last_error: Exception | None = None
+ for _ in range(RESOURCE_READ_MAX_ATTEMPTS):
+ try:
+ return self.process_resources.read()
+ except Exception as exc:
+ last_error = exc
</file context>
| return _host_memory_available() | ||
|
|
||
| def read(self) -> WorkerResourceReading: | ||
| if self._static is None: |
There was a problem hiding this comment.
P2: Runtime CPU or memory limit changes never reach telemetry after process startup. Refresh cgroup-backed resource values on each heartbeat; cache only truly stable identity data.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/infrahub/telemetry/resources.py, line 201:
<comment>Runtime CPU or memory limit changes never reach telemetry after process startup. Refresh cgroup-backed resource values on each heartbeat; cache only truly stable identity data.</comment>
<file context>
@@ -0,0 +1,260 @@
+ return _host_memory_available()
+
+ def read(self) -> WorkerResourceReading:
+ if self._static is None:
+ self._static = self._read_static()
+ return WorkerResourceReading(
</file context>
| memory_total = memory_limit if memory_limit is not None else _host_memory_total() | ||
| return _StaticResources( | ||
| host=socket.gethostname(), | ||
| processor_available=psutil.cpu_count(logical=True), |
There was a problem hiding this comment.
P2: Cpuset-restricted API or worker containers report the host vCPU count rather than CPUs available to the process. Derive this field from the process CPU affinity (with an appropriate platform fallback).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/infrahub/telemetry/resources.py, line 185:
<comment>Cpuset-restricted API or worker containers report the host vCPU count rather than CPUs available to the process. Derive this field from the process CPU affinity (with an appropriate platform fallback).</comment>
<file context>
@@ -0,0 +1,260 @@
+ memory_total = memory_limit if memory_limit is not None else _host_memory_total()
+ return _StaticResources(
+ host=socket.gethostname(),
+ processor_available=psutil.cpu_count(logical=True),
+ processor_assigned=_read_cgroup_cpu_quota(self._cgroup_root),
+ memory_total=memory_total,
</file context>
|
|
||
| **Why this priority**: This is the entire reason the feature exists. Without it, tier compliance can only be established by contacting the customer or inspecting their environment directly. It is the minimum viable slice: a snapshot that carries allocated cores and memory per component already delivers the audit. | ||
|
|
||
| **Independent Test**: Produce a telemetry snapshot on a running deployment and confirm it contains, for each of the three components, the cores available, cores assigned, memory available, and memory used, all in comparable units — and that a reviewer can compare those figures to a tier definition without any further data. |
There was a problem hiding this comment.
P2: The US1 Independent Test lists memory used as a field the snapshot should contain, but the actual payload emits memory_total and memory_available (free memory), with used derived as total − available by the consumer. A reader or QA engineer following this test will look for a memory_used field that doesn't exist. Either replace memory used with memory total to match the real fields, or clarify that usage is derived.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At dev/specs/infp-589-resource-telemetry/spec.md, line 21:
<comment>The US1 Independent Test lists `memory used` as a field the snapshot should contain, but the actual payload emits `memory_total` and `memory_available` (free memory), with `used` derived as `total − available` by the consumer. A reader or QA engineer following this test will look for a `memory_used` field that doesn't exist. Either replace `memory used` with `memory total` to match the real fields, or clarify that usage is derived.</comment>
<file context>
@@ -0,0 +1,113 @@
+
+**Why this priority**: This is the entire reason the feature exists. Without it, tier compliance can only be established by contacting the customer or inspecting their environment directly. It is the minimum viable slice: a snapshot that carries allocated cores and memory per component already delivers the audit.
+
+**Independent Test**: Produce a telemetry snapshot on a running deployment and confirm it contains, for each of the three components, the cores available, cores assigned, memory available, and memory used, all in comparable units — and that a reviewer can compare those figures to a tier definition without any further data.
+
+**Acceptance Scenarios**:
</file context>
psutil was pinned only in the dev dependency group, so production task-worker/api_server images (built with `uv sync --frozen --no-dev`) never had it installed. The resource-telemetry reader imports it unconditionally, which broke Prefect's collection loading on every Docker-stack-based CI job (backend-docker-integration, all E2E suites) with ModuleNotFoundError: No module named 'psutil'. Move the existing psutil==6.1.0 pin from [dependency-groups] dev to [project] dependencies and regenerate uv.lock. No new third-party package is introduced. Corrects research.md D1 and the spec.md assumption that wrongly claimed psutil was already a direct runtime dependency. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
0 issues found across 4 files (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would not auto-approve. Auto-approval blocked by 10 unresolved issues from previous reviews.
Re-trigger cubic
Apply review feedback from the collection PR: replace the three single-method protocols with one generic GathererInterface[T], drop the redundant .default() classmethods on the account/activity models, and collapse the eight per-field activity_24h test cases into a single test so the flow gathers once instead of eight times. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
0 issues found across 4 files (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would not auto-approve. Auto-approval blocked by 10 unresolved issues from previous reviews.
Re-trigger cubic
…nto resource-telemetry-infp-589 # Conflicts: # uv.lock
count_user_nodes issued one branch-aware count query per concrete kind in user-editable namespaces, which grows to hundreds of sequential round trips on a large schema. Replace the fan-out with CountNodesByKindsQuery: one pass over the graph that resolves each node's winning IS_PART_OF edge (branch hierarchy + timestamp) and counts active nodes grouped by kind. Component tests pin the branch semantics (deleted and branch-only nodes excluded) and assert equivalence with the per-kind NodeManager.count oracle. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
0 issues found across 3 files (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would not auto-approve. Auto-approval blocked by 10 unresolved issues from previous reviews.
Re-trigger cubic
Summary
Stacked on #9805. Extends the Phase 1 anonymous telemetry snapshot with per-component compute-allocation data (CPU cores, memory) so a deployment can be audited against its contracted license tier.
database.system_infowithprocessor_assigned(read live from the Neo4jserver.cypher.parallel.worker_limitsetting;nulluntil enforcement is configured).workersblock withprocessor_available/processor_assigned/memory_total/memory_available(git_agent fleet aggregate, deduplicated by host).serverblock with the same four fields for the api_server.resourcesblock, no renamed/removed fields, no payload-version bump this phase (additive only — seeresearch.mdD13).safe_metric); a component that fails to self-read after bounded retries logs a warning and reportsnullwithout blocking the snapshot; the worker aggregate undercounts rather than nulling the whole fleet.Design docs
Full spec/plan/research/data-model/contracts under
dev/specs/infp-589-resource-telemetry/, including the implementation report with the full chunk/commit ledger and review findings.Test plan
uv run pytest backend/tests/unit/telemetry/— 63 passeduv run pytest backend/tests/component/telemetry/(testcontainers Neo4j) — 44 passeduv run invoke format/backend.lint(ruff, mypy) — cleanFollow-ups (deferred, non-blocking, tracked in the implementation report)
gather_database_information/JMXget_system_infoinsafe_metric(pre-existing gap, predates this feature).payload_formatversion bump with the telemetry-receiving service before it ships.processor_assignedfields shipnulluntil per-tier enforcement (INFP-472) lands; they self-populate with no code change once it does.🤖 Generated with Claude Code
Summary by cubic
Adds per-component CPU and memory telemetry for the database, API server, and worker fleet to audit deployments against licensed tiers (INFP-589). Additive-only; no payload format change; metrics degrade independently; worker aggregate undercounts rather than nulling; promotes
psutilto a runtime dependency. Also speeds up telemetry user-node counting with a single branch-aware query.CountNodesByKindsQueryto reduce database round trips.Written for commit 68820d2. Summary will update on new commits.