diff --git a/src/selfhost/redis-token-cache.ts b/src/selfhost/redis-token-cache.ts index b590855fdf..ae94f5e578 100644 --- a/src/selfhost/redis-token-cache.ts +++ b/src/selfhost/redis-token-cache.ts @@ -6,25 +6,42 @@ // Also makes the cache shared across instances if the stack is ever scaled horizontally. import type { Redis } from "ioredis"; import type { InstallationTokenStore } from "../github/app"; +import { incr } from "./metrics"; + +const REDIS_TOKEN_CACHE_METRIC = "gittensory_redis_token_cache_total"; const keyFor = (installationId: number): string => `gh:insttoken:${installationId}`; +function recordTokenCacheMetric(result: "hit" | "miss"): void { + incr(REDIS_TOKEN_CACHE_METRIC, { result }); +} + export function createRedisTokenCache(redis: Redis): InstallationTokenStore { return { async get(installationId: number) { const raw = await redis.get(keyFor(installationId)); - if (!raw) return null; + if (!raw) { + recordTokenCacheMetric("miss"); + return null; + } try { const value = JSON.parse(raw) as { token?: unknown; expiresAtMs?: unknown; }; - return typeof value.token === "string" && - typeof value.expiresAtMs === "number" - ? { token: value.token, expiresAtMs: value.expiresAtMs } - : null; + if (typeof value.token !== "string") { + recordTokenCacheMetric("miss"); + return null; + } + if (typeof value.expiresAtMs !== "number") { + recordTokenCacheMetric("miss"); + return null; + } + recordTokenCacheMetric("hit"); + return { token: value.token, expiresAtMs: value.expiresAtMs }; } catch { + recordTokenCacheMetric("miss"); return null; } }, diff --git a/test/unit/selfhost-redis-token-cache.test.ts b/test/unit/selfhost-redis-token-cache.test.ts index c94b4658fb..967ad371fb 100644 --- a/test/unit/selfhost-redis-token-cache.test.ts +++ b/test/unit/selfhost-redis-token-cache.test.ts @@ -1,5 +1,6 @@ import type { Redis } from "ioredis"; -import { describe, expect, it } from "vitest"; +import { afterEach, describe, expect, it } from "vitest"; +import { renderMetrics, resetMetrics } from "../../src/selfhost/metrics"; import { createRedisTokenCache } from "../../src/selfhost/redis-token-cache"; /** Minimal ioredis stand-in that records the TTL passed to set(). */ @@ -23,20 +24,32 @@ function fakeRedis(): { return { redis, store, ttl: () => lastTtl }; } +afterEach(() => resetMetrics()); + describe("createRedisTokenCache (#perf installation-token persistence)", () => { it("get returns null for a missing installation", async () => { const { redis } = fakeRedis(); expect(await createRedisTokenCache(redis).get(42)).toBeNull(); + + expect(await renderMetrics()).toContain( + 'gittensory_redis_token_cache_total{result="miss"} 1', + ); }); it("set then get round-trips the token + expiry, with TTL ~ the token lifetime", async () => { const f = fakeRedis(); const cache = createRedisTokenCache(f.redis); const expiresAtMs = Date.now() + 3_600_000; - await cache.set(7, { token: "tok", expiresAtMs }); + await cache.set(7, { token: "sensitive-value", expiresAtMs }); expect(f.ttl()).toBeGreaterThan(3500); // ~3600s expect(f.ttl()).toBeLessThanOrEqual(3600); - expect(await cache.get(7)).toEqual({ token: "tok", expiresAtMs }); + expect(await cache.get(7)).toEqual({ token: "sensitive-value", expiresAtMs }); + + const metrics = await renderMetrics(); + expect(metrics).toContain( + 'gittensory_redis_token_cache_total{result="hit"} 1', + ); + expect(metrics).not.toContain("sensitive-value"); }); it("floors the TTL at 1s for an already-near-expiry token", async () => { @@ -52,14 +65,35 @@ describe("createRedisTokenCache (#perf installation-token persistence)", () => { const f = fakeRedis(); f.store.set("gh:insttoken:9", "{not json"); expect(await createRedisTokenCache(f.redis).get(9)).toBeNull(); + + expect(await renderMetrics()).toContain( + 'gittensory_redis_token_cache_total{result="miss"} 1', + ); + }); + + it("get returns null when the cached token is not a string", async () => { + const f = fakeRedis(); + f.store.set( + "gh:insttoken:9", + JSON.stringify({ token: 123, expiresAtMs: Date.now() + 60_000 }), + ); + expect(await createRedisTokenCache(f.redis).get(9)).toBeNull(); + + expect(await renderMetrics()).toContain( + 'gittensory_redis_token_cache_total{result="miss"} 1', + ); }); - it("get returns null when the stored shape is wrong", async () => { + it("get returns null when the cached expiry is not a number", async () => { const f = fakeRedis(); f.store.set( "gh:insttoken:9", - JSON.stringify({ token: 123, expiresAtMs: "soon" }), + JSON.stringify({ token: "sensitive-value", expiresAtMs: "soon" }), ); expect(await createRedisTokenCache(f.redis).get(9)).toBeNull(); + + expect(await renderMetrics()).toContain( + 'gittensory_redis_token_cache_total{result="miss"} 1', + ); }); });