feat(stack-files): in-process metrics and structured mutation logs (#1216)

* feat(stack-files): in-process metrics and structured mutation logs

Adds FileExplorerMetricsService, an in-memory counter and latency
histogram keyed by (nodeId, op) modelled on StackOpMetricsService.
record() is called once per file-route request from a small
recordFileOp helper that wraps the metric capture; rejection paths
that ran real filesystem work (overwrite confirms, write conflicts,
multer oversize) now record an error count and a warn log instead of
disappearing from the snapshot. recordUploadBytes tracks bytes that
actually persisted so a node taking many small uploads vs a few large
ones is visible in the dashboard.

Admin-only GET /api/file-explorer-metrics returns the snapshot in the
same shape as /api/stack-metrics so an operator chasing a slow node
has a single place to look. No external telemetry; everything is
process-local and resets on restart.

Mutation INFO lines now carry op, stack, path, and bytes/mode/
recursive/overwrite/toPath in the structured details so log scrapers
can pivot on the same identity the metric uses. The existing
developer_mode gate on logFileDiag is unchanged. A new
rejectFileMutation helper centralises the log+metric+response triple
on the three rejection sites (upload DIR_EXISTS, upload FILE_EXISTS,
write PRECONDITION_FAILED) so a future rejection cannot skip the
metric.

Tests cover the service in isolation (counts, p50/p95, ring buffer cap,
upload bytes tracking, snapshot sorting), the admin route auth and
shape, and the route layer end-to-end: a real upload surfaces in
/api/file-explorer-metrics, a FILE_EXISTS rejection bumps errorCount,
and the structured INFO line carries the expected fields.

* fix(stack-files): tighten download/upload latency tracking

Two metric-accuracy bugs caught in independent review:

Download metric was recorded as a success before result.stream.pipe(res)
ran. A mid-stream read failure or a client disconnect was not counted as
an error because the recorder fired at pipe time, not stream completion.
Now the recorder hangs off res.on('finish') for success and on both the
stream's error event and res.on('close') for failure, with a flag so a
normal completion (which emits both finish and close) does not produce
two recordings.

Upload latency was inconsistent across the success and failure branches.
The multer wrapper captured startedAt at route entry, but the async
handler created its own startedAt after multer had already buffered the
body. Successful uploads therefore reported only the post-multer time
and the multipart transfer/buffer cost vanished from the histogram. The
wrapper now stashes the route-entry timestamp on the request object and
the async handler reads it back, so every metric for a given upload
shares one window.

A new test pins the download recorder behaviour: a single successful
download must produce successCount=1, count=1, errorCount=0 in the
snapshot, which would have flagged the original double-fire path the
finish+close pair could have introduced.
This commit is contained in:
Anso
2026-05-25 01:46:08 -04:00
committed by GitHub
parent d8b6f8cf3b
commit 9f2f13f35a
7 changed files with 669 additions and 22 deletions
@@ -0,0 +1,68 @@
/**
* Integration tests for GET /api/file-explorer-metrics. Admin-only endpoint
* surfacing the in-process snapshot from FileExplorerMetricsService.
*/
import { describe, it, expect, beforeAll, afterAll, beforeEach, vi } from 'vitest';
import request from 'supertest';
import { setupTestDb, cleanupTestDb, loginAsTestAdmin } from './helpers/setupTestDb';
let tmpDir: string;
let app: import('express').Express;
let authCookie: string;
beforeAll(async () => {
tmpDir = await setupTestDb();
({ app } = await import('../index'));
authCookie = await loginAsTestAdmin(app);
});
afterAll(() => {
vi.restoreAllMocks();
cleanupTestDb(tmpDir);
});
beforeEach(async () => {
const { FileExplorerMetricsService } = await import('../services/FileExplorerMetricsService');
FileExplorerMetricsService.resetForTests();
});
describe('GET /api/file-explorer-metrics', () => {
it('returns 401 without an auth cookie', async () => {
const res = await request(app).get('/api/file-explorer-metrics');
expect(res.status).toBe(401);
});
it('returns an empty snapshot on a fresh process', async () => {
const res = await request(app).get('/api/file-explorer-metrics').set('Cookie', authCookie);
expect(res.status).toBe(200);
expect(res.body).toEqual({ entries: [], uploadBytesByNode: [] });
});
it('returns recorded entries and upload bytes with the expected shape', async () => {
const { FileExplorerMetricsService } = await import('../services/FileExplorerMetricsService');
const svc = FileExplorerMetricsService.getInstance();
svc.record(1, 'upload', 100, true);
svc.record(1, 'upload', 200, false);
svc.record(2, 'read', 50, true);
svc.recordUploadBytes(1, 1024);
svc.recordUploadBytes(2, 2048);
const res = await request(app).get('/api/file-explorer-metrics').set('Cookie', authCookie);
expect(res.status).toBe(200);
expect(res.body.entries).toHaveLength(2);
expect(res.body.entries[0]).toMatchObject({
nodeId: 1,
op: 'upload',
count: 2,
successCount: 1,
errorCount: 1,
avgMs: 150,
});
expect(typeof res.body.entries[0].p50Ms).toBe('number');
expect(typeof res.body.entries[0].p95Ms).toBe('number');
expect(res.body.uploadBytesByNode).toEqual([
{ nodeId: 1, totalBytes: 1024 },
{ nodeId: 2, totalBytes: 2048 },
]);
});
});
@@ -0,0 +1,100 @@
import { describe, it, expect, beforeEach } from 'vitest';
import { FileExplorerMetricsService } from '../services/FileExplorerMetricsService';
beforeEach(() => {
FileExplorerMetricsService.resetForTests();
});
describe('FileExplorerMetricsService', () => {
it('aggregates count, success, error, and avg/p50/p95 latency per (nodeId, op)', () => {
const svc = FileExplorerMetricsService.getInstance();
svc.record(1, 'upload', 10, true);
svc.record(1, 'upload', 30, true);
svc.record(1, 'upload', 50, false);
svc.record(1, 'read', 5, true);
svc.record(2, 'upload', 1000, true);
const { entries } = svc.snapshot();
const node1Upload = entries.find(e => e.nodeId === 1 && e.op === 'upload')!;
expect(node1Upload.count).toBe(3);
expect(node1Upload.successCount).toBe(2);
expect(node1Upload.errorCount).toBe(1);
expect(node1Upload.avgMs).toBe(30);
// percentile() floors (n-1) * p, so with sorted = [10, 30, 50]: p50 = sorted[1] = 30
// and p95 = sorted[floor((3-1)*0.95)] = sorted[1] = 30. The shared pattern with
// StackOpMetricsService accepts this off-by-floor; the histogram is for
// operator triage, not statistical reporting.
expect(node1Upload.p50Ms).toBe(30);
expect(node1Upload.p95Ms).toBe(30);
const node1Read = entries.find(e => e.nodeId === 1 && e.op === 'read')!;
expect(node1Read.count).toBe(1);
expect(node1Read.successCount).toBe(1);
expect(node1Read.errorCount).toBe(0);
const node2Upload = entries.find(e => e.nodeId === 2 && e.op === 'upload')!;
expect(node2Upload.count).toBe(1);
expect(node2Upload.avgMs).toBe(1000);
});
it('rejects negative or non-finite latency without growing the ring buffer', () => {
const svc = FileExplorerMetricsService.getInstance();
svc.record(1, 'upload', -1, true);
svc.record(1, 'upload', NaN, true);
svc.record(1, 'upload', Infinity, true);
expect(svc.size()).toBe(0);
expect(svc.snapshot().entries).toEqual([]);
});
it('caps the ring buffer at 1000 samples (older samples dropped on overflow)', () => {
const svc = FileExplorerMetricsService.getInstance();
// Record 1050 ops with increasing latencies. p95 should reflect the recent
// window, not the dropped low values from the very start.
for (let i = 0; i < 1050; i++) {
svc.record(1, 'upload', i, true);
}
const entry = svc.snapshot().entries.find(e => e.nodeId === 1 && e.op === 'upload')!;
expect(entry.count).toBe(1050);
// The ring buffer dropped the first 50, so p50 ≈ 524 (middle of 50..1049)
// and p95 ≈ 1001 (95th percentile of the same window). Use a relaxed
// tolerance because percentile() floors the index.
expect(entry.p50Ms).toBeGreaterThan(520);
expect(entry.p50Ms).toBeLessThan(560);
expect(entry.p95Ms).toBeGreaterThan(990);
});
it('tracks upload bytes per node and sorts the snapshot by nodeId', () => {
const svc = FileExplorerMetricsService.getInstance();
svc.recordUploadBytes(2, 1024);
svc.recordUploadBytes(1, 512);
svc.recordUploadBytes(2, 256);
const { uploadBytesByNode } = svc.snapshot();
expect(uploadBytesByNode).toEqual([
{ nodeId: 1, totalBytes: 512 },
{ nodeId: 2, totalBytes: 1280 },
]);
});
it('ignores negative or non-finite upload byte counts', () => {
const svc = FileExplorerMetricsService.getInstance();
svc.recordUploadBytes(1, -100);
svc.recordUploadBytes(1, NaN);
svc.recordUploadBytes(1, Infinity);
expect(svc.snapshot().uploadBytesByNode).toEqual([]);
});
it('snapshot entries sort by nodeId ascending then op alphabetically', () => {
const svc = FileExplorerMetricsService.getInstance();
svc.record(2, 'upload', 1, true);
svc.record(1, 'write', 1, true);
svc.record(1, 'read', 1, true);
const { entries } = svc.snapshot();
expect(entries.map(e => `${e.nodeId}:${e.op}`)).toEqual([
'1:read',
'1:write',
'2:upload',
]);
});
});
@@ -126,6 +126,81 @@ describe('GET /api/stacks/:stackName/files', () => {
DatabaseService.getInstance().updateGlobalSetting('developer_mode', '0');
});
it('emits a structured INFO line on a successful mutation (op/stack/path/bytes)', async () => {
const logSpy = vi.spyOn(console, 'log').mockImplementation(() => undefined);
await request(app)
.post(`/api/stacks/${STACK}/files/upload`)
.set('Cookie', adminCookie)
.attach('file', Buffer.from('observability payload'), 'observability-info.txt');
const mutateCall = logSpy.mock.calls.find(
([prefix, details]) =>
typeof prefix === 'string' &&
prefix.includes('[Files] mutate') &&
details &&
typeof details === 'object' &&
(details as { op?: unknown }).op === 'upload',
);
expect(mutateCall).toBeDefined();
const details = mutateCall![1] as Record<string, unknown>;
// sanitizeForLog coerces every value to a string before it lands in the log
// payload, so bytes/mode/etc. arrive as their string representation. Operators
// and log scrapers still see the right number; the test reflects that shape.
expect(details.stack).toBe(STACK);
expect(details.path).toBe('observability-info.txt');
expect(typeof details.bytes).toBe('string');
expect(Number(details.bytes)).toBeGreaterThan(0);
logSpy.mockRestore();
});
it('records an error metric when an upload is rejected with FILE_EXISTS (overwrite confirm path)', async () => {
const { FileExplorerMetricsService } = await import('../services/FileExplorerMetricsService');
FileExplorerMetricsService.resetForTests();
// Seed an existing file so the second upload hits the FILE_EXISTS branch.
await fs.writeFile(path.join(stacksDir, STACK, 'observability-conflict.txt'), 'pre-existing');
const res = await request(app)
.post(`/api/stacks/${STACK}/files/upload`)
.set('Cookie', adminCookie)
.attach('file', Buffer.from('replacement'), 'observability-conflict.txt');
expect(res.status).toBe(409);
expect(res.body.code).toBe('FILE_EXISTS');
const metricsRes = await request(app)
.get('/api/file-explorer-metrics')
.set('Cookie', adminCookie);
const uploadEntry = (metricsRes.body.entries as Array<{ op: string; errorCount: number }>).find(
e => e.op === 'upload',
);
expect(uploadEntry).toBeDefined();
expect(uploadEntry!.errorCount).toBeGreaterThanOrEqual(1);
});
it('records a metric on the upload route that surfaces in /api/file-explorer-metrics', async () => {
const { FileExplorerMetricsService } = await import('../services/FileExplorerMetricsService');
FileExplorerMetricsService.resetForTests();
await request(app)
.post(`/api/stacks/${STACK}/files/upload`)
.set('Cookie', adminCookie)
.attach('file', Buffer.from('metrics payload'), 'observability-metrics.txt');
const metricsRes = await request(app)
.get('/api/file-explorer-metrics')
.set('Cookie', adminCookie);
expect(metricsRes.status).toBe(200);
const uploadEntry = (metricsRes.body.entries as Array<{ op: string; successCount: number }>).find(
e => e.op === 'upload',
);
expect(uploadEntry).toBeDefined();
expect(uploadEntry!.successCount).toBeGreaterThanOrEqual(1);
const bytesEntry = (metricsRes.body.uploadBytesByNode as Array<{ totalBytes: number }>)[0];
expect(bytesEntry).toBeDefined();
expect(bytesEntry.totalBytes).toBeGreaterThan(0);
});
it('returns 400 for an invalid stack name containing path traversal', async () => {
const res = await request(app)
.get('/api/stacks/../evil/files')
@@ -314,6 +389,35 @@ describe('GET /api/stacks/:stackName/files/download', () => {
expect(res.headers['content-disposition']).toMatch(/attachment/);
expect(res.text).toContain('services');
});
it('records the download metric exactly once per successful response', async () => {
// The metric recorder is wired to both res.on("finish") and
// res.on("close"), guarded by a flag so a single completion does not
// double-fire. A regression that drops the flag would push successCount
// to 2 for one download.
const { FileExplorerMetricsService } = await import('../services/FileExplorerMetricsService');
FileExplorerMetricsService.resetForTests();
const res = await request(app)
.get(`/api/stacks/${STACK}/files/download`)
.query({ path: 'compose.yaml' })
.set('Cookie', adminCookie);
expect(res.status).toBe(200);
// Allow the res.on('close') tail event to fire after the test's await.
await new Promise<void>((r) => setTimeout(r, 50));
const metricsRes = await request(app)
.get('/api/file-explorer-metrics')
.set('Cookie', adminCookie);
const downloadEntry = (metricsRes.body.entries as Array<{ op: string; count: number; successCount: number; errorCount: number }>).find(
e => e.op === 'download',
);
expect(downloadEntry).toBeDefined();
expect(downloadEntry!.count).toBe(1);
expect(downloadEntry!.successCount).toBe(1);
expect(downloadEntry!.errorCount).toBe(0);
});
});
// ── POST /:stackName/files/upload ─────────────────────────────────────────────