add OpenTelemetry metrics instrumentation by PavelPashov · Pull Request #3110 · redis/node-redis

PavelPashov · 2025-10-24T14:15:00Z

Description

TODO

Add docs
Add examples

Checklist

Does npm test pass with this change (including linting)?
Is the new or changed code fully tested?
Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?

Note

Medium Risk
Touches core client lifecycle (queue/socket construction, command execution paths, pool/cluster wiring) to emit metrics and maintain per-client identity/registry; while mostly no-op unless OpenTelemetry.init is called, mistakes could still impact performance or shutdown/cleanup behavior.

Overview
Adds first-class OpenTelemetry metrics support via a new OpenTelemetry.init({ metrics: ... }) entrypoint (exported from @redis/client) plus a singleton OTelMetrics implementation and ClientRegistry used by observable gauges.

Instruments the client stack to emit metrics for command durations (including MULTI/PIPELINE), connection lifecycle/closure, pool wait time and pending requests, enterprise maintenance notifications/handoff, client-side cache hits/misses/evictions, pub/sub in+out message counts, and stream lag (via command onSuccess hooks). Includes new client identity IDs (standalone/cluster/pool roles) with registration/unregistration on close/destroy/quit, plus updated docs (docs/otel-metrics.md), an otel-metrics.js example, and extensive new test coverage.

^{Written by Cursor Bugbot for commit bc14732. This will update automatically on new commits. Configure here.}

jit-ci

❌ The following Jit checks failed to run:

license-compliance-checker
secret-detection-trufflehog
software-component-analysis-js
static-code-analysis-semgrep-pro

#jit_bypass_commit in this PR to bypass, Jit Admin privileges required.

More info in the Jit platform.

elimelt · 2025-12-08T21:20:11Z

This is an exciting feature, I hope it makes it to main!

jit-ci · 2026-01-19T08:28:50Z

❌ Security scan failed

Security scan failed: Branch feat/add-opentelemetry-metrics does not exist in the remote repository

💡 Need to bypass this check? Comment @sera bypass to override.

jit-ci · 2026-01-19T08:35:55Z

❌ Security scan failed

Security scan failed: Branch feat/add-opentelemetry-metrics does not exist in the remote repository

💡 Need to bypass this check? Comment @sera bypass to override.

jit-ci · 2026-01-19T09:28:19Z

❌ Security scan failed

Security scan failed: Branch feat/add-opentelemetry-metrics does not exist in the remote repository

💡 Need to bypass this check? Comment @sera bypass to override.

…overhead

…ne attributes

…strument

…overage

…onfig

…ource attrs

packages/client/lib/client/cache.ts

vladvildanov · 2026-02-25T14:28:16Z

packages/client/lib/client/cache.ts

+        );
+        // Estimate bytes saved by avoiding network round-trip
+        // Note: JSON.stringify approximation; actual RESP wire size may differ (especially for Buffers)
+        const bytesEstimate = JSON.stringify(cacheEntry.value).length;


What do you think about wrapping it with Buffer.byteLength(JSON.stringify(cacheEntry.value), 'utf8');? If data to be stored isn't just ASCII it makes sense to measure UTF-8 bytes count

That's a good catch!

packages/client/lib/client/cache.ts

packages/client/lib/client/index.ts

packages/client/lib/opentelemetry/metrics.ts

vladvildanov · 2026-02-26T10:53:43Z

packages/client/lib/opentelemetry/noop-metrics.ts

