Skip to content

feat(telemetry): capture latency percentiles and add rollup query endpoint - #132

Merged
pmaxhogan merged 1 commit into
mainfrom
feat/telemetry-latency-percentiles
Jul 20, 2026
Merged

pmaxhogan merged 1 commit into
mainfrom
feat/telemetry-latency-percentiles

Conversation

@pmaxhogan

Copy link
Copy Markdown
Owner

What

Fills in the telemetry latency percentiles that shipped in V1 as always-empty wire keys, end to end:

  • Client capture. A new app-global LatencyReservoir in driven-core (bounded ring buffer per metric, cap 4096, nearest-rank p50/p95). The scanner records per-file scan-processing latency; the executor records each completed upload op's latency normalized per MiB. One reservoir is shared (Arc) into every account's executor + orchestrator (mirroring the existing with_mem_gauge seam), wired once in assembly.
  • Ping build. build_payload now carries the drained percentiles instead of the hardcoded LatencyP50P95::default(). The window is snapshotted read-only at build and reset only after a successful send, so a dropped/aborted ping re-uses the same window (matching how the event-count deltas re-send an un-checkpointed window).
  • Worker ingest. writePing appends the 4 percentiles as AE doubles (double7..10), with a -1 sentinel for an empty metric so the rollup can tell "no samples" from a legitimate 0 ms (a sub-ms per-file scan rounds to 0).
  • Worker rollup query. New gated GET /telemetry/v1/stats/latency?days=N returning per-day aggregates via the Analytics Engine SQL API.

Why

DESIGN s13 lists "latency histograms (p50, p95) for scan and upload-per-file" in the payload, but nothing captured per-op durations, so latency_p50_p95_ms.{scan,upload_per_mb} were always empty and the worker had no read surface. This makes the signal real and queryable.

Consent / privacy

Capture is gated on the telemetry-enabled pref: the reservoir's enable flag is initialized from the persisted pref at boot (before any capture) and flipped in lockstep by apply_enabled_change. Turning telemetry off drops any captured samples. A latency sample is a bare millisecond duration - no path/name/content.

Endpoint contract

GET /telemetry/v1/stats/latency?days=7
Authorization: Bearer <QUERY_TOKEN>

200 OK
{
  "days": 7,
  "metrics": {
    "scan":          [ { "day": "2026-07-14", "avg_p50_ms": 3,  "avg_p95_ms": 12,  "max_p95_ms": 40,  "samples": 9 } ],
    "upload_per_mb": [ { "day": "2026-07-14", "avg_p50_ms": 50, "avg_p95_ms": 120, "max_p95_ms": 300, "samples": 4 } ]
  }
}

days defaults to 7, clamped to [1, 90]. Per metric, per UTC day: mean of the pinged p50s, mean + max of the pinged p95s, and the count of pings that reported the metric. Empty-latency pings (the -1 sentinel) are excluded per metric (WHERE <p50col> >= 0). Status: 401 missing/wrong bearer, 405 non-GET, 502 upstream AE failure, 503 not configured.

Where things live

  • Reservoir: crates/driven-core/src/telemetry.rs (new).
  • Scan capture: scanner::scan_with_latency (thin scan() wrapper keeps existing callers/tests unchanged).
  • Upload capture: ExecOne::run in executor.rs.
  • Wiring: src-tauri/src/assembly.rs; held on AppState's TelemetryRuntime.
  • Ping: src-tauri/src/telemetry.rs. Worker: telemetry-worker/src/index.ts + README.md.

Operational caveats (action required post-merge)

  • Set QUERY_TOKEN + CF_API_TOKEN as wrangler secrets (see telemetry-worker/README.md). deploy-telemetry.yml auto-deploys on merge, so /stats/latency ships 503-until-configured by design; the ingest path needs neither. CF_ACCOUNT_ID is optional (defaults to the Driven account).
  • The AE SQL query has only been tested against a mocked fetch - toDate() day-grouping, the double7..10 column mapping, and {meta,data} parsing have never hit real Analytics Engine. It needs a one-time live smoke-check after the secrets are set. This is the one thing the test suite structurally cannot cover.
  • upload_per_mb is an op-latency proxy, not pure transfer time: the timer wraps the whole upload op (hash -> crypto -> pacer gate -> network), so on a throttled/metered link it reflects pacer wait, not Drive throughput. A defensible reading of "upload op"; documented so the percentiles aren't misread.

