feat(plugin): bot_serve + page_age analytics — rollout success metrics; v0.26.0

[harper-joseph]

What

Two new bot-path analytics metrics, recorded once the request has resolved — the serve-side success metrics for the phased bot-traffic rollout:

Metric	Value	Dimensions (path, method, type)	Answers
bot_serve	count	source, cacheStatus, botName	origin offload (source !== 'origin' = requests the origin never saw) and cache hit rate (cacheStatus split: hit / stale / miss / skip / bypass), both per-bot
page_age	ms since the served page rendered	botName, deviceType	freshness at serve (median/p95 age of what each crawler actually received)

bot_request is unchanged and remains the ingress-volume metric — all three of its dimensions (hostname, botName, deviceType) are already in use, which is why the serve outcome is a separate metric rather than a label on it.

Design notes

• Hot-path cost: gated behind the existing config.analytics.enabled / recordUnmatched check (hoisted, evaluated once per request). recordAnalytics is Harper's in-memory buffered counter — no storage touch, no await, nothing added to response latency.
• page_age is cache-served only (source === 'cache'), so a render-now response doesn't drag the freshness distribution toward zero. lastCached is coerced like expiresAt (Date / number / serialized string); NaN and negative ages (cross-node clock skew) record nothing rather than poisoning the mean.
• Non-GET/HEAD requests now stamp cacheStatus: 'bypass', which also surfaces in the x-harper-cache debug header.
• The admin overview's unwired bot-traffic panel note is updated: the metric it was waiting on now exists; the remaining blocker is the node-local hdb_analytics vs cluster-aggregation decision (panel stays unwired per the standing decision).

Tests

New test/botServe.test.js pins the dimension order (the positional contract dashboards key on), the cache-served-only rule, the three lastCached shapes, and the NaN/negative-age guards. Full suite: 342 pass.

Version

v0.26.0 — main is at 0.24.0; #62 and #63 both already claim 0.25.0.

Follow-ups (deliberately out of scope)

• Daily distinct-URLs-per-bot (needs a cardinality mechanism, not a counter)
• Hydration-marker verification metric (browser package)

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request introduces new analytics metrics (bot_serve and page_age) to track bot traffic outcomes and cache freshness, alongside corresponding documentation updates and unit tests. The review feedback highlights a critical bug where a null value for resource.lastCached would incorrectly evaluate to 0 in JavaScript's Date constructor, leading to a massive, incorrect page_age metric. It is recommended to check that lastCached is truthy before parsing and to add a corresponding test case to prevent regression.

[gemini-code-assist]

In JavaScript, new Date(null).getTime() returns 0 rather than NaN. If resource.lastCached is null (which is a common representation for missing or unset date fields in database schemas), new Date(resource.lastCached).getTime() will evaluate to 0. This results in age being calculated as Date.now(), which is a very large positive number. This will poison the page_age metric with incorrect, extremely high values.

Additionally, to prevent runtime exceptions if resource itself is null or undefined, we should use optional chaining (resource?.lastCached).

Using a conditional check to ensure resource?.lastCached is truthy before parsing avoids both issues.

		const lastCached = resource?.lastCached;
		const age = lastCached ? Date.now() - new Date(lastCached).getTime() : NaN;
		if (age >= 0) {
			server.recordAnalytics(age, 'page_age', request.botName, deviceType);
		}

[harper-joseph]

Fixed in afb6691 — good catch, new Date(null) is epoch 0 so the NaN reasoning in my comment didn't cover null. Now guarded with the same truthiness-first pattern as the expiresAt read above it. (Skipped the resource?. chaining: resolveResource always returns an object on this path.) Also swept both open PRs for the same class — every other date coercion already guards truthiness first.

[gemini-code-assist]

Update the test to verify that page_age is correctly skipped when lastCached is null, ensuring the bug where new Date(null).getTime() returns 0 is covered and does not regress.

Suggested change

	test('page_age is skipped when lastCached is missing or in the future', () => {
	recordServeOutcome({}, request, { source: 'cache', cacheStatus: 'hit' }, 'desktop');
	recordServeOutcome({ lastCached: Date.now() + 60_000 }, request, { source: 'cache', cacheStatus: 'hit' }, 'desktop');
	assert.equal(analytics.length, 2);
	assert.ok(analytics.every(([, metric]) => metric === 'bot_serve'));
	});
	test('page_age is skipped when lastCached is missing, null, or in the future', () => {
	recordServeOutcome({}, request, { source: 'cache', cacheStatus: 'hit' }, 'desktop');
	recordServeOutcome({ lastCached: null }, request, { source: 'cache', cacheStatus: 'hit' }, 'desktop');
	recordServeOutcome({ lastCached: Date.now() + 60_000 }, request, { source: 'cache', cacheStatus: 'hit' }, 'desktop');
	assert.equal(analytics.length, 3);
	assert.ok(analytics.every(([, metric]) => metric === 'bot_serve'));
	});

[harper-joseph]

Added in afb6691, with a comment explaining why null specifically is the trap case.

[harper-joseph]

Addressed the review in afb6691: lastCached is now truthiness-guarded before Date coercion (new Date(null) → epoch 0, which would have recorded page_age ≈ Date.now()), and the test covers the null case explicitly. Re-reviewed both this PR and stacked #65 for the same coercion class: this was the only unguarded instance — the codebase's existing date reads all guard first, and #65's date handling only ever coerces values derived from Date.now(), never table data. 342 tests pass.