agents: gRPC JS asset methods must add requestId

PR-URL: #213 Reviewed-By: Trevor Norris <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]>
nodesource · Nov 8, 2024 · 624dde8 · 624dde8
1 parent dc4e10e
commit 624dde8
Show file tree

Hide file tree

Showing 8 changed files with 228 additions and 10 deletions.
diff --git a/agents/grpc/src/binding.cc b/agents/grpc/src/binding.cc
@@ -2,6 +2,7 @@
 #include "node_external_reference.h"
 #include "grpc_agent.h"
 #include "util.h"
+#include "nsolid/nsolid_util.h"
 #include "asserts-cpp/asserts.h"
 
 
@@ -25,6 +26,7 @@ static void Snapshot(const FunctionCallbackInfo<Value>& args) {
 
   grpcagent::CommandRequest req;
   req.set_command("snapshot");
+  req.set_requestid(utils::generate_unique_id());
   auto* command_args = req.mutable_args();
   auto* snapshot_args = command_args->mutable_profile();
   snapshot_args->set_thread_id(thread_id);
@@ -45,6 +47,7 @@ static void StartCPUProfile(const FunctionCallbackInfo<Value>& args) {
 
   grpcagent::CommandRequest req;
   req.set_command("profile");
+  req.set_requestid(utils::generate_unique_id());
   auto* command_args = req.mutable_args();
   auto* profile_args = command_args->mutable_profile();
   profile_args->set_thread_id(thread_id);
@@ -73,6 +76,7 @@ static void StartHeapProfile(const FunctionCallbackInfo<Value>& args) {
 
   grpcagent::CommandRequest req;
   req.set_command("heap_profile");
+  req.set_requestid(utils::generate_unique_id());
   auto* command_args = req.mutable_args();
   auto* profile_args = command_args->mutable_profile();
   profile_args->set_thread_id(thread_id);
@@ -106,6 +110,7 @@ static void StartHeapSampling(const FunctionCallbackInfo<Value>& args) {
 
   grpcagent::CommandRequest req;
   req.set_command("heap_sampling");
+  req.set_requestid(utils::generate_unique_id());
   auto* command_args = req.mutable_args();
   auto* profile_args = command_args->mutable_profile();
   profile_args->set_thread_id(thread_id);

diff --git a/agents/grpc/src/grpc_agent.cc b/agents/grpc/src/grpc_agent.cc
@@ -583,10 +583,9 @@ int GrpcAgent::start_cpu_profile_from_js(const grpcagent::CommandRequest& req) {
   ProfileOptions options = CPUProfileOptions();
   ErrorType ret = do_start_prof_init(req, ProfileType::kCpu, options);
   if (ret == ErrorType::ESuccess) {
-    std::string req_id = utils::generate_unique_id();
     start_profiling_msg_q_.enqueue({
       ret,
-      std::move(req_id),
+      req.requestid(),
       ProfileType::kCpu,
       std::move(options)
     });
@@ -613,7 +612,7 @@ int GrpcAgent::start_heap_profile_from_js(
   if (ret == ErrorType::ESuccess) {
     start_profiling_msg_q_.enqueue({
       ret,
-      utils::generate_unique_id(),
+      req.requestid(),
       ProfileType::kHeapProf,
       std::move(options)
     });
@@ -640,7 +639,7 @@ int GrpcAgent::start_heap_sampling_from_js(
   if (ret == ErrorType::ESuccess) {
     start_profiling_msg_q_.enqueue({
       ret,
-      utils::generate_unique_id(),
+      req.requestid(),
       ProfileType::kHeapSampl,
       std::move(options)
     });
@@ -667,7 +666,7 @@ int GrpcAgent::start_heap_snapshot_from_js(
   if (ret == ErrorType::ESuccess) {
     start_profiling_msg_q_.enqueue({
       ret,
-      utils::generate_unique_id(),
+      req.requestid(),
       ProfileType::kHeapSnapshot,
       std::move(options)
     });

diff --git a/test/agents/test-grpc-heap-profile.mjs b/test/agents/test-grpc-heap-profile.mjs
@@ -17,7 +17,12 @@ const {
 
 function checkProfileData(profile, metadata, requestId, agentId, options) {
   console.dir(profile, { depth: null });
-  assert.strictEqual(profile.common.requestId, requestId);
+  validateString(profile.common.requestId, 'requestId');
+  assert.ok(profile.common.requestId.length > 0);
+  if (requestId) {
+    assert.strictEqual(profile.common.requestId, requestId);
+  }
+
   assert.strictEqual(profile.common.command, 'heap_profile');
   // From here check at least that all the fields are present
   validateObject(profile.common.recorded, 'recorded');
@@ -324,6 +329,38 @@ tests.push({
   },
 });
 
+tests.push({
+  name: 'should also work from the JS api',
+  test: async () => {
+    return new Promise((resolve) => {
+      const grpcServer = new GRPCServer();
+      grpcServer.start(mustSucceed(async (port) => {
+        const env = {
+          NODE_DEBUG_NATIVE: 'nsolid_grpc_agent',
+          NSOLID_GRPC_INSECURE: 1,
+          NSOLID_GRPC: `localhost:${port}`
+        };
+
+        const opts = {
+          stdio: ['inherit', 'inherit', 'inherit', 'ipc'],
+          env,
+        };
+        const child = new TestClient([], opts);
+        const duration = 100;
+        grpcServer.once('heap_profile', mustCall(async (data) => {
+          checkProfileData(data.msg, data.metadata, null, agentId, { duration, threadId: 0 }, true);
+          await child.shutdown(0);
+          grpcServer.close();
+          resolve();
+        }));
+
+        const agentId = await child.id();
+        await child.heapProfile(duration);
+      }));
+    });
+  },
+});
+
 for (const { name, test } of tests) {
   console.log(`[heap profile] ${name}`);
   await test();

diff --git a/test/agents/test-grpc-heap-sampling.mjs b/test/agents/test-grpc-heap-sampling.mjs
@@ -17,7 +17,12 @@ const {
 
 function checkProfileData(profile, metadata, requestId, agentId, options) {
   console.dir(profile, { depth: null });
-  assert.strictEqual(profile.common.requestId, requestId);
+  validateString(profile.common.requestId, 'requestId');
+  assert.ok(profile.common.requestId.length > 0);
+  if (requestId) {
+    assert.strictEqual(profile.common.requestId, requestId);
+  }
+
   assert.strictEqual(profile.common.command, 'heap_sampling');
   // From here check at least that all the fields are present
   validateObject(profile.common.recorded, 'recorded');
@@ -303,6 +308,38 @@ tests.push({
   },
 });
 
+tests.push({
+  name: 'should also work from the JS api',
+  test: async () => {
+    return new Promise((resolve) => {
+      const grpcServer = new GRPCServer();
+      grpcServer.start(mustSucceed(async (port) => {
+        const env = {
+          NODE_DEBUG_NATIVE: 'nsolid_grpc_agent',
+          NSOLID_GRPC_INSECURE: 1,
+          NSOLID_GRPC: `localhost:${port}`
+        };
+
+        const opts = {
+          stdio: ['inherit', 'inherit', 'inherit', 'ipc'],
+          env,
+        };
+        const child = new TestClient([], opts);
+        const duration = 100;
+        grpcServer.once('heap_sampling', mustCall(async (data) => {
+          checkProfileData(data.msg, data.metadata, null, agentId, { duration, threadId: 0 }, true);
+          await child.shutdown(0);
+          grpcServer.close();
+          resolve();
+        }));
+
+        const agentId = await child.id();
+        await child.heapSampling(duration);
+      }));
+    });
+  },
+});
+
 for (const { name, test } of tests) {
   console.log(`[heap sampling] ${name}`);
   await test();

diff --git a/test/agents/test-grpc-profile.mjs b/test/agents/test-grpc-profile.mjs
@@ -17,7 +17,12 @@ const {
 
 function checkProfileData(profile, metadata, requestId, agentId, options) {
   console.dir(profile, { depth: null });
-  assert.strictEqual(profile.common.requestId, requestId);
+  validateString(profile.common.requestId, 'requestId');
+  assert.ok(profile.common.requestId.length > 0);
+  if (requestId) {
+    assert.strictEqual(profile.common.requestId, requestId);
+  }
+
   assert.strictEqual(profile.common.command, 'profile');
   // From here check at least that all the fields are present
   validateObject(profile.common.recorded, 'recorded');
@@ -351,6 +356,38 @@ tests.push({
   },
 });
 
+tests.push({
+  name: 'should also work from the JS api',
+  test: async () => {
+    return new Promise((resolve) => {
+      const grpcServer = new GRPCServer();
+      grpcServer.start(mustSucceed(async (port) => {
+        const env = {
+          NODE_DEBUG_NATIVE: 'nsolid_grpc_agent',
+          NSOLID_GRPC_INSECURE: 1,
+          NSOLID_GRPC: `localhost:${port}`
+        };
+
+        const opts = {
+          stdio: ['inherit', 'inherit', 'inherit', 'ipc'],
+          env,
+        };
+        const child = new TestClient([], opts);
+        const duration = 100;
+        grpcServer.once('profile', mustCall(async (data) => {
+          checkProfileData(data.msg, data.metadata, null, agentId, { duration, threadId: 0 }, true);
+          await child.shutdown(0);
+          grpcServer.close();
+          resolve();
+        }));
+
+        const agentId = await child.id();
+        await child.profile(duration);
+      }));
+    });
+  },
+});
+
 for (const { name, test } of tests) {
   console.log(`[cpu profile] ${name}`);
   await test();

diff --git a/test/agents/test-grpc-snapshot.mjs b/test/agents/test-grpc-snapshot.mjs
@@ -1,5 +1,5 @@
 // Flags: --expose-internals
-import { mustSucceed } from '../common/index.mjs';
+import { mustCall, mustSucceed } from '../common/index.mjs';
 import assert from 'node:assert';
 import validators from 'internal/validators';
 import {
@@ -15,7 +15,12 @@ const {
 
 function checkSnapshotData(snapshot, metadata, requestId, agentId, options) {
   console.dir(snapshot, { depth: null });
-  assert.strictEqual(snapshot.common.requestId, requestId);
+  validateString(snapshot.common.requestId, 'requestId');
+  assert.ok(snapshot.common.requestId.length > 0);
+  if (requestId) {
+    assert.strictEqual(snapshot.common.requestId, requestId);
+  }
+
   assert.strictEqual(snapshot.common.command, 'snapshot');
   // From here check at least that all the fields are present
   validateObject(snapshot.common.recorded, 'recorded');
@@ -258,6 +263,37 @@ tests.push({
   },
 });
 
+tests.push({
+  name: 'should also work from the JS api',
+  test: async () => {
+    return new Promise((resolve) => {
+      const grpcServer = new GRPCServer();
+      grpcServer.start(mustSucceed(async (port) => {
+        const env = {
+          NODE_DEBUG_NATIVE: 'nsolid_grpc_agent',
+          NSOLID_GRPC_INSECURE: 1,
+          NSOLID_GRPC: `localhost:${port}`
+        };
+
+        const opts = {
+          stdio: ['inherit', 'inherit', 'inherit', 'ipc'],
+          env,
+        };
+        const child = new TestClient([], opts);
+        grpcServer.once('snapshot', mustCall(async (data) => {
+          checkSnapshotData(data.msg, data.metadata, null, agentId, { threadId: 0 }, true);
+          await child.shutdown(0);
+          grpcServer.close();
+          resolve();
+        }));
+
+        const agentId = await child.id();
+        await child.snapshot();
+      }));
+    });
+  },
+});
+
 for (const { name, test } of tests) {
   console.log(`[heap profile] ${name}`);
   await test();

diff --git a/test/common/nsolid-grpc-agent/client.js b/test/common/nsolid-grpc-agent/client.js
@@ -105,6 +105,10 @@ if (isMainThread) {
       }
     } else if (msg.type === 'config') {
       process.send({ type: 'config', config: nsolid.config });
+    } else if (msg.type === 'heap_profile') {
+      nsolid.heapProfile(msg.duration);
+    } else if (msg.type === 'heap_sampling') {
+      nsolid.heapSampling(msg.duration);
     } else if (msg.type === 'id') {
       process.send({ type: 'id', id: nsolid.id });
     } else if (msg.type === 'log') {
@@ -117,13 +121,17 @@ if (isMainThread) {
       process.send({ type: 'log' });
     } else if (msg.type === 'metrics') {
       process.send({ type: 'metrics', metrics: nsolid.metrics() });
+    } else if (msg.type === 'profile') {
+      nsolid.profile(msg.duration);
     } else if (msg.type === 'shutdown') {
       clearInterval(interval);
       if (!msg.error) {
         process.exit(msg.code || 0);
       } else {
         throw new Error('error');
       }
+    } else if (msg.type === 'snapshot') {
+      nsolid.snapshot();
     } else if (msg.type === 'startupTimes') {
       process.recordStartupTime(msg.name);
       process.send(msg);