diff --git a/src/selfhost/metrics.ts b/src/selfhost/metrics.ts index 044d04c7a..a2b93e545 100644 --- a/src/selfhost/metrics.ts +++ b/src/selfhost/metrics.ts @@ -96,6 +96,7 @@ export const DEFAULT_METRIC_META: readonly (readonly [string, MetricMeta])[] = [ ["loopover_redis_gh_response_cache_total", { help: "Redis-backed GitHub response cache outcomes.", type: "counter" }], ["loopover_redis_gh_response_cache_hit_ratio", { help: "Redis GitHub response cache hit ratio (hits / (hits + misses)) at scrape time.", type: "gauge" }], ["loopover_redis_token_cache_total", { help: "Redis-backed GitHub token cache outcomes.", type: "counter" }], + ["loopover_redis_webhook_dedup_cache_total", { help: "Redis-backed webhook-dedup cache outcomes.", type: "counter" }], ["loopover_qdrant_queries_total", { help: "Qdrant vector query attempts.", type: "counter" }], ["loopover_qdrant_upserts_total", { help: "Qdrant vector upserted item count.", type: "counter" }], ["loopover_qdrant_errors_total", { help: "Qdrant vector operation errors.", type: "counter" }], diff --git a/src/selfhost/redis-cache.ts b/src/selfhost/redis-cache.ts index 50d692eb0..2404503eb 100644 --- a/src/selfhost/redis-cache.ts +++ b/src/selfhost/redis-cache.ts @@ -8,6 +8,17 @@ import { incr } from "./metrics"; const WEBHOOK_DELIVERY_CACHE_PREFIX = "delivery:"; +const REDIS_WEBHOOK_DEDUP_CACHE_METRIC = "loopover_redis_webhook_dedup_cache_total"; + +/** Records a webhook-dedup Redis cache outcome. Mirrors redis-token-cache.ts's recordTokenCacheMetric / + * redis-response-cache.ts's recordRedisResponseCacheMetric: a named recorder over a `{result}`-labeled + * counter, so a sustained Redis outage on this cache is as visible as one on either sibling. Kept separate + * from `loopover_webhook_dedup_total{backend="redis"}`, which counts actual dedup HITS -- folding errors + * into that counter would conflate "deliveries deduplicated" with "cache unavailable". */ +function recordWebhookDedupCacheMetric(result: "error"): void { + incr(REDIS_WEBHOOK_DEDUP_CACHE_METRIC, { result }); +} + export function webhookDeliveryCacheKey(deliveryId: string): string { return `${WEBHOOK_DELIVERY_CACHE_PREFIX}${deliveryId}`; } @@ -24,6 +35,9 @@ export async function isWebhookDeliveryDuplicate(cache: RedisCache, deliveryId: } return false; } catch { + // Fail open (unchanged): a cache outage must never block webhook processing -- but record it so the + // outage is visible, matching both sibling Redis caches. + recordWebhookDedupCacheMetric("error"); return false; } } @@ -33,7 +47,9 @@ export async function rememberWebhookDelivery(cache: RedisCache, deliveryId: str try { await cache.set(webhookDeliveryCacheKey(deliveryId), "1", ttlSeconds); } catch { - // best-effort — never block the response on a cache write failure + // best-effort — never block the response on a cache write failure (unchanged), but make the failure + // observable on the same metric surface as the sibling caches. + recordWebhookDedupCacheMetric("error"); } } diff --git a/test/unit/selfhost-redis-cache.test.ts b/test/unit/selfhost-redis-cache.test.ts index b4b56fa78..345b35ef0 100644 --- a/test/unit/selfhost-redis-cache.test.ts +++ b/test/unit/selfhost-redis-cache.test.ts @@ -121,11 +121,22 @@ describe("isWebhookDeliveryDuplicate (#2075)", () => { expect(await renderMetrics()).toContain('loopover_webhook_dedup_total{backend="redis"} 1'); }); - it("returns false without incrementing when Redis get throws", async () => { + it("returns false without counting a dedup hit, and records an error metric, when Redis get throws (#8363)", async () => { const brokenRedis = { async get() { throw new Error("connection refused"); } } as unknown as Redis; const cache = createRedisCache(brokenRedis); await expect(isWebhookDeliveryDuplicate(cache, "delivery-3")).resolves.toBe(false); - expect(await renderMetrics()).not.toContain('loopover_webhook_dedup_total{backend="redis"}'); + const rendered = await renderMetrics(); + // Fail-open behavior is unchanged: a cache outage is never counted as a deduplicated delivery. + expect(rendered).not.toContain('loopover_webhook_dedup_total{backend="redis"}'); + // ...but the outage is now observable, matching redis-token-cache / redis-response-cache. + expect(rendered).toContain('loopover_redis_webhook_dedup_cache_total{result="error"} 1'); + }); + + it("rememberWebhookDelivery records an error metric when the Redis set throws, and still resolves (#8363)", async () => { + const brokenRedis = { async set() { throw new Error("connection refused"); } } as unknown as Redis; + const cache = createRedisCache(brokenRedis); + await expect(rememberWebhookDelivery(cache, "delivery-5")).resolves.toBeUndefined(); + expect(await renderMetrics()).toContain('loopover_redis_webhook_dedup_cache_total{result="error"} 1'); }); it("rememberWebhookDelivery stores the delivery key for later dedup", async () => {