Changed Profile Action to have a per-process limit rather than per thread

api/envoy/extensions/watchdog/profile_action/v3alpha/profile_action.proto

@@ -25,11 +25,8 @@ message ProfileActionConfig {
   // File path to the directory to output profiles.
   string profile_path = 2 [(validate.rules).string = {min_bytes: 1}];
 
-  // Limits the max number of profiles that can be generated by a thread over
-  // its lifetime to avoid filling the disk. We keep a map of <tid, count>
-  // to track the number of profiles triggered by a particular thread. Only one
-  // thread is counted as triggering the profile even though multiple threads
-  // might have been eligible for triggering the profile.
+  // Limits the max number of profiles that can be generated by this action
+  // over its lifetime to avoid filling the disk.
   // If not set (i.e. it's 0), a default of 10 will be used.
-  uint64 max_profiles_per_thread = 3;
+  uint64 max_profiles = 3;
 }

source/extensions/watchdog/profile_action/BUILD

@@ -32,7 +32,7 @@ envoy_cc_extension(
     name = "config",
     srcs = ["config.cc"],
     hdrs = ["config.h"],
-    security_posture = "robust_to_untrusted_downstream_and_upstream",
+    security_posture = "data_plane_agnostic",
     status = "alpha",
     deps = [
         ":profile_action_lib",

source/extensions/watchdog/profile_action/config.h

@@ -12,7 +12,7 @@ namespace ProfileAction {
 
 class ProfileActionFactory : public Server::Configuration::GuardDogActionFactory {
 public:
-  ProfileActionFactory() : name_("envoy.watchdog.profile_action"){};
+  ProfileActionFactory() = default;
 
   Server::Configuration::GuardDogActionPtr createGuardDogActionFromProto(
       const envoy::config::bootstrap::v3::Watchdog::WatchdogAction& config,
@@ -22,13 +22,11 @@ class ProfileActionFactory : public Server::Configuration::GuardDogActionFactory
     return std::make_unique<ProfileActionConfig>();
   }
 
-  std::string name() const override { return name_; }
+  std::string name() const override { return "envoy.watchdog.profile_action"; }
 
 private:
   using ProfileActionConfig =
       envoy::extensions::watchdog::profile_action::v3alpha::ProfileActionConfig;
-
-  const std::string name_;
 };
 
 } // namespace ProfileAction

source/extensions/watchdog/profile_action/profile_action.cc

@@ -14,7 +14,7 @@ namespace Extensions {
 namespace Watchdog {
 namespace ProfileAction {
 namespace {
-static constexpr uint64_t DefaultMaxProfilePerTid = 10;
+static constexpr uint64_t DefaultMaxProfiles = 10;
 
 std::string generateProfileFilePath(const std::string& directory, const SystemTime& now) {
   auto timestamp = std::chrono::duration_cast<std::chrono::seconds>(now.time_since_epoch()).count();
@@ -31,9 +31,7 @@ ProfileAction::ProfileAction(
     : path_(config.profile_path()),
       duration_(
           std::chrono::milliseconds(PROTOBUF_GET_MS_OR_DEFAULT(config, profile_duration, 5000))),
-      max_profiles_per_tid_(config.max_profiles_per_thread() == 0
-                                ? DefaultMaxProfilePerTid
-                                : config.max_profiles_per_thread()),
+      max_profiles_(config.max_profiles() == 0 ? DefaultMaxProfiles : config.max_profiles()),
       running_profile_(false), profiles_started_(0), context_(context),
       timer_cb_(context_.dispatcher_.createTimer([this] {
         if (Profiler::Cpu::profilerEnabled()) {
@@ -58,9 +56,15 @@ void ProfileAction::run(
   }
 
   // Check if there's a tid that justifies profiling
-  auto trigger_tid = getTidTriggeringProfile(thread_ltt_pairs);
-  if (!trigger_tid.has_value()) {
-    ENVOY_LOG_MISC(warn, "Profile Action: None of the provided tids justify profiling");
+  if (thread_ltt_pairs.empty()) {
+    ENVOY_LOG_MISC(warn, "Profile Action: No tids were provided.");
+    return;
+  }
+
+  if (profiles_started_ >= max_profiles_) {
+    ENVOY_LOG_MISC(warn,
+                   "Profile Action: Unable to profile: enabled but already wrote {} profiles.",
+                   profiles_started_);
     return;
   }
 
@@ -78,7 +82,6 @@ void ProfileAction::run(
       // Update state
       running_profile_ = true;
       ++profiles_started_;
-      tid_to_profile_count_[*trigger_tid] += 1;
 
       // Schedule callback to stop
       timer_cb_->enableTimer(duration_);
@@ -90,19 +93,6 @@ void ProfileAction::run(
   }
 }
 
-// Helper to determine if we have a valid tid to start profiling.
-absl::optional<Thread::ThreadId> ProfileAction::getTidTriggeringProfile(
-    const std::vector<std::pair<Thread::ThreadId, MonotonicTime>>& thread_ltt_pairs) {
-
-  // Find a TID not over the max_profiles threshold
-  for (const auto& [tid, ltt] : thread_ltt_pairs) {
-    if (tid_to_profile_count_[tid] < max_profiles_per_tid_) {
-      return tid;
-    }
-  }
-
-  return absl::nullopt;
-}
 } // namespace ProfileAction
 } // namespace Watchdog
 } // namespace Extensions

source/extensions/watchdog/profile_action/profile_action.h

@@ -26,17 +26,12 @@ class ProfileAction : public Server::Configuration::GuardDogAction {
            MonotonicTime now) override;
 
 private:
-  // Helper to determine if we should run the profiler.
-  absl::optional<Thread::ThreadId> getTidTriggeringProfile(
-      const std::vector<std::pair<Thread::ThreadId, MonotonicTime>>& thread_ltt_pairs);
-
   const std::string path_;
   const std::chrono::milliseconds duration_;
-  const uint64_t max_profiles_per_tid_;
+  const uint64_t max_profiles_;
   bool running_profile_;
   std::string profile_filename_;
   uint64_t profiles_started_;
-  absl::flat_hash_map<Thread::ThreadId, uint64_t> tid_to_profile_count_;
   Server::Configuration::GuardDogActionFactoryContext& context_;
   Event::TimerPtr timer_cb_;
 };

test/extensions/watchdog/profile_action/config_test.cc

@@ -34,7 +34,7 @@ TEST(ProfileActionFactoryTest, CanCreateAction) {
 	      "value": {
 		"profile_duration": "2s",
 		"profile_path": "/tmp/envoy/",
-		"max_profiles_per_thread": "20"
+		"max_profiles": "20"
 	      }
             }
           },

test/extensions/watchdog/profile_action/profile_action_test.cc

@@ -292,12 +292,12 @@ TEST_F(ProfileActionTest, ShouldNotProfileIfNoTids) {
   EXPECT_EQ(countNumberOfProfileInPath(test_path_), 0);
 }
 
-TEST_F(ProfileActionTest, ShouldSaturateTids) {
+TEST_F(ProfileActionTest, ShouldSaturatedMaxProfiles) {
   // Create configuration that we'll run until it saturates.
   envoy::extensions::watchdog::profile_action::v3alpha::ProfileActionConfig config;
   config.set_profile_path(test_path_);
   config.mutable_profile_duration()->set_seconds(1);
-  config.set_max_profiles_per_thread(1);
+  config.set_max_profiles(1);
 
   // Create the ProfileAction before we start running the dispatcher
   // otherwise the timer created will in ProfileActions ctor will