Skip to content

Commit e08ee4f

Browse files
committed
fix(metrics-pipeline): use BigInt order keys and namespaced odometer counters
entryOrderKey returns a string built with BigInt math so ordering stays correct at real epoch magnitudes. Odometer keys are namespaced by definition name. The consumer reports null lag for a missing consumer group instead of 0, and empty gauge values parse as NaN rather than 0.
1 parent b6c626d commit e08ee4f

5 files changed

Lines changed: 25 additions & 11 deletions

File tree

internal-packages/metrics-pipeline/src/cachedValue.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,8 @@ export class CachedRedisNumber {
100100
redis: options.redis,
101101
key: options.key,
102102
parse: (raw) => {
103-
const n = raw == null ? Number.NaN : Number(raw);
103+
// Number("") is 0 (not NaN), so treat blank/whitespace as missing => fallback.
104+
const n = raw == null || raw.trim() === "" ? Number.NaN : Number(raw);
104105
return Number.isFinite(n) ? clamp(n) : fallback;
105106
},
106107
defaultValue: fallback,

internal-packages/metrics-pipeline/src/consumer.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ export class MetricsStreamConsumer<TRow> {
114114
description: "Failed inserts (batch left pending for retry)",
115115
valueType: ValueType.INT,
116116
});
117-
this.insertDuration = this.meter.createHistogram("queue_metrics.consumer.insert_duration_ms", {
117+
this.insertDuration = this.meter.createHistogram("queue_metrics.consumer.insert_duration", {
118118
description: "Sink insert latency",
119119
unit: "ms",
120120
valueType: ValueType.INT,
@@ -302,9 +302,10 @@ export async function probeShardStates(
302302
for (let shard = 0; shard < keys.length; shard++) {
303303
const key = keys[shard]!;
304304
const depth = Number(await redis.xlen(key)) || 0;
305-
// lag defaults to null (unknown) and only becomes a number when Redis reports one:
306-
// a nil lag means entries were trimmed past the group's read position (data loss).
307-
let lag: number | null = 0;
305+
// lag defaults to null (unknown) and only becomes a number when the group is found and
306+
// Redis reports one: a nil lag (or a missing group on an existing stream) means we can't
307+
// compute it, e.g. entries were trimmed past the group's read position (data loss).
308+
let lag: number | null = null;
308309
let pending = 0;
309310
try {
310311
const groups = (await redis.call("XINFO", "GROUPS", key)) as unknown[];

internal-packages/metrics-pipeline/src/emitter.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ export class MetricsStreamEmitter {
136136
if (!this.flag.enabled()) return;
137137
const op = String(fields.op ?? "unknown");
138138
const q = String(fields.q ?? "");
139-
const odometerKey = `queue_metrics_cum:${op}:${q}`;
139+
const odometerKey = `${this.def.name}_cum:${op}:${q}`;
140140
const stream = streamKey(this.def, shardFor(shardKey, this.def.shardCount));
141141
const extra: string[] = [];
142142
for (const [field, value] of Object.entries(fields)) {

internal-packages/metrics-pipeline/src/pipeline.test.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { describe, expect, it } from "vitest";
22
import { createMetricsGaugeComputeLua } from "./lua.js";
33
import { dedupTokenFromEntryIds } from "./idempotency.js";
44
import { fnv1a32, shardFor } from "./hash.js";
5-
import { allStreamKeys, entryTimeMs, streamKey } from "./types.js";
5+
import { allStreamKeys, entryOrderKey, entryTimeMs, streamKey } from "./types.js";
66

77
describe("shardFor", () => {
88
it("is deterministic and in range", () => {
@@ -33,6 +33,17 @@ describe("stream keys", () => {
3333
expect(entryTimeMs("1717000000000-5")).toBe(1717000000000);
3434
expect(entryTimeMs("nope")).toBeNull();
3535
});
36+
37+
it("entryOrderKey stays exact and strictly monotonic at real epoch magnitudes", () => {
38+
const ms = 1783000000000; // ~2026: ms*1e5 is past JS safe-integer range, so a number key
39+
const k = (seq: number) => BigInt(entryOrderKey(`${ms}-${seq}`));
40+
// adjacent seq within one ms must not collapse to the same key (the float bug)
41+
expect(k(0)).toBe(BigInt(ms) * 100000n);
42+
expect(k(1) - k(0)).toBe(1n);
43+
expect(k(2) - k(1)).toBe(1n);
44+
// a later ms always outranks any seq of an earlier ms
45+
expect(BigInt(entryOrderKey(`${ms + 1}-0`))).toBeGreaterThan(k(99999));
46+
});
3647
});
3748

3849
describe("createMetricsGaugeComputeLua", () => {

internal-packages/metrics-pipeline/src/types.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,10 @@ export function entryTimeMs(id: string): number | null {
3232
return Number.isFinite(ms) ? ms : null;
3333
}
3434

35-
// Strictly-monotonic-per-stream ordering key from a stream id (`<ms>-<seq>`): ms*1e5+seq.
36-
// Used to order cumulative readings for deltaSumTimestamp so within-ms ties don't misorder.
37-
export function entryOrderKey(id: string): number {
35+
// Ordering key from a stream id (`<ms>-<seq>`) = ms*1e5+seq, for deltaSumTimestamp. BigInt +
36+
// string because ms*1e5 exceeds JS safe-integer range at real epoch magnitudes (a number would
37+
// collapse nearby seq values); the ClickHouse order_key column is UInt64 and takes the string.
38+
export function entryOrderKey(id: string): string {
3839
const [ms, seq] = id.split("-");
39-
return (Number(ms) || 0) * 100000 + (Number(seq) || 0);
40+
return (BigInt(Number(ms) || 0) * 100000n + BigInt(Number(seq) || 0)).toString();
4041
}

0 commit comments

Comments
 (0)