Use std::optional for GetNodeApplicationFilename return value and add…

… doc string to method Signed-off-by: Dom Del Nano <[email protected]>
pixie-io · Jul 28, 2024 · 92f79ba · 92f79ba
1 parent 1207906
commit 92f79ba
Show file tree

Hide file tree

Showing 4 changed files with 14 additions and 7 deletions.
diff --git a/src/stirling/source_connectors/socket_tracer/uprobe_manager.cc b/src/stirling/source_connectors/socket_tracer/uprobe_manager.cc
@@ -484,9 +484,10 @@ StatusOr<int> UProbeManager::AttachNodeJsOpenSSLUprobes(const uint32_t pid,
   }
 
   const std::string exe_cmdline = proc_parser_->GetPIDCmdline(pid);
-  const std::string node_application_filepath = GetNodeApplicationFilename(exe_cmdline);
-  if (uprobe_opt_out_.contains(node_application_filepath)) {
-    VLOG(1) << absl::Substitute(kUprobeSkippedMessage, node_application_filepath);
+  const auto node_application_filepath = GetNodeApplicationFilename(exe_cmdline);
+  if (node_application_filepath.has_value() &&
+      uprobe_opt_out_.contains(node_application_filepath.value())) {
+    VLOG(1) << absl::Substitute(kUprobeSkippedMessage, node_application_filepath.value());
     return 0;
   }
   const auto host_proc_exe = ProcPidRootPath(pid, proc_exe);

diff --git a/src/stirling/utils/detect_application.cc b/src/stirling/utils/detect_application.cc
@@ -49,15 +49,21 @@ Application DetectApplication(const std::filesystem::path& exe) {
   return Application::kUnknown;
 }
 
-std::string GetNodeApplicationFilename(std::string_view cmdline) {
+// This method returns the main nodejs application file from a command line. See the following
+// examples below:
+//
+// "node /usr/bin/test.js" -> "test.js"
+// "node --node-memory-debug /usr/bin/test.js" -> "test.js"
+// "node /usr/bin/test" -> std::nullopt
+std::optional<std::string> GetNodeApplicationFilename(std::string_view cmdline) {
   std::vector<std::string> cmdline_parts = absl::StrSplit(cmdline, ' ');
   for (const auto& part : cmdline_parts) {
     if (absl::EndsWith(part, ".js")) {
       std::filesystem::path path(part);
       return path.filename();
     }
   }
-  return "";
+  return {};
 }
 
 bool operator<(const SemVer& lhs, const SemVer& rhs) {

diff --git a/src/stirling/utils/detect_application.h b/src/stirling/utils/detect_application.h
@@ -37,7 +37,7 @@ enum class Application {
 Application DetectApplication(const std::filesystem::path& exe);
 
 // Returns the filename of a node application from the command line.
-std::string GetNodeApplicationFilename(std::string_view cmdline);
+std::optional<std::string> GetNodeApplicationFilename(std::string_view cmdline);
 
 // Describes a semantic versioning number.
 struct SemVer {

diff --git a/src/stirling/utils/detect_application_test.cc b/src/stirling/utils/detect_application_test.cc
@@ -37,7 +37,7 @@ TEST(DetectApplicationTest, ResultsAreAsExpected) {
 TEST(GetNodeApplicatFilenameTest, ResultsAreAsExpected) {
   EXPECT_EQ(GetNodeApplicationFilename("node /usr/bin/test.js"), "test.js");
   EXPECT_EQ(GetNodeApplicationFilename("node --node-memory-debug /usr/bin/test.js"), "test.js");
-  EXPECT_EQ(GetNodeApplicationFilename("node /usr/bin/test"), "");
+  EXPECT_FALSE(GetNodeApplicationFilename("node /usr/bin/test").has_value());
 }
 
 TEST(GetSemVerTest, AsExpected) {