Public: anyone can read this; the crew can change it.

The usage page's Server-Timing names its phases: session, each read, render #4182

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

scout/forge:scout/3279-the-usage-page-s-server-timing-names-its → main 6 commits fork 1 thread · 1 unresolved

  • Merged. scout/3279-the-usage-page-s-server-timing-names-its was merged into main as f6d76cc.

Review

Sign in to review this change.
3 files changed +96 −3
src/web/pages/usage.ts +8 −2
@@ -7,6 +7,7 @@
77
88 import { mayManageFlags } from "../../core/flags.ts";
99 import { ROLES, SURFACES } from "../../core/usage_count.ts";
10 import { notePhase, takeSessionTime, timePhase } from "../../core/route_timing.ts";
1011 import { routeUsage, WINDOW_DAYS } from "../../core/usage_report.ts";
1112 import type { Ctx } from "../../http/context.ts";
1213 import { signInFirst } from "../chrome.ts";
@@ -20,6 +21,7 @@ const part = (counts: Readonly<Record<string, number>>, keys: readonly string[])
2021
2122 // GET /settings/usage
2223 export function usagePage(ctx: Ctx, patterns: () => readonly string[]): Response {
24 notePhase(ctx.request, "session", takeSessionTime(ctx.request));
2325 const user = ctx.auth.user;
2426 if (!user) return signInFirst(ctx);
2527 if (!mayManageFlags(user))
@@ -37,7 +39,9 @@ export function usagePage(ctx: Ctx, patterns: () => readonly string[]): Response
3739 },
3840 403,
3941 );
40 const rows = routeUsage(ctx.app.db, patterns(), new Date());
42 const known = timePhase(ctx.request, "patterns", patterns);
43 const rows = timePhase(ctx.request, "usage", () => routeUsage(ctx.app.db, known, new Date()));
44 const rendering = performance.now();
4145 const unused = rows.filter((r) => r.total === 0).length;
4246 const body = card(
4347 html`<p class="muted small card-body">${String(unused)} of ${String(rows.length)} routes had no request in
@@ -55,5 +59,7 @@ export function usagePage(ctx: Ctx, patterns: () => readonly string[]): Response
5559 </table>`,
5660 { head: html`${icon("activity")}<h2>Routes</h2>`, pad: false },
5761 );
58 return usageShell(ctx, "routes", body);
62 const res = usageShell(ctx, "routes", body);
63 notePhase(ctx.request, "render", performance.now() - rendering);
64 return res;
5965 }
test/server-timing-sweep.test.ts −1
@@ -50,7 +50,6 @@ const UNNAMED: Array<[path: string, issue: string]> = [
5050 ["/settings/keys", "#3390"],
5151 ["/settings/password", "#3390"],
5252 ["/settings/tokens", "#3390"],
53 ["/settings/usage", "#3279"],
5453 ["/signin/parent", "#3390"],
5554 ["/signin/parent/callback", "#3390"],
5655 ["/verify", "#3390"],
test/usage-timing.test.ts added +88
@@ -0,0 +1,88 @@
1 /** `/settings/usage` names session, patterns, usage and render in Server-Timing (#3413). */

The header cites #3413 but the sweep debt entry was #3279; name the right issue.

The header cites #3413, but the debt entry this pull removes was #3279. Point the comment at the right issue.

2 import { afterAll, beforeAll, expect, test } from "bun:test";
3 import { mkdtempSync, rmSync } from "node:fs";
4 import { tmpdir } from "node:os";
5 import { join } from "node:path";
6 import { createApp, type App } from "../src/app.ts";
7 import { createUser } from "../src/core/users.ts";
8 import { serve, type Servers } from "../src/http/server.ts";
9 import { proveAll } from "./helpers/verified.ts";
10 import { quietLogs } from "./helpers/globals.ts";
11
12 quietLogs();
13
14 let root = "";
15 let app: App;
16 let servers: Servers;
17
18 beforeAll(async () => {
19 root = mkdtempSync(join(tmpdir(), "forge-usage-timing-"));
20 app = createApp({ dataDir: root, port: 0, forgeRepo: "alice/forge" });
21 const u = await createUser(app.db, {
22 origin: "seed",
23 trusted: true,
24 username: "alice",
25 email: "[email protected]",
26 password: "a-good-passphrase",
27 adminIfFirst: true,
28 });
29 if (!u.ok) throw new Error(u.error);
30 proveAll(app);
31 servers = await serve(app);
32 });
33 afterAll(() => {
34 servers.stop();
35 app.close();
36 rmSync(root, { recursive: true, force: true });
37 });
38
39 test("GET /settings/usage names its phases", async () => {
40 const signin = await fetch(`${servers.url}/signin`, {
41 method: "POST",
42 headers: { origin: servers.url },
43 body: new URLSearchParams({ username: "alice", password: "a-good-passphrase" }),
44 redirect: "manual",
45 });
46 const cookie = (signin.headers.get("set-cookie") ?? "").split(";")[0]!;
47 const res = await fetch(`${servers.url}/settings/usage`, { headers: { cookie } });
48 const body = await res.text();
49 expect(res.status).toBe(200);
50 expect(body).toContain("Route usage");
51 const parts = (res.headers.get("server-timing") ?? "").split(", ");
52 for (const name of ["app", "session", "patterns", "usage", "render"])
53 expect(parts.find((p) => p.startsWith(`${name};`))).toMatch(/^[a-z]+;dur=\d+\.\d$/);
54 });
55
56 test("a non-admin's 403 names session but not usage or render", async () => {
57 const bob = await createUser(app.db, {
58 origin: "seed",
59 trusted: true,
60 username: "bob",
61 email: "[email protected]",
62 password: "a-good-passphrase",
63 });
64 if (!bob.ok) throw new Error(bob.error);
65 proveAll(app);
66 const signin = await fetch(`${servers.url}/signin`, {
67 method: "POST",
68 headers: { origin: servers.url },
69 body: new URLSearchParams({ username: "bob", password: "a-good-passphrase" }),
70 redirect: "manual",
71 });
72 const cookie = (signin.headers.get("set-cookie") ?? "").split(";")[0]!;
73 const res = await fetch(`${servers.url}/settings/usage`, { headers: { cookie } });
74 await res.text();
75 expect(res.status).toBe(403);
76 const timing = res.headers.get("server-timing") ?? "";
77 expect(timing).toContain("session;");
78 for (const name of ["patterns", "usage", "render"]) expect(timing).not.toContain(`${name};`);
79 });
80
81 test("a signed-out redirect names session and no later phase", async () => {
82 const res = await fetch(`${servers.url}/settings/usage`, { redirect: "manual" });
83 await res.text();
84 expect(res.status).toBe(303);
85 const timing = res.headers.get("server-timing") ?? "";
86 expect(timing).toContain("session;");
87 for (const name of ["patterns", "usage", "render"]) expect(timing).not.toContain(`${name};`);
88 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs