health check: remove exceptions (#37263)

Risk Level: low Testing: updated tests Docs Changes: n/a Release Notes: n/a envoyproxy/envoy-mobile#176 Signed-off-by: Alyssa Wilk <[email protected]>
envoyproxy · Nov 20, 2024 · 15e6a55 · 15e6a55
1 parent 7cf5344
commit 15e6a55
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 31 deletions.
diff --git a/source/common/upstream/health_checker_event_logger.h b/source/common/upstream/health_checker_event_logger.h
@@ -25,22 +25,19 @@ namespace Upstream {
 
 class HealthCheckEventLoggerImpl : public HealthCheckEventLogger {
 public:
-  HealthCheckEventLoggerImpl(const envoy::config::core::v3::HealthCheck& health_check_config,
-                             Server::Configuration::HealthCheckerFactoryContext& context)
-      : time_source_(context.serverFactoryContext().mainThreadDispatcher().timeSource()) {
-    // TODO(botengyao): Remove the file_ creation here into the file based health check
-    // event sink. In this way you can remove the file_ based code from the createHealthCheckEvent
+  static absl::StatusOr<std::unique_ptr<HealthCheckEventLoggerImpl>>
+  create(const envoy::config::core::v3::HealthCheck& health_check_config,
+         Server::Configuration::HealthCheckerFactoryContext& context) {
+    AccessLog::AccessLogFileSharedPtr file;
     if (!health_check_config.event_log_path().empty() /* deprecated */) {
       auto file_or_error = context.serverFactoryContext().accessLogManager().createAccessLog(
           Filesystem::FilePathAndType{Filesystem::DestinationType::File,
                                       health_check_config.event_log_path()});
-      THROW_IF_NOT_OK_REF(file_or_error.status());
-      file_ = file_or_error.value();
-    }
-    for (const auto& config : health_check_config.event_logger()) {
-      auto& factory = Config::Utility::getAndCheckFactory<HealthCheckEventSinkFactory>(config);
-      event_sinks_.push_back(factory.createHealthCheckEventSink(config.typed_config(), context));
+      RETURN_IF_NOT_OK_REF(file_or_error.status());
+      file = file_or_error.value();
     }
+    return std::unique_ptr<HealthCheckEventLoggerImpl>(
+        new HealthCheckEventLoggerImpl(health_check_config, std::move(file), context));
   }
 
   void logEjectUnhealthy(envoy::data::core::v3::HealthCheckerType health_checker_type,
@@ -59,6 +56,18 @@ class HealthCheckEventLoggerImpl : public HealthCheckEventLogger {
   void logNoLongerDegraded(envoy::data::core::v3::HealthCheckerType health_checker_type,
                            const HostDescriptionConstSharedPtr& host) override;
 
+protected:
+  HealthCheckEventLoggerImpl(const envoy::config::core::v3::HealthCheck& health_check_config,
+                             AccessLog::AccessLogFileSharedPtr&& file,
+                             Server::Configuration::HealthCheckerFactoryContext& context)
+      : time_source_(context.serverFactoryContext().mainThreadDispatcher().timeSource()),
+        file_(std::move(file)) {
+    for (const auto& config : health_check_config.event_logger()) {
+      auto& factory = Config::Utility::getAndCheckFactory<HealthCheckEventSinkFactory>(config);
+      event_sinks_.push_back(factory.createHealthCheckEventSink(config.typed_config(), context));
+    }
+  }
+
 private:
   void createHealthCheckEvent(
       envoy::data::core::v3::HealthCheckerType health_checker_type, const HostDescription& host,

diff --git a/source/common/upstream/health_checker_impl.cc b/source/common/upstream/health_checker_impl.cc
@@ -84,9 +84,9 @@ HealthCheckerFactory::create(const envoy::config::core::v3::HealthCheck& health_
 
   if (!health_check_config.event_log_path().empty() /* deprecated */ ||
       !health_check_config.event_logger().empty()) {
-    HealthCheckEventLoggerPtr event_logger;
-    event_logger = std::make_unique<HealthCheckEventLoggerImpl>(health_check_config, *context);
-    context->setEventLogger(std::move(event_logger));
+    auto logger_or_error = HealthCheckEventLoggerImpl::create(health_check_config, *context);
+    RETURN_IF_NOT_OK_REF(logger_or_error.status());
+    context->setEventLogger(std::move(*logger_or_error));
   }
   return factory->createCustomHealthChecker(health_check_config, *context);
 }

diff --git a/test/common/upstream/health_checker_impl_test.cc b/test/common/upstream/health_checker_impl_test.cc
@@ -6544,7 +6544,8 @@ TEST(HealthCheckEventLoggerImplTest, All) {
   // This is rendered as "2009-02-13T23:31:31.234Z".a
   time_system.setSystemTime(std::chrono::milliseconds(1234567891234));
 
-  HealthCheckEventLoggerImpl event_logger(health_check_config, context);
+  std::unique_ptr<HealthCheckEventLoggerImpl> event_logger =
+      *HealthCheckEventLoggerImpl::create(health_check_config, context);
 
   EXPECT_CALL(*file, write(absl::string_view{
                          "{\"health_checker_type\":\"HTTP\",\"host\":{\"socket_address\":{"
@@ -6555,7 +6556,7 @@ TEST(HealthCheckEventLoggerImplTest, All) {
                          "\"timestamp\":\"2009-02-13T23:31:31.234Z\","
                          "\"metadata\":{\"filter_metadata\":{},\"typed_filter_metadata\":{}},"
                          "\"locality\":{\"region\":\"\",\"zone\":\"\",\"sub_zone\":\"\"}}\n"}));
-  event_logger.logEjectUnhealthy(envoy::data::core::v3::HTTP, host, envoy::data::core::v3::ACTIVE);
+  event_logger->logEjectUnhealthy(envoy::data::core::v3::HTTP, host, envoy::data::core::v3::ACTIVE);
 
   EXPECT_CALL(*file, write(absl::string_view{
                          "{\"health_checker_type\":\"HTTP\",\"host\":{\"socket_address\":{"
@@ -6567,7 +6568,7 @@ TEST(HealthCheckEventLoggerImplTest, All) {
                          "\"metadata\":"
                          "{\"filter_metadata\":{},\"typed_filter_metadata\":{}},\"locality\":"
                          "{\"region\":\"\",\"zone\":\"\",\"sub_zone\":\"\"}}\n"}));
-  event_logger.logAddHealthy(envoy::data::core::v3::HTTP, host, false);
+  event_logger->logAddHealthy(envoy::data::core::v3::HTTP, host, false);
 
   EXPECT_CALL(*file, write(absl::string_view{
                          "{\"health_checker_type\":\"HTTP\",\"host\":{\"socket_address\":{"
@@ -6580,7 +6581,7 @@ TEST(HealthCheckEventLoggerImplTest, All) {
                          "{\"filter_metadata\":{},\"typed_filter_metadata\":{}},\"locality\":"
                          "{\"region\":\"\",\"zone\":\"\",\"sub_zone\":\"\"},"
                          "\"successful_health_check_event\":{}}\n"}));
-  event_logger.logSuccessfulHealthCheck(envoy::data::core::v3::HTTP, host);
+  event_logger->logSuccessfulHealthCheck(envoy::data::core::v3::HTTP, host);
 
   EXPECT_CALL(*file, write(absl::string_view{
                          "{\"health_checker_type\":\"HTTP\",\"host\":{\"socket_address\":{"
@@ -6592,8 +6593,8 @@ TEST(HealthCheckEventLoggerImplTest, All) {
                          "\"first_check\":false},"
                          "\"metadata\":{\"filter_metadata\":{},\"typed_filter_metadata\":{}},"
                          "\"locality\":{\"region\":\"\",\"zone\":\"\",\"sub_zone\":\"\"}}\n"}));
-  event_logger.logUnhealthy(envoy::data::core::v3::HTTP, host, envoy::data::core::v3::ACTIVE,
-                            false);
+  event_logger->logUnhealthy(envoy::data::core::v3::HTTP, host, envoy::data::core::v3::ACTIVE,
+                             false);
 
   EXPECT_CALL(*file, write(absl::string_view{
                          "{\"health_checker_type\":\"HTTP\",\"host\":{\"socket_address\":{"
@@ -6604,7 +6605,7 @@ TEST(HealthCheckEventLoggerImplTest, All) {
                          "\"degraded_healthy_host\":{},"
                          "\"metadata\":{\"filter_metadata\":{},\"typed_filter_metadata\":{}},"
                          "\"locality\":{\"region\":\"\",\"zone\":\"\",\"sub_zone\":\"\"}}\n"}));
-  event_logger.logDegraded(envoy::data::core::v3::HTTP, host);
+  event_logger->logDegraded(envoy::data::core::v3::HTTP, host);
 
   EXPECT_CALL(*file, write(absl::string_view{
                          "{\"health_checker_type\":\"HTTP\",\"host\":{\"socket_address\":{"
@@ -6615,7 +6616,7 @@ TEST(HealthCheckEventLoggerImplTest, All) {
                          "\"no_longer_degraded_host\":{},"
                          "\"metadata\":{\"filter_metadata\":{},\"typed_filter_metadata\":{}},"
                          "\"locality\":{\"region\":\"\",\"zone\":\"\",\"sub_zone\":\"\"}}\n"}));
-  event_logger.logNoLongerDegraded(envoy::data::core::v3::HTTP, host);
+  event_logger->logNoLongerDegraded(envoy::data::core::v3::HTTP, host);
 }
 
 TEST(HealthCheckEventLoggerImplTest, OneEventLogger) {
@@ -6644,9 +6645,10 @@ TEST(HealthCheckEventLoggerImplTest, OneEventLogger) {
   // This is rendered as "2009-02-13T23:31:31.234Z".a
   time_system.setSystemTime(std::chrono::milliseconds(1234567891234));
 
-  HealthCheckEventLoggerImpl event_logger(health_check_config, context);
+  std::unique_ptr<HealthCheckEventLoggerImpl> event_logger =
+      *HealthCheckEventLoggerImpl::create(health_check_config, context);
 
-  event_logger.logEjectUnhealthy(envoy::data::core::v3::HTTP, host, envoy::data::core::v3::ACTIVE);
+  event_logger->logEjectUnhealthy(envoy::data::core::v3::HTTP, host, envoy::data::core::v3::ACTIVE);
   EXPECT_EQ(
       file_log_data.value(),
       "{\"health_checker_type\":\"HTTP\",\"host\":{\"socket_address\":{"
@@ -6657,7 +6659,7 @@ TEST(HealthCheckEventLoggerImplTest, OneEventLogger) {
       "\"metadata\":{\"filter_metadata\":{},\"typed_filter_metadata\":{}},"
       "\"locality\":{\"region\":\"\",\"zone\":\"\",\"sub_zone\":\"\"}}\n");
 
-  event_logger.logAddHealthy(envoy::data::core::v3::HTTP, host, false);
+  event_logger->logAddHealthy(envoy::data::core::v3::HTTP, host, false);
   EXPECT_EQ(
       file_log_data.value(),
       "{\"health_checker_type\":\"HTTP\",\"host\":{\"socket_address\":{"
@@ -6669,7 +6671,7 @@ TEST(HealthCheckEventLoggerImplTest, OneEventLogger) {
       "{\"filter_metadata\":{},\"typed_filter_metadata\":{}},\"locality\":"
       "{\"region\":\"\",\"zone\":\"\",\"sub_zone\":\"\"}}\n");
 
-  event_logger.logSuccessfulHealthCheck(envoy::data::core::v3::HTTP, host);
+  event_logger->logSuccessfulHealthCheck(envoy::data::core::v3::HTTP, host);
   EXPECT_EQ(
       file_log_data.value(),
       "{\"health_checker_type\":\"HTTP\",\"host\":{\"socket_address\":{"
@@ -6680,8 +6682,8 @@ TEST(HealthCheckEventLoggerImplTest, OneEventLogger) {
       "{\"filter_metadata\":{},\"typed_filter_metadata\":{}},\"locality\":"
       "{\"region\":\"\",\"zone\":\"\",\"sub_zone\":\"\"},\"successful_health_check_event\":{}}\n");
 
-  event_logger.logUnhealthy(envoy::data::core::v3::HTTP, host, envoy::data::core::v3::ACTIVE,
-                            false);
+  event_logger->logUnhealthy(envoy::data::core::v3::HTTP, host, envoy::data::core::v3::ACTIVE,
+                             false);
   EXPECT_EQ(
       file_log_data.value(),
       "{\"health_checker_type\":\"HTTP\",\"host\":{\"socket_address\":{"
@@ -6693,7 +6695,7 @@ TEST(HealthCheckEventLoggerImplTest, OneEventLogger) {
       "\"metadata\":{\"filter_metadata\":{},\"typed_filter_metadata\":{}},"
       "\"locality\":{\"region\":\"\",\"zone\":\"\",\"sub_zone\":\"\"}}\n");
 
-  event_logger.logDegraded(envoy::data::core::v3::HTTP, host);
+  event_logger->logDegraded(envoy::data::core::v3::HTTP, host);
   EXPECT_EQ(
       file_log_data.value(),
       "{\"health_checker_type\":\"HTTP\",\"host\":{\"socket_address\":{"
@@ -6704,7 +6706,7 @@ TEST(HealthCheckEventLoggerImplTest, OneEventLogger) {
       "\"metadata\":{\"filter_metadata\":{},\"typed_filter_metadata\":{}},"
       "\"locality\":{\"region\":\"\",\"zone\":\"\",\"sub_zone\":\"\"}}\n");
 
-  event_logger.logNoLongerDegraded(envoy::data::core::v3::HTTP, host);
+  event_logger->logNoLongerDegraded(envoy::data::core::v3::HTTP, host);
   EXPECT_EQ(
       file_log_data.value(),
       "{\"health_checker_type\":\"HTTP\",\"host\":{\"socket_address\":{"

diff --git a/tools/code_format/config.yaml b/tools/code_format/config.yaml
@@ -142,7 +142,6 @@ paths:
     - source/common/router/router.cc
     - source/common/config/config_provider_impl.h
     - source/common/common/logger_delegates.cc
-    - source/common/upstream/health_checker_event_logger.h
     - source/common/upstream/outlier_detection_impl.h
     - source/common/grpc/async_client_impl.cc
     - source/common/grpc/google_grpc_creds_impl.cc