+} from "./types";
+import { noopFunction } from "./utils";
+
+export class NoopCommandMetrics implements IOTelCommandMetrics {


Why do you need this object? Just to ensure that no commands will be actually send to a collector?

Because we have multiple metric groups and don’t know what the user will opt into, I kept each group in its own class and defaulted them to noop. When the user initializes metrics, we swap in the real implementations based on config, so all checks happen once during OpenTelemetry.init(...). This keeps runtime code simple (no repeated conditional checks) and ensures disabled metrics have near-zero overhead

So this is basically Null object pattern where in case if Otel is disabled empty methods are called?

Yes, exactly

packages/client/lib/client/index.ts

packages/client/lib/client/cache.ts

packages/client/lib/cluster/index.ts

packages/client/lib/client/socket.ts

PavelPashov · 2026-02-27T14:58:56Z

packages/client/lib/RESP/types.ts

  TRANSFORM_LEGACY_REPLY?: boolean;
  transformReply: TransformReply | Record<RespVersions, TransformReply>;
  unstableResp3?: boolean;
+  onSuccess?: (args: ReadonlyArray<RedisArgument>, reply: unknown, clientId: string) => void;


@nkaradzhov we might want to rename onSuccess to something more metrics/otel related

PavelPashov · 2026-02-27T15:00:38Z

packages/client/lib/client/index.ts

+      if (command.onSuccess) {
+        command.onSuccess(parser.redisArgs, finalReply, this._self._clientId);


@nkaradzhov there might be a better way to record specific command related metrics that require the server response

nkaradzhov · 2026-03-04T11:32:33Z

packages/client/lib/opentelemetry/metrics.ts

+    // Build the appropriate function based on options
+    if (options.hasIncludeCommands || options.hasExcludeCommands) {
+      // Version with filtering
+      this.createRecordOperationDuration = this.#createWithFiltering.bind(this);
+    } else {
+      this.createRecordOperationDuration =
+        this.#createWithoutFiltering.bind(this);
+    }
+  }


why have two fns?

@nkaradzhov to make sure we don't do unnecessary checks after initiating the metrics singleton if the user hasn't provided any included or excluded commands

packages/client/lib/opentelemetry/metrics.ts

… in command and CSC metrics

cursor · 2026-03-05T09:31:16Z

packages/client/lib/client/cache.ts

      this.clear(false);
+      // Record invalidations as server-initiated evictions
+      if (oldSize > 0) {
+        OTelMetrics.instance.clientSideCacheMetrics.recordCacheEviction(CSC_EVICTION_REASON.INVALIDATION, oldSize);


Cache eviction metrics missing clientId for attribute resolution

Low Severity

All recordCacheEviction calls omit clientId, so resolveClientAttributes returns undefined and the resulting eviction metrics lack server.address, server.port, and db.client.connection.pool.name attributes. In contrast, recordCacheRequest and recordNetworkBytesSaved in the same class do pass client._clientId, making CSC eviction metrics inconsistent with other CSC metrics and impossible to correlate by server.

Additional Locations (2)

packages/client/lib/client/cache.ts#L663-L664

packages/client/lib/client/cache.ts#L694-L695

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

cursor · 2026-03-05T15:01:26Z

packages/client/lib/opentelemetry/metrics.ts

+    // because CSC stores transformed replies and does not retain raw byte size.
+    // Implement this at a lower protocol/parsing layer where response bytes are known.
+  }
+}


Network bytes saved metric is silently a no-op

Medium Severity

OTelClientSideCacheMetrics.recordNetworkBytesSaved has an empty body with a TODO comment, yet cache.ts calls it on every cache hit expecting the redis.client.csc.network_saved counter to be incremented. The metric instrument is registered and documented but will always report zero, silently providing incorrect observability data to users who enable client-side caching metrics.

Additional Locations (1)

packages/client/lib/client/cache.ts#L564-L568

PavelPashov force-pushed the feat/add-opentelemetry-metrics branch 4 times, most recently from a11117e to 5fb1791 Compare October 31, 2025 11:39

PavelPashov force-pushed the feat/add-opentelemetry-metrics branch 2 times, most recently from 1be4565 to 03db1a2 Compare November 5, 2025 11:03

jit-ci bot reviewed Nov 5, 2025

View reviewed changes

PavelPashov force-pushed the feat/add-opentelemetry-metrics branch 3 times, most recently from 1106f8a to 242e98d Compare November 6, 2025 13:55

PavelPashov force-pushed the feat/add-opentelemetry-metrics branch from 364ff86 to 30b0e8c Compare January 19, 2026 08:35

PavelPashov force-pushed the feat/add-opentelemetry-metrics branch 2 times, most recently from 0e91b34 to e2cc888 Compare February 20, 2026 15:32

PavelPashov marked this pull request as ready for review February 25, 2026 11:28

PavelPashov added 12 commits February 25, 2026 13:30

wip: add OpenTelemetry metrics instrumentation

2d569f7

add noop metrics

364ed40

fix: check if metrics are initilized in sendCommand

2e3cb2a

fix(otel): optimize metrics tracking by eliminating promise chaining …

cd31dfa

…overhead

refactor(otel): organize metrics into specialized metric groups

0d55b20

fix(otel): revert metrics to be tracked in the sendCommand method

67d99ca

