Skip to content

Commit 473812f

Browse files
szegediwatson
authored andcommitted
Various improvements to profiling sample label generator (#5316)
* - Allow for context.context not being defined (newer pprof-nodejs won't attach it to idle samples) - Bail out faster when it's not present - Handle soon-to-be-introduced context.asyncId * Add a comment, better express type expectations * Test the label generator
1 parent 8e5f7c6 commit 473812f

File tree

2 files changed

+105
-5
lines changed

2 files changed

+105
-5
lines changed

packages/dd-trace/src/profiling/profilers/wall.js

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -286,14 +286,25 @@ class NativeWallProfiler {
286286

287287
const labels = { ...getThreadLabels() }
288288

289-
const { context: { ref }, timestamp } = context
290-
const { spanId, rootSpanId, webTags, endpoint } = ref ?? {}
291-
292289
if (this._timelineEnabled) {
293290
// Incoming timestamps are in microseconds, we emit nanos.
294-
labels[END_TIMESTAMP_LABEL] = timestamp * 1000n
291+
labels[END_TIMESTAMP_LABEL] = context.timestamp * 1000n
292+
}
293+
294+
const asyncId = context.asyncId
295+
if (asyncId !== undefined && asyncId !== -1) {
296+
labels['async id'] = asyncId
295297
}
296298

299+
// Native profiler doesn't set context.context for some samples, such as idle samples or when
300+
// the context was otherwise unavailable when the sample was taken.
301+
const ref = context.context?.ref
302+
if (typeof ref !== 'object') {
303+
return labels
304+
}
305+
306+
const { spanId, rootSpanId, webTags, endpoint } = ref
307+
297308
if (spanId !== undefined) {
298309
labels[SPAN_ID_LABEL] = spanId
299310
}

packages/dd-trace/test/profiling/profilers/wall.spec.js

Lines changed: 90 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,11 @@ describe('profilers/native/wall', () => {
1616
time: {
1717
start: sinon.stub(),
1818
stop: sinon.stub().returns('profile'),
19+
setContext: sinon.stub(),
1920
v8ProfilerStuckEventLoopDetected: sinon.stub().returns(false),
2021
constants: {
21-
kSampleCount: 0
22+
kSampleCount: 0,
23+
NON_JS_THREADS_FUNCTION_NAME: 'Non JS threads activity'
2224
}
2325
}
2426
}
@@ -174,4 +176,91 @@ describe('profilers/native/wall', () => {
174176
collectCpuTime: false
175177
})
176178
})
179+
180+
it('should generate appropriate sample labels', () => {
181+
const profiler = new NativeWallProfiler({ timelineEnabled: true })
182+
profiler.start()
183+
profiler.stop()
184+
185+
function expectLabels (context, expected) {
186+
const actual = profiler._generateLabels({ node: {}, context })
187+
expect(actual).to.deep.equal(expected)
188+
}
189+
190+
expect(profiler._generateLabels({ node: { name: 'Non JS threads activity' } })).to.deep.equal({
191+
'thread name': 'Non-JS threads',
192+
'thread id': 'NA',
193+
'os thread id': 'NA'
194+
})
195+
196+
const shared = require('../../../src/profiling/profilers/shared')
197+
const nativeThreadId = shared.getThreadLabels()['os thread id']
198+
const threadInfo = {
199+
'thread name': 'Main Event Loop',
200+
'thread id': '0',
201+
'os thread id': nativeThreadId
202+
}
203+
204+
expectLabels(undefined, threadInfo)
205+
206+
const threadInfoWithTimestamp = {
207+
...threadInfo,
208+
end_timestamp_ns: 1234000n
209+
}
210+
211+
expectLabels({ timestamp: 1234n }, threadInfoWithTimestamp)
212+
213+
expectLabels({ timestamp: 1234n, asyncId: -1 }, threadInfoWithTimestamp)
214+
215+
expectLabels({ timestamp: 1234n, asyncId: 1 }, {
216+
...threadInfoWithTimestamp,
217+
'async id': 1
218+
})
219+
220+
expectLabels({ timestamp: 1234n, context: {} }, threadInfoWithTimestamp)
221+
222+
expectLabels({ timestamp: 1234n, context: { ref: {} } }, threadInfoWithTimestamp)
223+
224+
expectLabels({ timestamp: 1234n, context: { ref: { spanId: 'foo' } } }, {
225+
...threadInfoWithTimestamp,
226+
'span id': 'foo'
227+
})
228+
229+
expectLabels({ timestamp: 1234n, context: { ref: { rootSpanId: 'foo' } } }, {
230+
...threadInfoWithTimestamp,
231+
'local root span id': 'foo'
232+
})
233+
234+
expectLabels({
235+
timestamp: 1234n,
236+
context: { ref: { webTags: { 'http.method': 'GET', 'http.route': '/foo/bar' } } }
237+
}, {
238+
...threadInfoWithTimestamp,
239+
'trace endpoint': 'GET /foo/bar'
240+
})
241+
242+
expectLabels({ timestamp: 1234n, context: { ref: { endpoint: 'GET /foo/bar/2' } } }, {
243+
...threadInfoWithTimestamp,
244+
'trace endpoint': 'GET /foo/bar/2'
245+
})
246+
247+
// All at once
248+
expectLabels({
249+
timestamp: 1234n,
250+
asyncId: 2,
251+
context: {
252+
ref: {
253+
spanId: '1234567890',
254+
rootSpanId: '0987654321',
255+
webTags: { 'http.method': 'GET', 'http.route': '/foo/bar' }
256+
}
257+
}
258+
}, {
259+
...threadInfoWithTimestamp,
260+
'async id': 2,
261+
'span id': '1234567890',
262+
'local root span id': '0987654321',
263+
'trace endpoint': 'GET /foo/bar'
264+
})
265+
})
177266
})

0 commit comments

Comments
 (0)