Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/selfhost/metrics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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" }],
Expand Down
18 changes: 17 additions & 1 deletion src/selfhost/redis-cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}`;
}
Expand All @@ -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;
}
}
Expand All @@ -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");
}
}

Expand Down
15 changes: 13 additions & 2 deletions test/unit/selfhost-redis-cache.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down