Skip to content

Commit 8f338e4

Browse files
s1gr1ds1gr1d
committed
feat(redis): Add cache logic for redis-4
1 parent e84230e commit 8f338e4

File tree

9 files changed

+337
-114
lines changed

9 files changed

+337
-114
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: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
const { loggingTransport } = require('@sentry-internal/node-integration-tests');
2+
const Sentry = require('@sentry/node');
3+
4+
Sentry.init({
5+
dsn: 'https://[email protected]/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 initializeClient() {
18+
return createClient().connect();
19+
}
20+
21+
let client;
22+
23+
(async function () {
24+
client = await initializeClient();
25+
})();
26+
27+
async function run() {
28+
await Sentry.startSpan(
29+
{
30+
name: 'Test Span',
31+
op: 'test-span',
32+
},
33+
async () => {
34+
try {
35+
await client.set('redis-test-key', 'test-value');
36+
await client.set('redis-cache:test-key', 'test-value');
37+
38+
await client.set('redis-cache:test-key-set-EX', 'test-value', 'EX', 10);
39+
await client.setex('redis-cache:test-key-setex', 10, 'test-value');
40+
41+
await client.get('redis-test-key');
42+
await client.get('redis-cache:test-key');
43+
await client.get('redis-cache:unavailable-data');
44+
45+
await client.mget('redis-test-key', 'redis-cache:test-key', 'redis-cache:unavailable-data');
46+
} finally {
47+
await client.disconnect();
48+
}
49+
},
50+
);
51+
}
52+
53+
// eslint-disable-next-line @typescript-eslint/no-floating-promises
54+
run();

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

Lines changed: 99 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,102 @@ 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_TRANSACTION = {
145+
transaction: 'Test Span',
146+
spans: expect.arrayContaining([
147+
// SET
148+
expect.objectContaining({
149+
description: 'redis-cache:test-key',
150+
op: 'cache.put',
151+
origin: 'auto.db.otel.redis',
152+
data: expect.objectContaining({
153+
'sentry.origin': 'auto.db.otel.redis',
154+
'db.statement': 'SET redis-cache:test-key [1 other arguments]',
155+
'cache.key': ['redis-cache:test-key'],
156+
'cache.item_size': 2,
157+
'network.peer.address': 'localhost',
158+
'network.peer.port': 6379,
159+
}),
160+
}),
161+
// SET (with EX)
162+
expect.objectContaining({
163+
description: 'redis-cache:test-key-set-EX',
164+
op: 'cache.put',
165+
origin: 'auto.db.otel.redis',
166+
data: expect.objectContaining({
167+
'sentry.origin': 'auto.db.otel.redis',
168+
'db.statement': 'SET redis-cache:test-key-set-EX [3 other arguments]',
169+
'cache.key': ['redis-cache:test-key-set-EX'],
170+
'cache.item_size': 2,
171+
'network.peer.address': 'localhost',
172+
'network.peer.port': 6379,
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+
'network.peer.address': 'localhost',
186+
'network.peer.port': 6379,
187+
}),
188+
}),
189+
// GET
190+
expect.objectContaining({
191+
description: 'redis-cache:test-key',
192+
op: 'cache.get',
193+
origin: 'auto.db.otel.redis',
194+
data: expect.objectContaining({
195+
'sentry.origin': 'auto.db.otel.redis',
196+
'db.statement': 'GET redis-cache:test-key',
197+
'cache.hit': true,
198+
'cache.key': ['redis-cache:test-key'],
199+
'cache.item_size': 10,
200+
'network.peer.address': 'localhost',
201+
'network.peer.port': 6379,
202+
}),
203+
}),
204+
// GET (unavailable - no cache hit)
205+
expect.objectContaining({
206+
description: 'redis-cache:unavailable-data',
207+
op: 'cache.get',
208+
origin: 'auto.db.otel.redis',
209+
data: expect.objectContaining({
210+
'sentry.origin': 'auto.db.otel.redis',
211+
'db.statement': 'GET redis-cache:unavailable-data',
212+
'cache.hit': false,
213+
'cache.key': ['redis-cache:unavailable-data'],
214+
'network.peer.address': 'localhost',
215+
'network.peer.port': 6379,
216+
}),
217+
}),
218+
// MGET
219+
expect.objectContaining({
220+
description: 'redis-test-key, redis-cache:test-key, redis-cache:unavailable-data',
221+
op: 'cache.get',
222+
origin: 'auto.db.otel.redis',
223+
data: expect.objectContaining({
224+
'sentry.origin': 'auto.db.otel.redis',
225+
'db.statement': 'MGET [3 other arguments]',
226+
'cache.hit': true,
227+
'cache.key': ['redis-test-key', 'redis-cache:test-key', 'redis-cache:unavailable-data'],
228+
'network.peer.address': 'localhost',
229+
'network.peer.port': 6379,
230+
}),
231+
}),
232+
]),
233+
};
234+
235+
createRunner(__dirname, 'scenario-redis-4.js')
236+
.withDockerCompose({ workingDirectory: [__dirname], readyMatches: ['port=6379'] })
237+
.expect({ transaction: EXPECTED_TRANSACTION })
238+
.start(done);
239+
});
142240
});