Tests

  • Rust (driven-core): reservoir + percentile edge cases (empty / single / even / odd / large-uniform / ring-cap), consent no-op + disable-drops-samples, per_mb_ms normalization + zero guard.
  • Rust (driven-app): build_payload carries the drained percentiles; ping snapshots then resets on success; a failed send keeps the window; a disabled ping does not touch the reservoir; apply_enabled_change clears the reservoir on disable.
  • Worker (vitest): the 4 latency doubles + -1 sentinel (legit 0 preserved); /stats/latency 503-unconfigured, 401 no/wrong bearer, 405 wrong method, 200 per-day aggregates (mocked AE fetch), days clamping, 502 on upstream failure.

Gates green: cargo fmt --check, clippy --workspace --all-targets -D warnings, cargo test -p driven-core, cargo check --workspace; worker typecheck + lint + test (56).

Issue reference

The task said "Closes #5", but #5 resolves to a PR, not an issue (shared numbering - gh issue view 5 fails), so "Closes #5" would attach to a PR and close nothing. Referencing the V2 backlog tracking issue instead; correct me if a different issue was meant.

Refs #34

🤖 Generated with Claude Code

…point

Client (driven-core + src-tauri):
- New driven-core telemetry module: an app-global LatencyReservoir (bounded
  ring buffer per metric, cap 4096) with nearest-rank p50/p95, consent-gated
  capture, snapshot (read-only) + reset (post-send).
- Scanner records per-file scan-processing latency; executor records
  per-completed-upload latency normalized per MiB.
- Orchestrator + executor share the reservoir (with_latency_reservoir); wired
  once in assembly, enable-gate initialized from the persisted pref and flipped
  in lockstep by apply_enabled_change.
- Ping build_payload now carries the drained percentiles; the window resets only
  after a SUCCESSFUL send so a dropped ping re-uses it.

Worker (telemetry-worker):
- writePing appends scan/upload_per_mb p50/p95 as AE doubles (7..10), with a -1
  sentinel for an empty metric (distinguishes "no samples" from a legit 0 ms).
- New gated GET /telemetry/v1/stats/latency?days=N (default 7, cap 90): Bearer
  QUERY_TOKEN, per-day aggregates via the AE SQL API (CF_API_TOKEN); 503 until
  configured. README documents the secrets + contract.

Tests: reservoir/percentile edge cases + reservoir<->ping integration (rust);
latency doubles + stats endpoint auth/clamp/aggregate (vitest, mocked AE fetch).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QZQVP2tUuTLh8oL31D8heC
@github-actions

Copy link
Copy Markdown
Contributor

Coverage

Area main this PR delta
Rust (lib crates) 78.59% 78.64% +0.06 (OK)
UI (vue/ts) 88.51% 88.51% +0.00 (OK)

Gate: passed - no coverage regression (epsilon 0.1 pp).

@pmaxhogan
pmaxhogan merged commit 4e9fde6 into main Jul 20, 2026
18 checks passed
@pmaxhogan
pmaxhogan deleted the feat/telemetry-latency-percentiles branch July 20, 2026 17:01
@github-project-automation github-project-automation Bot moved this from Todo to Done in Driven Jul 20, 2026
pmaxhogan added a commit that referenced this pull request Jul 20, 2026
## What

Follow-up to #132. Adds a schema-version marker to every latency row and
filters on it in the rollup query, so pre-#132 rows are excluded from
`/telemetry/v1/stats/latency`.

## Why (the AE default-0 gotcha)

