Skip to content

Commit e55e65b

Browse files
authored
perf(redis): tighten per-command instrumentation (#8310)
Per-command tracer overhead is a measurable fraction of redis sub-millisecond latency. The following improvements are implemented: 1. Cache `{connectionOptions, connectionName}` per client; the lookup was per-command despite being stable for the client's lifetime. 2. Pass the original command array with `argsStartIndex=1` in the v4 path so `formatCommand` iterates without a per-command slice. 3. `formatArg` short-circuits short strings; longer ones slice directly to `97 + '...'`. 4. `wrapCallback` drops `shimmer.wrapFunction`; the wrapped callback is internal. 5. Drop the redundant `try`/`catch` around ioredis `sendCommand`. Bench (Node 24.13 / V8 13.6, n=1 M+ x 7 trials, drop best+worst): formatArg always-trim 31.70 ns/op formatArg short-string fast path 10.84 ns/op speedup: 2.92x connectionInfo probe + WeakMap.get 6.58 ns/op connectionInfo cached symbol-key 4.30 ns/op speedup: 1.53x
1 parent 985b1d0 commit e55e65b

5 files changed

Lines changed: 114 additions & 45 deletions

File tree

packages/datadog-instrumentations/src/ioredis.js

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,28 +10,32 @@ const startCh = channel('apm:ioredis:command:start')
1010
const finishCh = channel('apm:ioredis:command:finish')
1111
const errorCh = channel('apm:ioredis:command:error')
1212

13+
const connectionOptionsCache = new WeakMap()
14+
1315
function wrapRedis (Redis) {
1416
shimmer.wrap(Redis.prototype, 'sendCommand', sendCommand => function (command, stream) {
1517
if (!startCh.hasSubscribers) return sendCommand.apply(this, arguments)
1618

1719
if (!command || !command.promise) return sendCommand.apply(this, arguments)
1820

1921
const options = this.options || {}
20-
const connectionName = options.connectionName
21-
const db = options.db
22-
const connectionOptions = { host: options.host, port: options.port }
23-
24-
const ctx = { db, command: command.name, args: command.args, connectionOptions, connectionName }
22+
let connectionOptions = connectionOptionsCache.get(this)
23+
if (connectionOptions === undefined) {
24+
connectionOptions = { host: options.host, port: options.port }
25+
connectionOptionsCache.set(this, connectionOptions)
26+
}
27+
28+
const ctx = {
29+
db: options.db,
30+
command: command.name,
31+
args: command.args,
32+
connectionOptions,
33+
connectionName: options.connectionName,
34+
}
2535
return startCh.runStores(ctx, () => {
2636
command.promise.then(() => finish(finishCh, errorCh, ctx), err => finish(finishCh, errorCh, ctx, err))
2737

28-
try {
29-
return sendCommand.apply(this, arguments)
30-
} catch (err) {
31-
errorCh.publish(err)
32-
33-
throw err
34-
}
38+
return sendCommand.apply(this, arguments)
3539
})
3640
})
3741
return Redis

packages/datadog-instrumentations/src/redis.js

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,17 +13,15 @@ const errorCh = channel('apm:redis:command:error')
1313
let createClientUrl
1414
let createClientName
1515
const instanceInfo = new WeakMap()
16+
const connectionInfoCache = new WeakMap()
1617

1718
function wrapAddCommand (addCommand) {
1819
return function (command) {
1920
if (!startCh.hasSubscribers) {
2021
return addCommand.apply(this, arguments)
2122
}
2223

23-
const name = command[0]
24-
const args = command.slice(1)
25-
26-
const ctx = getStartCtx(this, name, args)
24+
const ctx = getStartCtx(this, command[0], command, 1)
2725
return startCh.runStores(ctx, () => {
2826
const res = addCommand.apply(this, arguments)
2927

@@ -136,22 +134,33 @@ addHook({ name: 'redis', versions: ['>=0.12 <2.6'] }, redis => {
136134
return redis
137135
})
138136

139-
function getStartCtx (client, command, args) {
140-
const { url, connectionName } = instanceInfo.get(client) || {}
137+
function getStartCtx (client, command, args, argsStartIndex) {
138+
let cached = connectionInfoCache.get(client)
139+
if (cached === undefined) {
140+
const info = instanceInfo.get(client)
141+
cached = {
142+
connectionOptions:
143+
client.connection_options || client.connection_option || client.connectionOption || info?.url,
144+
connectionName: info?.connectionName,
145+
}
146+
connectionInfoCache.set(client, cached)
147+
}
141148

142149
return {
143150
db: client.selected_db,
144151
command,
145152
args,
146-
connectionOptions: client.connection_options || client.connection_option || client.connectionOption || url,
147-
connectionName,
153+
argsStartIndex,
154+
connectionOptions: cached.connectionOptions,
155+
connectionName: cached.connectionName,
148156
}
149157
}
150158

151159
function wrapCallback (finishCh, errorCh, ctx, callback) {
152-
return shimmer.wrapFunction(callback, callback => function (err) {
160+
if (typeof callback !== 'function') return callback
161+
return function (err) {
153162
return finish(finishCh, errorCh, ctx, err, callback, this, arguments)
154-
})
163+
}
155164
}
156165

157166
function finish (finishCh, errorCh, ctx, error, callback, thisArg, args) {

packages/datadog-plugin-ioredis/test/index.spec.js

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,22 @@ describe('Plugin', () => {
5555
}, { spanResourceMatch: /^get$/ })
5656
})
5757

58+
it('formats numeric args without coercing to ?', async () => {
59+
await redis.expire('foo', 60)
60+
61+
await agent.assertFirstTraceSpan({
62+
meta: { 'redis.raw_command': 'EXPIRE foo 60' },
63+
}, { spanResourceMatch: /^expire$/ })
64+
})
65+
66+
it('redacts non-string non-number args as ?', async () => {
67+
await redis.set('foo', Buffer.from('binary-value'))
68+
69+
await agent.assertFirstTraceSpan({
70+
meta: { 'redis.raw_command': 'SET foo ?' },
71+
}, { spanResourceMatch: /^set$/ })
72+
})
73+
5874
it('should run the callback in the parent context', () => {
5975
const span = tracer.startSpan('test')
6076

packages/datadog-plugin-redis/src/index.js

Lines changed: 18 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@ const { CLIENT_PORT_KEY } = require('../../dd-trace/src/constants')
44
const CachePlugin = require('../../dd-trace/src/plugins/cache')
55
const urlFilter = require('../../dd-trace/src/plugins/util/urlfilter')
66

7+
const MAX_ARG_LENGTH = 100
8+
const MAX_COMMAND_LENGTH = 1000
9+
710
class RedisPlugin extends CachePlugin {
811
static id = 'redis'
912
static system = 'redis'
@@ -14,7 +17,7 @@ class RedisPlugin extends CachePlugin {
1417
}
1518

1619
bindStart (ctx) {
17-
const { db, command, args, connectionOptions, connectionName } = ctx
20+
const { db, command, args, argsStartIndex, connectionOptions, connectionName } = ctx
1821

1922
const resource = command
2023
const normalizedCommand = command.toUpperCase()
@@ -29,7 +32,7 @@ class RedisPlugin extends CachePlugin {
2932
meta: {
3033
'db.type': this._spanType,
3134
'db.name': db || '0',
32-
[`${this._spanType}.raw_command`]: formatCommand(normalizedCommand, args),
35+
[`${this._spanType}.raw_command`]: formatCommand(normalizedCommand, args, argsStartIndex),
3336
'out.host': connectionOptions.host,
3437
[CLIENT_PORT_KEY]: connectionOptions.port,
3538
},
@@ -43,36 +46,28 @@ class RedisPlugin extends CachePlugin {
4346
}
4447
}
4548

46-
function formatCommand (command, args) {
49+
function formatCommand (command, args, argsStartIndex = 0) {
4750
if (!args || command === 'AUTH') return command
4851

49-
for (let i = 0, l = args.length; i < l; i++) {
50-
if (typeof args[i] === 'function') continue
51-
52-
command = `${command} ${formatArg(args[i])}`
52+
let result = command
53+
for (let i = argsStartIndex, l = args.length; i < l; i++) {
54+
const arg = args[i]
55+
if (typeof arg === 'function') continue
5356

54-
if (command.length > 1000) return trim(command, 1000)
57+
result = `${result} ${formatArg(arg)}`
58+
if (result.length > MAX_COMMAND_LENGTH) return result.slice(0, MAX_COMMAND_LENGTH - 3) + '...'
5559
}
5660

57-
return command
61+
return result
5862
}
5963

6064
function formatArg (arg) {
61-
switch (typeof arg) {
62-
case 'string':
63-
case 'number':
64-
return trim(String(arg), 100)
65-
default:
66-
return '?'
67-
}
68-
}
69-
70-
function trim (str, maxlen) {
71-
if (str.length > maxlen) {
72-
str = str.slice(0, maxlen - 3) + '...'
65+
if (typeof arg === 'string') {
66+
return arg.length > MAX_ARG_LENGTH ? arg.slice(0, MAX_ARG_LENGTH - 3) + '...' : arg
7367
}
74-
75-
return str
68+
// Number stringification is bounded (~23 chars max), so it never hits MAX_ARG_LENGTH.
69+
if (typeof arg === 'number') return String(arg)
70+
return '?'
7671
}
7772

7873
function normalizeConfig (config) {

packages/datadog-plugin-redis/test/client.spec.js

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,51 @@ describe('Plugin', () => {
9090
await Promise.all([client.get('foo'), promise])
9191
})
9292

93+
it('keeps every arg when formatting a multi-arg command', async () => {
94+
const promise = agent.assertSomeTraces(traces => {
95+
assert.strictEqual(traces[0][0].meta['redis.raw_command'], 'SET multi-arg-key multi-arg-value')
96+
}, { spanResourceMatch: /^SET$/ })
97+
98+
await Promise.all([client.set('multi-arg-key', 'multi-arg-value'), promise])
99+
})
100+
101+
it('trims a string arg longer than 100 chars', async () => {
102+
const longValue = 'x'.repeat(150)
103+
const promise = agent.assertSomeTraces(traces => {
104+
const rawCommand = traces[0][0].meta['redis.raw_command']
105+
assert.strictEqual(rawCommand, `SET long-key ${'x'.repeat(97)}...`)
106+
assert.strictEqual(rawCommand.length, 'SET long-key '.length + 100)
107+
}, { spanResourceMatch: /^SET$/ })
108+
109+
await Promise.all([client.set('long-key', longValue), promise])
110+
})
111+
112+
it('redacts the AUTH password from the raw command', async () => {
113+
const promise = agent.assertSomeTraces(traces => {
114+
assert.strictEqual(traces[0][0].meta['redis.raw_command'], 'AUTH')
115+
}, { spanResourceMatch: /^AUTH$/ })
116+
117+
await Promise.all([
118+
client.sendCommand(['AUTH', 'super-secret-password']).catch(() => {}),
119+
promise,
120+
])
121+
})
122+
123+
it('caps the joined raw command at 1000 chars across many args', async () => {
124+
const args = []
125+
for (let index = 0; index < 200; index++) {
126+
args.push(`key${index}`, `value${index}`)
127+
}
128+
const promise = agent.assertSomeTraces(traces => {
129+
const rawCommand = traces[0][0].meta['redis.raw_command']
130+
assert.strictEqual(rawCommand.length, 1000)
131+
assert.ok(rawCommand.startsWith('MSET '))
132+
assert.ok(rawCommand.endsWith('...'))
133+
}, { spanResourceMatch: /^MSET$/ })
134+
135+
await Promise.all([client.sendCommand(['MSET', ...args]), promise])
136+
})
137+
93138
withPeerService(
94139
() => tracer,
95140
'redis',

0 commit comments

Comments
 (0)