From 0500fc0894e73081e17c107d1aabe91a3c72850b Mon Sep 17 00:00:00 2001 From: Kevin Baichoo Date: Thu, 27 Aug 2020 17:35:39 -0400 Subject: [PATCH] Watchdog Extension: Profile Action (#12636) Added a watchdog extension that triggers profiling. Risk Level: Medium (new extension that is optional) Testing: Unit tests Docs Changes: Included (added a reference to the generated extension proto.rst) Release Notes: Included Fixes #11388 Signed-off-by: Kevin Baichoo Signed-off-by: Clara Andrew-Wani --- CODEOWNERS | 2 + api/BUILD | 1 + .../watchdog/profile_action/v3alpha/BUILD | 9 + .../v3alpha/profile_action.proto | 35 ++ api/versioning/BUILD | 1 + docs/root/api-v3/config/config.rst | 1 + docs/root/api-v3/config/watchdog/watchdog.rst | 8 + docs/root/version_history/current.rst | 1 + .../watchdog/profile_action/v3alpha/BUILD | 9 + .../v3alpha/profile_action.proto | 35 ++ include/envoy/server/guarddog_config.h | 2 +- source/extensions/extensions_build_config.bzl | 6 +- .../extensions/watchdog/profile_action/BUILD | 46 +++ .../watchdog/profile_action/config.cc | 32 ++ .../watchdog/profile_action/config.h | 37 ++ .../watchdog/profile_action/profile_action.cc | 109 ++++++ .../watchdog/profile_action/profile_action.h | 49 +++ test/extensions/watchdog/profile_action/BUILD | 51 +++ .../watchdog/profile_action/config_test.cc | 57 +++ .../profile_action/profile_action_test.cc | 359 ++++++++++++++++++ test/server/guarddog_impl_test.cc | 4 +- tools/spelling/spelling_dictionary.txt | 1 + 22 files changed, 851 insertions(+), 4 deletions(-) create mode 100644 api/envoy/extensions/watchdog/profile_action/v3alpha/BUILD create mode 100644 api/envoy/extensions/watchdog/profile_action/v3alpha/profile_action.proto create mode 100644 docs/root/api-v3/config/watchdog/watchdog.rst create mode 100644 generated_api_shadow/envoy/extensions/watchdog/profile_action/v3alpha/BUILD create mode 100644 generated_api_shadow/envoy/extensions/watchdog/profile_action/v3alpha/profile_action.proto create mode 100644 source/extensions/watchdog/profile_action/BUILD create mode 100644 source/extensions/watchdog/profile_action/config.cc create mode 100644 source/extensions/watchdog/profile_action/config.h create mode 100644 source/extensions/watchdog/profile_action/profile_action.cc create mode 100644 source/extensions/watchdog/profile_action/profile_action.h create mode 100644 test/extensions/watchdog/profile_action/BUILD create mode 100644 test/extensions/watchdog/profile_action/config_test.cc create mode 100644 test/extensions/watchdog/profile_action/profile_action_test.cc diff --git a/CODEOWNERS b/CODEOWNERS index 9bc329986764..1edcf7c68c1e 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -128,6 +128,8 @@ extensions/filters/common/original_src @snowp @klarose /*/extensions/compression/common @junr03 @rojkov /*/extensions/compression/gzip @junr03 @rojkov /*/extensions/filters/http/decompressor @rojkov @dio +# Watchdog Extensions +/*/extensions/watchdog/profile_action @kbaichoo @htuch # Core upstream code extensions/upstreams/http @alyssawilk @snowp @mattklein123 extensions/upstreams/http/http @alyssawilk @snowp @mattklein123 diff --git a/api/BUILD b/api/BUILD index 42825f55c569..4aad4899a847 100644 --- a/api/BUILD +++ b/api/BUILD @@ -245,6 +245,7 @@ proto_library( "//envoy/extensions/upstreams/http/http/v3:pkg", "//envoy/extensions/upstreams/http/tcp/v3:pkg", "//envoy/extensions/wasm/v3:pkg", + "//envoy/extensions/watchdog/profile_action/v3alpha:pkg", "//envoy/service/accesslog/v3:pkg", "//envoy/service/auth/v3:pkg", "//envoy/service/cluster/v3:pkg", diff --git a/api/envoy/extensions/watchdog/profile_action/v3alpha/BUILD b/api/envoy/extensions/watchdog/profile_action/v3alpha/BUILD new file mode 100644 index 000000000000..ee92fb652582 --- /dev/null +++ b/api/envoy/extensions/watchdog/profile_action/v3alpha/BUILD @@ -0,0 +1,9 @@ +# DO NOT EDIT. This file is generated by tools/proto_format/proto_sync.py. + +load("@envoy_api//bazel:api_build_system.bzl", "api_proto_package") + +licenses(["notice"]) # Apache 2 + +api_proto_package( + deps = ["@com_github_cncf_udpa//udpa/annotations:pkg"], +) diff --git a/api/envoy/extensions/watchdog/profile_action/v3alpha/profile_action.proto b/api/envoy/extensions/watchdog/profile_action/v3alpha/profile_action.proto new file mode 100644 index 000000000000..2bb03f603132 --- /dev/null +++ b/api/envoy/extensions/watchdog/profile_action/v3alpha/profile_action.proto @@ -0,0 +1,35 @@ +syntax = "proto3"; + +package envoy.extensions.watchdog.profile_action.v3alpha; + +import "google/protobuf/duration.proto"; + +import "udpa/annotations/status.proto"; +import "udpa/annotations/versioning.proto"; +import "validate/validate.proto"; + +option java_package = "io.envoyproxy.envoy.extensions.watchdog.profile_action.v3alpha"; +option java_outer_classname = "ProfileActionProto"; +option java_multiple_files = true; +option (udpa.annotations.file_status).work_in_progress = true; +option (udpa.annotations.file_status).package_version_status = ACTIVE; + +// [#protodoc-title: Watchdog Action that does CPU profiling.] +// [#extension: envoy.watchdog.profile_action] + +// Configuration for the profile watchdog action. +message ProfileActionConfig { + // How long the profile should last. If not set defaults to 5 seconds. + google.protobuf.Duration profile_duration = 1; + + // 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 + // 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. + // If not set (i.e. it's 0), a default of 10 will be used. + uint64 max_profiles_per_thread = 3; +} diff --git a/api/versioning/BUILD b/api/versioning/BUILD index a9f80a557a70..c93b1f7d84c5 100644 --- a/api/versioning/BUILD +++ b/api/versioning/BUILD @@ -128,6 +128,7 @@ proto_library( "//envoy/extensions/upstreams/http/http/v3:pkg", "//envoy/extensions/upstreams/http/tcp/v3:pkg", "//envoy/extensions/wasm/v3:pkg", + "//envoy/extensions/watchdog/profile_action/v3alpha:pkg", "//envoy/service/accesslog/v3:pkg", "//envoy/service/auth/v3:pkg", "//envoy/service/cluster/v3:pkg", diff --git a/docs/root/api-v3/config/config.rst b/docs/root/api-v3/config/config.rst index 536bd9468979..3d87b90373eb 100644 --- a/docs/root/api-v3/config/config.rst +++ b/docs/root/api-v3/config/config.rst @@ -21,3 +21,4 @@ Extensions endpoint/endpoint upstream/upstream wasm/wasm + watchdog/watchdog diff --git a/docs/root/api-v3/config/watchdog/watchdog.rst b/docs/root/api-v3/config/watchdog/watchdog.rst new file mode 100644 index 000000000000..60f284384d59 --- /dev/null +++ b/docs/root/api-v3/config/watchdog/watchdog.rst @@ -0,0 +1,8 @@ +Watchdog +======== + +.. toctree:: + :glob: + :maxdepth: 2 + + ../../extensions/watchdog/profile_action/v3alpha/* diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index 0b0d4021e7d1..4e6e20aa03e0 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -100,6 +100,7 @@ New Features * udp_proxy: added :ref:`use_original_src_ip ` option to replicate the downstream remote address of the packets on the upstream side of Envoy. It is similar to :ref:`original source filter `. * watchdog: support randomizing the watchdog's kill timeout to prevent synchronized kills via a maximium jitter parameter :ref:`max_kill_timeout_jitter`. * watchdog: supports an extension point where actions can be registered to fire on watchdog events such as miss, megamiss, kill and multikill. See ref:`watchdog actions`. +* watchdog: watchdog action extension that does cpu profiling. See ref:`Profile Action `. * xds: added :ref:`extension config discovery` support for HTTP filters. * zlib: added option to use `zlib-ng `_ as zlib library. diff --git a/generated_api_shadow/envoy/extensions/watchdog/profile_action/v3alpha/BUILD b/generated_api_shadow/envoy/extensions/watchdog/profile_action/v3alpha/BUILD new file mode 100644 index 000000000000..ee92fb652582 --- /dev/null +++ b/generated_api_shadow/envoy/extensions/watchdog/profile_action/v3alpha/BUILD @@ -0,0 +1,9 @@ +# DO NOT EDIT. This file is generated by tools/proto_format/proto_sync.py. + +load("@envoy_api//bazel:api_build_system.bzl", "api_proto_package") + +licenses(["notice"]) # Apache 2 + +api_proto_package( + deps = ["@com_github_cncf_udpa//udpa/annotations:pkg"], +) diff --git a/generated_api_shadow/envoy/extensions/watchdog/profile_action/v3alpha/profile_action.proto b/generated_api_shadow/envoy/extensions/watchdog/profile_action/v3alpha/profile_action.proto new file mode 100644 index 000000000000..2bb03f603132 --- /dev/null +++ b/generated_api_shadow/envoy/extensions/watchdog/profile_action/v3alpha/profile_action.proto @@ -0,0 +1,35 @@ +syntax = "proto3"; + +package envoy.extensions.watchdog.profile_action.v3alpha; + +import "google/protobuf/duration.proto"; + +import "udpa/annotations/status.proto"; +import "udpa/annotations/versioning.proto"; +import "validate/validate.proto"; + +option java_package = "io.envoyproxy.envoy.extensions.watchdog.profile_action.v3alpha"; +option java_outer_classname = "ProfileActionProto"; +option java_multiple_files = true; +option (udpa.annotations.file_status).work_in_progress = true; +option (udpa.annotations.file_status).package_version_status = ACTIVE; + +// [#protodoc-title: Watchdog Action that does CPU profiling.] +// [#extension: envoy.watchdog.profile_action] + +// Configuration for the profile watchdog action. +message ProfileActionConfig { + // How long the profile should last. If not set defaults to 5 seconds. + google.protobuf.Duration profile_duration = 1; + + // 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 + // 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. + // If not set (i.e. it's 0), a default of 10 will be used. + uint64 max_profiles_per_thread = 3; +} diff --git a/include/envoy/server/guarddog_config.h b/include/envoy/server/guarddog_config.h index 3f775d737379..4c51778a7eb3 100644 --- a/include/envoy/server/guarddog_config.h +++ b/include/envoy/server/guarddog_config.h @@ -32,7 +32,7 @@ class GuardDogAction { * @param now the current time. */ virtual void run(envoy::config::bootstrap::v3::Watchdog::WatchdogAction::WatchdogEvent event, - std::vector> thread_ltt_pairs, + const std::vector>& thread_ltt_pairs, MonotonicTime now) PURE; }; diff --git a/source/extensions/extensions_build_config.bzl b/source/extensions/extensions_build_config.bzl index bce909221fa5..6b4929e0dd59 100644 --- a/source/extensions/extensions_build_config.bzl +++ b/source/extensions/extensions_build_config.bzl @@ -196,10 +196,14 @@ EXTENSIONS = { # # Http Upstreams (excepting envoy.upstreams.http.generic which is hard-coded into the build so not registered here) # - "envoy.upstreams.http.http": "//source/extensions/upstreams/http/http:config", "envoy.upstreams.http.tcp": "//source/extensions/upstreams/http/tcp:config", + # + # Watchdog actions + # + "envoy.watchdog.profile_action": "//source/extensions/watchdog/profile_action:config", + } # These can be changed to ["//visibility:public"], for downstream builds which diff --git a/source/extensions/watchdog/profile_action/BUILD b/source/extensions/watchdog/profile_action/BUILD new file mode 100644 index 000000000000..0a028627ccd4 --- /dev/null +++ b/source/extensions/watchdog/profile_action/BUILD @@ -0,0 +1,46 @@ +load( + "//bazel:envoy_build_system.bzl", + "envoy_cc_extension", + "envoy_cc_library", + "envoy_extension_package", +) + +licenses(["notice"]) # Apache 2 + +envoy_extension_package() + +envoy_cc_library( + name = "profile_action_lib", + srcs = ["profile_action.cc"], + hdrs = ["profile_action.h"], + external_deps = [ + "abseil_optional", + ], + deps = [ + "//include/envoy/api:api_interface", + "//include/envoy/common:time_interface", + "//include/envoy/event:timer_interface", + "//include/envoy/server:guarddog_config_interface", + "//include/envoy/thread:thread_interface", + "//source/common/profiler:profiler_lib", + "//source/common/protobuf:utility_lib", + "@envoy_api//envoy/extensions/watchdog/profile_action/v3alpha:pkg_cc_proto", + ], +) + +envoy_cc_extension( + name = "config", + srcs = ["config.cc"], + hdrs = ["config.h"], + security_posture = "robust_to_untrusted_downstream_and_upstream", + status = "alpha", + deps = [ + ":profile_action_lib", + "//include/envoy/registry", + "//source/common/common:assert_lib", + "//source/common/config:utility_lib", + "//source/common/protobuf", + "//source/common/protobuf:message_validator_lib", + "@envoy_api//envoy/extensions/watchdog/profile_action/v3alpha:pkg_cc_proto", + ], +) diff --git a/source/extensions/watchdog/profile_action/config.cc b/source/extensions/watchdog/profile_action/config.cc new file mode 100644 index 000000000000..035fa17641e0 --- /dev/null +++ b/source/extensions/watchdog/profile_action/config.cc @@ -0,0 +1,32 @@ +#include "extensions/watchdog/profile_action/config.h" + +#include "envoy/registry/registry.h" + +#include "common/config/utility.h" +#include "common/protobuf/message_validator_impl.h" + +#include "extensions/watchdog/profile_action/profile_action.h" + +namespace Envoy { +namespace Extensions { +namespace Watchdog { +namespace ProfileAction { + +Server::Configuration::GuardDogActionPtr ProfileActionFactory::createGuardDogActionFromProto( + const envoy::config::bootstrap::v3::Watchdog::WatchdogAction& config, + Server::Configuration::GuardDogActionFactoryContext& context) { + auto message = createEmptyConfigProto(); + Config::Utility::translateOpaqueConfig(config.config().typed_config(), ProtobufWkt::Struct(), + ProtobufMessage::getStrictValidationVisitor(), *message); + return std::make_unique(dynamic_cast(*message), context); +} + +/** + * Static registration for the ProfileAction factory. @see RegistryFactory. + */ +REGISTER_FACTORY(ProfileActionFactory, Server::Configuration::GuardDogActionFactory); + +} // namespace ProfileAction +} // namespace Watchdog +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/watchdog/profile_action/config.h b/source/extensions/watchdog/profile_action/config.h new file mode 100644 index 000000000000..39ad21a4ff2c --- /dev/null +++ b/source/extensions/watchdog/profile_action/config.h @@ -0,0 +1,37 @@ +#pragma once + +#include "envoy/extensions/watchdog/profile_action/v3alpha/profile_action.pb.h" +#include "envoy/server/guarddog_config.h" + +#include "common/protobuf/protobuf.h" + +namespace Envoy { +namespace Extensions { +namespace Watchdog { +namespace ProfileAction { + +class ProfileActionFactory : public Server::Configuration::GuardDogActionFactory { +public: + ProfileActionFactory() : name_("envoy.watchdog.profile_action"){}; + + Server::Configuration::GuardDogActionPtr createGuardDogActionFromProto( + const envoy::config::bootstrap::v3::Watchdog::WatchdogAction& config, + Server::Configuration::GuardDogActionFactoryContext& context) override; + + ProtobufTypes::MessagePtr createEmptyConfigProto() override { + return std::make_unique(); + } + + std::string name() const override { return name_; } + +private: + using ProfileActionConfig = + envoy::extensions::watchdog::profile_action::v3alpha::ProfileActionConfig; + + const std::string name_; +}; + +} // namespace ProfileAction +} // namespace Watchdog +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/watchdog/profile_action/profile_action.cc b/source/extensions/watchdog/profile_action/profile_action.cc new file mode 100644 index 000000000000..6d63c3cf23f9 --- /dev/null +++ b/source/extensions/watchdog/profile_action/profile_action.cc @@ -0,0 +1,109 @@ +#include "extensions/watchdog/profile_action/profile_action.h" + +#include + +#include "envoy/thread/thread.h" + +#include "common/profiler/profiler.h" +#include "common/protobuf/utility.h" + +#include "absl/strings/str_format.h" + +namespace Envoy { +namespace Extensions { +namespace Watchdog { +namespace ProfileAction { +namespace { +static constexpr uint64_t DefaultMaxProfilePerTid = 10; + +std::string generateProfileFilePath(const std::string& directory, const SystemTime& now) { + auto timestamp = std::chrono::duration_cast(now.time_since_epoch()).count(); + if (absl::EndsWith(directory, "/")) { + return absl::StrFormat("%s%s.%d", directory, "ProfileAction", timestamp); + } + return absl::StrFormat("%s/%s.%d", directory, "ProfileAction", timestamp); +} +} // namespace + +ProfileAction::ProfileAction( + envoy::extensions::watchdog::profile_action::v3alpha::ProfileActionConfig& config, + Server::Configuration::GuardDogActionFactoryContext& context) + : 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()), + running_profile_(false), profiles_started_(0), context_(context), + timer_cb_(context_.dispatcher_.createTimer([this] { + if (Profiler::Cpu::profilerEnabled()) { + Profiler::Cpu::stopProfiler(); + running_profile_ = false; + } else { + ENVOY_LOG_MISC(error, + "Profile Action's stop() was scheduled, but profiler isn't running!"); + } + + if (!context_.api_.fileSystem().fileExists(profile_filename_)) { + ENVOY_LOG_MISC(error, "Profile file {} wasn't created!", profile_filename_); + } + })) {} + +void ProfileAction::run( + envoy::config::bootstrap::v3::Watchdog::WatchdogAction::WatchdogEvent /*event*/, + const std::vector>& thread_ltt_pairs, + MonotonicTime /*now*/) { + if (running_profile_) { + return; + } + + // 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"); + return; + } + + auto& fs = context_.api_.fileSystem(); + if (!fs.directoryExists(path_)) { + ENVOY_LOG_MISC(error, "Profile Action: Directory path {} doesn't exist.", path_); + return; + } + + // Generate file path for output and try to profile + profile_filename_ = generateProfileFilePath(path_, context_.api_.timeSource().systemTime()); + + if (!Profiler::Cpu::profilerEnabled()) { + if (Profiler::Cpu::startProfiler(profile_filename_)) { + // Update state + running_profile_ = true; + ++profiles_started_; + tid_to_profile_count_[*trigger_tid] += 1; + + // Schedule callback to stop + timer_cb_->enableTimer(duration_); + } else { + ENVOY_LOG_MISC(error, "Profile Action failed to start the profiler."); + } + } else { + ENVOY_LOG_MISC(error, "Profile Action unable to start the profiler as it is in use elsewhere."); + } +} + +// Helper to determine if we have a valid tid to start profiling. +absl::optional ProfileAction::getTidTriggeringProfile( + const std::vector>& 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 +} // namespace Envoy diff --git a/source/extensions/watchdog/profile_action/profile_action.h b/source/extensions/watchdog/profile_action/profile_action.h new file mode 100644 index 000000000000..e91a2cfd3615 --- /dev/null +++ b/source/extensions/watchdog/profile_action/profile_action.h @@ -0,0 +1,49 @@ +#pragma once + +#include + +#include "envoy/extensions/watchdog/profile_action/v3alpha/profile_action.pb.h" +#include "envoy/server/guarddog_config.h" +#include "envoy/thread/thread.h" + +#include "absl/types/optional.h" + +namespace Envoy { +namespace Extensions { +namespace Watchdog { +namespace ProfileAction { + +/** + * A GuardDogAction that will start CPU profiling. + */ +class ProfileAction : public Server::Configuration::GuardDogAction { +public: + ProfileAction(envoy::extensions::watchdog::profile_action::v3alpha::ProfileActionConfig& config, + Server::Configuration::GuardDogActionFactoryContext& context); + + void run(envoy::config::bootstrap::v3::Watchdog::WatchdogAction::WatchdogEvent event, + const std::vector>& thread_ltt_pairs, + MonotonicTime now) override; + +private: + // Helper to determine if we should run the profiler. + absl::optional getTidTriggeringProfile( + const std::vector>& thread_ltt_pairs); + + const std::string path_; + const std::chrono::milliseconds duration_; + const uint64_t max_profiles_per_tid_; + bool running_profile_; + std::string profile_filename_; + uint64_t profiles_started_; + absl::flat_hash_map tid_to_profile_count_; + Server::Configuration::GuardDogActionFactoryContext& context_; + Event::TimerPtr timer_cb_; +}; + +using ProfileActionPtr = std::unique_ptr; + +} // namespace ProfileAction +} // namespace Watchdog +} // namespace Extensions +} // namespace Envoy diff --git a/test/extensions/watchdog/profile_action/BUILD b/test/extensions/watchdog/profile_action/BUILD new file mode 100644 index 000000000000..7af95504deae --- /dev/null +++ b/test/extensions/watchdog/profile_action/BUILD @@ -0,0 +1,51 @@ +load( + "//bazel:envoy_build_system.bzl", + "envoy_package", +) +load( + "//test/extensions:extensions_build_system.bzl", + "envoy_extension_cc_test", +) + +licenses(["notice"]) # Apache 2 + +envoy_package() + +envoy_extension_cc_test( + name = "profile_action_test", + srcs = ["profile_action_test.cc"], + extension_name = "envoy.watchdog.profile_action", + external_deps = [ + "abseil_synchronization", + ], + deps = [ + "//include/envoy/common:time_interface", + "//include/envoy/registry", + "//include/envoy/server:guarddog_config_interface", + "//source/common/filesystem:directory_lib", + "//source/common/profiler:profiler_lib", + "//source/extensions/watchdog/profile_action:config", + "//source/extensions/watchdog/profile_action:profile_action_lib", + "//test/mocks/event:event_mocks", + "//test/test_common:environment_lib", + "//test/test_common:simulated_time_system_lib", + "//test/test_common:utility_lib", + "@envoy_api//envoy/config/bootstrap/v3:pkg_cc_proto", + "@envoy_api//envoy/extensions/watchdog/profile_action/v3alpha:pkg_cc_proto", + ], +) + +envoy_extension_cc_test( + name = "config_test", + srcs = ["config_test.cc"], + extension_name = "envoy.watchdog.profile_action", + deps = [ + "//include/envoy/registry", + "//include/envoy/server:guarddog_config_interface", + "//source/extensions/watchdog/profile_action:config", + "//source/extensions/watchdog/profile_action:profile_action_lib", + "//test/mocks/event:event_mocks", + "//test/test_common:utility_lib", + "@envoy_api//envoy/extensions/watchdog/profile_action/v3alpha:pkg_cc_proto", + ], +) diff --git a/test/extensions/watchdog/profile_action/config_test.cc b/test/extensions/watchdog/profile_action/config_test.cc new file mode 100644 index 000000000000..ba29113fdc1b --- /dev/null +++ b/test/extensions/watchdog/profile_action/config_test.cc @@ -0,0 +1,57 @@ +#include "envoy/extensions/watchdog/profile_action/v3alpha/profile_action.pb.h" +#include "envoy/registry/registry.h" +#include "envoy/server/guarddog_config.h" + +#include "extensions/watchdog/profile_action/config.h" + +#include "test/mocks/event/mocks.h" +#include "test/test_common/utility.h" + +#include "gtest/gtest.h" + +namespace Envoy { +namespace Extensions { +namespace Watchdog { +namespace ProfileAction { +namespace { + +TEST(ProfileActionFactoryTest, CanCreateAction) { + auto factory = + Registry::FactoryRegistry::getFactory( + "envoy.watchdog.profile_action"); + ASSERT_NE(factory, nullptr); + + // Create config and mock context + envoy::config::bootstrap::v3::Watchdog::WatchdogAction config; + TestUtility::loadFromJson( + R"EOF( + { + "config": { + "name": "envoy.watchdog.profile_action", + "typed_config": { + "@type": "type.googleapis.com/udpa.type.v1.TypedStruct", + "type_url": "type.googleapis.com/envoy.extensions.watchdog.profile_action.v3alpha.ProfileActionConfig", + "value": { + "profile_duration": "2s", + "profile_path": "/tmp/envoy/", + "max_profiles_per_thread": "20" + } + } + }, + } + )EOF", + config); + + Event::MockDispatcher dispatcher; + Api::ApiPtr api = Api::createApiForTest(); + Server::Configuration::GuardDogActionFactoryContext context{*api, dispatcher}; + + EXPECT_CALL(dispatcher, createTimer_(_)); + EXPECT_NE(factory->createGuardDogActionFromProto(config, context), nullptr); +} + +} // namespace +} // namespace ProfileAction +} // namespace Watchdog +} // namespace Extensions +} // namespace Envoy diff --git a/test/extensions/watchdog/profile_action/profile_action_test.cc b/test/extensions/watchdog/profile_action/profile_action_test.cc new file mode 100644 index 000000000000..2dade866a329 --- /dev/null +++ b/test/extensions/watchdog/profile_action/profile_action_test.cc @@ -0,0 +1,359 @@ +#include + +#include "envoy/common/time.h" +#include "envoy/config/bootstrap/v3/bootstrap.pb.h" +#include "envoy/event/dispatcher.h" +#include "envoy/extensions/watchdog/profile_action/v3alpha/profile_action.pb.h" +#include "envoy/filesystem/filesystem.h" +#include "envoy/server/guarddog_config.h" +#include "envoy/thread/thread.h" + +#include "common/common/assert.h" +#include "common/filesystem/directory.h" +#include "common/profiler/profiler.h" + +#include "extensions/watchdog/profile_action/config.h" +#include "extensions/watchdog/profile_action/profile_action.h" + +#include "test/mocks/event/mocks.h" +#include "test/test_common/environment.h" +#include "test/test_common/simulated_time_system.h" +#include "test/test_common/test_time.h" +#include "test/test_common/utility.h" + +#include "absl/strings/substitute.h" +#include "absl/synchronization/mutex.h" +#include "gtest/gtest.h" + +namespace Envoy { +namespace Extensions { +namespace Watchdog { +namespace ProfileAction { +namespace { + +class ProfileActionTest : public testing::Test { +protected: + ProfileActionTest() + : time_system_(std::make_unique()), + api_(Api::createApiForTest(*time_system_)), dispatcher_(api_->allocateDispatcher("test")), + context_({*api_, *dispatcher_}), test_path_(generateTestPath()) {} + + // Generates a unique path for a testcase. + static std::string generateTestPath() { + const ::testing::TestInfo* const test_info = + ::testing::UnitTest::GetInstance()->current_test_info(); + + std::string test_path = TestEnvironment::temporaryPath( + absl::StrJoin({test_info->test_suite_name(), test_info->name()}, "/")); + TestEnvironment::createPath(test_path); + + return test_path; + } + + // Counts the number of non-empty profiles found within a directory. + int countNumberOfProfileInPath(const std::string& path) { + int nonempty_profiles_found = 0; + Filesystem::Directory directory(path); + + for (const Filesystem::DirectoryEntry& entry : directory) { + const std::string full_path = path + "/" + entry.name_; + + // Count if its a non-empty file with the prefix of profiles. + if (entry.type_ == Filesystem::FileType::Regular && + absl::StartsWith(entry.name_, "ProfileAction") && + api_->fileSystem().fileSize(full_path) > 0) { + nonempty_profiles_found++; + } + } + + return nonempty_profiles_found; + } + + void waitForOutstandingNotify() ABSL_EXCLUSIVE_LOCKS_REQUIRED(mutex_) { + mutex_.Await(absl::Condition( + +[](int* outstanding_notifies) -> bool { return *outstanding_notifies > 0; }, + &outstanding_notifies_)); + outstanding_notifies_ -= 1; + } + + std::unique_ptr time_system_; + Api::ApiPtr api_; + Event::DispatcherPtr dispatcher_; + Server::Configuration::GuardDogActionFactoryContext context_; + std::unique_ptr action_; + // Path for the test case to dump any profiles to. + const std::string test_path_; + // Used to synchronize with the dispatch thread + absl::Mutex mutex_; + int outstanding_notifies_ ABSL_GUARDED_BY(mutex_) = 0; +}; + +TEST_F(ProfileActionTest, CanDoSingleProfile) { + // Create configuration. + envoy::extensions::watchdog::profile_action::v3alpha::ProfileActionConfig config; + config.set_profile_path(test_path_); + config.mutable_profile_duration()->set_seconds(1); + + // Create the ProfileAction before we start running the dispatcher + // otherwise the timer created will in ProfileActions ctor will + // not be thread safe. + action_ = std::make_unique(config, context_); + Thread::ThreadPtr thread = api_->threadFactory().createThread( + [this]() -> void { dispatcher_->run(Event::Dispatcher::RunType::RunUntilExit); }); + + // Create vector of relevant threads + const auto now = api_->timeSource().monotonicTime(); + std::vector> tid_ltt_pairs = { + {Thread::ThreadId(10), now}}; + + // Check that we can do at least a single profile + dispatcher_->post([&tid_ltt_pairs, &now, this]() -> void { + action_->run(envoy::config::bootstrap::v3::Watchdog::WatchdogAction::MISS, tid_ltt_pairs, now); + absl::MutexLock lock(&mutex_); + outstanding_notifies_ += 1; + }); + + absl::MutexLock lock(&mutex_); + waitForOutstandingNotify(); + time_system_->advanceTimeWait(std::chrono::seconds(2)); + + dispatcher_->exit(); + thread->join(); + +#ifdef PROFILER_AVAILABLE + EXPECT_EQ(countNumberOfProfileInPath(test_path_), 1); +#else + // Profiler won't run in this case, so there should be no files generated. + EXPECT_EQ(countNumberOfProfileInPath(test_path_), 0); +#endif +} + +TEST_F(ProfileActionTest, CanDoMultipleProfiles) { + // Create configuration. + envoy::extensions::watchdog::profile_action::v3alpha::ProfileActionConfig config; + config.set_profile_path(test_path_); + config.mutable_profile_duration()->set_seconds(1); + // Create the ProfileAction before we start running the dispatcher + // otherwise the timer created will in ProfileActions ctor will + // not be thread safe. + action_ = std::make_unique(config, context_); + Thread::ThreadPtr thread = api_->threadFactory().createThread( + [this]() -> void { dispatcher_->run(Event::Dispatcher::RunType::RunUntilExit); }); + + // Create vector of relevant threads + const auto now = api_->timeSource().monotonicTime(); + std::vector> tid_ltt_pairs = { + {Thread::ThreadId(10), now}}; + + // Check that we can do at least a single profile + dispatcher_->post([&tid_ltt_pairs, &now, this]() -> void { + action_->run(envoy::config::bootstrap::v3::Watchdog::WatchdogAction::MISS, tid_ltt_pairs, now); + absl::MutexLock lock(&mutex_); + outstanding_notifies_ += 1; + }); + + absl::MutexLock lock(&mutex_); + waitForOutstandingNotify(); + time_system_->advanceTimeWait(std::chrono::seconds(2)); + +#ifdef PROFILER_AVAILABLE + ASSERT_EQ(countNumberOfProfileInPath(test_path_), 1); +#else + // Profiler won't run in this case, so there should be no files generated. + ASSERT_EQ(countNumberOfProfileInPath(test_path_), 0); +#endif + + // Check we can do multiple profiles + dispatcher_->post([&tid_ltt_pairs, &now, this]() -> void { + action_->run(envoy::config::bootstrap::v3::Watchdog::WatchdogAction::MISS, tid_ltt_pairs, now); + absl::MutexLock lock(&mutex_); + outstanding_notifies_ += 1; + }); + + waitForOutstandingNotify(); + time_system_->advanceTimeWait(std::chrono::seconds(2)); + + dispatcher_->exit(); + thread->join(); + +#ifdef PROFILER_AVAILABLE + EXPECT_EQ(countNumberOfProfileInPath(test_path_), 2); +#else + // Profiler won't run in this case, so there should be no files generated. + EXPECT_EQ(countNumberOfProfileInPath(test_path_), 0); +#endif +} + +TEST_F(ProfileActionTest, CannotTriggerConcurrentProfiles) { + // Create configuration. + envoy::extensions::watchdog::profile_action::v3alpha::ProfileActionConfig config; + TestUtility::loadFromJson(absl::Substitute(R"EOF({ "profile_path": "$0", })EOF", test_path_), + config); + // Create the ProfileAction before we start running the dispatcher + // otherwise the timer created will in ProfileActions ctor will + // not be thread safe. + action_ = std::make_unique(config, context_); + Thread::ThreadPtr thread = api_->threadFactory().createThread( + [this]() -> void { dispatcher_->run(Event::Dispatcher::RunType::RunUntilExit); }); + + // Create vector of relevant threads + const auto now = api_->timeSource().monotonicTime(); + std::vector> tid_ltt_pairs = { + {Thread::ThreadId(10), now}}; + + dispatcher_->post([&, this]() -> void { + action_->run(envoy::config::bootstrap::v3::Watchdog::WatchdogAction::MISS, tid_ltt_pairs, now); + + // This subsequent call should fail since the one prior starts a profile. + action_->run(envoy::config::bootstrap::v3::Watchdog::WatchdogAction::MISS, tid_ltt_pairs, now); + + absl::MutexLock lock(&mutex_); + outstanding_notifies_ += 1; + }); + + absl::MutexLock lock(&mutex_); + waitForOutstandingNotify(); + time_system_->advanceTimeWait(std::chrono::seconds(6)); + + dispatcher_->exit(); + thread->join(); +#ifdef PROFILER_AVAILABLE + EXPECT_EQ(countNumberOfProfileInPath(test_path_), 1); +#else + // Profiler won't run in this case, so there should be no files generated. + EXPECT_EQ(countNumberOfProfileInPath(test_path_), 0); +#endif +} + +TEST_F(ProfileActionTest, ShouldNotProfileIfDirectoryDoesNotExist) { + // Create configuration. + envoy::extensions::watchdog::profile_action::v3alpha::ProfileActionConfig config; + const std::string nonexistant_path = test_path_ + "/nonexistant_dir/"; + TestUtility::loadFromJson( + absl::Substitute(R"EOF({ "profile_path": "$0", })EOF", nonexistant_path), config); + // Create the ProfileAction before we start running the dispatcher + // otherwise the timer created will in ProfileActions ctor will + // not be thread safe. + action_ = std::make_unique(config, context_); + Thread::ThreadPtr thread = api_->threadFactory().createThread( + [this]() -> void { dispatcher_->run(Event::Dispatcher::RunType::RunUntilExit); }); + + // Create vector of relevant threads + const auto now = api_->timeSource().monotonicTime(); + std::vector> tid_ltt_pairs = { + {Thread::ThreadId(10), now}}; + + dispatcher_->post([&, this]() -> void { + action_->run(envoy::config::bootstrap::v3::Watchdog::WatchdogAction::MISS, tid_ltt_pairs, now); + absl::MutexLock lock(&mutex_); + outstanding_notifies_ += 1; + }); + + absl::MutexLock lock(&mutex_); + waitForOutstandingNotify(); + time_system_->advanceTimeWait(std::chrono::seconds(6)); + + dispatcher_->exit(); + thread->join(); + + EXPECT_EQ(countNumberOfProfileInPath(test_path_), 0); + EXPECT_FALSE(api_->fileSystem().directoryExists(nonexistant_path)); +} + +TEST_F(ProfileActionTest, ShouldNotProfileIfNoTids) { + // Create configuration. + envoy::extensions::watchdog::profile_action::v3alpha::ProfileActionConfig config; + TestUtility::loadFromJson(absl::Substitute(R"EOF({ "profile_path": "$0"})EOF", test_path_), + config); + // Create the ProfileAction before we start running the dispatcher + // otherwise the timer created will in ProfileActions ctor will + // not be thread safe. + action_ = std::make_unique(config, context_); + Thread::ThreadPtr thread = api_->threadFactory().createThread( + [this]() -> void { dispatcher_->run(Event::Dispatcher::RunType::RunUntilExit); }); + + // Test that no profiles are created given empty vector of valid TIDs + dispatcher_->post([this]() -> void { + std::vector> tid_ltt_pairs; + action_->run(envoy::config::bootstrap::v3::Watchdog::WatchdogAction::MISS, tid_ltt_pairs, + api_->timeSource().monotonicTime()); + absl::MutexLock lock(&mutex_); + outstanding_notifies_ += 1; + }); + + absl::MutexLock lock(&mutex_); + waitForOutstandingNotify(); + time_system_->advanceTimeWait(std::chrono::seconds(2)); + + dispatcher_->exit(); + thread->join(); + + // No profiles should have been created + EXPECT_EQ(countNumberOfProfileInPath(test_path_), 0); +} + +TEST_F(ProfileActionTest, ShouldSaturateTids) { + // 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); + + // Create the ProfileAction before we start running the dispatcher + // otherwise the timer created will in ProfileActions ctor will + // not be thread safe. + action_ = std::make_unique(config, context_); + Thread::ThreadPtr thread = api_->threadFactory().createThread( + [this]() -> void { dispatcher_->run(Event::Dispatcher::RunType::RunUntilExit); }); + + // Create vector of relevant threads + const auto now = api_->timeSource().monotonicTime(); + std::vector> tid_ltt_pairs = { + {Thread::ThreadId(10), now}}; + + dispatcher_->post([&, this]() -> void { + action_->run(envoy::config::bootstrap::v3::Watchdog::WatchdogAction::MISS, tid_ltt_pairs, now); + absl::MutexLock lock(&mutex_); + outstanding_notifies_ += 1; + }); + + absl::MutexLock lock(&mutex_); + waitForOutstandingNotify(); + time_system_->advanceTimeWait(std::chrono::seconds(2)); + + // check that the profile is created! +#ifdef PROFILER_AVAILABLE + EXPECT_EQ(countNumberOfProfileInPath(test_path_), 1); +#else + // Profiler won't run in this case, so there should be no files generated. + EXPECT_EQ(countNumberOfProfileInPath(test_path_), 0); +#endif + + // Do another run of the watchdog action. It shouldn't have run again. + dispatcher_->post([&, this]() -> void { + action_->run(envoy::config::bootstrap::v3::Watchdog::WatchdogAction::MISS, tid_ltt_pairs, now); + absl::MutexLock lock(&mutex_); + outstanding_notifies_ += 1; + }); + + waitForOutstandingNotify(); + + // If the callback had scheduled (it shouldn't as we've saturated the profile + // count) advancing time to make it run. + time_system_->advanceTimeWait(std::chrono::seconds(2)); + dispatcher_->exit(); + thread->join(); + +#ifdef PROFILER_AVAILABLE + EXPECT_EQ(countNumberOfProfileInPath(test_path_), 1); +#else + // Profiler won't run in this case, so there should be no files generated. + EXPECT_EQ(countNumberOfProfileInPath(test_path_), 0); +#endif +} + +} // namespace +} // namespace ProfileAction +} // namespace Watchdog +} // namespace Extensions +} // namespace Envoy diff --git a/test/server/guarddog_impl_test.cc b/test/server/guarddog_impl_test.cc index c3a4c91aeabb..49fe6f73f3f3 100644 --- a/test/server/guarddog_impl_test.cc +++ b/test/server/guarddog_impl_test.cc @@ -444,7 +444,7 @@ class RecordGuardDogAction : public Configuration::GuardDogAction { RecordGuardDogAction(std::vector& events) : events_(events) {} void run(envoy::config::bootstrap::v3::Watchdog::WatchdogAction::WatchdogEvent event, - std::vector> thread_ltt_pairs, + const std::vector>& thread_ltt_pairs, MonotonicTime /*now*/) override { std::string event_string = envoy::config::bootstrap::v3::Watchdog::WatchdogAction::WatchdogEvent_Name(event); @@ -470,7 +470,7 @@ class AssertGuardDogAction : public Configuration::GuardDogAction { AssertGuardDogAction() = default; void run(envoy::config::bootstrap::v3::Watchdog::WatchdogAction::WatchdogEvent /*event*/, - std::vector> /*thread_ltt_pairs*/, + const std::vector>& /*thread_ltt_pairs*/, MonotonicTime /*now*/) override { RELEASE_ASSERT(false, "ASSERT_GUARDDOG_ACTION"); } diff --git a/tools/spelling/spelling_dictionary.txt b/tools/spelling/spelling_dictionary.txt index 3adbb366388c..4647007122f2 100644 --- a/tools/spelling/spelling_dictionary.txt +++ b/tools/spelling/spelling_dictionary.txt @@ -28,6 +28,7 @@ CDS CEL DSR LTT +TIDs ceil CHACHA CHLO