outlier: removing exceptions (#37262)

Risk Level: low Testing: updated tests Docs Changes: n/a Release Notes: n/a envoyproxy/envoy-mobile#176 Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
envoyproxy · Nov 21, 2024 · dd6b7b7 · dd6b7b7
1 parent 2aebf12
commit dd6b7b7
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 16 deletions.
diff --git a/source/common/upstream/cluster_manager_impl.cc b/source/common/upstream/cluster_manager_impl.cc
@@ -345,8 +345,9 @@ ClusterManagerImpl::ClusterManagerImpl(
   if (cm_config.has_outlier_detection()) {
     const std::string event_log_file_path = cm_config.outlier_detection().event_log_path();
     if (!event_log_file_path.empty()) {
-      outlier_event_logger_ = std::make_shared<Outlier::EventLoggerImpl>(
-          log_manager, event_log_file_path, time_source_);
+      outlier_event_logger_ = THROW_OR_RETURN_VALUE(
+          Outlier::EventLoggerImpl::create(log_manager, event_log_file_path, time_source_),
+          std::unique_ptr<Outlier::EventLoggerImpl>);
     }
   }
 

diff --git a/source/common/upstream/outlier_detection_impl.h b/source/common/upstream/outlier_detection_impl.h
@@ -493,21 +493,25 @@ class DetectorImpl : public Detector, public std::enable_shared_from_this<Detect
 
 class EventLoggerImpl : public EventLogger {
 public:
-  EventLoggerImpl(AccessLog::AccessLogManager& log_manager, const std::string& file_name,
-                  TimeSource& time_source)
-      : time_source_(time_source) {
+  static absl::StatusOr<std::unique_ptr<EventLoggerImpl>>
+  create(AccessLog::AccessLogManager& log_manager, const std::string& file_name,
+         TimeSource& time_source) {
     auto file_or_error = log_manager.createAccessLog(
         Filesystem::FilePathAndType{Filesystem::DestinationType::File, file_name});
-    THROW_IF_NOT_OK_REF(file_or_error.status());
-    file_ = file_or_error.value();
+    RETURN_IF_NOT_OK_REF(file_or_error.status());
+    return std::unique_ptr<EventLoggerImpl>(
+        new EventLoggerImpl(std::move(*file_or_error), time_source));
   }
-
   // Upstream::Outlier::EventLogger
   void logEject(const HostDescriptionConstSharedPtr& host, Detector& detector,
                 envoy::data::cluster::v3::OutlierEjectionType type, bool enforced) override;
 
   void logUneject(const HostDescriptionConstSharedPtr& host) override;
 
+protected:
+  EventLoggerImpl(AccessLog::AccessLogFileSharedPtr&& file, TimeSource& time_source)
+      : file_(std::move(file)), time_source_(time_source) {}
+
 private:
   void setCommonEventParams(envoy::data::cluster::v3::OutlierDetectionEvent& event,
                             const HostDescriptionConstSharedPtr& host,

diff --git a/test/common/upstream/outlier_detection_impl_test.cc b/test/common/upstream/outlier_detection_impl_test.cc
@@ -2646,7 +2646,8 @@ TEST(OutlierDetectionEventLoggerImplTest, All) {
   EXPECT_CALL(log_manager, createAccessLog(Filesystem::FilePathAndType{
                                Filesystem::DestinationType::File, "foo"}))
       .WillOnce(Return(file));
-  EventLoggerImpl event_logger(log_manager, "foo", time_system);
+  std::unique_ptr<EventLoggerImpl> event_logger =
+      *EventLoggerImpl::create(log_manager, "foo", time_system);
 
   StringViewSaver log1;
   EXPECT_CALL(host->outlier_detector_, lastUnejectionTime()).WillOnce(ReturnRef(monotonic_time));
@@ -2659,7 +2660,7 @@ TEST(OutlierDetectionEventLoggerImplTest, All) {
                   "\"num_ejections\":0,\"enforced\":true,\"eject_consecutive_event\":{}}\n")))
       .WillOnce(SaveArg<0>(&log1));
 
-  event_logger.logEject(host, detector, envoy::data::cluster::v3::CONSECUTIVE_5XX, true);
+  event_logger->logEject(host, detector, envoy::data::cluster::v3::CONSECUTIVE_5XX, true);
   *Json::Factory::loadFromString(log1);
 
   StringViewSaver log2;
@@ -2672,7 +2673,7 @@ TEST(OutlierDetectionEventLoggerImplTest, All) {
                          "\"num_ejections\":0,\"enforced\":false}\n")))
       .WillOnce(SaveArg<0>(&log2));
 
-  event_logger.logUneject(host);
+  event_logger->logUneject(host);
   *Json::Factory::loadFromString(log2);
 
   // now test with time since last action.
@@ -2698,7 +2699,7 @@ TEST(OutlierDetectionEventLoggerImplTest, All) {
                          "\"host_success_rate\":0,\"cluster_average_success_rate\":0,"
                          "\"cluster_success_rate_ejection_threshold\":0}}\n")))
       .WillOnce(SaveArg<0>(&log3));
-  event_logger.logEject(host, detector, envoy::data::cluster::v3::SUCCESS_RATE, false);
+  event_logger->logEject(host, detector, envoy::data::cluster::v3::SUCCESS_RATE, false);
   *Json::Factory::loadFromString(log3);
 
   StringViewSaver log4;
@@ -2710,7 +2711,7 @@ TEST(OutlierDetectionEventLoggerImplTest, All) {
                          "\"upstream_url\":\"10.0.0.1:443\",\"action\":\"UNEJECT\","
                          "\"num_ejections\":0,\"enforced\":false}\n")))
       .WillOnce(SaveArg<0>(&log4));
-  event_logger.logUneject(host);
+  event_logger->logUneject(host);
   *Json::Factory::loadFromString(log4);
 
   StringViewSaver log5;
@@ -2727,7 +2728,7 @@ TEST(OutlierDetectionEventLoggerImplTest, All) {
                   "\"num_ejections\":0,\"enforced\":false,\"eject_failure_percentage_event\":{"
                   "\"host_success_rate\":0}}\n")))
       .WillOnce(SaveArg<0>(&log5));
-  event_logger.logEject(host, detector, envoy::data::cluster::v3::FAILURE_PERCENTAGE, false);
+  event_logger->logEject(host, detector, envoy::data::cluster::v3::FAILURE_PERCENTAGE, false);
   *Json::Factory::loadFromString(log5);
 
   StringViewSaver log6;
@@ -2739,7 +2740,7 @@ TEST(OutlierDetectionEventLoggerImplTest, All) {
                          "\"upstream_url\":\"10.0.0.1:443\",\"action\":\"UNEJECT\","
                          "\"num_ejections\":0,\"enforced\":false}\n")))
       .WillOnce(SaveArg<0>(&log6));
-  event_logger.logUneject(host);
+  event_logger->logUneject(host);
   *Json::Factory::loadFromString(log6);
 }
 

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