From 0d525f475ae92ca61ef8b12f8c8019d3bfd6584e Mon Sep 17 00:00:00 2001 From: Gregor MacLennan Date: Fri, 21 Aug 2026 14:11:24 +0100 Subject: [PATCH 1/3] feat(backend): report the backend's memory footprint as Sentry gauges MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `:ComapeoCore` process is the one Android kills first when memory is short — it runs in the background with `oom_score_adj 200` and the kernel's `oom_score` scales with footprint — so "what does this change cost in memory" is worth being able to answer cheaply and repeatedly. Until now there was no answer: the only memory signal was a `heapUsed` gauge every 60s, and no process-level number at all. `backend/lib/memory-snapshot.js` reads `/proc/self/status` and `v8.getHeapStatistics()`. Three new gauges join `heap_used_bytes`: `heap_physical_bytes` (heap committed and touched — the figure that tracks anonymous RSS, where `heap_used_bytes` tracks the live object graph), and on Android `rss_bytes` and `rss_peak_bytes`. The peak is `VmHWM`, which is effectively what the low-memory killer scores the process on, and being monotonic it needs no high-rate sampling to find. `/proc` does not exist on iOS, where node shares the app process and an rss would describe the UI too, so the reader returns null there and the pair is simply not emitted — the platform gate is the filesystem, not a flag that can drift. All four carry `runtime` (the nodejs-mobile revision, so two runtime builds can be compared in a staged rollout) plus `device_class` / `os_major`. Memory is the metric whose whole point is the cheap device, and `heap_used_bytes` shipping without them means its 16k samples to date cannot be attributed to a device class at all. They sit at the diagnostic tier, matching `heap_used_bytes`. They describe the process's own resource use at a fixed cadence that is deliberately not activity-triggered, name nothing the user did, and at boot are overwhelmingly a property of the build and device rather than the data. Free *device* memory stays usage-tier where it already is. Reasoning in docs/BENCHMARKING.md so it can be argued with later. An extra sample fires 3s after `ready`: a 60s-only sampler silently biases the data towards processes that survived, and 88 of the FGS exits reported from production in the last 90 days sit in the `<10s` uptime bucket. One snapshot measured ~28 us on an arm64 emulator, so this is roughly 0.3 ms of CPU across a ten-minute session. The boot sample also logs one `[comapeo.memory] boot` line per launch, unconditionally: no PII, and it lets a build be measured on a device with telemetry switched off. --- AGENTS.md | 1 + backend/index.js | 23 +++- backend/lib/memory-snapshot.js | 150 +++++++++++++++++++++++++++ backend/lib/memory-snapshot.test.mjs | 131 +++++++++++++++++++++++ backend/lib/metrics.js | 62 +++++++++-- backend/lib/metrics.test.mjs | 71 +++++++++++-- docs/ARCHITECTURE.md | 5 +- docs/BENCHMARKING.md | 137 ++++++++++++++++++++++++ 8 files changed, 561 insertions(+), 19 deletions(-) create mode 100644 backend/lib/memory-snapshot.js create mode 100644 backend/lib/memory-snapshot.test.mjs create mode 100644 docs/BENCHMARKING.md diff --git a/AGENTS.md b/AGENTS.md index a5028331..295fe643 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -113,6 +113,7 @@ Mirrored verbatim from Expo's official [`expo/skills`](https://github.com/expo/s - [`docs/ARCHITECTURE.md`](docs/ARCHITECTURE.md) — process model, IPC channels and framing, boot handshake, lifecycle state machines, error handling, Sentry observability, alternatives considered. - [`docs/BUILD.md`](docs/BUILD.md) — the `build-backend.ts` pipeline, the versioned addon-filename scheme, runtime addon loading, the native-modules source-of-truth model. - [`docs/TESTING.md`](docs/TESTING.md) — the seven test layers, the workflows, the merge queue and required checks, the e2e device suite, the secrets/trust boundary. +- [`docs/BENCHMARKING.md`](docs/BENCHMARKING.md) — measuring the backend's memory footprint: the Sentry gauges that answer the question across the fleet, what they measure and why, including the consent-tier reasoning. - [`CONTRIBUTING.md`](./CONTRIBUTING.md) — setup, every `npm run` script, per-layer test commands, commit/PR/release conventions. Open work is tracked in [GitHub issues](https://github.com/digidem/comapeo-core-react-native/issues). diff --git a/backend/index.js b/backend/index.js index f6b1a6e2..2b5ff376 100644 --- a/backend/index.js +++ b/backend/index.js @@ -7,6 +7,7 @@ import { ComapeoRpc } from "./lib/comapeo-rpc.js"; import { flushCompileCacheAfterBoot } from "./lib/compile-cache.js"; import { createComapeo } from "./lib/create-comapeo.js"; import { createMapServer } from "./lib/create-map-server.js"; +import { memorySnapshot } from "./lib/memory-snapshot.js"; import { SimpleRpcServer } from "./lib/simple-rpc.js"; import * as sentry from "./lib/sentry.js"; import * as metrics from "./lib/metrics.js"; @@ -16,6 +17,15 @@ import { observeSyncSessions } from "./lib/sync-observer.js"; // when Sentry is off (the metrics layer never got its SDK). const MEMORY_SAMPLE_INTERVAL_MS = 60_000; +// One extra sample this long after `ready`, so a short-lived process still +// reports its boot footprint — a 60s-only sampler silently biases the fleet +// data towards the processes that survived, which are not the interesting +// ones. Short-lived is common: 88 of the FGS exits reported from production +// in the last 90 days sit in the `<10s` uptime bucket. `ready` lands ~1.8s in +// and `VmHWM` stops climbing at ~2s, so 3s captures the peak and still +// reports inside that window. +const BOOT_MEMORY_SAMPLE_DELAY_MS = 3_000; + // `KEEP_THESE_FROM_BACKEND` in `scripts/build-backend.ts` mirrors this // directory into the on-device bundle. const MIGRATIONS_FOLDER_PATH = fileURLToPath( @@ -333,11 +343,22 @@ async function withPhase(phase, fn) { * iOS suspends the app, a background suspension is correctly NOT counted as delay. */ function startMemorySampler() { + // Boot sample. The log line is unconditional — one line per launch, no PII, + // and it is what `scripts/benchmark-boot.sh` reads to compare builds on a + // device that may have telemetry switched off entirely. The metric emission + // is still gated, by `backendMemorySample` itself. + const bootTimer = setTimeout(() => { + const snapshot = memorySnapshot(); + console.log(`[comapeo.memory] boot ${JSON.stringify(snapshot)}`); + metrics.backendMemorySample(snapshot); + }, BOOT_MEMORY_SAMPLE_DELAY_MS); + bootTimer.unref?.(); + if (!metrics.isEnabled()) return; const eld = monitorEventLoopDelay({ resolution: 10 }); eld.enable(); const timer = setInterval(() => { - metrics.backendMemorySample(); + metrics.backendMemorySample(memorySnapshot()); metrics.eventLoopDelaySample(eld.max / 1e6); eld.reset(); }, MEMORY_SAMPLE_INTERVAL_MS); diff --git a/backend/lib/memory-snapshot.js b/backend/lib/memory-snapshot.js new file mode 100644 index 00000000..6674b5a5 --- /dev/null +++ b/backend/lib/memory-snapshot.js @@ -0,0 +1,150 @@ +// Process + V8 memory snapshot for the backend. +// +// Two sources that deliberately stay separate because they measure different +// things: +// +// - V8 heap statistics describe the JavaScript heap only, and are +// meaningful on both platforms. +// - `/proc/self/status` describes the whole OS process. That is only "the +// backend" on Android, where node runs in its own `:ComapeoCore` process. +// On iOS node shares the app process, so the same numbers would describe +// the UI too — which is why `metrics.backendMemorySample` has always +// omitted `process.memoryUsage().rss`. iOS has no `/proc`, so the reader +// returns `null` there and the caller emits nothing: the platform gate is +// the filesystem, not a flag that could drift out of sync. +// +// `VmHWM` is the peak resident set since process start. It is the number that +// decides whether Android's low-memory killer picks this process (oom_score +// scales with footprint), and it is monotonic — so one late read still +// captures the boot peak, and no high-rate sampling is needed to find it. +// +// `total_physical_size` is the V8 heap memory actually committed and touched, +// as opposed to `used_heap_size` (live objects) or `total_heap_size` +// (reserved). It is the heap figure that tracks the process's anonymous RSS, +// so it is the one that moves when the runtime's memory layout changes. + +import fs from "node:fs"; +import v8 from "node:v8"; + +const PROC_SELF_STATUS = "/proc/self/status"; + +/** + * @typedef {{ + * rssBytes: number, + * peakRssBytes: number, + * anonBytes: number, + * fileBytes: number, + * swapBytes: number, + * }} ProcessMemory + * + * @typedef {{ + * usedBytes: number, + * physicalBytes: number, + * totalBytes: number, + * limitBytes: number, + * externalBytes: number, + * }} HeapStats + * + * @typedef {{ + * runtime: string, + * heap: HeapStats, + * process: ProcessMemory | null, + * }} MemorySnapshot + */ + +/** `/proc` fields we care about, all reported in kB. */ +const PROC_FIELDS = { + VmRSS: "rssBytes", + VmHWM: "peakRssBytes", + RssAnon: "anonBytes", + RssFile: "fileBytes", + VmSwap: "swapBytes", +}; + +/** + * Parses the `Key: kB` lines of `/proc//status` into bytes. + * Split out from the read so it can be tested against fixture text. + * + * @param {string} text Contents of a `/proc//status` file. + * @returns {Record} Bytes, keyed by the names in `PROC_FIELDS`. + */ +export function parseProcStatus(text) { + /** @type {Record} */ + const out = {}; + for (const line of text.split("\n")) { + const match = /^(\w+):\s+(\d+)\s*kB$/.exec(line); + if (!match) continue; + const key = PROC_FIELDS[/** @type {keyof typeof PROC_FIELDS} */ (match[1])]; + if (key) out[key] = Number(match[2]) * 1024; + } + return out; +} + +/** + * Whole-process memory, or `null` where `/proc` is unavailable (iOS) or + * unreadable. Best-effort by design: this is telemetry, never a boot + * dependency. + * + * @param {{ readFileSync?: (path: string, encoding: string) => string }} [deps] test seam + * @returns {ProcessMemory | null} + */ +export function readProcessMemory(deps = {}) { + const readFileSync = deps.readFileSync ?? fs.readFileSync; + let text; + try { + text = readFileSync(PROC_SELF_STATUS, "utf8"); + } catch { + return null; + } + const parsed = parseProcStatus(text); + // VmRSS and VmHWM are the two that carry the meaning; without them the + // sample is not worth emitting. + if (parsed.rssBytes === undefined || parsed.peakRssBytes === undefined) { + return null; + } + return { + rssBytes: parsed.rssBytes, + peakRssBytes: parsed.peakRssBytes, + anonBytes: parsed.anonBytes ?? 0, + fileBytes: parsed.fileBytes ?? 0, + swapBytes: parsed.swapBytes ?? 0, + }; +} + +/** + * V8 heap statistics, in bytes. + * + * @param {{ getHeapStatistics?: () => v8.HeapInfo }} [deps] test seam + */ +export function readHeapStats(deps = {}) { + const getHeapStatistics = deps.getHeapStatistics ?? v8.getHeapStatistics; + const heap = getHeapStatistics(); + return { + usedBytes: heap.used_heap_size, + physicalBytes: heap.total_physical_size, + totalBytes: heap.total_heap_size, + limitBytes: heap.heap_size_limit, + externalBytes: heap.external_memory ?? 0, + }; +} + +/** + * One combined snapshot. `runtime` is the nodejs-mobile revision + * (`process.versions.mobile`, e.g. `24.19.0-0`) — the dimension you slice by + * when comparing one runtime build against another, and the reason a fleet + * gauge can answer "did the new libnode help" without a bespoke experiment. + * + * @param {{ + * readFileSync?: (path: string, encoding: string) => string, + * getHeapStatistics?: () => v8.HeapInfo, + * versions?: Record, + * }} [deps] test seam + */ +export function memorySnapshot(deps = {}) { + const versions = deps.versions ?? process.versions; + return { + runtime: versions.mobile ?? "unknown", + heap: readHeapStats(deps), + process: readProcessMemory(deps), + }; +} diff --git a/backend/lib/memory-snapshot.test.mjs b/backend/lib/memory-snapshot.test.mjs new file mode 100644 index 00000000..fe5dfdd6 --- /dev/null +++ b/backend/lib/memory-snapshot.test.mjs @@ -0,0 +1,131 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +import { + memorySnapshot, + parseProcStatus, + readHeapStats, + readProcessMemory, +} from "./memory-snapshot.js"; + +const PROC_STATUS = `Name:\tnode +Umask:\t0077 +State:\tS (sleeping) +Tgid:\t14704 +VmPeak:\t26003584 kB +VmSize:\t25937544 kB +VmLck:\t 0 kB +VmHWM:\t 261000 kB +VmRSS:\t 192600 kB +RssAnon:\t 92500 kB +RssFile:\t 99100 kB +RssShmem:\t 580 kB +VmSwap:\t 26500 kB +Threads:\t14 +`; + +/** @type {import("node:v8").HeapInfo} */ +const HEAP = { + total_heap_size: 35_233_792, + total_heap_size_executable: 2_838_528, + total_physical_size: 27_484_160, + total_available_size: 551_480_616, + used_heap_size: 16_598_748, + heap_size_limit: 569_114_624, + malloced_memory: 548_908, + peak_malloced_memory: 28_204_576, + does_zap_garbage: 0, + number_of_native_contexts: 1, + number_of_detached_contexts: 0, + total_global_handles_size: 8_192, + used_global_handles_size: 4_096, + external_memory: 10_081_653, +}; + +test("parseProcStatus converts the kB fields we care about to bytes", () => { + const parsed = parseProcStatus(PROC_STATUS); + assert.equal(parsed.peakRssBytes, 261_000 * 1024); + assert.equal(parsed.rssBytes, 192_600 * 1024); + assert.equal(parsed.anonBytes, 92_500 * 1024); + assert.equal(parsed.fileBytes, 99_100 * 1024); + assert.equal(parsed.swapBytes, 26_500 * 1024); +}); + +test("parseProcStatus ignores fields outside the allowlist", () => { + const parsed = parseProcStatus(PROC_STATUS); + assert.equal(parsed.VmPeak, undefined); + assert.equal(parsed.VmSize, undefined); + assert.equal(Object.keys(parsed).length, 5); +}); + +test("readProcessMemory returns bytes from /proc/self/status", () => { + const mem = readProcessMemory({ readFileSync: () => PROC_STATUS }); + assert.deepEqual(mem, { + rssBytes: 192_600 * 1024, + peakRssBytes: 261_000 * 1024, + anonBytes: 92_500 * 1024, + fileBytes: 99_100 * 1024, + swapBytes: 26_500 * 1024, + }); +}); + +test("readProcessMemory returns null where /proc is unavailable (iOS)", () => { + const mem = readProcessMemory({ + readFileSync: () => { + throw Object.assign(new Error("ENOENT"), { code: "ENOENT" }); + }, + }); + assert.equal(mem, null); +}); + +test("readProcessMemory returns null when the meaningful fields are missing", () => { + const mem = readProcessMemory({ readFileSync: () => "Name:\tnode\nThreads:\t3\n" }); + assert.equal(mem, null); +}); + +test("readProcessMemory tolerates a status file without the optional fields", () => { + const mem = readProcessMemory({ + readFileSync: () => "VmHWM:\t 100 kB\nVmRSS:\t 80 kB\n", + }); + assert.deepEqual(mem, { + rssBytes: 80 * 1024, + peakRssBytes: 100 * 1024, + anonBytes: 0, + fileBytes: 0, + swapBytes: 0, + }); +}); + +test("readHeapStats maps the V8 fields we report", () => { + const heap = readHeapStats({ getHeapStatistics: () => HEAP }); + assert.deepEqual(heap, { + usedBytes: 16_598_748, + physicalBytes: 27_484_160, + totalBytes: 35_233_792, + limitBytes: 569_114_624, + externalBytes: 10_081_653, + }); +}); + +test("memorySnapshot carries the nodejs-mobile revision as `runtime`", () => { + const snapshot = memorySnapshot({ + readFileSync: () => PROC_STATUS, + getHeapStatistics: () => HEAP, + versions: { mobile: "24.19.0-0" }, + }); + assert.equal(snapshot.runtime, "24.19.0-0"); + assert.equal(snapshot.heap.physicalBytes, 27_484_160); + assert.equal(snapshot.process?.peakRssBytes, 261_000 * 1024); +}); + +test("memorySnapshot falls back to `unknown` off nodejs-mobile", () => { + const snapshot = memorySnapshot({ + readFileSync: () => { + throw new Error("no /proc"); + }, + getHeapStatistics: () => HEAP, + versions: {}, + }); + assert.equal(snapshot.runtime, "unknown"); + assert.equal(snapshot.process, null); +}); diff --git a/backend/lib/metrics.js b/backend/lib/metrics.js index b58342ff..21debb59 100644 --- a/backend/lib/metrics.js +++ b/backend/lib/metrics.js @@ -197,19 +197,65 @@ export function syncSession(outcome, ms, peersBucket, bytesBucket) { } } -// ── Backend health (60s sampler) ──────────────────────────────── +// ── Backend health (boot sample, then 60s sampler) ────────────── /** - * Heap-used gauge. `rss` is intentionally omitted: node's `rss` is the whole - * OS process, which on iOS is the entire app (node runs in-process), so it - * wouldn't measure "the backend". `heapUsed` is the V8 JS heap and is - * meaningful on both platforms. + * Backend footprint gauges. + * + * The heap pair is emitted everywhere: `heap_used_bytes` is the live object + * graph, `heap_physical_bytes` the heap memory actually committed and + * touched. The second is what tracks the process's anonymous RSS, so it is + * the one that moves when the runtime's memory layout changes — a V8 build + * flag can halve the first while barely denting the process, or the reverse, + * and only having both tells you which happened. + * + * The RSS pair rides on `snapshot.process`, which is non-null only where + * `/proc` exists **and** node owns the process — i.e. Android's + * `:ComapeoCore`. On iOS node runs in-process, so an `rss` there would be the + * whole app, not the backend; `memory-snapshot.js` returns null and these two + * are simply not emitted. `rss_peak_bytes` is `VmHWM`, the high-water mark + * that Android's low-memory killer effectively scores the process on. + * + * All four sit at the **diagnostic** tier, matching `heap_used_bytes`, which + * has always been diagnostic. They describe the process's own resource use at + * a fixed low cadence, name nothing the user did, and — measured at boot — + * are overwhelmingly a property of the build and the device rather than of + * the data. Free *device* memory is deliberately still absent: that lives in + * the `node_resources` context and stays usage-tier, because read-at-capture + * frequency is itself usage-shape data. + * + * `runtime` is the nodejs-mobile revision, the dimension you group by to + * compare two runtime builds in the field. It is one value per shipped build + * — low cardinality, and no more identifying than the app version already on + * every event. + * + * These carry `device_class` / `os_major` too, unlike the duration metrics + * where they are a nice-to-have. Memory is the metric whose whole point is + * the low-RAM device: `heap_used_bytes` shipped without them and, three + * months in, its 15k samples cannot answer "is the tail coming from cheap + * hardware" at all — which is the only question worth asking of it. + * + * @param {import("./memory-snapshot.js").MemorySnapshot} snapshot */ -export function backendMemorySample() { +export function backendMemorySample(snapshot) { const metrics = api(); if (!metrics) return; - const mem = process.memoryUsage(); - gauge("comapeo.backend.heap_used_bytes", mem.heapUsed, "byte", {}); + const attrs = { ...deviceTags(), runtime: snapshot.runtime }; + gauge("comapeo.backend.heap_used_bytes", snapshot.heap.usedBytes, "byte", attrs); + gauge( + "comapeo.backend.heap_physical_bytes", + snapshot.heap.physicalBytes, + "byte", + attrs, + ); + if (!snapshot.process) return; + gauge("comapeo.backend.rss_bytes", snapshot.process.rssBytes, "byte", attrs); + gauge( + "comapeo.backend.rss_peak_bytes", + snapshot.process.peakRssBytes, + "byte", + attrs, + ); } /** diff --git a/backend/lib/metrics.test.mjs b/backend/lib/metrics.test.mjs index 951be625..99a51e88 100644 --- a/backend/lib/metrics.test.mjs +++ b/backend/lib/metrics.test.mjs @@ -27,6 +27,28 @@ function fakeSentry() { }; } +/** A `memorySnapshot()` shaped fixture; Android values by default. */ +function snapshot(overrides = {}) { + return { + runtime: "24.19.0-0", + heap: { + usedBytes: 16_598_748, + physicalBytes: 27_484_160, + totalBytes: 35_233_792, + limitBytes: 569_114_624, + externalBytes: 10_081_653, + }, + process: { + rssBytes: 192_600 * 1024, + peakRssBytes: 261_000 * 1024, + anonBytes: 92_500 * 1024, + fileBytes: 99_100 * 1024, + swapBytes: 26_500 * 1024, + }, + ...overrides, + }; +} + function initWith(sdk, overrides = {}) { metrics.init({ Sentry: sdk, @@ -43,7 +65,7 @@ beforeEach(() => metrics.resetForTests()); test("no-ops entirely when Sentry is off (init never ran)", () => { // No init → no SDK. Calls must not throw and record nothing. metrics.rpcServer("read.doc", "ok", 12); - metrics.backendMemorySample(); + metrics.backendMemorySample(snapshot()); metrics.storageSizeBucket("<10MB"); // Nothing to assert beyond "did not throw"; the absence of an SDK is // the whole point. @@ -102,16 +124,47 @@ test("syncSession emits one duration metric; peers/bytes buckets are usage-gated ); }); -test("backendMemorySample emits the heap-used gauge in bytes", () => { +test("backendMemorySample emits the heap gauges in bytes, tagged by runtime", () => { + const { sdk, calls } = fakeSentry(); + initWith(sdk); + // No `process` block — this is the iOS shape, where node shares the app + // process so an rss would describe the UI too. Uptime was dropped (a + // sampled monotonic gauge has no actionable aggregate). + metrics.backendMemorySample(snapshot({ process: null })); + assert.deepEqual( + calls.gauge.map((g) => g.name), + ["comapeo.backend.heap_used_bytes", "comapeo.backend.heap_physical_bytes"], + ); + assert.ok(calls.gauge.every((g) => g.unit === "byte")); + assert.ok(calls.gauge.every((g) => g.attributes.runtime === "24.19.0-0")); +}); + +test("backendMemorySample carries device tags — the slice memory is about", () => { + const { sdk, calls } = fakeSentry(); + initWith(sdk); + metrics.backendMemorySample(snapshot()); + assert.ok( + calls.gauge.every( + (g) => g.attributes.device_class === "mid" && g.attributes.os_major === "android.14", + ), + ); +}); + +test("backendMemorySample adds the rss pair where node owns the process", () => { const { sdk, calls } = fakeSentry(); initWith(sdk); - metrics.backendMemorySample(); - const names = calls.gauge.map((g) => g.name); - // rss is intentionally omitted (it measures the whole process, misleading - // on iOS where node runs in-process); uptime was dropped (a sampled - // monotonic gauge has no actionable aggregate). - assert.deepEqual(names, ["comapeo.backend.heap_used_bytes"]); - assert.equal(calls.gauge[0].unit, "byte"); + metrics.backendMemorySample(snapshot()); + assert.deepEqual( + calls.gauge.map((g) => g.name), + [ + "comapeo.backend.heap_used_bytes", + "comapeo.backend.heap_physical_bytes", + "comapeo.backend.rss_bytes", + "comapeo.backend.rss_peak_bytes", + ], + ); + const peak = calls.gauge.find((g) => g.name === "comapeo.backend.rss_peak_bytes"); + assert.equal(peak.value, 261_000 * 1024); }); test("before_metric_send filter drops a forbidden tag name routed through count()", () => { diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index e030cc89..640bc566 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -886,7 +886,10 @@ by `isForbiddenMetric`. | `comapeo.boot.phase_duration_ms` | distribution · ms | `phase`, `device_class`, `os_major` | Node — per boot phase | Where boot time goes (which phase dominates), per device class — the input to boot- and ANR-margin optimisation. | | `comapeo.boot.outcome` | count | `outcome` (`started`/`error`), `error_phase?` | Node — once per boot | Boot success-vs-failure rate and which phase fails — the backend reliability signal. | | `comapeo.storage.size_bucket` | count | `bucket` (`<10MB`/`10-100MB`/`100MB-1GB`/`>1GB`) | Node — one-shot at STARTED (recursive size of the private storage dir) | The population's storage-footprint distribution — context for correlating large DBs with slow sync/query. Coarse, one-shot: context, not an alerting signal. | -| `comapeo.backend.heap_used_bytes` | gauge · byte | — | Node — 60s sampler (V8 JS heap; `rss` is omitted, it measures the whole process) | Backend JS-heap ceiling, and across a session's samples a heap-growth (leak) signal. Coarse on iOS, where the runtime suspends in the background. | +| `comapeo.backend.heap_used_bytes` | gauge · byte | `runtime`, `device_class`, `os_major` | Node — boot sample (`ready` + 3s) then 60s sampler; V8 live object graph | Backend JS-heap ceiling, and across a session's samples a heap-growth (leak) signal. Coarse on iOS, where the runtime suspends in the background. | +| `comapeo.backend.heap_physical_bytes` | gauge · byte | `runtime`, `device_class`, `os_major` | Node — same cadence; V8 heap memory committed and touched | The heap figure that tracks the process's anonymous RSS. Paired with `heap_used_bytes` it separates "we allocate more" from "V8 commits more for the same objects" — which is what a runtime build-flag change moves. | +| `comapeo.backend.rss_bytes` | gauge · byte | `runtime`, `device_class`, `os_major` | Node — same cadence, **Android only** (`/proc/self/status`) | Resident footprint of the `:ComapeoCore` process. Omitted on iOS, where node shares the app process so the number would describe the UI too. | +| `comapeo.backend.rss_peak_bytes` | gauge · byte | `runtime`, `device_class`, `os_major` | Node — same cadence, **Android only** (`VmHWM`) | Peak resident set since process start — what the low-memory killer effectively scores the process on, and the number to watch when judging whether a change makes the FGS a likelier victim. Monotonic, so the boot sample already carries the boot peak. See [BENCHMARKING.md](BENCHMARKING.md). | | `comapeo.backend.event_loop_delay_ms` | distribution · ms | `device_class`, `os_major` | Node — 60s sampler; each emission is the window **max** from `monitorEventLoopDelay` | The worst event-loop stall per minute (sync work blocking async I/O). As a distribution, Sentry gives fleet percentiles of the per-minute worst stall, sliceable by device class. | | `comapeo.sync.session.duration_ms` | distribution · ms | `outcome`, `device_class`, `os_major` | Node — **scaffolding, not yet wired** | (When wired) sync-session latency by outcome and device — core sync performance. | | `comapeo.sync.session.peers_bucket` | count | `bucket` (usage-gated) | Node — **scaffolding, not yet wired** | (When wired) collaboration scale — how many peers took part in a sync. | diff --git a/docs/BENCHMARKING.md b/docs/BENCHMARKING.md new file mode 100644 index 00000000..e77f09f0 --- /dev/null +++ b/docs/BENCHMARKING.md @@ -0,0 +1,137 @@ +# Benchmarking the backend's footprint + +The `:ComapeoCore` process is the one that gets killed first when Android is +short of memory: it runs in the background with `oom_score_adj 200` for life, +and the kernel's `oom_score` scales with footprint, so every megabyte off the +backend makes it a less attractive victim than the foreground UI. That makes +"what does this change cost in memory" a question worth being able to answer +cheaply and repeatedly. + +The backend measures its own footprint and reports it across the fleet as +Sentry gauges, from every install with diagnostics on. The numbers come from +[`backend/lib/memory-snapshot.js`](../backend/lib/memory-snapshot.js). + +## What gets measured, and why those numbers + +`memorySnapshot()` returns three things. + +**Peak RSS (`VmHWM`).** The high-water mark of resident memory since the +process started. This is the headline number: it is what the low-memory killer +effectively scores the process on, and because it is monotonic, one late read +still captures the boot peak — no high-rate sampling needed to find it. + +**Current RSS, split into anonymous / file-backed / swapped.** The split +matters because only anonymous memory moves when you change how much the +backend allocates. File-backed pages are dominated by the ~46 MB of +`libnode.so` mapped straight out of the APK, and they will not shift no matter +what you do to the JavaScript. A change that looks like a 5% win on total RSS +is often a 10% win on the part you actually influenced. + +**V8 heap statistics.** `used_heap_size` is the live object graph; +`total_physical_size` is the heap memory actually committed and touched. Report +both: a V8 build flag can shrink the first while barely denting the process, or +commit far more than it uses. `total_physical_size` is the one that tracks +anonymous RSS. `heap_size_limit` is there as a control — if it moved, the +comparison is not measuring what you think it is. + +The process-level numbers come from `/proc/self/status`, which exists only on +Android. On iOS node runs in-process, so the same fields would describe the UI +too; the reader returns `null` there and only the V8 numbers are reported. The +platform gate is the filesystem, not a flag that could drift. + +## The fleet view: Sentry gauges + +The same snapshot feeds four gauges, emitted once about three seconds after +`ready` and then every 60 seconds: + +| Metric | Unit | Platforms | +|---|---|---| +| `comapeo.backend.heap_used_bytes` | byte | both | +| `comapeo.backend.heap_physical_bytes` | byte | both | +| `comapeo.backend.rss_bytes` | byte | Android | +| `comapeo.backend.rss_peak_bytes` | byte | Android | + +All four carry `device_class`, `os_major` and `runtime`. + +`runtime` is `process.versions.mobile`, the nodejs-mobile revision — the +dimension you group by to compare two runtime builds in the field, and the +reason a staged rollout can answer "did the new libnode help" without a +bespoke experiment. + +`device_class` and `os_major` are there because memory is the metric whose +whole point is the cheap device. `heap_used_bytes` originally shipped without +them, and the consequence is concrete: after three months and ~16k samples, +its Android tail (p50 70 MB, p99 287 MB, max 518 MB — the ceiling is about +542 MB) cannot be attributed to a device class at all, which is the only +question worth asking of it. + +It only separates builds that carry different revisions, though. A libnode +built from an unmerged branch reports whatever tag it was based on, so two such +builds are indistinguishable by this attribute — locally that does not matter +(you know which APK you installed), but anything shipped to devices for +comparison needs its own revision. Bump the nodejs-mobile tag before a staged +rollout you intend to measure. + +The extra boot-time sample exists because a process killed before the first +60-second tick is precisely the case worth knowing about; without it, the +fleet data would be silently biased towards processes that survived. + +### Cost + +One `memorySnapshot()` call measured **~28 µs** on an arm64 emulator (500 +iterations, Release build): a 1.3 KB `/proc/self/status` read plus +`v8.getHeapStatistics()`, neither of which triggers a GC or a syscall storm. +Two calls in the first minute, then one a minute — about 0.3 ms of CPU across a +ten-minute session. The `/proc` read and the parse are also skipped entirely on +iOS, where the file does not exist. + +### Which consent tier, and why + +**All four sit at the diagnostic tier**, alongside `heap_used_bytes`, which has +been diagnostic since it was added. + +The rule this repo applies is that anything whose value or frequency reveals +what, when, or how much a user does belongs behind the application-usage +opt-in; process resource health does not. These gauges are on the health side: + +- They describe the backend's own footprint. They name nothing the user did — + no method names, no project or peer counts, no sync volumes, all of which + stay usage-gated where they already are. +- The headline number is a boot measurement, and the boot peak is + overwhelmingly a property of the build and the device rather than of the + data: compiling the 3 MB bundle, initialising V8, and the two Argon2 + derivations dominate it, and it is essentially unchanged with no client + attached. +- The cadence is fixed and coarse, and deliberately not tied to activity. A + gauge sampled *after each sync* would be usage-shape data no matter what it + measured; one sampled every 60 seconds regardless is not. +- The inference risk that does exist — heap size drifting upward with dataset + size over a long session — is identical to `heap_used_bytes`, which is + already collected at this tier. Adding RSS alongside it changes the units, + not the class of information. + +Two things stay out on purpose: + +- **Free device memory** (`os.freemem()`). It is already available as the + `node_resources` event context and is **usage-tier**, because read-at-capture + frequency is itself usage-shape data. Do not promote it to a diagnostic-tier + metric to sit next to these; that would quietly reclassify it. +- **Anything device-identifying as an attribute.** Total RAM, core count, ABI, + OS version and app version are already on every event, and the bucketed + `device_class` / `os_major` tags are already on the duration metrics. Slice + by those; do not re-send them. + +If this reasoning is ever revisited, the thing to re-examine is the 60-second +series, not the boot sample — the boot sample is the part that is clearly about +the build. + +## Adding a metric + +`docs/sentry-integration.md` §8 has the full rules. The short version for +anything in this area: go through the `metrics.js` wrappers (they inject +`platform` and run the forbidden-attribute filter), name it +`comapeo.._` with an explicit unit, keep attributes +low-cardinality and non-identifying, decide the tier explicitly, and guard the +*work* behind `metrics.isEnabled()` if computing the number is expensive — +these gauges are cheap enough that they do not need it, but a storage walk +does. From 7d1a3288f2e27b8d749ec3c9cf9931eaf4fd9b02 Mon Sep 17 00:00:00 2001 From: Gregor MacLennan Date: Sat, 22 Aug 2026 15:26:06 +0100 Subject: [PATCH 2/3] refactor(backend): mark boot memory samples and share the runtime helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups on the memory gauges. The four gauges now carry `sample` (`boot` or `interval`). Without it the boot sample joins the same series as the 60s sampler, so a release that changes how long processes live moves the percentiles on its own and reads as a footprint change. `runtime` was derived twice — once for the snapshot, once for the `nodejs_mobile` event tag — and threaded through every snapshot argument. It now comes from one exported `runtimeVersion()` and from the metrics config, alongside its `platform` / `device_class` / `os_major` siblings, so events and metrics cannot drift apart. The snapshot keeps its own `runtime` for the `[comapeo.memory] boot` log line. The four open-coded gauge emissions fold into one name-to-value table, and the comment blocks that restated docs/BENCHMARKING.md point at it instead. The reasoning they carried that the doc did not already have — the `<10s`-uptime figure behind the boot sample and the cardinality argument for `runtime` — moved into the doc. --- backend/index.js | 13 ++---- backend/lib/memory-snapshot.js | 47 +++++++------------ backend/lib/metrics.js | 84 +++++++++++++--------------------- backend/lib/metrics.test.mjs | 28 +++++++++--- backend/lib/sentry.js | 3 +- docs/ARCHITECTURE.md | 8 ++-- docs/BENCHMARKING.md | 19 ++++++-- 7 files changed, 96 insertions(+), 106 deletions(-) diff --git a/backend/index.js b/backend/index.js index 2b5ff376..a2753608 100644 --- a/backend/index.js +++ b/backend/index.js @@ -18,12 +18,9 @@ import { observeSyncSessions } from "./lib/sync-observer.js"; const MEMORY_SAMPLE_INTERVAL_MS = 60_000; // One extra sample this long after `ready`, so a short-lived process still -// reports its boot footprint — a 60s-only sampler silently biases the fleet -// data towards the processes that survived, which are not the interesting -// ones. Short-lived is common: 88 of the FGS exits reported from production -// in the last 90 days sit in the `<10s` uptime bucket. `ready` lands ~1.8s in -// and `VmHWM` stops climbing at ~2s, so 3s captures the peak and still -// reports inside that window. +// reports its boot footprint. `ready` lands ~1.8s in and `VmHWM` stops +// climbing at ~2s, so 3s captures the peak and still reports inside the +// window a killed process survives. const BOOT_MEMORY_SAMPLE_DELAY_MS = 3_000; // `KEEP_THESE_FROM_BACKEND` in `scripts/build-backend.ts` mirrors this @@ -350,7 +347,7 @@ function startMemorySampler() { const bootTimer = setTimeout(() => { const snapshot = memorySnapshot(); console.log(`[comapeo.memory] boot ${JSON.stringify(snapshot)}`); - metrics.backendMemorySample(snapshot); + metrics.backendMemorySample(snapshot, "boot"); }, BOOT_MEMORY_SAMPLE_DELAY_MS); bootTimer.unref?.(); @@ -358,7 +355,7 @@ function startMemorySampler() { const eld = monitorEventLoopDelay({ resolution: 10 }); eld.enable(); const timer = setInterval(() => { - metrics.backendMemorySample(memorySnapshot()); + metrics.backendMemorySample(memorySnapshot(), "interval"); metrics.eventLoopDelaySample(eld.max / 1e6); eld.reset(); }, MEMORY_SAMPLE_INTERVAL_MS); diff --git a/backend/lib/memory-snapshot.js b/backend/lib/memory-snapshot.js index 6674b5a5..0ca27a7c 100644 --- a/backend/lib/memory-snapshot.js +++ b/backend/lib/memory-snapshot.js @@ -1,33 +1,23 @@ -// Process + V8 memory snapshot for the backend. -// -// Two sources that deliberately stay separate because they measure different -// things: -// -// - V8 heap statistics describe the JavaScript heap only, and are -// meaningful on both platforms. -// - `/proc/self/status` describes the whole OS process. That is only "the -// backend" on Android, where node runs in its own `:ComapeoCore` process. -// On iOS node shares the app process, so the same numbers would describe -// the UI too — which is why `metrics.backendMemorySample` has always -// omitted `process.memoryUsage().rss`. iOS has no `/proc`, so the reader -// returns `null` there and the caller emits nothing: the platform gate is -// the filesystem, not a flag that could drift out of sync. -// -// `VmHWM` is the peak resident set since process start. It is the number that -// decides whether Android's low-memory killer picks this process (oom_score -// scales with footprint), and it is monotonic — so one late read still -// captures the boot peak, and no high-rate sampling is needed to find it. -// -// `total_physical_size` is the V8 heap memory actually committed and touched, -// as opposed to `used_heap_size` (live objects) or `total_heap_size` -// (reserved). It is the heap figure that tracks the process's anonymous RSS, -// so it is the one that moves when the runtime's memory layout changes. +// V8 heap statistics plus, where `/proc` exists (Android only), whole-process +// memory. What each number means and why it is the one reported: +// docs/BENCHMARKING.md. import fs from "node:fs"; import v8 from "node:v8"; const PROC_SELF_STATUS = "/proc/self/status"; +/** + * nodejs-mobile revision (`process.versions.mobile`, e.g. `24.19.0-0`). Shared + * so the Sentry `nodejs_mobile` event tag and the `runtime` metric attribute + * always join on the same value. + * + * @param {Record} [versions] test seam + */ +export function runtimeVersion(versions = process.versions) { + return versions.mobile ?? "unknown"; +} + /** * @typedef {{ * rssBytes: number, @@ -129,10 +119,8 @@ export function readHeapStats(deps = {}) { } /** - * One combined snapshot. `runtime` is the nodejs-mobile revision - * (`process.versions.mobile`, e.g. `24.19.0-0`) — the dimension you slice by - * when comparing one runtime build against another, and the reason a fleet - * gauge can answer "did the new libnode help" without a bespoke experiment. + * One combined snapshot, `runtime` included so the log line names the + * nodejs-mobile build it measured. * * @param {{ * readFileSync?: (path: string, encoding: string) => string, @@ -141,9 +129,8 @@ export function readHeapStats(deps = {}) { * }} [deps] test seam */ export function memorySnapshot(deps = {}) { - const versions = deps.versions ?? process.versions; return { - runtime: versions.mobile ?? "unknown", + runtime: runtimeVersion(deps.versions), heap: readHeapStats(deps), process: readProcessMemory(deps), }; diff --git a/backend/lib/metrics.js b/backend/lib/metrics.js index 21debb59..afde43c7 100644 --- a/backend/lib/metrics.js +++ b/backend/lib/metrics.js @@ -18,6 +18,7 @@ // so the chunk stays unloaded when Sentry is off. import { isForbiddenMetric } from "../before-send.js"; +import { runtimeVersion } from "./memory-snapshot.js"; /** @type {typeof import("@sentry/node-core") | null} */ let Sentry = null; @@ -26,6 +27,7 @@ let Sentry = null; * platform: string, * deviceClass: string, * osMajor: string, + * runtime: string, * applicationUsageData: boolean, * } | null} */ @@ -46,6 +48,9 @@ export function init(args) { platform: args.platform, deviceClass: args.deviceClass, osMajor: args.osMajor, + // Derived, not injected: sharing `runtimeVersion()` with the + // `nodejs_mobile` event tag is what lets events and metrics join. + runtime: runtimeVersion(), applicationUsageData: args.applicationUsageData, }; } @@ -67,6 +72,9 @@ function defaultTags() { return { platform: config?.platform ?? "unknown" }; } +// Mirrored by `asMetricAttributes()` in android's `DeviceTags.kt` and +// `ios/DeviceTags.swift`; the names must stay in lock-step or native and +// backend metrics stop joining. function deviceTags() { return { device_class: config?.deviceClass ?? "unknown", @@ -200,62 +208,32 @@ export function syncSession(outcome, ms, peersBucket, bytesBucket) { // ── Backend health (boot sample, then 60s sampler) ────────────── /** - * Backend footprint gauges. - * - * The heap pair is emitted everywhere: `heap_used_bytes` is the live object - * graph, `heap_physical_bytes` the heap memory actually committed and - * touched. The second is what tracks the process's anonymous RSS, so it is - * the one that moves when the runtime's memory layout changes — a V8 build - * flag can halve the first while barely denting the process, or the reverse, - * and only having both tells you which happened. - * - * The RSS pair rides on `snapshot.process`, which is non-null only where - * `/proc` exists **and** node owns the process — i.e. Android's - * `:ComapeoCore`. On iOS node runs in-process, so an `rss` there would be the - * whole app, not the backend; `memory-snapshot.js` returns null and these two - * are simply not emitted. `rss_peak_bytes` is `VmHWM`, the high-water mark - * that Android's low-memory killer effectively scores the process on. - * - * All four sit at the **diagnostic** tier, matching `heap_used_bytes`, which - * has always been diagnostic. They describe the process's own resource use at - * a fixed low cadence, name nothing the user did, and — measured at boot — - * are overwhelmingly a property of the build and the device rather than of - * the data. Free *device* memory is deliberately still absent: that lives in - * the `node_resources` context and stays usage-tier, because read-at-capture - * frequency is itself usage-shape data. - * - * `runtime` is the nodejs-mobile revision, the dimension you group by to - * compare two runtime builds in the field. It is one value per shipped build - * — low cardinality, and no more identifying than the app version already on - * every event. - * - * These carry `device_class` / `os_major` too, unlike the duration metrics - * where they are a nice-to-have. Memory is the metric whose whole point is - * the low-RAM device: `heap_used_bytes` shipped without them and, three - * months in, its 15k samples cannot answer "is the tail coming from cheap - * hardware" at all — which is the only question worth asking of it. + * Backend footprint gauges, at the diagnostic tier. What each number measures, + * why the RSS pair is Android-only and why this tier: docs/BENCHMARKING.md. * * @param {import("./memory-snapshot.js").MemorySnapshot} snapshot + * @param {"boot" | "interval"} sample Which of the two call sites emitted it — + * without it the boot samples are indistinguishable from the 60s series and + * a population change reads as a footprint change. */ -export function backendMemorySample(snapshot) { - const metrics = api(); - if (!metrics) return; - const attrs = { ...deviceTags(), runtime: snapshot.runtime }; - gauge("comapeo.backend.heap_used_bytes", snapshot.heap.usedBytes, "byte", attrs); - gauge( - "comapeo.backend.heap_physical_bytes", - snapshot.heap.physicalBytes, - "byte", - attrs, - ); - if (!snapshot.process) return; - gauge("comapeo.backend.rss_bytes", snapshot.process.rssBytes, "byte", attrs); - gauge( - "comapeo.backend.rss_peak_bytes", - snapshot.process.peakRssBytes, - "byte", - attrs, - ); +export function backendMemorySample(snapshot, sample) { + if (!api()) return; + const attrs = { + ...deviceTags(), + runtime: config?.runtime ?? "unknown", + sample, + }; + const values = { + "comapeo.backend.heap_used_bytes": snapshot.heap.usedBytes, + "comapeo.backend.heap_physical_bytes": snapshot.heap.physicalBytes, + ...(snapshot.process && { + "comapeo.backend.rss_bytes": snapshot.process.rssBytes, + "comapeo.backend.rss_peak_bytes": snapshot.process.peakRssBytes, + }), + }; + for (const [name, value] of Object.entries(values)) { + gauge(name, value, "byte", attrs); + } } /** diff --git a/backend/lib/metrics.test.mjs b/backend/lib/metrics.test.mjs index 99a51e88..5a582805 100644 --- a/backend/lib/metrics.test.mjs +++ b/backend/lib/metrics.test.mjs @@ -1,6 +1,7 @@ import { test, beforeEach } from "node:test"; import assert from "node:assert/strict"; +import { runtimeVersion } from "./memory-snapshot.js"; import * as metrics from "./metrics.js"; /** @@ -65,7 +66,7 @@ beforeEach(() => metrics.resetForTests()); test("no-ops entirely when Sentry is off (init never ran)", () => { // No init → no SDK. Calls must not throw and record nothing. metrics.rpcServer("read.doc", "ok", 12); - metrics.backendMemorySample(snapshot()); + metrics.backendMemorySample(snapshot(), "boot"); metrics.storageSizeBucket("<10MB"); // Nothing to assert beyond "did not throw"; the absence of an SDK is // the whole point. @@ -128,21 +129,22 @@ test("backendMemorySample emits the heap gauges in bytes, tagged by runtime", () const { sdk, calls } = fakeSentry(); initWith(sdk); // No `process` block — this is the iOS shape, where node shares the app - // process so an rss would describe the UI too. Uptime was dropped (a - // sampled monotonic gauge has no actionable aggregate). - metrics.backendMemorySample(snapshot({ process: null })); + // process so an rss would describe the UI too. + metrics.backendMemorySample(snapshot({ process: null }), "interval"); assert.deepEqual( calls.gauge.map((g) => g.name), ["comapeo.backend.heap_used_bytes", "comapeo.backend.heap_physical_bytes"], ); assert.ok(calls.gauge.every((g) => g.unit === "byte")); - assert.ok(calls.gauge.every((g) => g.attributes.runtime === "24.19.0-0")); + // The config value, shared with the `nodejs_mobile` event tag — not the + // snapshot's own `runtime`, which the fixture sets to something else. + assert.ok(calls.gauge.every((g) => g.attributes.runtime === runtimeVersion())); }); test("backendMemorySample carries device tags — the slice memory is about", () => { const { sdk, calls } = fakeSentry(); initWith(sdk); - metrics.backendMemorySample(snapshot()); + metrics.backendMemorySample(snapshot(), "interval"); assert.ok( calls.gauge.every( (g) => g.attributes.device_class === "mid" && g.attributes.os_major === "android.14", @@ -150,10 +152,22 @@ test("backendMemorySample carries device tags — the slice memory is about", () ); }); +test("backendMemorySample marks which call site sampled", () => { + const { sdk, calls } = fakeSentry(); + initWith(sdk); + metrics.backendMemorySample(snapshot(), "boot"); + metrics.backendMemorySample(snapshot(), "interval"); + assert.equal(calls.gauge.length, 8); + assert.ok(calls.gauge.slice(0, 4).every((g) => g.attributes.sample === "boot")); + assert.ok( + calls.gauge.slice(4).every((g) => g.attributes.sample === "interval"), + ); +}); + test("backendMemorySample adds the rss pair where node owns the process", () => { const { sdk, calls } = fakeSentry(); initWith(sdk); - metrics.backendMemorySample(snapshot()); + metrics.backendMemorySample(snapshot(), "interval"); assert.deepEqual( calls.gauge.map((g) => g.name), [ diff --git a/backend/lib/sentry.js b/backend/lib/sentry.js index 9cbe41bf..a19131d6 100644 --- a/backend/lib/sentry.js +++ b/backend/lib/sentry.js @@ -7,6 +7,7 @@ /** @typedef {import("./sentry-frame.js").SentryFrame} SentryFrame */ import { scrubEvent, scrubLog } from "../before-send.js"; +import { runtimeVersion } from "./memory-snapshot.js"; import * as metrics from "./metrics.js"; import { createNodeResourcesProcessor } from "./node-resources.js"; @@ -190,7 +191,7 @@ export function init({ Sentry: sdk, argv, envelopeToFrame: toFrame, storageDir } tags: { proc: "fgs", layer: "node", - nodejs_mobile: process.versions.mobile ?? "unknown", + nodejs_mobile: runtimeVersion(), }, // Native-derived user.id (monthly/permanent hash) — same value the // FGS and RN layers set, so one launch reports one user. diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 640bc566..f480c27d 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -886,10 +886,10 @@ by `isForbiddenMetric`. | `comapeo.boot.phase_duration_ms` | distribution · ms | `phase`, `device_class`, `os_major` | Node — per boot phase | Where boot time goes (which phase dominates), per device class — the input to boot- and ANR-margin optimisation. | | `comapeo.boot.outcome` | count | `outcome` (`started`/`error`), `error_phase?` | Node — once per boot | Boot success-vs-failure rate and which phase fails — the backend reliability signal. | | `comapeo.storage.size_bucket` | count | `bucket` (`<10MB`/`10-100MB`/`100MB-1GB`/`>1GB`) | Node — one-shot at STARTED (recursive size of the private storage dir) | The population's storage-footprint distribution — context for correlating large DBs with slow sync/query. Coarse, one-shot: context, not an alerting signal. | -| `comapeo.backend.heap_used_bytes` | gauge · byte | `runtime`, `device_class`, `os_major` | Node — boot sample (`ready` + 3s) then 60s sampler; V8 live object graph | Backend JS-heap ceiling, and across a session's samples a heap-growth (leak) signal. Coarse on iOS, where the runtime suspends in the background. | -| `comapeo.backend.heap_physical_bytes` | gauge · byte | `runtime`, `device_class`, `os_major` | Node — same cadence; V8 heap memory committed and touched | The heap figure that tracks the process's anonymous RSS. Paired with `heap_used_bytes` it separates "we allocate more" from "V8 commits more for the same objects" — which is what a runtime build-flag change moves. | -| `comapeo.backend.rss_bytes` | gauge · byte | `runtime`, `device_class`, `os_major` | Node — same cadence, **Android only** (`/proc/self/status`) | Resident footprint of the `:ComapeoCore` process. Omitted on iOS, where node shares the app process so the number would describe the UI too. | -| `comapeo.backend.rss_peak_bytes` | gauge · byte | `runtime`, `device_class`, `os_major` | Node — same cadence, **Android only** (`VmHWM`) | Peak resident set since process start — what the low-memory killer effectively scores the process on, and the number to watch when judging whether a change makes the FGS a likelier victim. Monotonic, so the boot sample already carries the boot peak. See [BENCHMARKING.md](BENCHMARKING.md). | +| `comapeo.backend.heap_used_bytes` | gauge · byte | `sample` (`boot`/`interval`), `runtime`, `device_class`, `os_major` | Node — boot sample (`ready` + 3s) then 60s sampler; V8 live object graph | Backend JS-heap ceiling, and across a session's samples a heap-growth (leak) signal. Coarse on iOS, where the runtime suspends in the background. | +| `comapeo.backend.heap_physical_bytes` | gauge · byte | `sample` (`boot`/`interval`), `runtime`, `device_class`, `os_major` | Node — same cadence; V8 heap memory committed and touched | The heap figure that tracks the process's anonymous RSS. Paired with `heap_used_bytes` it separates "we allocate more" from "V8 commits more for the same objects" — which is what a runtime build-flag change moves. | +| `comapeo.backend.rss_bytes` | gauge · byte | `sample` (`boot`/`interval`), `runtime`, `device_class`, `os_major` | Node — same cadence, **Android only** (`/proc/self/status`) | Resident footprint of the `:ComapeoCore` process. Omitted on iOS, where node shares the app process so the number would describe the UI too. | +| `comapeo.backend.rss_peak_bytes` | gauge · byte | `sample` (`boot`/`interval`), `runtime`, `device_class`, `os_major` | Node — same cadence, **Android only** (`VmHWM`) | Peak resident set since process start — what the low-memory killer effectively scores the process on, and the number to watch when judging whether a change makes the FGS a likelier victim. Monotonic, so the boot sample already carries the boot peak. See [BENCHMARKING.md](BENCHMARKING.md). | | `comapeo.backend.event_loop_delay_ms` | distribution · ms | `device_class`, `os_major` | Node — 60s sampler; each emission is the window **max** from `monitorEventLoopDelay` | The worst event-loop stall per minute (sync work blocking async I/O). As a distribution, Sentry gives fleet percentiles of the per-minute worst stall, sliceable by device class. | | `comapeo.sync.session.duration_ms` | distribution · ms | `outcome`, `device_class`, `os_major` | Node — **scaffolding, not yet wired** | (When wired) sync-session latency by outcome and device — core sync performance. | | `comapeo.sync.session.peers_bucket` | count | `bucket` (usage-gated) | Node — **scaffolding, not yet wired** | (When wired) collaboration scale — how many peers took part in a sync. | diff --git a/docs/BENCHMARKING.md b/docs/BENCHMARKING.md index e77f09f0..13ee2d73 100644 --- a/docs/BENCHMARKING.md +++ b/docs/BENCHMARKING.md @@ -51,12 +51,21 @@ The same snapshot feeds four gauges, emitted once about three seconds after | `comapeo.backend.rss_bytes` | byte | Android | | `comapeo.backend.rss_peak_bytes` | byte | Android | -All four carry `device_class`, `os_major` and `runtime`. +All four carry `device_class`, `os_major`, `runtime` and `sample`. + +`sample` is `boot` or `interval`, naming which of the two call sites emitted +the gauge. Without it the boot samples and the 60-second series are one +population, so a release that changes how long processes live — or how often +the boot sample lands at all — moves the percentiles on its own and reads as a +footprint change. Filter to one before comparing two builds. `runtime` is `process.versions.mobile`, the nodejs-mobile revision — the dimension you group by to compare two runtime builds in the field, and the reason a staged rollout can answer "did the new libnode help" without a -bespoke experiment. +bespoke experiment. It is one value per shipped build, so it costs nothing in +cardinality and is no more identifying than the app version already on every +event. The same value is the `nodejs_mobile` tag on Sentry events (both read +`runtimeVersion()` in `memory-snapshot.js`), so events and metrics join on it. `device_class` and `os_major` are there because memory is the metric whose whole point is the cheap device. `heap_used_bytes` originally shipped without @@ -74,7 +83,11 @@ rollout you intend to measure. The extra boot-time sample exists because a process killed before the first 60-second tick is precisely the case worth knowing about; without it, the -fleet data would be silently biased towards processes that survived. +fleet data would be silently biased towards processes that survived. That case +is common: 88 of the FGS exits reported from production in the last 90 days +sit in the `<10s` uptime bucket. It fires 3s after `ready` — `ready` lands +~1.8s in and `VmHWM` stops climbing at ~2s, so 3s captures the boot peak and +still reports inside the window a short-lived process survives. ### Cost From bc630b0c0a60ebc930caaf8d2c7ed29f06532652 Mon Sep 17 00:00:00 2001 From: Gregor MacLennan Date: Mon, 24 Aug 2026 14:49:54 +0100 Subject: [PATCH 3/3] refactor(backend): drop the test-only deps seam from memory-snapshot Pure processMemoryFrom/heapStatsFrom carry the mapping tests on fixtures; memorySnapshot() takes no parameters and a smoke test runs the real path, which exercises both sides of the /proc platform gate for real (Linux CI has /proc, macOS dev machines don't). --- backend/lib/memory-snapshot.js | 52 +++++++++---------- backend/lib/memory-snapshot.test.mjs | 74 +++++++++++----------------- 2 files changed, 53 insertions(+), 73 deletions(-) diff --git a/backend/lib/memory-snapshot.js b/backend/lib/memory-snapshot.js index 0ca27a7c..cf71ba08 100644 --- a/backend/lib/memory-snapshot.js +++ b/backend/lib/memory-snapshot.js @@ -12,7 +12,7 @@ const PROC_SELF_STATUS = "/proc/self/status"; * so the Sentry `nodejs_mobile` event tag and the `runtime` metric attribute * always join on the same value. * - * @param {Record} [versions] test seam + * @param {Record} [versions] */ export function runtimeVersion(versions = process.versions) { return versions.mobile ?? "unknown"; @@ -53,7 +53,6 @@ const PROC_FIELDS = { /** * Parses the `Key: kB` lines of `/proc//status` into bytes. - * Split out from the read so it can be tested against fixture text. * * @param {string} text Contents of a `/proc//status` file. * @returns {Record} Bytes, keyed by the names in `PROC_FIELDS`. @@ -71,21 +70,13 @@ export function parseProcStatus(text) { } /** - * Whole-process memory, or `null` where `/proc` is unavailable (iOS) or - * unreadable. Best-effort by design: this is telemetry, never a boot - * dependency. + * Whole-process memory from `/proc//status` text, or `null` when the + * meaningful fields are absent. * - * @param {{ readFileSync?: (path: string, encoding: string) => string }} [deps] test seam + * @param {string} text Contents of a `/proc//status` file. * @returns {ProcessMemory | null} */ -export function readProcessMemory(deps = {}) { - const readFileSync = deps.readFileSync ?? fs.readFileSync; - let text; - try { - text = readFileSync(PROC_SELF_STATUS, "utf8"); - } catch { - return null; - } +export function processMemoryFrom(text) { const parsed = parseProcStatus(text); // VmRSS and VmHWM are the two that carry the meaning; without them the // sample is not worth emitting. @@ -102,13 +93,12 @@ export function readProcessMemory(deps = {}) { } /** - * V8 heap statistics, in bytes. + * The V8 heap fields we report, in bytes. * - * @param {{ getHeapStatistics?: () => v8.HeapInfo }} [deps] test seam + * @param {v8.HeapInfo} heap + * @returns {HeapStats} */ -export function readHeapStats(deps = {}) { - const getHeapStatistics = deps.getHeapStatistics ?? v8.getHeapStatistics; - const heap = getHeapStatistics(); +export function heapStatsFrom(heap) { return { usedBytes: heap.used_heap_size, physicalBytes: heap.total_physical_size, @@ -120,18 +110,22 @@ export function readHeapStats(deps = {}) { /** * One combined snapshot, `runtime` included so the log line names the - * nodejs-mobile build it measured. + * nodejs-mobile build it measured. `process` is `null` where `/proc` is + * unavailable (iOS) or unreadable — best-effort by design: this is telemetry, + * never a boot dependency. * - * @param {{ - * readFileSync?: (path: string, encoding: string) => string, - * getHeapStatistics?: () => v8.HeapInfo, - * versions?: Record, - * }} [deps] test seam + * @returns {MemorySnapshot} */ -export function memorySnapshot(deps = {}) { +export function memorySnapshot() { + let process_ = null; + try { + process_ = processMemoryFrom(fs.readFileSync(PROC_SELF_STATUS, "utf8")); + } catch { + // No `/proc` here. + } return { - runtime: runtimeVersion(deps.versions), - heap: readHeapStats(deps), - process: readProcessMemory(deps), + runtime: runtimeVersion(), + heap: heapStatsFrom(v8.getHeapStatistics()), + process: process_, }; } diff --git a/backend/lib/memory-snapshot.test.mjs b/backend/lib/memory-snapshot.test.mjs index fe5dfdd6..c9b58658 100644 --- a/backend/lib/memory-snapshot.test.mjs +++ b/backend/lib/memory-snapshot.test.mjs @@ -2,10 +2,11 @@ import test from "node:test"; import assert from "node:assert/strict"; import { + heapStatsFrom, memorySnapshot, parseProcStatus, - readHeapStats, - readProcessMemory, + processMemoryFrom, + runtimeVersion, } from "./memory-snapshot.js"; const PROC_STATUS = `Name:\tnode @@ -58,9 +59,8 @@ test("parseProcStatus ignores fields outside the allowlist", () => { assert.equal(Object.keys(parsed).length, 5); }); -test("readProcessMemory returns bytes from /proc/self/status", () => { - const mem = readProcessMemory({ readFileSync: () => PROC_STATUS }); - assert.deepEqual(mem, { +test("processMemoryFrom maps a full status file", () => { + assert.deepEqual(processMemoryFrom(PROC_STATUS), { rssBytes: 192_600 * 1024, peakRssBytes: 261_000 * 1024, anonBytes: 92_500 * 1024, @@ -69,25 +69,12 @@ test("readProcessMemory returns bytes from /proc/self/status", () => { }); }); -test("readProcessMemory returns null where /proc is unavailable (iOS)", () => { - const mem = readProcessMemory({ - readFileSync: () => { - throw Object.assign(new Error("ENOENT"), { code: "ENOENT" }); - }, - }); - assert.equal(mem, null); -}); - -test("readProcessMemory returns null when the meaningful fields are missing", () => { - const mem = readProcessMemory({ readFileSync: () => "Name:\tnode\nThreads:\t3\n" }); - assert.equal(mem, null); +test("processMemoryFrom returns null when the meaningful fields are missing", () => { + assert.equal(processMemoryFrom("Name:\tnode\nThreads:\t3\n"), null); }); -test("readProcessMemory tolerates a status file without the optional fields", () => { - const mem = readProcessMemory({ - readFileSync: () => "VmHWM:\t 100 kB\nVmRSS:\t 80 kB\n", - }); - assert.deepEqual(mem, { +test("processMemoryFrom tolerates a status file without the optional fields", () => { + assert.deepEqual(processMemoryFrom("VmHWM:\t 100 kB\nVmRSS:\t 80 kB\n"), { rssBytes: 80 * 1024, peakRssBytes: 100 * 1024, anonBytes: 0, @@ -96,9 +83,8 @@ test("readProcessMemory tolerates a status file without the optional fields", () }); }); -test("readHeapStats maps the V8 fields we report", () => { - const heap = readHeapStats({ getHeapStatistics: () => HEAP }); - assert.deepEqual(heap, { +test("heapStatsFrom maps the V8 fields we report", () => { + assert.deepEqual(heapStatsFrom(HEAP), { usedBytes: 16_598_748, physicalBytes: 27_484_160, totalBytes: 35_233_792, @@ -107,25 +93,25 @@ test("readHeapStats maps the V8 fields we report", () => { }); }); -test("memorySnapshot carries the nodejs-mobile revision as `runtime`", () => { - const snapshot = memorySnapshot({ - readFileSync: () => PROC_STATUS, - getHeapStatistics: () => HEAP, - versions: { mobile: "24.19.0-0" }, - }); - assert.equal(snapshot.runtime, "24.19.0-0"); - assert.equal(snapshot.heap.physicalBytes, 27_484_160); - assert.equal(snapshot.process?.peakRssBytes, 261_000 * 1024); +test("runtimeVersion reads the nodejs-mobile revision", () => { + assert.equal(runtimeVersion({ mobile: "24.19.0-0" }), "24.19.0-0"); + assert.equal(runtimeVersion({}), "unknown"); }); -test("memorySnapshot falls back to `unknown` off nodejs-mobile", () => { - const snapshot = memorySnapshot({ - readFileSync: () => { - throw new Error("no /proc"); - }, - getHeapStatistics: () => HEAP, - versions: {}, - }); - assert.equal(snapshot.runtime, "unknown"); - assert.equal(snapshot.process, null); +// The real production path, end to end. `/proc/self/status` genuinely exists +// on Linux and genuinely doesn't on macOS/iOS, so between dev machines and CI +// both sides of the platform gate run for real. +test("memorySnapshot returns a well-formed snapshot of this process", () => { + const snapshot = memorySnapshot(); + assert.equal(typeof snapshot.runtime, "string"); + assert.ok(snapshot.heap.usedBytes > 0); + assert.ok(snapshot.heap.physicalBytes > 0); + assert.ok(snapshot.heap.limitBytes > 0); + if (process.platform === "linux") { + assert.ok(snapshot.process !== null); + assert.ok(snapshot.process.rssBytes > 0); + assert.ok(snapshot.process.peakRssBytes >= snapshot.process.rssBytes); + } else { + assert.equal(snapshot.process, null); + } });