Skip to content

Commit 702b9ce

Browse files
committed
feat(observability): record Redis connection state on failed slot operations
A Redis command that never gets a reply fails identically whether the connection was still being established, was reconnecting with the command parked in the offline queue, or was a socket that had silently died. ioredis reports all three the same way — `Error: Command timed out` with only its own timer frames in the stack, no app frame naming the call, and no lifecycle event saying which happened. Nothing recorded anywhere distinguishes them, so the cause can only be inferred from timing. Adds `describeRedisConnection()`, a snapshot of client status, connection and ready ages, offline-queue depth, lifecycle counters, and whether the configured host is an IP or a DNS name. `status` alone usually decides it; queue depth confirms, since a parked command was waiting on connection setup while one written to a `ready` socket that never answered means the socket died unreported. Host kind rules DNS resolution in or out, which no server-side telemetry can see. Attaches it to the usage-reservation slot operations. Those are the first Redis calls a queued workflow makes, so an unusable connection surfaces there first. Connect and ready now log elapsed-since-construction, making the wait before a connection becomes usable directly measurable — today it is spent inside a command's deadline, where it reads as a command timeout rather than as connection latency. Derives only non-sensitive facts from REDIS_URL; the URL carries the AUTH token and is never logged.
1 parent 603f1c2 commit 702b9ce

4 files changed

Lines changed: 283 additions & 16 deletions

File tree

apps/sim/lib/billing/calculations/usage-reservation.ts

Lines changed: 50 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import {
99
type ReservationDenialReason,
1010
} from '@/lib/core/admission/transient-failure'
1111
import { isBillingEnabled, isHosted } from '@/lib/core/config/env-flags'
12-
import { getRedisClient } from '@/lib/core/config/redis'
12+
import { describeRedisConnection, getRedisClient } from '@/lib/core/config/redis'
1313
import { getExecutionReservationTtlMs } from '@/lib/core/execution-limits'
1414