‎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.38.1",
8989
"@opentelemetry/instrumentation-nestjs-core": "0.37.1",
9090
"@opentelemetry/instrumentation-pg": "0.41.0",
91+
"@opentelemetry/instrumentation-redis-4": "0.40.0",
9192
"@opentelemetry/resources": "^1.23.0",
9293
"@opentelemetry/sdk-trace-base": "^1.23.0",
9394
"@opentelemetry/semantic-conventions": "^1.23.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: 67 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,
@@ -26,64 +29,80 @@ const INTEGRATION_NAME = 'Redis';
2629

2730
let _redisOptions: RedisOptions = {};
2831

29-
export const instrumentRedis = generateInstrumentOnce(INTEGRATION_NAME, () => {
32+
const cacheResponseHook: RedisResponseCustomAttributeFunction = (span: Span, redisCommand, cmdArgs, response) => {
33+
span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, 'auto.db.otel.redis');
34+
35+
const safeKey = getCacheKeySafely(redisCommand, cmdArgs);
36+
const cacheOperation = getCacheOperation(redisCommand);
37+
38+
if (
39+
!safeKey ||
40+
!cacheOperation ||
41+
!_redisOptions?.cachePrefixes ||
42+
!shouldConsiderForCache(redisCommand, safeKey, _redisOptions.cachePrefixes)
43+
) {
44+
// not relevant for cache
45+
return;
46+
}
47+
48+
// 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
49+
// We are using params based on the docs: https://opentelemetry.io/docs/specs/semconv/attributes-registry/network/
50+
const networkPeerAddress = spanToJSON(span).data?.['net.peer.name'];
51+
const networkPeerPort = spanToJSON(span).data?.['net.peer.port'];
52+
if (networkPeerPort && networkPeerAddress) {
53+
span.setAttributes({ 'network.peer.address': networkPeerAddress, 'network.peer.port': networkPeerPort });
54+
}
55+
56+
const cacheItemSize = calculateCacheItemSize(response);
57+
58+
if (cacheItemSize) {
59+
span.setAttribute(SEMANTIC_ATTRIBUTE_CACHE_ITEM_SIZE, cacheItemSize);
60+
}
61+
62+
if (GET_COMMANDS.includes(redisCommand) && cacheItemSize !== undefined) {
63+
span.setAttribute(SEMANTIC_ATTRIBUTE_CACHE_HIT, cacheItemSize > 0);
64+
}
65+
66+
span.setAttributes({
67+
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: cacheOperation,
68+
[SEMANTIC_ATTRIBUTE_CACHE_KEY]: safeKey,
69+
});
70+
71+
const spanDescription = safeKey.join(', ');
72+
73+
span.updateName(spanDescription.length > 1024 ? `${spanDescription.substring(0, 1024)}...` : spanDescription);
74+
};
75+
76+
const instrumentIORedis = generateInstrumentOnce('IORedis', () => {
3077
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-
},
78+
responseHook: cacheResponseHook,
7479
});
7580
});
7681

82+
const instrumentRedis4 = generateInstrumentOnce('Redis-4', () => {
83+
return new RedisInstrumentation({
84+
responseHook: cacheResponseHook,
85+
});
86+
});
87+
88+
/** To be able to preload all Redis OTel instrumentations with just one ID ("Redis"), all the instrumentations are generated in this one function */
89+
export const instrumentRedis = Object.assign(
90+
(): void => {
91+
instrumentIORedis();
92+
instrumentRedis4();
93+
94+
// todo: implement them gradually
95+
// new LegacyRedisInstrumentation({}),
96+
},
97+
{ id: INTEGRATION_NAME },
98+
);
99+
77100
const _redisIntegration = ((options: RedisOptions = {}) => {
78101
return {
79102
name: INTEGRATION_NAME,
80103
setupOnce() {
81104
_redisOptions = options;
82105
instrumentRedis();
83-
84-
// todo: implement them gradually
85-
// new LegacyRedisInstrumentation({}),
86-
// new RedisInstrumentation({}),
87106
},
88107
};
89108
}) satisfies IntegrationFn;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ export function getCacheKeySafely(redisCommand: string, cmdArgs: IORedisCommandA
4444
}
4545
};
4646

47-
if (SINGLE_ARG_COMMANDS.includes(redisCommand) && cmdArgs.length > 0) {
47+
if (SINGLE_ARG_COMMANDS.includes(redisCommand.toLowerCase()) && cmdArgs.length > 0) {
4848
return processArg(cmdArgs[0]);
4949
}
5050

‎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)