perf(metrics): optimize command metrics with factory pattern and inli…

ce74dc1

…ne attributes

feat: Add new OTEL_ATTRIBUTES constants

0eedb01

feat: Add client-side-caching metric group with 4 new metric names

a14d159

feat: Create error categorization stub function

46ebd09

feat: Add pool name formatting utility function

cfe88dc

feat: Refactor recordConnectionCreateTime to use closure pattern

bccbdf7

PavelPashov added 11 commits February 25, 2026 13:30

feat: add error classification helper and enrich metrics attributes

ce23121

refactor: replace command wrapper with onSuccess hook for metrics

89aaf1b

feat(client): add client identity tracking for OpenTelemetry metrics

912e27d

feat(otel): convert metrics to observable gauges with client registry

ad2fd8d

feat(otel): refine metrics coverage and remove connection use_time in…

e843d0d

…strument

refactor(otel): resolve metric attributes via clientId registry lookup

6d14d9c

refactor(otel): propagate clientId through runtime metrics paths

4305d30

test(otel): expand metrics coverage and add test utilities

19c95b8

test(otel): fix flaky test

34cd991

feat(otel): align metric attributes/config and expand observability c…

ee5f2b0

…overage

test(otel): add maintenance metrics e2e scenario with standalone FI c…

d0f42d3

…onfig

PavelPashov force-pushed the feat/add-opentelemetry-metrics branch from af9f0fb to d0f42d3 Compare February 25, 2026 11:34

PavelPashov changed the title ~~wip: add OpenTelemetry metrics instrumentation~~ add OpenTelemetry metrics instrumentation Feb 25, 2026

refactor(otel): use instrumentation scope name and stop injecting res…

3e19cad

…ource attrs

vladvildanov requested changes Feb 25, 2026

View reviewed changes

vladvildanov reviewed Feb 26, 2026

View reviewed changes

PavelPashov added 2 commits February 26, 2026 14:37

fix(opentelemetry): rename stream bucket config

baac384

docs(opentelemetry): add metrics docs/examples

08c7413

cursor bot reviewed Feb 26, 2026

View reviewed changes

packages/client/lib/client/index.ts Show resolved Hide resolved

packages/client/lib/client/cache.ts Outdated Show resolved Hide resolved

packages/client/lib/cluster/index.ts Outdated Show resolved Hide resolved

packages/client/lib/client/socket.ts Show resolved Hide resolved

PavelPashov commented Feb 27, 2026

View reviewed changes

nkaradzhov reviewed Mar 4, 2026

View reviewed changes

fix(otel): scope redirection error dedupe to cluster retry path

f8a51c9

cursor bot reviewed Mar 4, 2026

View reviewed changes

packages/client/lib/opentelemetry/metrics.ts Show resolved Hide resolved

fix(opentelemetry): normalize db.namespace and server.port to strings…

181d086

… in command and CSC metrics

cursor bot reviewed Mar 5, 2026

View reviewed changes

PavelPashov force-pushed the feat/add-opentelemetry-metrics branch from bf97457 to adbe066 Compare March 5, 2026 14:57

cursor bot reviewed Mar 5, 2026

View reviewed changes

fix(otel): disable recordNetworkBytesSaved for CSC

bc14732

PavelPashov force-pushed the feat/add-opentelemetry-metrics branch from adbe066 to bc14732 Compare March 5, 2026 15:01

		if (command.onSuccess) {
		command.onSuccess(parser.redisArgs, finalReply, this._self._clientId);

Conversation

PavelPashov commented Oct 24, 2025 • edited by cursor bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

jit-ci bot left a comment

Choose a reason for hiding this comment

Uh oh!

elimelt commented Dec 8, 2025

Uh oh!

jit-ci bot commented Jan 19, 2026

❌ Security scan failed

Uh oh!

jit-ci bot commented Jan 19, 2026

❌ Security scan failed

Uh oh!

jit-ci bot commented Jan 19, 2026

❌ Security scan failed

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cursor bot Mar 5, 2026

Choose a reason for hiding this comment

Cache eviction metrics missing clientId for attribute resolution

Uh oh!

cursor bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor bot Mar 5, 2026

Choose a reason for hiding this comment

Network bytes saved metric is silently a no-op

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

PavelPashov commented Oct 24, 2025 •

edited by cursor bot

Loading

Cache eviction metrics missing `clientId` for attribute resolution