1515
const logger = createLogger('UsageReservation')
@@ -433,6 +433,34 @@ export type ReserveExecutionSlotResult =
433433
* proven. A newly-created local reservation is rolled back if pointer
434434
* registration fails; TTL is only the bounded crash fallback.
435435
*/
436+
/**
437+
* Records connection state alongside a failed slot operation.
438+
*
439+
* These three functions are the first Redis calls a queued workflow makes, so
440+
* when the connection is not usable they are where it surfaces — as an
441+
* `Error: Command timed out` carrying no app frame and no indication of which
442+
* of several very different causes applied. Pairing the failure with
443+
* `describeRedisConnection()` is what makes the next occurrence self-diagnosing
444+
* instead of another inference from timing alone.
445+
*/
446+
async function withReservationDiagnostics<T>(
447+
operation: string,
448+
reservationId: string,
449+
run: () => Promise<T>
450+
): Promise<T> {
451+
try {
452+
return await run()
453+
} catch (error) {
454+
logger.error('Usage reservation Redis operation failed', {
455+
operation,
456+
reservationId,
457+
error: toError(error).message,
458+
redis: describeRedisConnection(),
459+
})
460+
throw error
461+
}
462+
}
463+
436464
export async function reserveExecutionSlot(
437465
params: ReserveExecutionSlotParams
438466
): Promise<ReserveExecutionSlotResult> {
@@ -510,6 +538,7 @@ export async function reserveExecutionSlot(
510538
error: toError(error).message,
511539
entityKey,
512540
reservationId,
541+
redis: describeRedisConnection(),
513542
})
514543
throw new UsageReservationUnavailableError(
515544
'Usage admission is temporarily unavailable. Please retry.',
@@ -622,7 +651,11 @@ export async function refreshExecutionSlotExpiry(
622651

623652
const boundedReservationId = requireBoundedIdentifier(reservationId, 'reservation id')
624653
const pointerKey = `${POINTER_KEY_PREFIX}${boundedReservationId}`
625-
const descriptorValue = await redis.get(pointerKey)
654+
const descriptorValue = await withReservationDiagnostics(
655+
'refresh:read-pointer',
656+
boundedReservationId,
657+
() => redis.get(pointerKey)
658+
)
626659
if (!descriptorValue) return false
627660
const descriptor = parseDescriptor(descriptorValue)
628661
if (!descriptor) {
@@ -633,22 +666,25 @@ export async function refreshExecutionSlotExpiry(
633666
const expiryAt = Math.min(expiresAt, now + getExecutionReservationTtlMs())
634667
const keys = buildLocalKeys(descriptor, boundedReservationId)
635668
const keyArgs = localKeyArguments(keys)
636-
const localResult = await redis.eval(
637-
REFRESH_LOCAL_SCRIPT,
638-
keyArgs.length,
639-
...keyArgs,
669+
const localResult = await withReservationDiagnostics(
670+
'refresh:extend-local',
640671
boundedReservationId,
641-
descriptorValue,
642-
expiryAt.toString()
672+
() =>
673+
redis.eval(
674+
REFRESH_LOCAL_SCRIPT,
675+
keyArgs.length,
676+
...keyArgs,
677+
boundedReservationId,
678+
descriptorValue,
679+
expiryAt.toString()
680+
)
643681
)
644682
if (localResult !== 1) return false
645683

646-
const pointerResult = await redis.eval(
647-
REFRESH_POINTER_SCRIPT,
648-
1,
649-
pointerKey,
650-
descriptorValue,
651-
expiryAt.toString()
684+
const pointerResult = await withReservationDiagnostics(
685+
'refresh:extend-pointer',
686+
boundedReservationId,
687+
() => redis.eval(REFRESH_POINTER_SCRIPT, 1, pointerKey, descriptorValue, expiryAt.toString())
652688
)
653689
if (pointerResult !== 1) {
654690
throw new UsageReservationUnavailableError(

apps/sim/lib/core/config/redis.test.ts

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ vi.mock('ioredis', () => ({
2727
import {
2828
acquireLock,
2929
closeRedisConnection,
30+
describeRedisConnection,
3031
extendLock,
3132
getRedisClient,
3233
onRedisReconnect,
@@ -38,6 +39,8 @@ describe('redis config', () => {
3839
vi.clearAllMocks()
3940
vi.useFakeTimers()
4041
resetForTesting()
42+
mockRedisInstance.status = 'ready'
43+
Object.assign(mockRedisInstance, { offlineQueue: undefined })
4144
mockEnv.REDIS_URL = 'redis://localhost:6379'
4245
mockEnv.REDIS_TLS_SERVERNAME = undefined
4346
MockRedisConstructor.mockImplementation(
@@ -159,6 +162,76 @@ describe('redis config', () => {
159162
})
160163
})
161164

165+
describe('describeRedisConnection', () => {
166+
it('reports no client before one is built', () => {
167+
const d = describeRedisConnection()
168+
169+
expect(d.status).toBe('no-client')
170+
expect(d.clientAgeMs).toBeNull()
171+
expect(d.readyAgeMs).toBeNull()
172+
expect(d.connects).toBe(0)
173+
})
174+
175+
it('separates a connecting client from a ready one', () => {
176+
// The constructor copies the mock's fields, so each state has to be set
177+
// before the client is built.
178+
mockRedisInstance.status = 'connecting'
179+
getRedisClient()
180+
expect(describeRedisConnection().status).toBe('connecting')
181+
182+
resetForTesting()
183+
mockRedisInstance.status = 'ready'
184+
getRedisClient()
185+
expect(describeRedisConnection().status).toBe('ready')
186+
})
187+
188+
it('surfaces the offline queue depth that proves a command was waiting on the connection', () => {
189+
Object.assign(mockRedisInstance, { offlineQueue: { length: 3 } })
190+
getRedisClient()
191+
192+
expect(describeRedisConnection().queuedCommands).toBe(3)
193+
})
194+
195+
it('counts lifecycle events so a reconnect is distinguishable from a first connect', async () => {
196+
getRedisClient()
197+
const handler = (event: string) =>
198+
mockRedisInstance.on.mock.calls.find((c: unknown[]) => c[0] === event)?.[1] as
199+
| (() => void)
200+
| undefined
201+
202+
handler('connect')?.()
203+
handler('ready')?.()
204+
const afterConnect = describeRedisConnection()
205+
expect(afterConnect.connects).toBe(1)
206+
expect(afterConnect.readyAgeMs).not.toBeNull()
207+
208+
const errorHandler = mockRedisInstance.on.mock.calls.find(
209+
(c: unknown[]) => c[0] === 'error'
210+
)?.[1] as ((e: Error) => void) | undefined
211+
errorHandler?.(new Error('ECONNRESET'))
212+
213+
const afterError = describeRedisConnection()
214+
expect(afterError.errors).toBe(1)
215+
expect(afterError.lastErrorMessage).toBe('ECONNRESET')
216+
})
217+
218+
it('classifies the host without ever exposing the URL that carries the auth token', () => {
219+
mockEnv.REDIS_URL = 'rediss://10.0.0.5:6379'
220+
mockEnv.REDIS_TLS_SERVERNAME = 'primary.example.cache.amazonaws.com'
221+
222+
const d = describeRedisConnection()
223+
224+
expect(d).toMatchObject({ hostKind: 'ip', tls: true, sniOverride: true })
225+
expect(JSON.stringify(d)).not.toContain('10.0.0.5')
226+
})
227+
228+
it('reports a DNS host so resolution latency can be ruled in or out', () => {
229+
mockEnv.REDIS_URL = 'rediss://primary.example.cache.amazonaws.com:6379'
230+
231+
expect(describeRedisConnection()).toMatchObject({ hostKind: 'dns', sniOverride: false })
232+
})
233+
})
234+
162235
describe('closeRedisConnection', () => {
163236
it('should clear the PING interval', async () => {
164237
getRedisClient()

apps/sim/lib/core/config/redis.ts

Lines changed: 133 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,13 @@ interface RedisState {
5959
pingInterval: NodeJS.Timeout | null
6060
pingInFlight: boolean
6161
reconnectListeners: Array<() => void>
62+
clientCreatedAt: number | null
63+
lastReadyAt: number | null
64+
lastPingOkAt: number | null
65+
connects: number
66+
reconnects: number
67+
errors: number
68+
lastErrorMessage: string | null
6269
}
6370

6471
const g = globalThis as typeof globalThis & { _redisState?: RedisState }
@@ -69,10 +76,104 @@ if (!g._redisState) {
6976
pingInterval: null,
7077
pingInFlight: false,
7178
reconnectListeners: [],
79+
clientCreatedAt: null,
80+
lastReadyAt: null,
81+
lastPingOkAt: null,
82+
connects: 0,
83+
reconnects: 0,
84+
errors: 0,
85+
lastErrorMessage: null,
7286
}
7387
}
7488
const state = g._redisState
7589

90+
/**
91+
* A command that never gets a reply fails identically whichever of three states
92+
* the client was in — still establishing its connection, reconnecting with the
93+
* command parked in the offline queue, or holding a socket that has silently
94+
* died. ioredis reports all three the same way: an `Error: Command timed out`
95+
* whose stack contains only its own timer frames, with no app frame naming the
96+
* call and no lifecycle event to say which happened.
97+
*
98+
* This snapshot is what separates them. `status` alone is usually decisive
99+
* (`connecting`/`reconnecting`/`ready`), and `queuedCommands` confirms it: a
100+
* command parked in the offline queue was waiting on connection setup, while a
101+
* command written to a `ready` socket that never answered means the socket is
102+
* dead in a way nothing reported.
103+
*/
104+
export interface RedisConnectionDiagnostics {
105+
status: string
106+
/** Age of the client object — distinguishes one created for this unit of work from an inherited one. */
107+
clientAgeMs: number | null
108+
/** Time since the connection last reached `ready`. */
109+
readyAgeMs: number | null
110+
/** Time since the last PING round-trip actually completed; the health check runs every 15s. */
111+
msSinceLastPingOk: number | null
112+
/** Depth of ioredis's offline queue — non-zero means commands are waiting on the connection. */
113+
queuedCommands: number | null
114+
connects: number
115+
reconnects: number
116+
errors: number
117+
lastErrorMessage: string | null
118+
/** Whether REDIS_URL targets an IP literal or a DNS name. Names DNS resolution in or out. */
119+
hostKind: 'ip' | 'dns' | 'unknown'
120+
tls: boolean
121+
/** Whether the TLS SNI override is in play (set when the host is a bare IP). */
122+
sniOverride: boolean
123+
}
124+
125+
/** ioredis keeps no public accessor for its offline queue, but its depth is the tiebreaker above. */
126+
interface OfflineQueueView {
127+
offlineQueue?: { length?: number }
128+
}
129+
130+
function describeRedisUrl(
131+
url: string | null
132+
): Pick<RedisConnectionDiagnostics, 'hostKind' | 'tls' | 'sniOverride'> {
133+
if (!url) return { hostKind: 'unknown', tls: false, sniOverride: false }
134+
try {
135+
const parsed = new URL(url)
136+
const isIp = /^\d{1,3}(\.\d{1,3}){3}$/.test(parsed.hostname)
137+
const tls = parsed.protocol === 'rediss:'
138+
return { hostKind: isIp ? 'ip' : 'dns', tls, sniOverride: tls && isIp }
139+
} catch {
140+
return { hostKind: 'unknown', tls: false, sniOverride: false }
141+
}
142+
}
143+
144+
/**
145+
* Connection state at a point in time, safe to attach to any log line.
146+
*
147+
* Derives only non-sensitive facts from REDIS_URL — never the URL itself, which
148+
* carries the AUTH token.
149+
*/
150+
export function describeRedisConnection(): RedisConnectionDiagnostics {
151+
const now = Date.now()
152+
let url: string | null = null
153+
try {
154+
url = getConfiguredRedisUrl()
155+
} catch {
156+
url = null
157+
}
158+
159+
const client = state.client
160+
// double-cast-allowed: ioredis omits offlineQueue from its public type, and its depth is what separates a command waiting on connection setup from one written to a live socket
161+
const queued = (client as unknown as OfflineQueueView | null)?.offlineQueue?.length
162+
163+
return {
164+
status: client?.status ?? 'no-client',
165+
clientAgeMs: state.clientCreatedAt === null ? null : now - state.clientCreatedAt,
166+
readyAgeMs: state.lastReadyAt === null ? null : now - state.lastReadyAt,
167+
msSinceLastPingOk: state.lastPingOkAt === null ? null : now - state.lastPingOkAt,
168+
queuedCommands: typeof queued === 'number' ? queued : null,
169+
connects: state.connects,
170+
reconnects: state.reconnects,
171+
errors: state.errors,
172+
lastErrorMessage: state.lastErrorMessage,
173+
...describeRedisUrl(url),
174+
}
175+
}
176+
76177
const PING_INTERVAL_MS = 15_000
77178
const MAX_PING_FAILURES = 2
78179

@@ -103,6 +204,7 @@ function startPingHealthCheck(redis: Redis): void {
103204
try {
104205
await redis.ping()
105206
state.pingFailures = 0
207+
state.lastPingOkAt = Date.now()
106208
} catch (error) {
107209
state.pingFailures++
108210
logger.warn('Redis PING failed', {
@@ -172,6 +274,7 @@ export function getRedisClient(): Redis | null {
172274
const base = Math.min(1000 * 2 ** (times - 1), 10000)
173275
const jitter = randomFloat() * base * 0.3
174276
const delay = Math.round(base + jitter)
277+
state.reconnects++
175278
logger.warn('Redis reconnecting', { attempt: times, nextRetryMs: delay })
176279
return delay
177280
},
@@ -182,9 +285,30 @@ export function getRedisClient(): Redis | null {
182285
},
183286
})
184287

185-
state.client.on('connect', () => logger.info('Redis connected'))
186-
state.client.on('ready', () => logger.info('Redis ready'))
288+
state.clientCreatedAt = Date.now()
289+
state.lastReadyAt = null
290+
state.lastPingOkAt = null
291+
292+
state.client.on('connect', () => {
293+
state.connects++
294+
// Elapsed since construction, because the wait before a connection becomes
295+
// usable is the number this path has never been able to produce: it is
296+
// spent inside a command's deadline, where it surfaces as a command
297+
// timeout rather than as connection latency.
298+
logger.info('Redis connected', {
299+
elapsedMs: state.clientCreatedAt === null ? null : Date.now() - state.clientCreatedAt,
300+
attempt: state.connects,
301+
})
302+
})
303+
state.client.on('ready', () => {
304+
state.lastReadyAt = Date.now()
305+
logger.info('Redis ready', {
306+
elapsedMs: state.clientCreatedAt === null ? null : Date.now() - state.clientCreatedAt,
307+
})
308+
})
187309
state.client.on('error', (err: Error) => {
310+
state.errors++
311+
state.lastErrorMessage = err.message
188312
logger.error('Redis error', { error: err.message, code: (err as any).code })
189313
})
190314
state.client.on('close', () => logger.warn('Redis connection closed'))
@@ -355,4 +479,11 @@ export function resetForTesting(): void {
355479
state.pingFailures = 0
356480
state.pingInFlight = false
357481
state.reconnectListeners.length = 0
482+
state.clientCreatedAt = null
483+
state.lastReadyAt = null
484+
state.lastPingOkAt = null
485+
state.connects = 0
486+
state.reconnects = 0
487+
state.errors = 0
488+
state.lastErrorMessage = null
358489
}

0 commit comments

Comments
 (0)