The Analytics Engine SQL API has **no NULLs**: any `double` a row never
wrote is materialized as **`0`** at query time. Rows written by the
pre-latency Worker (before #132) have no `double7..10`, so they read
`scan_p50 == 0` — a materialized 0, not a real sample. That passed the
rollup's `WHERE <p50col> >= 0` sentinel filter and showed up as legit `0
ms` samples. Caught in the live smoke of the endpoint: a day that
predates the deploy reported `samples > 0, avg 0`.

The `-1` sentinel can't fix this on its own — it only distinguishes
empty from present *within* a row that actually wrote the doubles; a
legacy row never wrote them, so there's no sentinel to read.

## Fix

- **Write** (`writePing`): append a schema-version marker `double11 = 1`
(`LATENCY_SCHEMA_VERSION`) on every row that carries the latency
doubles.
- **Query** (`queryLatencyMetric`): add `AND double11 >= 1` to each
per-metric `WHERE` (in addition to the existing `>= 0` sentinel). A
legacy row materializes the marker as `0` and is excluded; a new
empty-latency row is marked (`double11 = 1`) but still excluded by its
`-1` sentinel; a real `0 ms` still counts.
- Both sites comment the AE missing-double=0 behavior. README dataset
table + rollup section updated (`double11` row, dual-filter
explanation).

## Tests (worker vitest)

- `writePing` marks every new latency-schema row with `double11 = 1`.
- A new-but-empty-latency row is marked (`double11 = 1`) yet still
carries the `-1` sentinels — proving the marker and sentinel filters are
independent (the sentinel is what excludes it).
- Both per-metric rollup queries include `double11 >= 1` (the mechanism
that excludes pre-latency rows, whose marker materializes as 0).

Gates green: worker `typecheck` + `lint` + `test` (58, +2 new).

## Notes

Cut from latest `origin/main` (which already includes #132). Endpoint
still needs the same wrangler secrets as #132; the live smoke that
surfaced this bug already has them set. Do not merge — for review.

Refs #34

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
@pmaxhogan pmaxhogan mentioned this pull request Jul 20, 2026
23 tasks
pmaxhogan added a commit that referenced this pull request Jul 24, 2026
🤖 I have created a release *beep* *boop*
---


## [2.1.0](v2.0.1...v2.1.0)
(2026-07-24)


### Features

* **core:** adaptive upload parallelism with throughput probe and
disk-saturation gate
([#143](#143))
([8ecced6](8ecced6))
* **core:** filesystem timestamp-granularity probe with ctime fallback
and per-directory gitignore cascade
([#141](#141))
([344262c](344262c))
* **drive:** support Google Shared Drive destinations end-to-end
([#142](#142))
([d9c3161](d9c3161))
* **net:** native OS reachability backends with automatic fallback
([#138](#138))
([319e85f](319e85f))
* **net:** SOCKS5 and PAC proxy support for all outbound connections
([#145](#145))
([2f0b7d1](2f0b7d1))
* **net:** support a custom corporate root CA for all outbound
connections ([#134](#134))
([929e93d](929e93d))
* per-source toggle to back up OneDrive cloud-only placeholder files
([#133](#133))
([6863ea3](6863ea3))
* **telemetry:** capture latency percentiles and add rollup query
endpoint ([#132](#132))
([4e9fde6](4e9fde6))
* **telemetry:** preview exactly what a telemetry ping sends
([#139](#139))
([95fbd9a](95fbd9a))


### Bug Fixes

* **core:** commit file_state for a create that skipped post-upload so
the next scan updates instead of re-creating
([#146](#146))
([f5230d1](f5230d1))
* **deps:** bump tauri-winrt-notification to drop vulnerable quick-xml
(closes [#89](#89))
([#129](#129))
([232fd8f](232fd8f))
* **telemetry:** exclude pre-schema rows from latency rollup
([#137](#137))
([1ae6220](1ae6220))
* **ui:** add cursor pointer to buttons and link-buttons
([#136](#136))
([dbd4809](dbd4809))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant