Skip to content

Commit ce6fbcd

Browse files
s1gr1ds1gr1d
authored
feat(redis): Add cache logic for redis-4 (#12429)
Follow-up as cache logic for `ioredis` was already added.
1 parent ceaeeba commit ce6fbcd

File tree

9 files changed

+293
-62
lines changed

9 files changed

+293
-62
lines changed

‎dev-packages/node-integration-tests/package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@
5555
"node-schedule": "^2.1.1",
5656
"pg": "^8.7.3",
5757
"proxy": "^2.1.1",
58+
"redis-4": "npm:redis@^4.6.14",
5859
"reflect-metadata": "0.2.1",
5960
"rxjs": "^7.8.1",
6061
"yargs": "^16.2.0"

‎dev-packages/node-integration-tests/suites/tracing/redis-cache/scenario-redis-4.js‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
const { loggingTransport } = require('@sentry-internal/node-integration-tests');
2+
const Sentry = require('@sentry/node');
3+
4+
Sentry.init({
5+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
6+
release: '1.0',
7+
tracesSampleRate: 1.0,
8+
transport: loggingTransport,
9+
integrations: [Sentry.redisIntegration({ cachePrefixes: ['redis-cache:'] })],
10+
});
11+
12+
// Stop the process from exiting before the transaction is sent
13+
setInterval(() => {}, 1000);
14+
15+
const { createClient } = require('redis-4');
16+
17+
async function run() {
18+
const redisClient = await createClient().connect();
19+
20+
await Sentry.startSpan(
21+
{
22+
name: 'Test Span Redis 4',
23+
op: 'test-span-redis-4',
24+
},
25+
async () => {
26+
try {
27+
await redisClient.set('redis-test-key', 'test-value');
28+
await redisClient.set('redis-cache:test-key', 'test-value');
29+
30+
await redisClient.set('redis-cache:test-key-set-EX', 'test-value', { EX: 10 });
31+
await redisClient.setEx('redis-cache:test-key-setex', 10, 'test-value');
32+
33+
await redisClient.get('redis-test-key');
34+
await redisClient.get('redis-cache:test-key');
35+
await redisClient.get('redis-cache:unavailable-data');
36+
37+
await redisClient.mGet(['redis-test-key', 'redis-cache:test-key', 'redis-cache:unavailable-data']);
38+
} finally {
39+
await redisClient.disconnect();
40+
}
41+
},
42+
);
43+
}
44+
45+
// eslint-disable-next-line @typescript-eslint/no-floating-promises
46+
run();

‎dev-packages/node-integration-tests/suites/tracing/redis-cache/test.ts‎

Lines changed: 92 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ describe('redis cache auto instrumentation', () => {
4242
.start(done);
4343
});
4444

45-
test('should create cache spans for prefixed keys', done => {
45+
test('should create cache spans for prefixed keys (ioredis)', done => {
4646
const EXPECTED_TRANSACTION = {
4747
transaction: 'Test Span',
4848
spans: expect.arrayContaining([
@@ -139,4 +139,95 @@ describe('redis cache auto instrumentation', () => {
139139
.expect({ transaction: EXPECTED_TRANSACTION })
140140
.start(done);
141141
});
142+
143+
test('should create cache spans for prefixed keys (redis-4)', done => {
144+
const EXPECTED_REDIS_CONNECT = {
145+
transaction: 'redis-connect',
146+
};
147+
148+
const EXPECTED_TRANSACTION = {
149+
transaction: 'Test Span Redis 4',
150+
spans: expect.arrayContaining([
151+
// SET
152+
expect.objectContaining({
153+
description: 'redis-cache:test-key',
154+
op: 'cache.put',
155+
origin: 'auto.db.otel.redis',
156+
data: expect.objectContaining({
157+
'sentry.origin': 'auto.db.otel.redis',
158+
'db.statement': 'SET redis-cache:test-key [1 other arguments]',
159+
'cache.key': ['redis-cache:test-key'],
160+
'cache.item_size': 2,
161+
}),
162+
}),
163+
// SET (with EX)
164+
expect.objectContaining({
165+
description: 'redis-cache:test-key-set-EX',
166+
op: 'cache.put',
167+
origin: 'auto.db.otel.redis',
168+
data: expect.objectContaining({
169+
'sentry.origin': 'auto.db.otel.redis',
170+
'db.statement': 'SET redis-cache:test-key-set-EX [3 other arguments]',
171+
'cache.key': ['redis-cache:test-key-set-EX'],
172+
'cache.item_size': 2,
173+
}),
174+
}),
175+
// SETEX
176+
expect.objectContaining({
177+
description: 'redis-cache:test-key-setex',
178+
op: 'cache.put',
179+
origin: 'auto.db.otel.redis',
180+
data: expect.objectContaining({
181+
'sentry.origin': 'auto.db.otel.redis',
182+
'db.statement': 'SETEX redis-cache:test-key-setex [2 other arguments]',
183+
'cache.key': ['redis-cache:test-key-setex'],
184+
'cache.item_size': 2,
185+
}),
186+
}),
187+
// GET
188+
expect.objectContaining({
189+
description: 'redis-cache:test-key',
190+
op: 'cache.get',
191+
origin: 'auto.db.otel.redis',
192+
data: expect.objectContaining({
193+
'sentry.origin': 'auto.db.otel.redis',
194+
'db.statement': 'GET redis-cache:test-key',
195+
'cache.hit': true,
196+
'cache.key': ['redis-cache:test-key'],
197+
'cache.item_size': 10,
198+
}),
199+
}),
200+
// GET (unavailable - no cache hit)
201+
expect.objectContaining({
202+
description: 'redis-cache:unavailable-data',
203+
op: 'cache.get',
204+
origin: 'auto.db.otel.redis',
205+
data: expect.objectContaining({
206+
'sentry.origin': 'auto.db.otel.redis',
207+
'db.statement': 'GET redis-cache:unavailable-data',
208+
'cache.hit': false,
209+
'cache.key': ['redis-cache:unavailable-data'],
210+
}),
211+
}),
212+
// MGET
213+
expect.objectContaining({
214+
description: 'redis-test-key, redis-cache:test-key, redis-cache:unavailable-data',
215+
op: 'cache.get',
216+
origin: 'auto.db.otel.redis',
217+
data: expect.objectContaining({
218+
'sentry.origin': 'auto.db.otel.redis',
219+
'db.statement': 'MGET [3 other arguments]',
220+
'cache.hit': true,
221+
'cache.key': ['redis-test-key', 'redis-cache:test-key', 'redis-cache:unavailable-data'],
222+
}),
223+
}),
224+
]),
225+
};
226+
227+
createRunner(__dirname, 'scenario-redis-4.js')
228+
.withDockerCompose({ workingDirectory: [__dirname], readyMatches: ['port=6379'] })
229+
.expect({ transaction: EXPECTED_REDIS_CONNECT })
230+
.expect({ transaction: EXPECTED_TRANSACTION })
231+
.start(done);
232+
});
142233
});

‎packages/node/package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@
8888
"@opentelemetry/instrumentation-mysql2": "0.39.0",
8989
"@opentelemetry/instrumentation-nestjs-core": "0.38.0",
9090
"@opentelemetry/instrumentation-pg": "0.42.0",
91+
"@opentelemetry/instrumentation-redis-4": "0.40.0",
9192
"@opentelemetry/resources": "^1.25.0",
9293
"@opentelemetry/sdk-trace-base": "^1.25.0",
9394
"@opentelemetry/semantic-conventions": "^1.25.0",

‎packages/node/src/integrations/tracing/index.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import { instrumentMysql, mysqlIntegration } from './mysql';
1313
import { instrumentMysql2, mysql2Integration } from './mysql2';
1414
import { instrumentNest, nestIntegration } from './nest';
1515
import { instrumentPostgres, postgresIntegration } from './postgres';
16-
import { redisIntegration } from './redis';
16+
import { instrumentRedis, redisIntegration } from './redis';
1717

1818
/**
1919
* With OTEL, all performance integrations will be added, as OTEL only initializes them when the patched package is actually required.
@@ -60,5 +60,6 @@ export function getOpenTelemetryInstrumentationToPreload(): (((options?: any) =>
6060
instrumentPostgres,
6161
instrumentHapi,
6262
instrumentGraphql,
63+
instrumentRedis,
6364
];
6465
}

‎packages/node/src/integrations/tracing/redis.ts‎

Lines changed: 69 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
1+
import type { Span } from '@opentelemetry/api';
2+
import type { RedisResponseCustomAttributeFunction } from '@opentelemetry/instrumentation-ioredis';
13
import { IORedisInstrumentation } from '@opentelemetry/instrumentation-ioredis';
4+
import { RedisInstrumentation } from '@opentelemetry/instrumentation-redis-4';
25
import {
36
SEMANTIC_ATTRIBUTE_CACHE_HIT,
47
SEMANTIC_ATTRIBUTE_CACHE_ITEM_SIZE,
@@ -9,12 +12,14 @@ import {
912
spanToJSON,
1013
} from '@sentry/core';
1114
import type { IntegrationFn } from '@sentry/types';
15+
import { truncate } from '@sentry/utils';
1216
import { generateInstrumentOnce } from '../../otel/instrument';
1317
import {
1418
GET_COMMANDS,
1519
calculateCacheItemSize,
1620
getCacheKeySafely,
1721
getCacheOperation,
22+
isInCommands,
1823
shouldConsiderForCache,
1924
} from '../../utils/redisCache';
2025

@@ -26,64 +31,80 @@ const INTEGRATION_NAME = 'Redis';
2631

2732
let _redisOptions: RedisOptions = {};
2833

29-
export const instrumentRedis = generateInstrumentOnce(INTEGRATION_NAME, () => {
34+
const cacheResponseHook: RedisResponseCustomAttributeFunction = (span: Span, redisCommand, cmdArgs, response) => {
35+
span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, 'auto.db.otel.redis');
36+
37+
const safeKey = getCacheKeySafely(redisCommand, cmdArgs);
38+
const cacheOperation = getCacheOperation(redisCommand);
39+
40+
if (
41+
!safeKey ||
42+
!cacheOperation ||
43+
!_redisOptions?.cachePrefixes ||
44+
!shouldConsiderForCache(redisCommand, safeKey, _redisOptions.cachePrefixes)
45+
) {
46+
// not relevant for cache
47+
return;
48+
}
49+
50+
// otel/ioredis seems to be using the old standard, as there was a change to those params: https://github.com/open-telemetry/opentelemetry-specification/issues/3199
51+
// We are using params based on the docs: https://opentelemetry.io/docs/specs/semconv/attributes-registry/network/
52+
const networkPeerAddress = spanToJSON(span).data?.['net.peer.name'];
53+
const networkPeerPort = spanToJSON(span).data?.['net.peer.port'];
54+
if (networkPeerPort && networkPeerAddress) {
55+
span.setAttributes({ 'network.peer.address': networkPeerAddress, 'network.peer.port': networkPeerPort });
56+
}
57+
58+
const cacheItemSize = calculateCacheItemSize(response);
59+
60+
if (cacheItemSize) {
61+
span.setAttribute(SEMANTIC_ATTRIBUTE_CACHE_ITEM_SIZE, cacheItemSize);
62+
}
63+
64+
if (isInCommands(GET_COMMANDS, redisCommand) && cacheItemSize !== undefined) {
65+
span.setAttribute(SEMANTIC_ATTRIBUTE_CACHE_HIT, cacheItemSize > 0);
66+
}
67+
68+
span.setAttributes({
69+
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: cacheOperation,
70+
[SEMANTIC_ATTRIBUTE_CACHE_KEY]: safeKey,
71+
});
72+
73+
const spanDescription = safeKey.join(', ');
74+
75+
span.updateName(truncate(spanDescription, 1024));
76+
};
77+
78+
const instrumentIORedis = generateInstrumentOnce('IORedis', () => {
3079
return new IORedisInstrumentation({
31-
responseHook: (span, redisCommand, cmdArgs, response) => {
32-
span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, 'auto.db.otel.redis');
33-
34-
const safeKey = getCacheKeySafely(redisCommand, cmdArgs);
35-
const cacheOperation = getCacheOperation(redisCommand);
36-
37-
if (
38-
!safeKey ||
39-
!cacheOperation ||
40-
!_redisOptions?.cachePrefixes ||
41-
!shouldConsiderForCache(redisCommand, safeKey, _redisOptions.cachePrefixes)
42-
) {
43-
// not relevant for cache
44-
return;
45-
}
46-
47-
// otel/ioredis seems to be using the old standard, as there was a change to those params: https://github.com/open-telemetry/opentelemetry-specification/issues/3199
48-
// We are using params based on the docs: https://opentelemetry.io/docs/specs/semconv/attributes-registry/network/
49-
const networkPeerAddress = spanToJSON(span).data?.['net.peer.name'];
50-
const networkPeerPort = spanToJSON(span).data?.['net.peer.port'];
51-
if (networkPeerPort && networkPeerAddress) {
52-
span.setAttributes({ 'network.peer.address': networkPeerAddress, 'network.peer.port': networkPeerPort });
53-
}
54-
55-
const cacheItemSize = calculateCacheItemSize(response);
56-
57-
if (cacheItemSize) {
58-
span.setAttribute(SEMANTIC_ATTRIBUTE_CACHE_ITEM_SIZE, cacheItemSize);
59-
}
60-
61-
if (GET_COMMANDS.includes(redisCommand) && cacheItemSize !== undefined) {
62-
span.setAttribute(SEMANTIC_ATTRIBUTE_CACHE_HIT, cacheItemSize > 0);
63-
}
64-
65-
span.setAttributes({
66-
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: cacheOperation,
67-
[SEMANTIC_ATTRIBUTE_CACHE_KEY]: safeKey,
68-
});
69-
70-
const spanDescription = safeKey.join(', ');
71-
72-
span.updateName(spanDescription.length > 1024 ? `${spanDescription.substring(0, 1024)}...` : spanDescription);
73-
},
80+
responseHook: cacheResponseHook,
7481
});
7582
});
7683

84+
const instrumentRedis4 = generateInstrumentOnce('Redis-4', () => {
85+
return new RedisInstrumentation({
86+
responseHook: cacheResponseHook,
87+
});
88+
});
89+
90+
/** To be able to preload all Redis OTel instrumentations with just one ID ("Redis"), all the instrumentations are generated in this one function */
91+
export const instrumentRedis = Object.assign(
92+
(): void => {
93+
instrumentIORedis();
94+
instrumentRedis4();
95+
96+
// todo: implement them gradually
97+
// new LegacyRedisInstrumentation({}),
98+
},
99+
{ id: INTEGRATION_NAME },
100+
);
101+
77102
const _redisIntegration = ((options: RedisOptions = {}) => {
78103
return {
79104
name: INTEGRATION_NAME,
80105
setupOnce() {
81106
_redisOptions = options;
82107
instrumentRedis();
83-
84-
// todo: implement them gradually
85-
// new LegacyRedisInstrumentation({}),
86-
// new RedisInstrumentation({}),
87108
},
88109
};
89110
}) satisfies IntegrationFn;

‎packages/node/src/utils/redisCache.ts‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,19 @@ export const GET_COMMANDS = ['get', 'mget'];
77
export const SET_COMMANDS = ['set', 'setex'];
88
// todo: del, expire
99

10+
/** Checks if a given command is in the list of redis commands.
11+
* Useful because commands can come in lowercase or uppercase (depending on the library). */
12+
export function isInCommands(redisCommands: string[], command: string): boolean {
13+
return redisCommands.includes(command.toLowerCase());
14+
}
15+
1016
/** Determine cache operation based on redis statement */
1117
export function getCacheOperation(
1218
command: string,
1319
): 'cache.get' | 'cache.put' | 'cache.remove' | 'cache.flush' | undefined {
14-
const lowercaseStatement = command.toLowerCase();
15-
16-
if (GET_COMMANDS.includes(lowercaseStatement)) {
20+
if (isInCommands(GET_COMMANDS, command)) {
1721
return 'cache.get';
18-
} else if (SET_COMMANDS.includes(lowercaseStatement)) {
22+
} else if (isInCommands(SET_COMMANDS, command)) {
1923
return 'cache.put';
2024
} else {
2125
return undefined;
@@ -44,7 +48,7 @@ export function getCacheKeySafely(redisCommand: string, cmdArgs: IORedisCommandA
4448
}
4549
};
4650

47-
if (SINGLE_ARG_COMMANDS.includes(redisCommand) && cmdArgs.length > 0) {
51+
if (isInCommands(SINGLE_ARG_COMMANDS, redisCommand) && cmdArgs.length > 0) {
4852
return processArg(cmdArgs[0]);
4953
}
5054

‎packages/node/test/integrations/tracing/redis.test.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,18 @@ describe('Redis', () => {
1313
expect(result).toBe(undefined);
1414
});
1515

16-
it('should return a string representation of a single argument', () => {
16+
it('should return a string array representation of a single argument', () => {
1717
const cmdArgs = ['key1'];
1818
const result = getCacheKeySafely('get', cmdArgs);
1919
expect(result).toStrictEqual(['key1']);
2020
});
2121

22+
it('should return a string array representation of a single argument (uppercase)', () => {
23+
const cmdArgs = ['key1'];
24+
const result = getCacheKeySafely('GET', cmdArgs);
25+
expect(result).toStrictEqual(['key1']);
26+
});
27+
2228
it('should return only the key for multiple arguments', () => {
2329
const cmdArgs = ['key1', 'the-value'];
2430
const result = getCacheKeySafely('get', cmdArgs);

0 commit comments

Comments
 (0)