Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 1.8k
feat(node): use diagnostics_channel for redis >= 5.12.0#20573
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
2caff8950973587b5e4662b84a64629eb8cfe584497cabfb83c568bb6fd04e578b2244File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| version: '3.9' | ||
| services: | ||
| db: | ||
| image: redis:latest | ||
| restart: always | ||
| container_name: integration-tests-redis-dc | ||
| ports: | ||
| - '6379:6379' | ||
| healthcheck: | ||
| test: ['CMD-SHELL', 'redis-cli ping | grep -q PONG'] | ||
| interval: 2s | ||
| timeout: 3s | ||
| retries: 30 | ||
| start_period: 5s |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| const { loggingTransport } = require('@sentry-internal/node-integration-tests'); | ||
| const Sentry = require('@sentry/node'); | ||
| Sentry.init({ | ||
| dsn: 'https://public@dsn.ingest.sentry.io/1337', | ||
| release: '1.0', | ||
| tracesSampleRate: 1.0, | ||
| transport: loggingTransport, | ||
| integrations: [Sentry.redisIntegration({ cachePrefixes: ['dc-cache:'] })], | ||
| }); | ||
| // Stop the process from exiting before the transaction is sent | ||
| setInterval(() => {}, 1000); | ||
| async function run() { | ||
| // Yield a microtick so the DC subscriber (deferred via Promise.resolve().then) | ||
| // is registered before node-redis eagerly creates its native TracingChannels on require(). | ||
| await Promise.resolve(); | ||
| const { createClient } = require('redis-5'); | ||
| const redisClient = await createClient({ socket: { host: '127.0.0.1', port: 6379 } }).connect(); | ||
| await Sentry.startSpan( | ||
| { | ||
| name: 'Test Span Redis 5 DC', | ||
| op: 'test-span-redis-5-dc', | ||
| }, | ||
| async () => { | ||
| try { | ||
| await redisClient.set('dc-test-key', 'test-value'); | ||
| await redisClient.set('dc-cache:test-key', 'test-value'); | ||
| await redisClient.set('dc-cache:test-key-ex', 'test-value', { EX: 10 }); | ||
| await redisClient.get('dc-test-key'); | ||
| await redisClient.get('dc-cache:test-key'); | ||
| await redisClient.get('dc-cache:unavailable-data'); | ||
| await redisClient.mGet(['dc-test-key', 'dc-cache:test-key', 'dc-cache:unavailable-data']); | ||
| } finally { | ||
| await redisClient.disconnect(); | ||
| } | ||
| }, | ||
| ); | ||
| } | ||
| run(); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,110 @@ | ||
| import { afterAll, describe, expect, test } from 'vitest'; | ||
| import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; | ||
| describe('redis v5 diagnostics_channel auto instrumentation', () => { | ||
| afterAll(() => { | ||
| cleanupChildProcesses(); | ||
| }); | ||
| test('should create spans for redis v5 commands via diagnostics_channel', { timeout: 60_000 }, async () => { | ||
| const EXPECTED_TRANSACTION = { | ||
| transaction: 'Test Span Redis 5 DC', | ||
| spans: expect.arrayContaining([ | ||
| expect.objectContaining({ | ||
| op: 'db.redis', | ||
| origin: 'auto.db.redis.diagnostic_channel', | ||
| data: expect.objectContaining({ | ||
| 'sentry.op': 'db.redis', | ||
| 'sentry.origin': 'auto.db.redis.diagnostic_channel', | ||
| 'db.system': 'redis', | ||
| 'db.statement': 'SET dc-test-key [1 other arguments]', | ||
| }), | ||
| }), | ||
| // cache SET: span name updated to key by cacheResponseHook | ||
| expect.objectContaining({ | ||
| description: 'dc-cache:test-key', | ||
| op: 'cache.put', | ||
| origin: 'auto.db.redis.diagnostic_channel', | ||
| data: expect.objectContaining({ | ||
| 'sentry.origin': 'auto.db.redis.diagnostic_channel', | ||
| 'db.statement': 'SET dc-cache:test-key [1 other arguments]', | ||
| 'cache.key': ['dc-cache:test-key'], | ||
| 'cache.item_size': 2, | ||
| }), | ||
| }), | ||
| // cache SET with EX option: redis v5 sends SET key value EX 10 as the command | ||
| expect.objectContaining({ | ||
| description: 'dc-cache:test-key-ex', | ||
| op: 'cache.put', | ||
| origin: 'auto.db.redis.diagnostic_channel', | ||
| data: expect.objectContaining({ | ||
| 'sentry.origin': 'auto.db.redis.diagnostic_channel', | ||
| 'db.statement': 'SET dc-cache:test-key-ex [3 other arguments]', | ||
| 'cache.key': ['dc-cache:test-key-ex'], | ||
| 'cache.item_size': 2, | ||
| }), | ||
| }), | ||
| expect.objectContaining({ | ||
| op: 'db.redis', | ||
| origin: 'auto.db.redis.diagnostic_channel', | ||
| data: expect.objectContaining({ | ||
| 'sentry.op': 'db.redis', | ||
| 'sentry.origin': 'auto.db.redis.diagnostic_channel', | ||
| 'db.system': 'redis', | ||
| 'db.statement': 'GET dc-test-key', | ||
| }), | ||
| }), | ||
| // cache GET (hit) | ||
| expect.objectContaining({ | ||
| description: 'dc-cache:test-key', | ||
| op: 'cache.get', | ||
| origin: 'auto.db.redis.diagnostic_channel', | ||
| data: expect.objectContaining({ | ||
| 'sentry.origin': 'auto.db.redis.diagnostic_channel', | ||
| 'db.statement': 'GET dc-cache:test-key', | ||
| 'cache.hit': true, | ||
| 'cache.key': ['dc-cache:test-key'], | ||
| 'cache.item_size': 10, | ||
| }), | ||
| }), | ||
| // cache GET (miss) | ||
| expect.objectContaining({ | ||
| description: 'dc-cache:unavailable-data', | ||
| op: 'cache.get', | ||
| origin: 'auto.db.redis.diagnostic_channel', | ||
| data: expect.objectContaining({ | ||
| 'sentry.origin': 'auto.db.redis.diagnostic_channel', | ||
| 'db.statement': 'GET dc-cache:unavailable-data', | ||
| 'cache.hit': false, | ||
| 'cache.key': ['dc-cache:unavailable-data'], | ||
| }), | ||
| }), | ||
| // MGET: node-redis sanitizes args for diagnostics_channel (keys become '?'), | ||
| // so cache detection cannot match prefixes — remains a plain db.redis span. | ||
| expect.objectContaining({ | ||
| op: 'db.redis', | ||
| origin: 'auto.db.redis.diagnostic_channel', | ||
| data: expect.objectContaining({ | ||
| 'sentry.op': 'db.redis', | ||
| 'sentry.origin': 'auto.db.redis.diagnostic_channel', | ||
| 'db.system': 'redis', | ||
| 'db.statement': 'MGET [3 other arguments]', | ||
| }), | ||
| }), | ||
| ]), | ||
| }; | ||
| // node-redis emits a node-redis:connect DC event for the initial connection. | ||
| // That fires before startSpan so it arrives as the first envelope. | ||
| const EXPECTED_CONNECT = { | ||
| transaction: 'redis-connect', | ||
| }; | ||
| await createRunner(__dirname, 'scenario-redis-5.js') | ||
| .withDockerCompose({ workingDirectory: [__dirname] }) | ||
| .expect({ transaction: EXPECTED_CONNECT }) | ||
| .expect({ transaction: EXPECTED_TRANSACTION }) | ||
| .start() | ||
| .completed(); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -6,7 +6,6 @@ import { | ||
| SEMANTIC_ATTRIBUTE_CACHE_ITEM_SIZE, | ||
| SEMANTIC_ATTRIBUTE_CACHE_KEY, | ||
| SEMANTIC_ATTRIBUTE_SENTRY_OP, | ||
| SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, | ||
| spanToJSON, | ||
| truncate, | ||
| } from '@sentry/core'; | ||
| @@ -23,6 +22,7 @@ import { | ||
| import type { IORedisResponseCustomAttributeFunction } from './vendored/types'; | ||
| import { IORedisInstrumentation } from './vendored/ioredis-instrumentation'; | ||
| import { RedisInstrumentation } from './vendored/redis-instrumentation'; | ||
| import { subscribeRedisDiagnosticChannels } from './redis-dc-subscriber'; | ||
| interface RedisOptions { | ||
| /** | ||
| @@ -53,8 +53,6 @@ export const cacheResponseHook: IORedisResponseCustomAttributeFunction = ( | ||
| cmdArgs: IORedisCommandArgs, | ||
| response: unknown, | ||
| ) => { | ||
| span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, 'auto.db.otel.redis'); | ||
cursor[bot] marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| const safeKey = getCacheKeySafely(redisCommand, cmdArgs); | ||
| const cacheOperation = getCacheOperation(redisCommand); | ||
| @@ -120,6 +118,11 @@ export const instrumentRedis = Object.assign( | ||
| (): void => { | ||
| instrumentIORedis(); | ||
| instrumentRedisModule(); | ||
| // node-redis >= 5.12.0 publishes via diagnostics_channel. The subscriber uses | ||
| // `@sentry/opentelemetry/tracing-channel`, which needs the Sentry OTel context manager | ||
| // to be registered before it can `bindStore`. `initOpenTelemetry()` runs after integration | ||
| // `setupOnce`, so defer to the next tick. | ||
| void Promise.resolve().then(() => subscribeRedisDiagnosticChannels(cacheResponseHook)); | ||
isaacs marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. isaacs marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| // todo: implement them gradually | ||
| // new LegacyRedisInstrumentation({}), | ||
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.