- Preserve idle samples with zero wall time and no cpu time rollup to next

- Fix optionality of various elements of the time profile context

Author: szegedi

bindings/profilers/wall.cc

@@ -61,12 +61,6 @@ namespace dd {
 // Maximum number of rounds in the GetV8ToEpochOffset
 static constexpr int MAX_EPOCH_OFFSET_ATTEMPTS = 20;
 
-bool isIdleOrProgram(const v8::CpuProfileNode* node) {
-  auto* function_name = node->GetFunctionNameStr();
-  return strcmp(function_name, "(idle)") == 0 ||
-         strcmp(function_name, "(program)") == 0;
-}
-
 int getTotalHitCount(const v8::CpuProfileNode* node, bool* noHitLeaf) {
   int count = node->GetHitCount();
   auto child_count = node->GetChildrenCount();
@@ -450,23 +444,17 @@ std::shared_ptr<ContextsByNode> WallProfiler::GetContextsByNode(
           array = it->second.contexts;
           ++it->second.hitcount;
         }
-        if (sampleContext.context) {
-          // Conforms to TimeProfileNodeContext defined in v8-types.ts
-          Local<Object> timedContext = Object::New(isolate);
-          timedContext
-              ->Set(v8Context,
-                    contextKey,
-                    sampleContext.context.get()->Get(isolate))
-              .Check();
-          timedContext
-              ->Set(v8Context,
-                    timestampKey,
-                    BigInt::New(isolate, sampleTimestamp + V8toEpochOffset))
-              .Check();
-
-          // if current sample is idle/program, reports its cpu time to the next
-          // sample
-          if (collectCpuTime_ && !isIdleOrProgram(sample)) {
+        // Conforms to TimeProfileNodeContext defined in v8-types.ts
+        Local<Object> timedContext = Object::New(isolate);
+        timedContext
+            ->Set(v8Context,
+                  timestampKey,
+                  BigInt::New(isolate, sampleTimestamp + V8toEpochOffset))
+            .Check();
+        auto* function_name = sample->GetFunctionNameStr();
+        // If current sample is program, reports its cpu time to the next sample
+        if (strcmp(function_name, "(program)") != 0) {
+          if (collectCpuTime_) {
             timedContext
                 ->Set(v8Context,
                       cpuTimeKey,
@@ -475,13 +463,24 @@ std::shared_ptr<ContextsByNode> WallProfiler::GetContextsByNode(
                 .Check();
             lastCpuTime = sampleContext.cpu_time;
           }
-          timedContext
-              ->Set(v8Context,
-                    asyncIdKey,
-                    NewNumberFromInt64(isolate, sampleContext.async_id))
-              .Check();
-          array->Set(v8Context, array->Length(), timedContext).Check();
+          // If current sample is neither program nor idle, associate a sampling
+          // context and async ID
+          if (strcmp(function_name, "(idle)") != 0) {
+            if (sampleContext.context) {
+              timedContext
+                  ->Set(v8Context,
+                        contextKey,
+                        sampleContext.context.get()->Get(isolate))
+                  .Check();
+            }
+            timedContext
+                ->Set(v8Context,
+                      asyncIdKey,
+                      NewNumberFromInt64(isolate, sampleContext.async_id))
+                .Check();
+          }
         }
+        array->Set(v8Context, array->Length(), timedContext).Check();
 
         // Sample context was consumed, fetch the next one
         ++contextIt;

ts/src/profile-serializer.ts

@@ -270,7 +270,9 @@ function computeTotalHitCount(root: TimeProfileNode): number {
 
 /** Perform some modifications on time profile:
  *  - Add non-JS thread activity node if available
- *  - Remove `(idle)` and `(program)` nodes
+ *  - remove `(program)` nodes
+ *  - remove `(idle)` nodes with no context
+ *  - set `(idle)` nodes' wall time to zero when they have a context
  *  - Convert `(garbage collector)` node to `Garbage Collection`
  *  - Put `non-JS thread activity` node and `Garbage Collection` under a top level `Node.js` node
  * This function does not change the input profile.
@@ -297,7 +299,10 @@ function updateTimeProfile(prof: TimeProfile): TimeProfile {
   }
 
   for (const child of prof.topDownRoot.children as TimeProfileNode[]) {
-    if (child.name === '(idle)' || child.name === '(program)') {
+    if (child.name === '(program)') {
+      continue;
+    }
+    if (child.name === '(idle)' && child.contexts?.length === 0) {
       continue;
     }
     if (child.name === '(garbage collector)') {
@@ -377,17 +382,19 @@ export function serializeTimeProfile(
   ) => {
     let unlabelledHits = entry.node.hitCount;
     let unlabelledCpuTime = 0;
+    const isIdle = entry.node.name === '(idle)';
     for (const context of entry.node.contexts || []) {
       const labels = generateLabels
         ? generateLabels({node: entry.node, context})
-        : context.context;
+        : context.context ?? {};
       if (Object.keys(labels).length > 0) {
         // Only assign wall time if there are hits, some special nodes such as `(Non-JS threads)`
         // have zero hit count (since they do not count as wall time) and should not be assigned any
-        // wall time.
-        const values = unlabelledHits > 0 ? [1, intervalNanos] : [0, 0];
+        // wall time. Also, `(idle)` nodes should be assigned zero wall time.
+        const values =
+          unlabelledHits > 0 ? [1, isIdle ? 0 : intervalNanos] : [0, 0];
         if (prof.hasCpuTime) {
-          values.push(context.cpuTime);
+          values.push(context.cpuTime ?? 0);
         }
         const sample = new Sample({
           locationId: entry.stack,
@@ -397,14 +404,14 @@ export function serializeTimeProfile(
         samples.push(sample);
         unlabelledHits--;
       } else if (prof.hasCpuTime) {
-        unlabelledCpuTime += context.cpuTime;
+        unlabelledCpuTime += context.cpuTime ?? 0;
       }
     }
-    if (unlabelledHits > 0 || unlabelledCpuTime > 0) {
+    if ((!isIdle && unlabelledHits > 0) || unlabelledCpuTime > 0) {
       const labels = generateLabels ? generateLabels({node: entry.node}) : {};
       const values =
         unlabelledHits > 0
-          ? [unlabelledHits, unlabelledHits * intervalNanos]
+          ? [unlabelledHits, isIdle ? 0 : unlabelledHits * intervalNanos]
           : [0, 0];
       if (prof.hasCpuTime) {
         values.push(unlabelledCpuTime);

ts/src/v8-types.ts

@@ -38,10 +38,10 @@ export interface ProfileNode {
 }
 
 export interface TimeProfileNodeContext {
-  context: object;
+  context?: object;
   timestamp: bigint; // end of sample taking; in microseconds since epoch
-  cpuTime: number; // cpu time in nanoseconds
-  asyncId: number; // async_hooks.executionAsyncId() at the time of sample taking
+  cpuTime?: number; // cpu time in nanoseconds
+  asyncId?: number; // async_hooks.executionAsyncId() at the time of sample taking
 }
 
 export interface TimeProfileNode extends ProfileNode {

ts/test/test-time-profiler.ts

@@ -36,13 +36,7 @@ describe('Time Profiler', () => {
     it('should exclude program and idle time', async () => {
       const profile = await time.profile(PROFILE_OPTIONS);
       assert.ok(profile.stringTable);
-      assert.deepEqual(
-        [
-          profile.stringTable.strings!.indexOf('(program)'),
-          profile.stringTable.strings!.indexOf('(idle)'),
-        ],
-        [-1, -1]
-      );
+      assert.equal(profile.stringTable.strings!.indexOf('(program)'), -1);
     });
 
     it('should update state', function () {
@@ -140,7 +134,7 @@ describe('Time Profiler', () => {
           'context.asyncId should be a number'
         );
         const labels: LabelSet = {};
-        for (const [key, value] of Object.entries(context.context)) {
+        for (const [key, value] of Object.entries(context.context ?? {})) {
           if (typeof value === 'string') {
             labels[key] = value;
             if (