diff --git a/docs/http-metrics.md b/docs/http-metrics.md index 02d1bf0b..4965f6f4 100644 --- a/docs/http-metrics.md +++ b/docs/http-metrics.md @@ -12,6 +12,11 @@ These three signals — request rate, error rate and p95 latency — are what an specifically and cannot answer that question; it is unchanged and still means what it always meant. +The same cardinality rule applies to every metric family: labels must come from +fixed buckets or server-defined values. Never put raw request paths, asset +codes, issuer addresses, query values or websocket subscription strings in a +Prometheus label. + ## What is exported ### `http_requests_total` @@ -52,6 +57,11 @@ So: `methods x routes x 5` rather than `methods x routes x ~60`. If a concrete need for an exact code appears, add it then — it is easy to add a label and painful to remove one dashboards depend on. +- **Websocket pairs use fixed buckets.** `ws_clients_total`, + `ws_messages_sent_total` and `ws_messages_dropped_total` retain their `pair` + label, but pair-specific subscriptions are recorded as `other`; `*` remains + the aggregate across all subscriptions. Arbitrary pair names and issuer + addresses stay in fanout state only and cannot create Prometheus series. ## Bucket choice diff --git a/src/__tests__/fanout.test.ts b/src/__tests__/fanout.test.ts index 147340a0..5740de90 100644 --- a/src/__tests__/fanout.test.ts +++ b/src/__tests__/fanout.test.ts @@ -1,5 +1,9 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' +const { metricLabelValues } = vi.hoisted(() => ({ + metricLabelValues: [] as Array<{ metric: string; value: string }>, +})) + vi.mock('ioredis', () => { const mockRedis = { publish: vi.fn().mockResolvedValue(1), @@ -32,12 +36,15 @@ vi.mock('../events', () => ({ })) vi.mock('prom-client', () => { - function MockMetric() { - const labels = vi.fn(() => new MockMetric()) + function MockMetric(options: { name: string }) { + const metricName = options.name this.inc = vi.fn() this.dec = vi.fn() this.set = vi.fn() - this.labels = labels + this.labels = vi.fn((value: string) => { + metricLabelValues.push({ metric: metricName, value }) + return this + }) this.observe = vi.fn() this.startTimer = vi.fn(() => vi.fn()) } @@ -54,6 +61,7 @@ import { fanOutManager } from '../ws/fanout' describe('FanOutManager', () => { beforeEach(() => { vi.clearAllMocks() + metricLabelValues.length = 0 }) afterEach(async () => { @@ -96,6 +104,34 @@ describe('FanOutManager', () => { expect(unsub2).toBeInstanceOf(Function) }) + it('uses bounded metric labels for arbitrary pair subscriptions and keeps the aggregate', () => { + const unsubscribers: Array<() => void> = [] + for (let i = 0; i < 100; i++) { + unsubscribers.push(fanOutManager.register(`TOKEN${i}/ISSUER${i}`, { + id: `unknown-pair-${i}`, + send: vi.fn(), + close: vi.fn(), + })) + } + + const clientLabels = metricLabelValues + .filter(({ metric }) => metric === 'ws_clients_total') + .map(({ value }) => value) + expect(new Set(clientLabels)).toEqual(new Set(['other', '*'])) + expect(clientLabels.filter(value => value === 'other')).toHaveLength(100) + expect(clientLabels.filter(value => value === '*')).toHaveLength(100) + + unsubscribers.forEach(unsubscribe => unsubscribe()) + }) + + it('counts a wildcard subscriber once in the aggregate series', () => { + fanOutManager.register('*', { id: 'wildcard-client', send: vi.fn(), close: vi.fn() }) + + expect(metricLabelValues.filter(({ metric, value }) => + metric === 'ws_clients_total' && value === '*' + )).toHaveLength(1) + }) + it('destroy cleans up all state', async () => { const mockSocket = { id: 'client-cleanup', @@ -119,4 +155,4 @@ describe('FanOutClient per-pair subscription', () => { fanOutManager.register('ETH/USDC', mockSocket) }) -}) \ No newline at end of file +}) diff --git a/src/ws/fanout.ts b/src/ws/fanout.ts index 49548278..e708610f 100644 --- a/src/ws/fanout.ts +++ b/src/ws/fanout.ts @@ -30,6 +30,15 @@ export const ws_messages_dropped_total = new Counter({ const REDIS_CHANNEL = `lens:${activeNetwork}:price:updates` const DEFAULT_COALESCE_INTERVAL_MS = 100 // max 10 updates/sec per client +/** + * Pair names can contain arbitrary asset codes and issuer identifiers supplied + * by websocket clients. Keep them out of Prometheus labels: `other` is the + * bounded bucket for pair-specific subscriptions and `*` remains the aggregate. + */ +function pairMetricLabel(pairKey: string): 'other' | null { + return pairKey === '*' ? null : 'other' +} + // ─── Client wrapper with backpressure & coalescing ──────────────────────────── export interface FanOutClient { id: string @@ -111,7 +120,8 @@ class FanOutManager { } clients.add(id) - ws_clients_total.labels(pairKey).inc() + const metricPair = pairMetricLabel(pairKey) + if (metricPair) ws_clients_total.labels(metricPair).inc() ws_clients_total.labels('*').inc() // Return unsubscribe function @@ -133,7 +143,8 @@ class FanOutManager { } } - ws_clients_total.labels(client.pairKey).dec() + const metricPair = pairMetricLabel(client.pairKey) + if (metricPair) ws_clients_total.labels(metricPair).dec() ws_clients_total.labels('*').dec() } @@ -214,7 +225,8 @@ class FanOutManager { private sendToClient(client: InternalClient, payload: string): void { try { client.socket.send(payload) - ws_messages_sent_total.labels(client.pairKey).inc() + const metricPair = pairMetricLabel(client.pairKey) + if (metricPair) ws_messages_sent_total.labels(metricPair).inc() ws_messages_sent_total.labels('*').inc() } catch { // Client likely disconnected @@ -241,4 +253,4 @@ class FanOutManager { } export const fanOutManager = new FanOutManager() -export default FanOutManager \ No newline at end of file +export default FanOutManager