Skip to content

fix(telemetry): cold container-stats cache reports no_data, not running_count 0 (#1617)#1625

Merged
vybe merged 1 commit into
devfrom
fix/1617-telemetry-cold-cache-no-data
Jul 15, 2026
Merged

fix(telemetry): cold container-stats cache reports no_data, not running_count 0 (#1617)#1625
vybe merged 1 commit into
devfrom
fix/1617-telemetry-cold-cache-no-data

Conversation

@dolho

@dolho dolho commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Summary

GET /api/telemetry/containers serves a per-uvicorn-worker, stale-while-revalidate in-process cache. The cold-cache branch returned _empty_payload(0) — a literal running_count: 0 with stale: true — which is byte-identical to a healthy "zero containers running" answer. Because the cache is per worker, consecutive requests could flip between 0 and the true count depending on which worker served them, and a rarely-queried worker kept a cold cache indefinitely (observed cache_age_seconds: 5726). In a real incident this read as "the backend lost its Docker connection" and drove a wrong root-cause report (#1617).

Fix

Make the cold state self-describing so it can't be mistaken for a real zero:

  • Cold cacherunning_count: null + no_data: true.
  • Genuinely-empty fleet (refresh ran, nothing running) → running_count: 0 + no_data: false (unchanged).
  • Fresh + stale branches also carry no_data: false for a consistent contract.
  • Docstring now states stale = cache freshness only, not Docker connectivity (unreachable Docker is the existing HTTP 503).

Directly implements suggested fix 1 (running_count: null / explicit no_data) and #3 (document stale semantics) from the issue.

Deferred (issue's optional "consider"): moving the cache to Redis for cross-worker consistency. The misleading-diagnostics defect is fully closed by making cold state self-describing; a shared-cache rework is a larger, separate change and out of scope for a P3 diagnostic-quality fix.

Consumer safety

No in-repo consumer reads /api/telemetry/containers — the frontend uses only /api/telemetry/host; the running_count references in OverviewPanel/network.js/executions.js come from /api/executions/stats (a different endpoint/model). The endpoint's only known consumer is external diagnostic tooling (per the issue), so widening the contract with no_data + a nullable running_count is backward-additive.

Changes

  • src/backend/routers/telemetry.py: cold branch → running_count: null + no_data: true; no_data: false on fresh/stale; docstring clarifies stale semantics.
  • tests/unit/test_1096_telemetry_cache.py: update cold-start expectation to the new shape; new test_cold_no_data_is_distinct_from_real_zero_running proving cold≠real-zero.
  • tests/test_telemetry.py: live-API tests poll past the cold window (skip/tolerate no_data).

Test Plan

  • pytest tests/unit/test_1096_telemetry_cache.py -q → 12 passed
  • Router + test modules parse clean
  • Live-API tests/test_telemetry.py::TestContainerTelemetry (test-runner suite, needs a running stack)

Fixes #1617

🤖 Generated with Claude Code

…ng_count 0 (#1617)

GET /api/telemetry/containers serves a per-uvicorn-worker, stale-while-
revalidate in-process cache. The cold-cache branch returned
`_empty_payload(0)` — a literal `running_count: 0` with `stale: true` — which
is byte-identical to a healthy "zero containers running" answer. Because the
cache is per worker, consecutive requests could flip between 0 and the true
count depending on which worker served them, and a rarely-queried worker kept
a cold cache indefinitely. In a real incident this was read as "the backend
lost its Docker connection" and drove a wrong root-cause report.

Fix (contract, no behavior change to the measured paths):
- Cold cache now returns `running_count: null` + `no_data: true` so consumers
  can tell "not yet measured" from a real zero. The genuinely-empty fleet
  (refresh ran, found nothing running) still returns `running_count: 0` /
  `no_data: false`.
- Add `no_data: false` to the fresh + stale branches for a consistent contract.
- Document that `stale` refers to CACHE FRESHNESS only, not Docker
  connectivity (unreachable Docker is the existing HTTP 503).

Redis-shared cache (the issue's optional "consider") is intentionally deferred
— the misleading-diagnostics defect is fully closed by making cold state
self-describing; a shared cache is a larger, separate change.

Tests: update the #1096 cold-start unit test to the new shape, add a unit test
proving cold(no_data)/None is distinct from a refreshed real zero, and make the
live-API telemetry tests poll past the cold window.

Related to #1617

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

⚠️ Nightly unit-suite check skipped — merge conflict against dev.

Resolve by running git merge dev locally and pushing the result. The next nightly run will re-test once the conflict is gone.

@vybe vybe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Validated via /validate-pr.

Contract change is safe: I grepped the whole repo — nothing consumes /api/telemetry/containers outside the tests. The frontend running_count hits (stores/executions.js, network.js, OverviewPanel.vue) all read /api/executions/stats, a different endpoint. So making running_count nullable breaks no consumer.

  • no_data: true / running_count: null correctly distinguishes 'not yet measured' from a real zero; test_cold_no_data_is_distinct_from_real_zero_running proves both sides of that.
  • Named regression test present for the reported issue.

Security scan clean.

Two notes, neither blocking:

  1. Body edited — it read 'suggested fixes #1', which the issue-status-on-merge workflow regex (/\b(?:fix(?:e[sd])?|close[sd]?|resolve[sd]?)\s+#(\d+)/gi) matched as a closing ref to #1 (a long-merged PR), and would have stamped status-in-dev on it while leaving #1617 stranded in status-in-progress. Now reads 'suggested fix 1' + Fixes #1617.
  2. Follow-up: docs/memory/feature-flows/host-telemetry.md:146 documents this endpoint's response schema and still shows running_count as always-int with no no_data field. Not merge-blocking (bug-fix tier), but the flow doc is now stale.

@vybe
vybe merged commit a28657e into dev Jul 15, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants