fix(server-utils): Dedupe ioredis orchestrion span for offline-queued commands (#22279)

Offline commands that were drained on connect produced duplicated spans,
which we now prevent by just deduping in a `WeakSet`.

Saw this when testing locally, this was not catched by e2e before.

Author: chargome

dev-packages/e2e-tests/test-applications/react-router-8-orchestrion/tests/db.test.ts

@@ -36,6 +36,10 @@ test('Instruments ioredis automatically via orchestrion', async ({ baseURL }) =>
       }),
     }),
   );
+
+  // Each command maps to exactly one span (no offline-queue duplicate).
+  const setSpans = spans.filter(span => span.description === 'set test-key [1 other arguments]');
+  expect(setSpans).toHaveLength(1);
 });
 
 test('Instruments mysql automatically via orchestrion', async ({ baseURL }) => {

packages/server-utils/src/integrations/tracing-channel/ioredis.ts

@@ -67,6 +67,36 @@ function connectionAttributes(host: string | undefined, port: number | undefined
   };
 }
 
+// ioredis re-enters `sendCommand` with the same command object when it drains
+// the offline queue on connect which leads to duplicate spans.
+// Track commands we've already traced so each logical command produces one span.
+const tracedCommands = new WeakSet<object>();
+
+/**
+ * Builds the db span for an `orchestrion:ioredis:command` payload, or returns `undefined` to skip
+ * it: for a non-command payload, or the offline-queue re-send of an already-traced command.
+ *
+ * Exported for unit testing.
+ */
+export function startIORedisCommandSpan(data: IORedisCommandContext): Span | undefined {
+  const command = data.arguments?.[0] as RedisCommand | undefined;
+  if (!command || typeof command !== 'object') {
+    return undefined;
+  }
+  // guard against duplicate spans
+  if (tracedCommands.has(command)) {
+    return undefined;
+  }
+  tracedCommands.add(command);
+  const { host, port } = getConnectionOptions(data.self);
+  const statement = defaultDbStatementSerializer(command.name, command.args ?? []);
+  return startInactiveSpan({
+    name: statement,
+    op: 'db',
+    attributes: { ...connectionAttributes(host, port), [DB_STATEMENT]: statement },
+  });
+}
+
 const _ioredisChannelIntegration = ((options: IORedisChannelIntegrationOptions = {}) => {
   const responseHook = options.responseHook;
 
@@ -92,35 +122,19 @@ const _ioredisChannelIntegration = ((options: IORedisChannelIntegrationOptions =
       // binding that `initOpenTelemetry()` registers after integration `setupOnce` —
       // defer until it's available (matches the native redis diagnostics-channel subscriber).
       waitForTracingChannelBinding(() => {
-        bindTracingChannelToSpan(
-          commandChannel,
-          data => {
+        bindTracingChannelToSpan(commandChannel, startIORedisCommandSpan, {
+          // ioredis' `requireParentSpan` default: only create a span under an active span.
+          requiresParentSpan: true,
+          beforeSpanEnd(span, data) {
+            if ('error' in data || !responseHook) {
+              return;
+            }
             const command = data.arguments?.[0] as RedisCommand | undefined;
-            if (!command || typeof command !== 'object') {
-              return undefined;
+            if (command) {
+              runResponseHook(responseHook, span, command, data.result);
             }
-            const { host, port } = getConnectionOptions(data.self);
-            const statement = defaultDbStatementSerializer(command.name, command.args ?? []);
-            return startInactiveSpan({
-              name: statement,
-              op: 'db',
-              attributes: { ...connectionAttributes(host, port), [DB_STATEMENT]: statement },
-            });
           },
-          {
-            // ioredis' `requireParentSpan` default: only create a span under an active span.
-            requiresParentSpan: true,
-            beforeSpanEnd(span, data) {
-              if ('error' in data || !responseHook) {
-                return;
-              }
-              const command = data.arguments?.[0] as RedisCommand | undefined;
-              if (command) {
-                runResponseHook(responseHook, span, command, data.result);
-              }
-            },
-          },
-        );
+        });
 
         bindTracingChannelToSpan(
           connectChannel,

packages/server-utils/test/integrations/tracing-channel/ioredis.test.ts

@@ -0,0 +1,61 @@
+import type { Span } from '@sentry/core';
+import * as SentryCore from '@sentry/core';
+import { afterEach, beforeEach, describe, expect, it, type MockInstance, vi } from 'vitest';
+import { startIORedisCommandSpan } from '../../../src/integrations/tracing-channel/ioredis';
+
+const CONNECTION = { host: 'localhost', port: 6379 };
+
+function ctx(command: unknown): { arguments: unknown[]; self: { options: typeof CONNECTION } } {
+  return { arguments: [command], self: { options: CONNECTION } };
+}
+
+describe('startIORedisCommandSpan', () => {
+  let startInactiveSpanSpy: MockInstance;
+
+  beforeEach(() => {
+    startInactiveSpanSpy = vi.spyOn(SentryCore, 'startInactiveSpan').mockReturnValue({} as Span);
+  });
+
+  afterEach(() => {
+    vi.restoreAllMocks();
+  });
+
+  it('builds a db span with the orchestrion origin and stable db/net attributes', () => {
+    startIORedisCommandSpan(ctx({ name: 'set', args: ['test-key', 'test-value'] }));
+
+    expect(startInactiveSpanSpy).toHaveBeenCalledWith(
+      expect.objectContaining({
+        name: 'set test-key [1 other arguments]',
+        op: 'db',
+        attributes: expect.objectContaining({
+          'db.system': 'redis',
+          'db.connection_string': 'redis://localhost:6379',
+          'net.peer.name': 'localhost',
+          'net.peer.port': 6379,
+          'db.statement': 'set test-key [1 other arguments]',
+          'sentry.origin': 'auto.db.orchestrion.redis',
+        }),
+      }),
+    );
+  });
+
+  it('emits a single span when the same command is re-sent from the offline queue', () => {
+    const command = { name: 'set', args: ['test-key', 'test-value'] };
+
+    expect(startIORedisCommandSpan(ctx(command))).toBeDefined();
+    expect(startIORedisCommandSpan(ctx(command))).toBeUndefined();
+    expect(startInactiveSpanSpy).toHaveBeenCalledTimes(1);
+  });
+
+  it('spans distinct command objects with the same statement', () => {
+    startIORedisCommandSpan(ctx({ name: 'get', args: ['k'] }));
+    startIORedisCommandSpan(ctx({ name: 'get', args: ['k'] }));
+
+    expect(startInactiveSpanSpy).toHaveBeenCalledTimes(2);
+  });
+
+  it('skips payloads without a command object', () => {
+    expect(startIORedisCommandSpan({ arguments: [], self: { options: CONNECTION } })).toBeUndefined();
+    expect(startInactiveSpanSpy).not.toHaveBeenCalled();
+  });
+});