Skip to content

/api/v1/metrics/history re-sends the whole window on every poll (no ?since= and no conditional request) #112

Description

@LarsLaskowski

Summary

GET /api/v1/metrics/history always returns the complete retained window. There is no way for a client to ask for only what it has not seen, and no conditional-request support (no ETag / If-None-Match, no Last-Modified / If-Modified-Since).

The bundled dashboard polls this endpoint once a minute (app.js, historyTimer). At default settings — history_window_minutes: 60, poll_interval_seconds: 5 → 720 points per series across 7 scalar series plus one per filesystem and two per interface — that is roughly 5 000+ points, ~250 KB of uncompressed JSON, re-serialised and re-sent every 60 seconds, of which only about 12 points per series are actually new.

The server work is the expensive part on a Pi: Collector.History() deep-copies every ring buffer under RLock, then json.Encoder formats every timestamp as RFC 3339, then gzip compresses the result — all to send data the client already has.

Why it matters here specifically

  • The target hardware is a Pi Zero. This is the single most expensive request the service serves.
  • The cost scales with history_window_minutes, which is operator-configurable well beyond the default (config.maxHistoryCapacity permits 1 000 000 points per series).
  • It multiplies by the number of open dashboards and third-party pollers.
  • gzip (added in Compress /api/v1 responses with gzip #95) reduces bytes on the wire by roughly 10× but increases CPU per request.

Suggested fix

Two independent improvements. Implement whichever you prefer; doing only one is a real win. Do not feel obliged to do both in one PR.

Option A: conditional requests (smaller, fully backwards compatible)

History only changes once per fast tick. Give the payload a validator derived from a generation counter the collector bumps on each fastTick, and answer 304 Not Modified when it is unchanged.

  • Add a monotonically increasing counter to Collector, incremented in fastTick under the existing lock, exposed via a method on the MetricsProvider interface (or returned alongside History()).
  • In handleHistory, set ETag: "<generation>" and return 304 when If-None-Match matches.

This is cheap and safe, but note the ceiling: the dashboard polls history every 60 s while the fast tick runs every 5 s, so the generation will essentially always have changed between two dashboard polls. A 304 would almost never fire for the bundled dashboard. It helps third-party clients that poll faster than they need to, and it pairs well with response caching, but be realistic about the benefit — say so in the PR rather than overstating it.

Option B: ?since= incremental delivery (bigger win, needs API design care)

Accept an RFC 3339 timestamp and return only points strictly newer than it:

GET /api/v1/metrics/history?since=2026-07-12T18:31:00Z

The client keeps the last timestamp it saw, requests only the delta, and appends. This turns a ~250 KB response into a few hundred bytes for the steady-state dashboard poll.

Design points that must be decided deliberately:

  • Is this a breaking change? No, provided ?since= is optional and omitting it preserves today's full-window behaviour exactly. Per docs/CONTRIBUTING.md that keeps it within /api/v1. Do not change the default response shape — that would require an /api/v2 bump.
  • Invalid since values. A malformed timestamp should return 400 Bad Request with a clear message, not be silently ignored. Decide and document what a since in the future returns (empty series, presumably) and what a since older than the retained window returns (the full window).
  • Per-device series. disk_used_percent, network_rx_bytes_per_sec and network_tx_bytes_per_sec are maps. Filtering must apply per key, and a device whose series becomes empty after filtering should be omitted — matching the existing omitempty behaviour.
  • Client gap recovery. If the dashboard is backgrounded (or the Pi restarts and history is restored from disk, possibly re-ordered relative to what the client holds), a naive append produces a corrupt local series. The client needs a fallback: if the returned oldest point is newer than expected, or on any parse failure, discard local state and re-request the full window. This client-side robustness is the part most likely to be got wrong — do not skip it.
  • Ring buffers are stored oldest-first, so filtering is a prefix scan (sort.Search on the timestamp) rather than a full walk. Reuse the trimming logic already in importHistory (internal/collector/persist.go) rather than writing a second implementation.

Recommendation: start with Option B, since it addresses the actual dominant cost. Option A can follow later.

Testing requirements

Per docs/TESTS.md, tests are mandatory.

For Option B, add to internal/httpapi/handlers_test.go (using the existing fakeMetrics) and internal/collector/collector_test.go:

  1. ?since= returns only points strictly newer than the given timestamp, for a scalar series.
  2. ?since= filters per-device map series independently, and a device left with zero points is omitted from the response entirely.
  3. Omitting ?since= returns the full window — the backwards-compatibility regression test, the most important one here.
  4. A malformed ?since= returns 400 and does not fall back to the full window.
  5. A since newer than every point returns empty series (and the map fields omitted), not an error.
  6. A since older than the whole window returns everything.
  7. Boundary: a point whose timestamp is exactly equal to since is excluded (strictly-newer semantics). Pick a rule, test it, document it — the off-by-one here is what causes duplicated or dropped points client-side.

For Option A: a matching If-None-Match returns 304 with an empty body; a stale one returns 200 with the full payload; the ETag changes after a fastTick.

If the dashboard is updated to use the new parameter, follow the internal/web/xss_test.go source-scanning precedent for a guard test, and state plainly in the PR that the browser behaviour itself was verified manually.

Files to touch

  • internal/httpapi/handlers.go — query parsing, 400 handling, conditional response
  • internal/httpapi/server.go — only if the MetricsProvider interface changes
  • internal/collector/collector.go — a HistorySince(t time.Time) method, or filtering in the handler
  • internal/web/assets/app.js — client-side incremental fetch plus the gap-recovery fallback
  • internal/httpapi/handlers_test.go, internal/collector/collector_test.go — tests
  • docs/API.mdrequired. Document the parameter, its semantics, the boundary rule, the 400 case, and that omitting it is unchanged
  • docs/ARCHITECTURE.md — the HTTP layer and dashboard sections

Related

Overlaps with the server-side request-throttling issue, which proposes caching the serialised history payload as one of its options. Check whether that work is in progress before starting — the two touch the same code path and an uncoordinated implementation will conflict.


⚠️ Note on this issue

This issue was drafted by an AI code review of the repository. What is written here is not law — the analysis was produced by an AI and may contain mistakes. The ~250 KB / 5 000-point figures are calculated from the default configuration, not measured on real hardware — measure the actual response size and server-side timing before deciding how much complexity this justifies.

Before implementing, first verify that this issue is still factually correct. Implementation may happen considerably later than this issue was written, and the source may have been changed in the meantime by unrelated work (or the problem may already be addressed). Re-read the referenced files at their current state and adjust — or close — the issue if it no longer applies.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or requestperformancePerformance improvements

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions