fix(telemetry): user event counters can be lost between the Redis read and the clear

[xfournet]

Description

Telemetry user-event counters can be lost between the moment they are written to Redis and the moment they are cleared. The read and the clear are driven by two independent timers and are not atomic, so increments landing in the gap are erased without ever being collected.

This is a code reading, not a measurement: the windows below are established from the source, their real magnitude in production has not been instrumented.

How the data actually flows

Counters live in a single Redis hash, telemetry_events (redis.ts:736), written with HINCRBY (redis.ts:745).

From there, three independent timers:

Stage	What it does	Interval
fetchTelemetryData (telemetryManager.ts:445)	reads Redis, writes the values into the in-memory fields of TelemetryMeterManager	SCHEDULE_TIME, 1 min (:109)
_doRunCollect (BatchExportingMetricReader.ts:62)	collect() triggers the gauge callbacks, which read memory, then calls collectCallback → redisClearTelemetry()	TELEMETRY_COLLECT_INTERVAL, 1 min dev / 1 h prod (:119)
_runExportOnce	pushes the accumulated _resourceMetrics to the exporter	TELEMETRY_EXPORT_INTERVAL, 3 h dev / 6 h prod (:125)

The gauges are createObservableGauge whose callback does observableResult.observe(this[observer]) (TelemetryMeterManager.ts:667) — they never read Redis. Redis is read only by fetchTelemetryData.

Gap 1 — the clear erases what was never read

redisClearTelemetry() deletes the whole hash (redis.ts:762), including the increments that arrived after the last fetchTelemetryData. Those were never copied into memory, so they were never collected, and they are destroyed.

The loss window is not the microseconds between collect() and the DEL: it is the whole interval since the last fetchTelemetryData, up to one minute. The two timers are unsynchronised, so it varies from cycle to cycle.

Rough order of magnitude in production, assuming a uniform event rate: one minute out of sixty, so around 1.5% of the events. In dev mode both intervals are one minute, which is far worse.

Gap 2 — two readers, only one clears

Both exporters are BatchExportingMetricReader instances registered on the same meter provider, with the same collectIntervalMillis, but on separate timers, and only the file exporter carries the collectCallback (telemetryManager.ts:367).

When the file reader collects and clears Redis, the in-memory values stay until the next fetchTelemetryData refreshes them from a now-empty hash. The OTLP reader, collecting on its own schedule, can therefore observe the same values again, whose Redis backing has already been wiped. With AggregationTemporality.DELTA, the two exporters do not see the same windows.

Gap 3 — the clear is skipped once after every export

if (this._resourceMetrics.resource !== EMPTY_RESOURCE) {
  // append the data points
  if (this._collectCallback) {
    this._collectCallback();
  }
} else {
  this._resourceMetrics = resourceMetrics;   // no clear
}

(BatchExportingMetricReader.ts:71-88)

_doRunExport resets resource to EMPTY_RESOURCE after each export (:122), so the first collect following an export only seeds _resourceMetrics and does not clear Redis. The clearing rhythm is therefore not regular.

Severity

Moderate: this is product telemetry, not user data. The symptom is counters slightly under-reported, with nothing visible to a user. Worth fixing for the accuracy of the metrics, not urgently.

Possible directions

Not decided, listed for discussion:

• Make read-and-clear atomic per gauge, for instance GETDEL-style semantics or a HGETALL + DEL in a transaction, so nothing can slip in between.
• Have the collection read Redis at collect time rather than relying on a separate timer to snapshot into memory, which would remove the two-timer gap entirely.
• Decide explicitly how the two exporters should share a single source of truth, rather than having one of them own the clearing.