From cab9e1f642dacf96ba27f257810d7bb106e9a744 Mon Sep 17 00:00:00 2001
From: Alex Konradi <akonradi@google.com>
Date: Wed, 15 Jul 2020 15:06:16 -0400
Subject: [PATCH 01/17] Add 'scaling' to OverloadManager action states

Extend the existing overload action states with a 'scaling' state with a
value in the range [0, 1]. The scaling state is not used yet, but will
be used to support actions that can take partial effect even if resource
pressure is below saturation.

Signed-off-by: Alex Konradi <akonradi@google.com>
---
 include/envoy/server/overload_manager.h    | 30 +++++---
 source/common/http/conn_manager_impl.cc    |  5 +-
 source/common/memory/heap_shrinker.cc      |  7 +-
 source/server/admin/admin.h                |  2 +-
 source/server/overload_manager_impl.cc     | 79 +++++++++++++++-------
 source/server/overload_manager_impl.h      | 11 +--
 source/server/worker_impl.cc               |  7 +-
 test/common/http/conn_manager_impl_test.cc |  4 +-
 test/common/memory/heap_shrinker_test.cc   |  4 +-
 test/mocks/server/overload_manager.cc      |  2 +-
 test/server/overload_manager_impl_test.cc  | 26 ++++---
 11 files changed, 110 insertions(+), 67 deletions(-)

diff --git a/include/envoy/server/overload_manager.h b/include/envoy/server/overload_manager.h
index 24ddd16cfd6c..3acba8676988 100644
--- a/include/envoy/server/overload_manager.h
+++ b/include/envoy/server/overload_manager.h
@@ -11,15 +11,27 @@
 namespace Envoy {
 namespace Server {
 
-enum class OverloadActionState {
-  /**
-   * Indicates that an overload action is active because at least one of its triggers has fired.
-   */
-  Active,
-  /**
-   * Indicates that an overload action is inactive because none of its triggers have fired.
-   */
-  Inactive
+/**
+ * Tracks the state of an overload action. The state is a number between 0 and 1 that represents the
+ * level of saturation. The values are categorized in two groups:
+ * - Saturated (value = 1): indicates that an overload action is active because at least one of its
+ *   triggers has reached saturation.
+ * - Scaling (0 <= value < 1): indicates that an overload action is not saturated.
+ */
+class OverloadActionState {
+public:
+  static constexpr OverloadActionState inactive() { return OverloadActionState(0); }
+
+  static constexpr OverloadActionState saturated() { return OverloadActionState(1.0); }
+
+  explicit constexpr OverloadActionState(float value)
+      : action_value_(value < 0 ? 0 : value < 1 ? value : 1) {}
+
+  float value() const { return action_value_; }
+  bool isSaturated() const { return action_value_ == 1; }
+
+private:
+  float action_value_;
 };
 
 /**
diff --git a/source/common/http/conn_manager_impl.cc b/source/common/http/conn_manager_impl.cc
index 602cba3aebd0..1a5c7d46fdbd 100644
--- a/source/common/http/conn_manager_impl.cc
+++ b/source/common/http/conn_manager_impl.cc
@@ -833,8 +833,7 @@ void ConnectionManagerImpl::ActiveStream::decodeHeaders(RequestHeaderMapPtr&& he
   filter_manager_.maybeEndDecode(end_stream);
 
   // Drop new requests when overloaded as soon as we have decoded the headers.
-  if (connection_manager_.overload_stop_accepting_requests_ref_ ==
-      Server::OverloadActionState::Active) {
+  if (connection_manager_.overload_stop_accepting_requests_ref_.isSaturated()) {
     // In this one special case, do not create the filter chain. If there is a risk of memory
     // overload it is more important to avoid unnecessary allocation than to create the filters.
     state_.created_filter_chain_ = true;
@@ -1824,7 +1823,7 @@ void ConnectionManagerImpl::ActiveStream::encodeHeadersInternal(ResponseHeaderMa
   }
 
   if (connection_manager_.drain_state_ == DrainState::NotDraining &&
-      connection_manager_.overload_disable_keepalive_ref_ == Server::OverloadActionState::Active) {
+      connection_manager_.overload_disable_keepalive_ref_.isSaturated()) {
     ENVOY_STREAM_LOG(debug, "disabling keepalive due to envoy overload", *this);
     connection_manager_.drain_state_ = DrainState::Closing;
     connection_manager_.stats_.named_.downstream_cx_overload_disable_keepalive_.inc();
diff --git a/source/common/memory/heap_shrinker.cc b/source/common/memory/heap_shrinker.cc
index da0413bbfce3..5df99c8b42d0 100644
--- a/source/common/memory/heap_shrinker.cc
+++ b/source/common/memory/heap_shrinker.cc
@@ -15,10 +15,9 @@ HeapShrinker::HeapShrinker(Event::Dispatcher& dispatcher, Server::OverloadManage
                            Stats::Scope& stats)
     : active_(false) {
   const auto action_name = Server::OverloadActionNames::get().ShrinkHeap;
-  if (overload_manager.registerForAction(action_name, dispatcher,
-                                         [this](Server::OverloadActionState state) {
-                                           active_ = (state == Server::OverloadActionState::Active);
-                                         })) {
+  if (overload_manager.registerForAction(
+          action_name, dispatcher,
+          [this](Server::OverloadActionState state) { active_ = state.isSaturated(); })) {
     Envoy::Stats::StatNameManagedStorage stat_name(
         absl::StrCat("overload.", action_name, ".shrink_count"), stats.symbolTable());
     shrink_counter_ = &stats.counterFromStatName(stat_name.statName());
diff --git a/source/server/admin/admin.h b/source/server/admin/admin.h
index 093ed76e4156..d1f3b24a25e7 100644
--- a/source/server/admin/admin.h
+++ b/source/server/admin/admin.h
@@ -255,7 +255,7 @@ class AdminImpl : public Admin,
     struct NullThreadLocalOverloadState : public ThreadLocalOverloadState {
       const OverloadActionState& getState(const std::string&) override { return inactive_; }
 
-      const OverloadActionState inactive_ = OverloadActionState::Inactive;
+      const OverloadActionState inactive_ = OverloadActionState::inactive();
     };
 
     NullOverloadManager(ThreadLocal::SlotAllocator& slot_allocator)
diff --git a/source/server/overload_manager_impl.cc b/source/server/overload_manager_impl.cc
index 1e4e085bb890..c825cf995c88 100644
--- a/source/server/overload_manager_impl.cc
+++ b/source/server/overload_manager_impl.cc
@@ -24,16 +24,19 @@ class ThresholdTriggerImpl : public OverloadAction::Trigger {
       : threshold_(config.value()) {}
 
   bool updateValue(double value) override {
-    const bool fired = isFired();
-    value_ = value;
-    return fired != isFired();
+    const OverloadActionState state = actionState();
+    state_.emplace(value >= threshold_ ? OverloadActionState::saturated()
+                                       : OverloadActionState::inactive());
+    return state.value() != actionState().value();
   }
 
-  bool isFired() const override { return value_.has_value() && value_ >= threshold_; }
+  OverloadActionState actionState() const override {
+    return state_.value_or(OverloadActionState::inactive());
+  }
 
 private:
   const double threshold_;
-  absl::optional<double> value_;
+  absl::optional<OverloadActionState> state_;
 };
 
 /**
@@ -44,12 +47,19 @@ class ThreadLocalOverloadStateImpl : public ThreadLocalOverloadState {
   const OverloadActionState& getState(const std::string& action) override {
     auto it = actions_.find(action);
     if (it == actions_.end()) {
-      it = actions_.insert(std::make_pair(action, OverloadActionState::Inactive)).first;
+      it = actions_.insert(std::make_pair(action, OverloadActionState::inactive())).first;
     }
     return it->second;
   }
 
-  void setState(const std::string& action, OverloadActionState state) { actions_[action] = state; }
+  void setState(const std::string& action, OverloadActionState state) {
+    auto it = actions_.find(action);
+    if (it == actions_.end()) {
+      actions_.emplace(action, state);
+    } else {
+      it->second = state;
+    }
+  }
 
 private:
   absl::node_hash_map<std::string, OverloadActionState> actions_;
@@ -72,8 +82,11 @@ Stats::Gauge& makeGauge(Stats::Scope& scope, absl::string_view a, absl::string_v
 
 OverloadAction::OverloadAction(const envoy::config::overload::v3::OverloadAction& config,
                                Stats::Scope& stats_scope)
-    : active_gauge_(
-          makeGauge(stats_scope, config.name(), "active", Stats::Gauge::ImportMode::Accumulate)) {
+    : state_(OverloadActionState::inactive()),
+      active_gauge_(
+          makeGauge(stats_scope, config.name(), "active", Stats::Gauge::ImportMode::Accumulate)),
+      scaling_gauge_(
+          makeGauge(stats_scope, config.name(), "scaling", Stats::Gauge::ImportMode::Accumulate)) {
   for (const auto& trigger_config : config.triggers()) {
     TriggerPtr trigger;
 
@@ -92,29 +105,41 @@ OverloadAction::OverloadAction(const envoy::config::overload::v3::OverloadAction
   }
 
   active_gauge_.set(0);
+  scaling_gauge_.set(0);
 }
 
 bool OverloadAction::updateResourcePressure(const std::string& name, double pressure) {
-  const bool active = isActive();
+  const OverloadActionState old_state = getState();
 
   auto it = triggers_.find(name);
   ASSERT(it != triggers_.end());
-  if (it->second->updateValue(pressure)) {
-    if (it->second->isFired()) {
-      active_gauge_.set(1);
-      const auto result = fired_triggers_.insert(name);
-      ASSERT(result.second);
-    } else {
-      active_gauge_.set(0);
-      const auto result = fired_triggers_.erase(name);
-      ASSERT(result == 1);
+  if (!it->second->updateValue(pressure)) {
+    return false;
+  }
+  const auto trigger_new_state = it->second->actionState();
+  if (trigger_new_state.isSaturated()) {
+    active_gauge_.set(1);
+    scaling_gauge_.set(0);
+  } else {
+    active_gauge_.set(0);
+    scaling_gauge_.set(1);
+  }
+
+  {
+    OverloadActionState new_state = OverloadActionState::inactive();
+    for (auto& trigger : triggers_) {
+      const auto trigger_state = trigger.second->actionState();
+      if (trigger_state.value() > new_state.value()) {
+        new_state = trigger_state;
+      }
     }
+    state_ = new_state;
   }
 
-  return active != isActive();
+  return state_.value() != old_state.value();
 }
 
-bool OverloadAction::isActive() const { return !fired_triggers_.empty(); }
+OverloadActionState OverloadAction::getState() const { return state_; }
 
 OverloadManagerImpl::OverloadManagerImpl(Event::Dispatcher& dispatcher, Stats::Scope& stats_scope,
                                          ThreadLocal::SlotAllocator& slot_allocator,
@@ -223,12 +248,14 @@ void OverloadManagerImpl::updateResourcePressure(const std::string& resource, do
                   const std::string& action = entry.second;
                   auto action_it = actions_.find(action);
                   ASSERT(action_it != actions_.end());
+                  const OverloadActionState old_state = action_it->second.getState();
                   if (action_it->second.updateResourcePressure(resource, pressure)) {
-                    const bool is_active = action_it->second.isActive();
-                    const auto state =
-                        is_active ? OverloadActionState::Active : OverloadActionState::Inactive;
-                    ENVOY_LOG(info, "Overload action {} became {}", action,
-                              is_active ? "active" : "inactive");
+                    const auto state = action_it->second.getState();
+
+                    if (old_state.isSaturated() != state.isSaturated()) {
+                      ENVOY_LOG(info, "Overload action {} became {}", action,
+                                (state.isSaturated() ? "saturated" : "scaling"));
+                    }
                     tls_->runOnAllThreads([this, action, state] {
                       tls_->getTyped<ThreadLocalOverloadStateImpl>().setState(action, state);
                     });
diff --git a/source/server/overload_manager_impl.h b/source/server/overload_manager_impl.h
index 4405bfeaf3aa..4ee12988a948 100644
--- a/source/server/overload_manager_impl.h
+++ b/source/server/overload_manager_impl.h
@@ -30,8 +30,8 @@ class OverloadAction {
   // has changed state.
   bool updateResourcePressure(const std::string& name, double pressure);
 
-  // Returns whether the action is currently active or not.
-  bool isActive() const;
+  // Returns the current action state, which is the max state across all registered triggers.
+  OverloadActionState getState() const;
 
   class Trigger {
   public:
@@ -40,15 +40,16 @@ class OverloadAction {
     // Updates the current value of the metric and returns whether the trigger has changed state.
     virtual bool updateValue(double value) PURE;
 
-    // Returns whether the trigger is currently fired or not.
-    virtual bool isFired() const PURE;
+    // Returns the action state for the trigger.
+    virtual OverloadActionState actionState() const PURE;
   };
   using TriggerPtr = std::unique_ptr<Trigger>;
 
 private:
   absl::node_hash_map<std::string, TriggerPtr> triggers_;
-  absl::node_hash_set<std::string> fired_triggers_;
+  OverloadActionState state_;
   Stats::Gauge& active_gauge_;
+  Stats::Gauge& scaling_gauge_;
 };
 
 class OverloadManagerImpl : Logger::Loggable<Logger::Id::main>, public OverloadManager {
diff --git a/source/server/worker_impl.cc b/source/server/worker_impl.cc
index eae51fa68837..b5bbe8d91cbb 100644
--- a/source/server/worker_impl.cc
+++ b/source/server/worker_impl.cc
@@ -143,13 +143,10 @@ void WorkerImpl::threadRoutine(GuardDog& guard_dog) {
 }
 
 void WorkerImpl::stopAcceptingConnectionsCb(OverloadActionState state) {
-  switch (state) {
-  case OverloadActionState::Active:
+  if (state.isSaturated()) {
     handler_->disableListeners();
-    break;
-  case OverloadActionState::Inactive:
+  } else {
     handler_->enableListeners();
-    break;
   }
 }
 
diff --git a/test/common/http/conn_manager_impl_test.cc b/test/common/http/conn_manager_impl_test.cc
index 88b6712252d9..afae38e17149 100644
--- a/test/common/http/conn_manager_impl_test.cc
+++ b/test/common/http/conn_manager_impl_test.cc
@@ -5609,7 +5609,7 @@ TEST(HttpConnectionManagerTracingStatsTest, verifyTracingStats) {
 }
 
 TEST_F(HttpConnectionManagerImplTest, NoNewStreamWhenOverloaded) {
-  Server::OverloadActionState stop_accepting_requests = Server::OverloadActionState::Active;
+  Server::OverloadActionState stop_accepting_requests = Server::OverloadActionState::saturated();
   ON_CALL(overload_manager_.overload_state_,
           getState(Server::OverloadActionNames::get().StopAcceptingRequests))
       .WillByDefault(ReturnRef(stop_accepting_requests));
@@ -5640,7 +5640,7 @@ TEST_F(HttpConnectionManagerImplTest, NoNewStreamWhenOverloaded) {
 }
 
 TEST_F(HttpConnectionManagerImplTest, DisableKeepAliveWhenOverloaded) {
-  Server::OverloadActionState disable_http_keep_alive = Server::OverloadActionState::Active;
+  Server::OverloadActionState disable_http_keep_alive = Server::OverloadActionState::saturated();
   ON_CALL(overload_manager_.overload_state_,
           getState(Server::OverloadActionNames::get().DisableHttpKeepAlive))
       .WillByDefault(ReturnRef(disable_http_keep_alive));
diff --git a/test/common/memory/heap_shrinker_test.cc b/test/common/memory/heap_shrinker_test.cc
index 5889424f5435..e23336191dc9 100644
--- a/test/common/memory/heap_shrinker_test.cc
+++ b/test/common/memory/heap_shrinker_test.cc
@@ -61,7 +61,7 @@ TEST_F(HeapShrinkerTest, ShrinkWhenTriggered) {
 
   Envoy::Stats::Counter& shrink_count =
       stats_.counter("overload.envoy.overload_actions.shrink_heap.shrink_count");
-  action_cb(Server::OverloadActionState::Active);
+  action_cb(Server::OverloadActionState::saturated());
   step();
   EXPECT_EQ(1, shrink_count.value());
 
@@ -77,7 +77,7 @@ TEST_F(HeapShrinkerTest, ShrinkWhenTriggered) {
   step();
   EXPECT_EQ(2, shrink_count.value());
 
-  action_cb(Server::OverloadActionState::Inactive);
+  action_cb(Server::OverloadActionState::inactive());
   step();
   step();
   EXPECT_EQ(2, shrink_count.value());
diff --git a/test/mocks/server/overload_manager.cc b/test/mocks/server/overload_manager.cc
index d0fd9b545ec6..1402be606930 100644
--- a/test/mocks/server/overload_manager.cc
+++ b/test/mocks/server/overload_manager.cc
@@ -11,7 +11,7 @@ namespace Server {
 using ::testing::ReturnRef;
 
 MockThreadLocalOverloadState::MockThreadLocalOverloadState()
-    : disabled_state_(OverloadActionState::Inactive) {
+    : disabled_state_(OverloadActionState::inactive()) {
   ON_CALL(*this, getState).WillByDefault(ReturnRef(disabled_state_));
 }
 
diff --git a/test/server/overload_manager_impl_test.cc b/test/server/overload_manager_impl_test.cc
index 73715e249c07..582c0a1de282 100644
--- a/test/server/overload_manager_impl_test.cc
+++ b/test/server/overload_manager_impl_test.cc
@@ -1,4 +1,5 @@
 #include "envoy/config/overload/v3/overload.pb.h"
+#include "envoy/server/overload_manager.h"
 #include "envoy/server/resource_monitor.h"
 #include "envoy/server/resource_monitor_config.h"
 
@@ -19,8 +20,12 @@
 #include "gtest/gtest.h"
 
 using testing::_;
+using testing::AllOf;
+using testing::DoubleEq;
 using testing::Invoke;
 using testing::NiceMock;
+using testing::Not;
+using testing::Property;
 
 namespace Envoy {
 namespace Server {
@@ -176,7 +181,7 @@ TEST_F(OverloadManagerImplTest, CallbackOnlyFiresWhenStateChanges) {
   int cb_count = 0;
   manager->registerForAction("envoy.overload_actions.dummy_action", dispatcher_,
                              [&](OverloadActionState state) {
-                               is_active = state == OverloadActionState::Active;
+                               is_active = state.isSaturated();
                                cb_count++;
                              });
   manager->registerForAction("envoy.overload_actions.unknown_action", dispatcher_,
@@ -198,7 +203,8 @@ TEST_F(OverloadManagerImplTest, CallbackOnlyFiresWhenStateChanges) {
   factory1_.monitor_->setPressure(0.5);
   timer_cb_();
   EXPECT_FALSE(is_active);
-  EXPECT_EQ(action_state, OverloadActionState::Inactive);
+  EXPECT_THAT(action_state, AllOf(Property(&OverloadActionState::isSaturated, false),
+                                  Property(&OverloadActionState::value, 0)));
   EXPECT_EQ(0, cb_count);
   EXPECT_EQ(0, active_gauge.value());
   EXPECT_EQ(50, pressure_gauge1.value());
@@ -207,7 +213,7 @@ TEST_F(OverloadManagerImplTest, CallbackOnlyFiresWhenStateChanges) {
   factory1_.monitor_->setPressure(0.95);
   timer_cb_();
   EXPECT_TRUE(is_active);
-  EXPECT_EQ(action_state, OverloadActionState::Active);
+  EXPECT_TRUE(action_state.isSaturated());
   EXPECT_EQ(1, cb_count);
   EXPECT_EQ(1, active_gauge.value());
   EXPECT_EQ(95, pressure_gauge1.value());
@@ -216,7 +222,7 @@ TEST_F(OverloadManagerImplTest, CallbackOnlyFiresWhenStateChanges) {
   factory1_.monitor_->setPressure(0.94);
   timer_cb_();
   EXPECT_TRUE(is_active);
-  EXPECT_EQ(action_state, OverloadActionState::Active);
+  EXPECT_TRUE(action_state.isSaturated());
   EXPECT_EQ(1, cb_count);
   EXPECT_EQ(94, pressure_gauge1.value());
 
@@ -224,7 +230,7 @@ TEST_F(OverloadManagerImplTest, CallbackOnlyFiresWhenStateChanges) {
   factory2_.monitor_->setPressure(0.9);
   timer_cb_();
   EXPECT_TRUE(is_active);
-  EXPECT_EQ(action_state, OverloadActionState::Active);
+  EXPECT_TRUE(action_state.isSaturated());
   EXPECT_EQ(1, cb_count);
   EXPECT_EQ(90, pressure_gauge2.value());
 
@@ -232,7 +238,7 @@ TEST_F(OverloadManagerImplTest, CallbackOnlyFiresWhenStateChanges) {
   factory1_.monitor_->setPressure(0.5);
   timer_cb_();
   EXPECT_TRUE(is_active);
-  EXPECT_EQ(action_state, OverloadActionState::Active);
+  EXPECT_TRUE(action_state.isSaturated());
   EXPECT_EQ(1, cb_count);
   EXPECT_EQ(50, pressure_gauge1.value());
   EXPECT_EQ(90, pressure_gauge2.value());
@@ -241,7 +247,8 @@ TEST_F(OverloadManagerImplTest, CallbackOnlyFiresWhenStateChanges) {
   factory2_.monitor_->setPressure(0.3);
   timer_cb_();
   EXPECT_FALSE(is_active);
-  EXPECT_EQ(action_state, OverloadActionState::Inactive);
+  EXPECT_THAT(action_state, AllOf(Property(&OverloadActionState::isSaturated, false),
+                                  Property(&OverloadActionState::value, 0)));
   EXPECT_EQ(2, cb_count);
   EXPECT_EQ(30, pressure_gauge2.value());
 
@@ -250,7 +257,7 @@ TEST_F(OverloadManagerImplTest, CallbackOnlyFiresWhenStateChanges) {
   factory2_.monitor_->setPressure(0.96);
   timer_cb_();
   EXPECT_TRUE(is_active);
-  EXPECT_EQ(action_state, OverloadActionState::Active);
+  EXPECT_TRUE(action_state.isSaturated());
   EXPECT_EQ(3, cb_count);
   EXPECT_EQ(97, pressure_gauge1.value());
   EXPECT_EQ(96, pressure_gauge2.value());
@@ -260,7 +267,8 @@ TEST_F(OverloadManagerImplTest, CallbackOnlyFiresWhenStateChanges) {
   factory2_.monitor_->setPressure(0.42);
   timer_cb_();
   EXPECT_FALSE(is_active);
-  EXPECT_EQ(action_state, OverloadActionState::Inactive);
+  EXPECT_THAT(action_state, AllOf(Property(&OverloadActionState::isSaturated, false),
+                                  Property(&OverloadActionState::value, 0)));
   EXPECT_EQ(4, cb_count);
   EXPECT_EQ(41, pressure_gauge1.value());
   EXPECT_EQ(42, pressure_gauge2.value());

From 864ab5efe2dd6cdc95ecb259c79bd23a38cec838 Mon Sep 17 00:00:00 2001
From: Alex Konradi <akonradi@google.com>
Date: Wed, 15 Jul 2020 17:26:29 -0400
Subject: [PATCH 02/17] Add Range trigger to Overload Manager

Add a Range trigger that will produce overload action states that are in
between the inactive and saturated states. This will enable the creation
of actions that take more effect as load increases.

Signed-off-by: Alex Konradi <akonradi@google.com>
---
 api/envoy/config/overload/v3/overload.proto   |  18 +++-
 .../envoy/config/overload/v3/overload.proto   |  18 +++-
 source/server/overload_manager_impl.cc        |  36 +++++++
 test/server/overload_manager_impl_test.cc     | 102 +++++++++++++++++-
 4 files changed, 164 insertions(+), 10 deletions(-)

diff --git a/api/envoy/config/overload/v3/overload.proto b/api/envoy/config/overload/v3/overload.proto
index d564e0d0ae3d..c9939d456b74 100644
--- a/api/envoy/config/overload/v3/overload.proto
+++ b/api/envoy/config/overload/v3/overload.proto
@@ -50,10 +50,18 @@ message ThresholdTrigger {
       "envoy.config.overload.v2alpha.ThresholdTrigger";
 
   // If the resource pressure is greater than or equal to this value, the trigger
-  // will fire.
+  // will enter saturation.
   double value = 1 [(validate.rules).double = {lte: 1.0 gte: 0.0}];
 }
 
+message RangeTrigger {
+  // If the resource pressure is greater than this value, the trigger will enter the :ref:`scaling <arch_overview_overload_manager-triggers-state>` state.
+  double min_value = 1 [(validate.rules).double = {lte: 1.0 gte: 0.0}];
+
+  // If the resource pressure is greater than this value, the trigger will enter saturation.
+  double max_value = 2 [(validate.rules).double = {lte: 1.0 gte: 0.0}];
+}
+
 message Trigger {
   option (udpa.annotations.versioning).previous_message_type =
       "envoy.config.overload.v2alpha.Trigger";
@@ -65,6 +73,8 @@ message Trigger {
     option (validate.required) = true;
 
     ThresholdTrigger threshold = 2;
+
+    RangeTrigger range = 3;
   }
 }
 
@@ -77,9 +87,9 @@ message OverloadAction {
   // DNS to ensure uniqueness.
   string name = 1 [(validate.rules).string = {min_bytes: 1}];
 
-  // A set of triggers for this action. If any of these triggers fire the overload action
-  // is activated. Listeners are notified when the overload action transitions from
-  // inactivated to activated, or vice versa.
+  // A set of triggers for this action. The state of the action is the maximum
+  // state of all triggers, which can be inactive, scaling, or saturated.
+  // Listeners are notified when the overload action changes state.
   repeated Trigger triggers = 2 [(validate.rules).repeated = {min_items: 1}];
 }
 
diff --git a/generated_api_shadow/envoy/config/overload/v3/overload.proto b/generated_api_shadow/envoy/config/overload/v3/overload.proto
index 337150657b14..1f01f151f7b6 100644
--- a/generated_api_shadow/envoy/config/overload/v3/overload.proto
+++ b/generated_api_shadow/envoy/config/overload/v3/overload.proto
@@ -48,10 +48,18 @@ message ThresholdTrigger {
       "envoy.config.overload.v2alpha.ThresholdTrigger";
 
   // If the resource pressure is greater than or equal to this value, the trigger
-  // will fire.
+  // will enter saturation.
   double value = 1 [(validate.rules).double = {lte: 1.0 gte: 0.0}];
 }
 
+message RangeTrigger {
+  // If the resource pressure is greater than this value, the trigger will enter the :ref:`scaling <arch_overview_overload_manager-triggers-state>` state.
+  double min_value = 1 [(validate.rules).double = {lte: 1.0 gte: 0.0}];
+
+  // If the resource pressure is greater than this value, the trigger will enter saturation.
+  double max_value = 2 [(validate.rules).double = {lte: 1.0 gte: 0.0}];
+}
+
 message Trigger {
   option (udpa.annotations.versioning).previous_message_type =
       "envoy.config.overload.v2alpha.Trigger";
@@ -63,6 +71,8 @@ message Trigger {
     option (validate.required) = true;
 
     ThresholdTrigger threshold = 2;
+
+    RangeTrigger range = 3;
   }
 }
 
@@ -75,9 +85,9 @@ message OverloadAction {
   // DNS to ensure uniqueness.
   string name = 1 [(validate.rules).string = {min_bytes: 1}];
 
-  // A set of triggers for this action. If any of these triggers fire the overload action
-  // is activated. Listeners are notified when the overload action transitions from
-  // inactivated to activated, or vice versa.
+  // A set of triggers for this action. The state of the action is the maximum
+  // state of all triggers, which can be inactive, scaling, or saturated.
+  // Listeners are notified when the overload action changes state.
   repeated Trigger triggers = 2 [(validate.rules).repeated = {min_items: 1}];
 }
 
diff --git a/source/server/overload_manager_impl.cc b/source/server/overload_manager_impl.cc
index c825cf995c88..e4b7994b38a1 100644
--- a/source/server/overload_manager_impl.cc
+++ b/source/server/overload_manager_impl.cc
@@ -1,5 +1,6 @@
 #include "server/overload_manager_impl.h"
 
+#include "envoy/common/exception.h"
 #include "envoy/config/overload/v3/overload.pb.h"
 #include "envoy/stats/scope.h"
 
@@ -39,6 +40,38 @@ class ThresholdTriggerImpl : public OverloadAction::Trigger {
   absl::optional<OverloadActionState> state_;
 };
 
+class RangeTriggerImpl final : public OverloadAction::Trigger {
+public:
+  RangeTriggerImpl(const envoy::config::overload::v3::RangeTrigger& config)
+      : minimum_(config.min_value()),
+        maximum_(config.max_value()) {
+    if (minimum_ >= maximum_) {
+      throw EnvoyException("min_value must be less than max_value");
+    }
+  }
+
+  bool updateValue(double value) override {
+    const OverloadActionState old_state = actionState();
+    if (value <= minimum_) {
+      state_ = OverloadActionState::inactive();
+    } else if (value >= maximum_) {
+      state_ = OverloadActionState::saturated();
+    } else {
+      state_ = OverloadActionState((value - minimum_) / (maximum_ - minimum_));
+    }
+    return state_->value() != old_state.value();
+  }
+
+  OverloadActionState actionState() const override {
+    return state_.value_or(OverloadActionState::inactive());
+  }
+
+private:
+  const double minimum_;
+  const double maximum_;
+  absl::optional<OverloadActionState> state_;
+};
+
 /**
  * Thread-local copy of the state of each configured overload action.
  */
@@ -94,6 +127,9 @@ OverloadAction::OverloadAction(const envoy::config::overload::v3::OverloadAction
     case envoy::config::overload::v3::Trigger::TriggerOneofCase::kThreshold:
       trigger = std::make_unique<ThresholdTriggerImpl>(trigger_config.threshold());
       break;
+    case envoy::config::overload::v3::Trigger::TriggerOneofCase::kRange:
+      trigger = std::make_unique<RangeTriggerImpl>(trigger_config.range());
+      break;
     default:
       NOT_REACHED_GCOVR_EXCL_LINE;
     }
diff --git a/test/server/overload_manager_impl_test.cc b/test/server/overload_manager_impl_test.cc
index 582c0a1de282..b4f2af052104 100644
--- a/test/server/overload_manager_impl_test.cc
+++ b/test/server/overload_manager_impl_test.cc
@@ -88,8 +88,10 @@ class OverloadManagerImplTest : public testing::Test {
 protected:
   OverloadManagerImplTest()
       : factory1_("envoy.resource_monitors.fake_resource1"),
-        factory2_("envoy.resource_monitors.fake_resource2"), register_factory1_(factory1_),
-        register_factory2_(factory2_), api_(Api::createApiForTest(stats_)) {}
+        factory2_("envoy.resource_monitors.fake_resource2"),
+        factory3_("envoy.resource_monitors.fake_resource3"), register_factory1_(factory1_),
+        register_factory2_(factory2_), register_factory3_(factory3_),
+        api_(Api::createApiForTest(stats_)) {}
 
   void setDispatcherExpectation() {
     timer_ = new NiceMock<Event::MockTimer>();
@@ -117,6 +119,9 @@ class OverloadManagerImplTest : public testing::Test {
       resource_monitors {
         name: "envoy.resource_monitors.fake_resource2"
       }
+      resource_monitors {
+        name: "envoy.resource_monitors.fake_resource3"
+      }
       actions {
         name: "envoy.overload_actions.dummy_action"
         triggers {
@@ -131,6 +136,13 @@ class OverloadManagerImplTest : public testing::Test {
             value: 0.8
           }
         }
+        triggers {
+          name: "envoy.resource_monitors.fake_resource3"
+          range {
+            min_value: 0.5
+            max_value: 0.8
+          }
+        }
       }
     )EOF";
   }
@@ -162,8 +174,10 @@ class OverloadManagerImplTest : public testing::Test {
 
   FakeResourceMonitorFactory<Envoy::ProtobufWkt::Struct> factory1_;
   FakeResourceMonitorFactory<Envoy::ProtobufWkt::Timestamp> factory2_;
+  FakeResourceMonitorFactory<Envoy::ProtobufWkt::Timestamp> factory3_;
   Registry::InjectFactory<Configuration::ResourceMonitorFactory> register_factory1_;
   Registry::InjectFactory<Configuration::ResourceMonitorFactory> register_factory2_;
+  Registry::InjectFactory<Configuration::ResourceMonitorFactory> register_factory3_;
   NiceMock<Event::MockDispatcher> dispatcher_;
   NiceMock<Event::MockTimer>* timer_; // not owned
   Stats::TestUtil::TestStore stats_;
@@ -276,6 +290,50 @@ TEST_F(OverloadManagerImplTest, CallbackOnlyFiresWhenStateChanges) {
   manager->stop();
 }
 
+TEST_F(OverloadManagerImplTest, RangeTrigger) {
+  setDispatcherExpectation();
+
+  auto manager(createOverloadManager(getConfig()));
+  manager->start();
+  const auto& action_state =
+      manager->getThreadLocalOverloadState().getState("envoy.overload_actions.dummy_action");
+  Stats::Gauge& active_gauge = stats_.gauge("overload.envoy.overload_actions.dummy_action.active",
+                                            Stats::Gauge::ImportMode::Accumulate);
+  Stats::Gauge& scaling_gauge = stats_.gauge("overload.envoy.overload_actions.dummy_action.scaling",
+                                             Stats::Gauge::ImportMode::Accumulate);
+
+  factory3_.monitor_->setPressure(0.5);
+  timer_cb_();
+
+  EXPECT_THAT(action_state, AllOf(Property(&OverloadActionState::isSaturated, false),
+                                  Property(&OverloadActionState::value, 0)));
+  EXPECT_EQ(0, active_gauge.value());
+  EXPECT_EQ(0, scaling_gauge.value());
+
+  // The trigger for fake_resource3 is a range trigger with a min of 0.5 and a max of 0.8. Set the
+  // current pressure value to halfway in that range.
+  factory3_.monitor_->setPressure(0.65);
+  timer_cb_();
+
+  EXPECT_EQ(action_state.value(), 0.5 /* = 0.65 / (0.8 - 0.5) */);
+  EXPECT_EQ(0, active_gauge.value());
+  EXPECT_EQ(1, scaling_gauge.value());
+
+  factory3_.monitor_->setPressure(0.8);
+  timer_cb_();
+
+  EXPECT_TRUE(action_state.isSaturated());
+  EXPECT_EQ(1, active_gauge.value());
+  EXPECT_EQ(0, scaling_gauge.value());
+
+  factory3_.monitor_->setPressure(0.9);
+  timer_cb_();
+
+  EXPECT_TRUE(action_state.isSaturated());
+  EXPECT_EQ(1, active_gauge.value());
+  EXPECT_EQ(0, scaling_gauge.value());
+}
+
 TEST_F(OverloadManagerImplTest, FailedUpdates) {
   setDispatcherExpectation();
   auto manager(createOverloadManager(getConfig()));
@@ -347,6 +405,46 @@ TEST_F(OverloadManagerImplTest, DuplicateOverloadAction) {
                           "Duplicate overload action .*");
 }
 
+TEST_F(OverloadManagerImplTest, RangeTriggerMaxLessThanMin) {
+  const std::string config = R"EOF(
+    resource_monitors {
+      name: "envoy.resource_monitors.fake_resource1"
+    }
+    actions {
+      name: "envoy.overload_actions.dummy_action"
+      triggers {
+        name: "envoy.resource_monitors.fake_resource1"
+        range {
+          min_value: 0.9
+          max_value: 0.8
+        }
+      }
+    }
+  )EOF";
+
+  EXPECT_THROW_WITH_REGEX(createOverloadManager(config), EnvoyException, "min_value.*max_value.*");
+}
+
+TEST_F(OverloadManagerImplTest, RangeTriggerMaxEqualsMin) {
+  const std::string config = R"EOF(
+    resource_monitors {
+      name: "envoy.resource_monitors.fake_resource1"
+    }
+    actions {
+      name: "envoy.overload_actions.dummy_action"
+      triggers {
+        name: "envoy.resource_monitors.fake_resource1"
+        range {
+          min_value: 0.9
+          max_value: 0.9
+        }
+      }
+    }
+  )EOF";
+
+  EXPECT_THROW_WITH_REGEX(createOverloadManager(config), EnvoyException, "min_value.*max_value.*");
+}
+
 TEST_F(OverloadManagerImplTest, UnknownTrigger) {
   const std::string config = R"EOF(
     actions {

From accf2e33c570a0986838f7f9fe2cce535801e0d5 Mon Sep 17 00:00:00 2001
From: Alex Konradi <akonradi@google.com>
Date: Wed, 29 Jul 2020 11:51:43 -0400
Subject: [PATCH 03/17] Add documentation for action states

Signed-off-by: Alex Konradi <akonradi@google.com>
---
 .../overload_manager/overload_manager.rst     |  3 +-
 .../operations/overload_manager.rst           | 40 +++++++++++++++++++
 2 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/docs/root/configuration/operations/overload_manager/overload_manager.rst b/docs/root/configuration/operations/overload_manager/overload_manager.rst
index 2dd2e7fe5cc7..b4cb9c356036 100644
--- a/docs/root/configuration/operations/overload_manager/overload_manager.rst
+++ b/docs/root/configuration/operations/overload_manager/overload_manager.rst
@@ -99,4 +99,5 @@ with the following statistics:
   :header: Name, Type, Description
   :widths: 1, 1, 2
 
-  active, Gauge, "Active state of the action (0=inactive, 1=active)"
+  active, Gauge, "Active state of the action (0=inactive, 1=active) - only available for actions with threshold triggers"
+  scale_value, Gauge, "Scaled value of the action as a percent (0=inactive, 100=active) - only available for actions with range triggers"
diff --git a/docs/root/intro/arch_overview/operations/overload_manager.rst b/docs/root/intro/arch_overview/operations/overload_manager.rst
index af00c5948bcb..dc8d6df4b405 100644
--- a/docs/root/intro/arch_overview/operations/overload_manager.rst
+++ b/docs/root/intro/arch_overview/operations/overload_manager.rst
@@ -12,3 +12,43 @@ upstream services.
 The overload manager is :ref:`configured <config_overload_manager>` by specifying a set of
 resources to monitor and a set of overload actions that will be taken when some of those
 resources exceed certain pressure thresholds.
+
+Architecture
+------------
+
+The overload manager works by periodically polling the *pressure* of a set of **resources**,
+feeding those through **triggers**, and taking **actions** based on the triggers. The set of
+resource monitors, triggers, and actions are specified at startup.
+
+Resources
+~~~~~~~~~
+
+A resource is a thing that can be monitored by the overload manager, and whose *pressure* is
+represented by a real value in the range [0, 1]. The pressure of a resource is evaluated by a
+*resource monitor*. See the :ref:`configuration page <config_overload_manager>` for setting up
+resource monitors.
+
+Triggers
+~~~~~~~~
+
+Triggers are evaluated on each resource pressure update, and convert a resource pressure value
+into one of two action states:
+
+.. _arch_overview_overload_manager-triggers-state:
+
+.. csv-table::
+  :header: state, description
+  :widths: 1, 2
+
+  saturated, the resource pressure is at or above the critical point; drastic action should be taken
+  scaling,   the resource pressure is below the critical point; action may be taken
+
+On update, the action states are presented to the configured overload actions. What effect an
+overload action has based on an action state depends on its configuration and implementation.
+
+Actions
+~~~~~~~
+
+When a trigger changes state, the value is sent to registered actions, which can then affect how
+connections and requests are processed. Each action interprets the input states differently, and
+some may ignore the *scaling* state altogether, taking effect only when *saturated*.
\ No newline at end of file

From 981ff107842aea9e253dbb5267abcff4f4e1e221 Mon Sep 17 00:00:00 2001
From: Alex Konradi <akonradi@google.com>
Date: Thu, 30 Jul 2020 10:43:27 -0400
Subject: [PATCH 04/17] Fix scaling -> scale_value gauge for actions

Rename the gauge to match the documentation

Signed-off-by: Alex Konradi <akonradi@google.com>
---
 source/server/overload_manager_impl.cc | 15 +++++----------
 source/server/overload_manager_impl.h  |  2 +-
 2 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/source/server/overload_manager_impl.cc b/source/server/overload_manager_impl.cc
index e4b7994b38a1..8234e3d32cf6 100644
--- a/source/server/overload_manager_impl.cc
+++ b/source/server/overload_manager_impl.cc
@@ -118,8 +118,8 @@ OverloadAction::OverloadAction(const envoy::config::overload::v3::OverloadAction
     : state_(OverloadActionState::inactive()),
       active_gauge_(
           makeGauge(stats_scope, config.name(), "active", Stats::Gauge::ImportMode::Accumulate)),
-      scaling_gauge_(
-          makeGauge(stats_scope, config.name(), "scaling", Stats::Gauge::ImportMode::Accumulate)) {
+      scale_value_gauge_(
+          makeGauge(stats_scope, config.name(), "scale_value", Stats::Gauge::ImportMode::Accumulate)) {
   for (const auto& trigger_config : config.triggers()) {
     TriggerPtr trigger;
 
@@ -141,7 +141,7 @@ OverloadAction::OverloadAction(const envoy::config::overload::v3::OverloadAction
   }
 
   active_gauge_.set(0);
-  scaling_gauge_.set(0);
+  scale_value_gauge_.set(0);
 }
 
 bool OverloadAction::updateResourcePressure(const std::string& name, double pressure) {
@@ -153,13 +153,8 @@ bool OverloadAction::updateResourcePressure(const std::string& name, double pres
     return false;
   }
   const auto trigger_new_state = it->second->actionState();
-  if (trigger_new_state.isSaturated()) {
-    active_gauge_.set(1);
-    scaling_gauge_.set(0);
-  } else {
-    active_gauge_.set(0);
-    scaling_gauge_.set(1);
-  }
+  active_gauge_.set(trigger_new_state.isSaturated() ? 1 : 0);
+  scale_value_gauge_.set(trigger_new_state.value() * 100);
 
   {
     OverloadActionState new_state = OverloadActionState::inactive();
diff --git a/source/server/overload_manager_impl.h b/source/server/overload_manager_impl.h
index 4ee12988a948..959c1f41550a 100644
--- a/source/server/overload_manager_impl.h
+++ b/source/server/overload_manager_impl.h
@@ -49,7 +49,7 @@ class OverloadAction {
   absl::node_hash_map<std::string, TriggerPtr> triggers_;
   OverloadActionState state_;
   Stats::Gauge& active_gauge_;
-  Stats::Gauge& scaling_gauge_;
+  Stats::Gauge& scale_value_gauge_;
 };
 
 class OverloadManagerImpl : Logger::Loggable<Logger::Id::main>, public OverloadManager {

From ecad37bde6aff098a74f67b43f924c360e9a095b Mon Sep 17 00:00:00 2001
From: Alex Konradi <akonradi@google.com>
Date: Thu, 30 Jul 2020 10:46:28 -0400
Subject: [PATCH 05/17] Address feedback on docs

Both stats are present for all actions.

Signed-off-by: Alex Konradi <akonradi@google.com>
---
 .../operations/overload_manager/overload_manager.rst          | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/docs/root/configuration/operations/overload_manager/overload_manager.rst b/docs/root/configuration/operations/overload_manager/overload_manager.rst
index b4cb9c356036..06702070f00b 100644
--- a/docs/root/configuration/operations/overload_manager/overload_manager.rst
+++ b/docs/root/configuration/operations/overload_manager/overload_manager.rst
@@ -99,5 +99,5 @@ with the following statistics:
   :header: Name, Type, Description
   :widths: 1, 1, 2
 
-  active, Gauge, "Active state of the action (0=inactive, 1=active) - only available for actions with threshold triggers"
-  scale_value, Gauge, "Scaled value of the action as a percent (0=inactive, 100=active) - only available for actions with range triggers"
+  active, Gauge, "Active state of the action (0=inactive, 1=active)"
+  scale_value, Gauge, "Scaled value of the action as a percent (0=inactive, 100=active)"

From d1f482c22792f7c46dd6950fee233d0f115e9a9d Mon Sep 17 00:00:00 2001
From: Alex Konradi <akonradi@google.com>
Date: Thu, 30 Jul 2020 11:00:27 -0400
Subject: [PATCH 06/17] Run fix_format

Signed-off-by: Alex Konradi <akonradi@google.com>
---
 api/BUILD                              | 1 -
 source/server/overload_manager_impl.cc | 7 +++----
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/api/BUILD b/api/BUILD
index 8c608fdeca4a..3317f5a64c55 100644
--- a/api/BUILD
+++ b/api/BUILD
@@ -246,7 +246,6 @@ proto_library(
         "//envoy/service/discovery/v3:pkg",
         "//envoy/service/endpoint/v3:pkg",
         "//envoy/service/event_reporting/v3:pkg",
-        "//envoy/service/extension/v3:pkg",
         "//envoy/service/health/v3:pkg",
         "//envoy/service/listener/v3:pkg",
         "//envoy/service/load_stats/v3:pkg",
diff --git a/source/server/overload_manager_impl.cc b/source/server/overload_manager_impl.cc
index 8234e3d32cf6..e3799e611b32 100644
--- a/source/server/overload_manager_impl.cc
+++ b/source/server/overload_manager_impl.cc
@@ -43,8 +43,7 @@ class ThresholdTriggerImpl : public OverloadAction::Trigger {
 class RangeTriggerImpl final : public OverloadAction::Trigger {
 public:
   RangeTriggerImpl(const envoy::config::overload::v3::RangeTrigger& config)
-      : minimum_(config.min_value()),
-        maximum_(config.max_value()) {
+      : minimum_(config.min_value()), maximum_(config.max_value()) {
     if (minimum_ >= maximum_) {
       throw EnvoyException("min_value must be less than max_value");
     }
@@ -118,8 +117,8 @@ OverloadAction::OverloadAction(const envoy::config::overload::v3::OverloadAction
     : state_(OverloadActionState::inactive()),
       active_gauge_(
           makeGauge(stats_scope, config.name(), "active", Stats::Gauge::ImportMode::Accumulate)),
-      scale_value_gauge_(
-          makeGauge(stats_scope, config.name(), "scale_value", Stats::Gauge::ImportMode::Accumulate)) {
+      scale_value_gauge_(makeGauge(stats_scope, config.name(), "scale_value",
+                                   Stats::Gauge::ImportMode::Accumulate)) {
   for (const auto& trigger_config : config.triggers()) {
     TriggerPtr trigger;
 

From 8dd558442ce059bbe6d584e2abe5d0740be0a4dc Mon Sep 17 00:00:00 2001
From: Alex Konradi <akonradi@google.com>
Date: Fri, 31 Jul 2020 14:21:04 -0400
Subject: [PATCH 07/17] Fix overload manager impl test

Correct test for stat name change in prior commit

Signed-off-by: Alex Konradi <akonradi@google.com>
---
 test/server/overload_manager_impl_test.cc | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/test/server/overload_manager_impl_test.cc b/test/server/overload_manager_impl_test.cc
index b4f2af052104..8508baa531d1 100644
--- a/test/server/overload_manager_impl_test.cc
+++ b/test/server/overload_manager_impl_test.cc
@@ -204,6 +204,8 @@ TEST_F(OverloadManagerImplTest, CallbackOnlyFiresWhenStateChanges) {
 
   Stats::Gauge& active_gauge = stats_.gauge("overload.envoy.overload_actions.dummy_action.active",
                                             Stats::Gauge::ImportMode::Accumulate);
+  Stats::Gauge& scale_value_gauge = stats_.gauge("overload.envoy.overload_actions.dummy_action.scale_value",
+                                             Stats::Gauge::ImportMode::Accumulate);
   Stats::Gauge& pressure_gauge1 =
       stats_.gauge("overload.envoy.resource_monitors.fake_resource1.pressure",
                    Stats::Gauge::ImportMode::NeverImport);
@@ -221,6 +223,7 @@ TEST_F(OverloadManagerImplTest, CallbackOnlyFiresWhenStateChanges) {
                                   Property(&OverloadActionState::value, 0)));
   EXPECT_EQ(0, cb_count);
   EXPECT_EQ(0, active_gauge.value());
+  EXPECT_EQ(0, scale_value_gauge.value());
   EXPECT_EQ(50, pressure_gauge1.value());
 
   // Update exceeds fake_resource1 trigger threshold, callback is expected
@@ -230,6 +233,7 @@ TEST_F(OverloadManagerImplTest, CallbackOnlyFiresWhenStateChanges) {
   EXPECT_TRUE(action_state.isSaturated());
   EXPECT_EQ(1, cb_count);
   EXPECT_EQ(1, active_gauge.value());
+  EXPECT_EQ(100, scale_value_gauge.value());
   EXPECT_EQ(95, pressure_gauge1.value());
 
   // Callback should not be invoked if action state does not change
@@ -299,7 +303,7 @@ TEST_F(OverloadManagerImplTest, RangeTrigger) {
       manager->getThreadLocalOverloadState().getState("envoy.overload_actions.dummy_action");
   Stats::Gauge& active_gauge = stats_.gauge("overload.envoy.overload_actions.dummy_action.active",
                                             Stats::Gauge::ImportMode::Accumulate);
-  Stats::Gauge& scaling_gauge = stats_.gauge("overload.envoy.overload_actions.dummy_action.scaling",
+  Stats::Gauge& scale_value_gauge = stats_.gauge("overload.envoy.overload_actions.dummy_action.scale_value",
                                              Stats::Gauge::ImportMode::Accumulate);
 
   factory3_.monitor_->setPressure(0.5);
@@ -308,7 +312,7 @@ TEST_F(OverloadManagerImplTest, RangeTrigger) {
   EXPECT_THAT(action_state, AllOf(Property(&OverloadActionState::isSaturated, false),
                                   Property(&OverloadActionState::value, 0)));
   EXPECT_EQ(0, active_gauge.value());
-  EXPECT_EQ(0, scaling_gauge.value());
+  EXPECT_EQ(0, scale_value_gauge.value());
 
   // The trigger for fake_resource3 is a range trigger with a min of 0.5 and a max of 0.8. Set the
   // current pressure value to halfway in that range.
@@ -317,21 +321,21 @@ TEST_F(OverloadManagerImplTest, RangeTrigger) {
 
   EXPECT_EQ(action_state.value(), 0.5 /* = 0.65 / (0.8 - 0.5) */);
   EXPECT_EQ(0, active_gauge.value());
-  EXPECT_EQ(1, scaling_gauge.value());
+  EXPECT_EQ(50, scale_value_gauge.value());
 
   factory3_.monitor_->setPressure(0.8);
   timer_cb_();
 
   EXPECT_TRUE(action_state.isSaturated());
   EXPECT_EQ(1, active_gauge.value());
-  EXPECT_EQ(0, scaling_gauge.value());
+  EXPECT_EQ(100, scale_value_gauge.value());
 
   factory3_.monitor_->setPressure(0.9);
   timer_cb_();
 
   EXPECT_TRUE(action_state.isSaturated());
   EXPECT_EQ(1, active_gauge.value());
-  EXPECT_EQ(0, scaling_gauge.value());
+  EXPECT_EQ(100, scale_value_gauge.value());
 }
 
 TEST_F(OverloadManagerImplTest, FailedUpdates) {

From 622d9dae724eee3b7026bd4ce912222667097cca Mon Sep 17 00:00:00 2001
From: Alex Konradi <akonradi@google.com>
Date: Fri, 31 Jul 2020 14:28:15 -0400
Subject: [PATCH 08/17] fix formatting

Signed-off-by: Alex Konradi <akonradi@google.com>
---
 test/server/overload_manager_impl_test.cc | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/test/server/overload_manager_impl_test.cc b/test/server/overload_manager_impl_test.cc
index 8508baa531d1..c850f763c446 100644
--- a/test/server/overload_manager_impl_test.cc
+++ b/test/server/overload_manager_impl_test.cc
@@ -204,8 +204,9 @@ TEST_F(OverloadManagerImplTest, CallbackOnlyFiresWhenStateChanges) {
 
   Stats::Gauge& active_gauge = stats_.gauge("overload.envoy.overload_actions.dummy_action.active",
                                             Stats::Gauge::ImportMode::Accumulate);
-  Stats::Gauge& scale_value_gauge = stats_.gauge("overload.envoy.overload_actions.dummy_action.scale_value",
-                                             Stats::Gauge::ImportMode::Accumulate);
+  Stats::Gauge& scale_value_gauge =
+      stats_.gauge("overload.envoy.overload_actions.dummy_action.scale_value",
+                   Stats::Gauge::ImportMode::Accumulate);
   Stats::Gauge& pressure_gauge1 =
       stats_.gauge("overload.envoy.resource_monitors.fake_resource1.pressure",
                    Stats::Gauge::ImportMode::NeverImport);
@@ -303,8 +304,9 @@ TEST_F(OverloadManagerImplTest, RangeTrigger) {
       manager->getThreadLocalOverloadState().getState("envoy.overload_actions.dummy_action");
   Stats::Gauge& active_gauge = stats_.gauge("overload.envoy.overload_actions.dummy_action.active",
                                             Stats::Gauge::ImportMode::Accumulate);
-  Stats::Gauge& scale_value_gauge = stats_.gauge("overload.envoy.overload_actions.dummy_action.scale_value",
-                                             Stats::Gauge::ImportMode::Accumulate);
+  Stats::Gauge& scale_value_gauge =
+      stats_.gauge("overload.envoy.overload_actions.dummy_action.scale_value",
+                   Stats::Gauge::ImportMode::Accumulate);
 
   factory3_.monitor_->setPressure(0.5);
   timer_cb_();

From bf771190de5f8755e6940b249282a7b18f9207c3 Mon Sep 17 00:00:00 2001
From: Alex Konradi <akonradi@google.com>
Date: Fri, 31 Jul 2020 18:11:14 -0400
Subject: [PATCH 09/17] Address some review feedback

Signed-off-by: Alex Konradi <akonradi@google.com>
---
 api/envoy/config/overload/v3/overload.proto   |  8 ++++--
 .../overload_manager/overload_manager.rst     |  4 +--
 .../operations/overload_manager.rst           |  7 +++--
 .../envoy/config/overload/v3/overload.proto   |  8 ++++--
 include/envoy/server/overload_manager.h       |  2 +-
 source/server/overload_manager_impl.cc        |  8 +++---
 source/server/overload_manager_impl.h         |  2 +-
 test/server/overload_manager_impl_test.cc     | 28 +++++++++----------
 8 files changed, 36 insertions(+), 31 deletions(-)

diff --git a/api/envoy/config/overload/v3/overload.proto b/api/envoy/config/overload/v3/overload.proto
index c9939d456b74..395aaa294fc9 100644
--- a/api/envoy/config/overload/v3/overload.proto
+++ b/api/envoy/config/overload/v3/overload.proto
@@ -55,7 +55,9 @@ message ThresholdTrigger {
 }
 
 message RangeTrigger {
-  // If the resource pressure is greater than this value, the trigger will enter the :ref:`scaling <arch_overview_overload_manager-triggers-state>` state.
+  // If the resource pressure is greater than this value, the trigger will be in the
+  // :ref:`scaling <arch_overview_overload_manager-triggers-state>` state with value
+  // `(pressure - min_value) / (max_value - min_value)`.
   double min_value = 1 [(validate.rules).double = {lte: 1.0 gte: 0.0}];
 
   // If the resource pressure is greater than this value, the trigger will enter saturation.
@@ -88,8 +90,8 @@ message OverloadAction {
   string name = 1 [(validate.rules).string = {min_bytes: 1}];
 
   // A set of triggers for this action. The state of the action is the maximum
-  // state of all triggers, which can be inactive, scaling, or saturated.
-  // Listeners are notified when the overload action changes state.
+  // state of all triggers, which can be scaling between 0 and 1 or saturated. Listeners
+  // are notified when the overload action changes state.
   repeated Trigger triggers = 2 [(validate.rules).repeated = {min_items: 1}];
 }
 
diff --git a/docs/root/configuration/operations/overload_manager/overload_manager.rst b/docs/root/configuration/operations/overload_manager/overload_manager.rst
index 06702070f00b..6f2286225db4 100644
--- a/docs/root/configuration/operations/overload_manager/overload_manager.rst
+++ b/docs/root/configuration/operations/overload_manager/overload_manager.rst
@@ -99,5 +99,5 @@ with the following statistics:
   :header: Name, Type, Description
   :widths: 1, 1, 2
 
-  active, Gauge, "Active state of the action (0=inactive, 1=active)"
-  scale_value, Gauge, "Scaled value of the action as a percent (0=inactive, 100=active)"
+  active, Gauge, "Active state of the action (0=scaling, 1=saturated)"
+  scale_percent, Gauge, "Scaled value of the action as a percent (0-99=scaling, 100=saturated)"
diff --git a/docs/root/intro/arch_overview/operations/overload_manager.rst b/docs/root/intro/arch_overview/operations/overload_manager.rst
index dc8d6df4b405..ee1f20197884 100644
--- a/docs/root/intro/arch_overview/operations/overload_manager.rst
+++ b/docs/root/intro/arch_overview/operations/overload_manager.rst
@@ -32,7 +32,7 @@ Triggers
 ~~~~~~~~
 
 Triggers are evaluated on each resource pressure update, and convert a resource pressure value
-into one of two action states:
+into an action state in one of two ranges:
 
 .. _arch_overview_overload_manager-triggers-state:
 
@@ -43,8 +43,9 @@ into one of two action states:
   saturated, the resource pressure is at or above the critical point; drastic action should be taken
   scaling,   the resource pressure is below the critical point; action may be taken
 
-On update, the action states are presented to the configured overload actions. What effect an
-overload action has based on an action state depends on its configuration and implementation.
+When a resource pressure value is updated, the relevant triggers are reevaluated. For each action
+with at least one trigger, the resulting action state is the maximum value over the configured
+triggers. What effect the action state has depends on the action's configuration and implementation.
 
 Actions
 ~~~~~~~
diff --git a/generated_api_shadow/envoy/config/overload/v3/overload.proto b/generated_api_shadow/envoy/config/overload/v3/overload.proto
index 1f01f151f7b6..c7cb2be58016 100644
--- a/generated_api_shadow/envoy/config/overload/v3/overload.proto
+++ b/generated_api_shadow/envoy/config/overload/v3/overload.proto
@@ -53,7 +53,9 @@ message ThresholdTrigger {
 }
 
 message RangeTrigger {
-  // If the resource pressure is greater than this value, the trigger will enter the :ref:`scaling <arch_overview_overload_manager-triggers-state>` state.
+  // If the resource pressure is greater than this value, the trigger will be in the
+  // :ref:`scaling <arch_overview_overload_manager-triggers-state>` state with value
+  // `(pressure - min_value) / (max_value - min_value)`.
   double min_value = 1 [(validate.rules).double = {lte: 1.0 gte: 0.0}];
 
   // If the resource pressure is greater than this value, the trigger will enter saturation.
@@ -86,8 +88,8 @@ message OverloadAction {
   string name = 1 [(validate.rules).string = {min_bytes: 1}];
 
   // A set of triggers for this action. The state of the action is the maximum
-  // state of all triggers, which can be inactive, scaling, or saturated.
-  // Listeners are notified when the overload action changes state.
+  // state of all triggers, which can be scaling between 0 and 1 or saturated. Listeners
+  // are notified when the overload action changes state.
   repeated Trigger triggers = 2 [(validate.rules).repeated = {min_items: 1}];
 }
 
diff --git a/include/envoy/server/overload_manager.h b/include/envoy/server/overload_manager.h
index 3acba8676988..77c6c21c097d 100644
--- a/include/envoy/server/overload_manager.h
+++ b/include/envoy/server/overload_manager.h
@@ -25,7 +25,7 @@ class OverloadActionState {
   static constexpr OverloadActionState saturated() { return OverloadActionState(1.0); }
 
   explicit constexpr OverloadActionState(float value)
-      : action_value_(value < 0 ? 0 : value < 1 ? value : 1) {}
+      : action_value_(std::min(1.0f, std::max(0.0f, value))) {}
 
   float value() const { return action_value_; }
   bool isSaturated() const { return action_value_ == 1; }
diff --git a/source/server/overload_manager_impl.cc b/source/server/overload_manager_impl.cc
index e3799e611b32..e87f0b932fd5 100644
--- a/source/server/overload_manager_impl.cc
+++ b/source/server/overload_manager_impl.cc
@@ -117,8 +117,8 @@ OverloadAction::OverloadAction(const envoy::config::overload::v3::OverloadAction
     : state_(OverloadActionState::inactive()),
       active_gauge_(
           makeGauge(stats_scope, config.name(), "active", Stats::Gauge::ImportMode::Accumulate)),
-      scale_value_gauge_(makeGauge(stats_scope, config.name(), "scale_value",
-                                   Stats::Gauge::ImportMode::Accumulate)) {
+      scale_percent_gauge_(makeGauge(stats_scope, config.name(), "scale_percent",
+                                     Stats::Gauge::ImportMode::Accumulate)) {
   for (const auto& trigger_config : config.triggers()) {
     TriggerPtr trigger;
 
@@ -140,7 +140,7 @@ OverloadAction::OverloadAction(const envoy::config::overload::v3::OverloadAction
   }
 
   active_gauge_.set(0);
-  scale_value_gauge_.set(0);
+  scale_percent_gauge_.set(0);
 }
 
 bool OverloadAction::updateResourcePressure(const std::string& name, double pressure) {
@@ -153,7 +153,7 @@ bool OverloadAction::updateResourcePressure(const std::string& name, double pres
   }
   const auto trigger_new_state = it->second->actionState();
   active_gauge_.set(trigger_new_state.isSaturated() ? 1 : 0);
-  scale_value_gauge_.set(trigger_new_state.value() * 100);
+  scale_percent_gauge_.set(trigger_new_state.value() * 100);
 
   {
     OverloadActionState new_state = OverloadActionState::inactive();
diff --git a/source/server/overload_manager_impl.h b/source/server/overload_manager_impl.h
index 959c1f41550a..e19a74b55a10 100644
--- a/source/server/overload_manager_impl.h
+++ b/source/server/overload_manager_impl.h
@@ -49,7 +49,7 @@ class OverloadAction {
   absl::node_hash_map<std::string, TriggerPtr> triggers_;
   OverloadActionState state_;
   Stats::Gauge& active_gauge_;
-  Stats::Gauge& scale_value_gauge_;
+  Stats::Gauge& scale_percent_gauge_;
 };
 
 class OverloadManagerImpl : Logger::Loggable<Logger::Id::main>, public OverloadManager {
diff --git a/test/server/overload_manager_impl_test.cc b/test/server/overload_manager_impl_test.cc
index c850f763c446..d63de1c7aebe 100644
--- a/test/server/overload_manager_impl_test.cc
+++ b/test/server/overload_manager_impl_test.cc
@@ -21,10 +21,8 @@
 
 using testing::_;
 using testing::AllOf;
-using testing::DoubleEq;
 using testing::Invoke;
 using testing::NiceMock;
-using testing::Not;
 using testing::Property;
 
 namespace Envoy {
@@ -204,8 +202,8 @@ TEST_F(OverloadManagerImplTest, CallbackOnlyFiresWhenStateChanges) {
 
   Stats::Gauge& active_gauge = stats_.gauge("overload.envoy.overload_actions.dummy_action.active",
                                             Stats::Gauge::ImportMode::Accumulate);
-  Stats::Gauge& scale_value_gauge =
-      stats_.gauge("overload.envoy.overload_actions.dummy_action.scale_value",
+  Stats::Gauge& scale_percent_gauge =
+      stats_.gauge("overload.envoy.overload_actions.dummy_action.scale_percent",
                    Stats::Gauge::ImportMode::Accumulate);
   Stats::Gauge& pressure_gauge1 =
       stats_.gauge("overload.envoy.resource_monitors.fake_resource1.pressure",
@@ -224,7 +222,7 @@ TEST_F(OverloadManagerImplTest, CallbackOnlyFiresWhenStateChanges) {
                                   Property(&OverloadActionState::value, 0)));
   EXPECT_EQ(0, cb_count);
   EXPECT_EQ(0, active_gauge.value());
-  EXPECT_EQ(0, scale_value_gauge.value());
+  EXPECT_EQ(0, scale_percent_gauge.value());
   EXPECT_EQ(50, pressure_gauge1.value());
 
   // Update exceeds fake_resource1 trigger threshold, callback is expected
@@ -234,7 +232,7 @@ TEST_F(OverloadManagerImplTest, CallbackOnlyFiresWhenStateChanges) {
   EXPECT_TRUE(action_state.isSaturated());
   EXPECT_EQ(1, cb_count);
   EXPECT_EQ(1, active_gauge.value());
-  EXPECT_EQ(100, scale_value_gauge.value());
+  EXPECT_EQ(100, scale_percent_gauge.value());
   EXPECT_EQ(95, pressure_gauge1.value());
 
   // Callback should not be invoked if action state does not change
@@ -304,8 +302,8 @@ TEST_F(OverloadManagerImplTest, RangeTrigger) {
       manager->getThreadLocalOverloadState().getState("envoy.overload_actions.dummy_action");
   Stats::Gauge& active_gauge = stats_.gauge("overload.envoy.overload_actions.dummy_action.active",
                                             Stats::Gauge::ImportMode::Accumulate);
-  Stats::Gauge& scale_value_gauge =
-      stats_.gauge("overload.envoy.overload_actions.dummy_action.scale_value",
+  Stats::Gauge& scale_percent_gauge =
+      stats_.gauge("overload.envoy.overload_actions.dummy_action.scale_percent",
                    Stats::Gauge::ImportMode::Accumulate);
 
   factory3_.monitor_->setPressure(0.5);
@@ -314,7 +312,7 @@ TEST_F(OverloadManagerImplTest, RangeTrigger) {
   EXPECT_THAT(action_state, AllOf(Property(&OverloadActionState::isSaturated, false),
                                   Property(&OverloadActionState::value, 0)));
   EXPECT_EQ(0, active_gauge.value());
-  EXPECT_EQ(0, scale_value_gauge.value());
+  EXPECT_EQ(0, scale_percent_gauge.value());
 
   // The trigger for fake_resource3 is a range trigger with a min of 0.5 and a max of 0.8. Set the
   // current pressure value to halfway in that range.
@@ -323,21 +321,21 @@ TEST_F(OverloadManagerImplTest, RangeTrigger) {
 
   EXPECT_EQ(action_state.value(), 0.5 /* = 0.65 / (0.8 - 0.5) */);
   EXPECT_EQ(0, active_gauge.value());
-  EXPECT_EQ(50, scale_value_gauge.value());
+  EXPECT_EQ(50, scale_percent_gauge.value());
 
   factory3_.monitor_->setPressure(0.8);
   timer_cb_();
 
   EXPECT_TRUE(action_state.isSaturated());
   EXPECT_EQ(1, active_gauge.value());
-  EXPECT_EQ(100, scale_value_gauge.value());
+  EXPECT_EQ(100, scale_percent_gauge.value());
 
   factory3_.monitor_->setPressure(0.9);
   timer_cb_();
 
   EXPECT_TRUE(action_state.isSaturated());
   EXPECT_EQ(1, active_gauge.value());
-  EXPECT_EQ(100, scale_value_gauge.value());
+  EXPECT_EQ(100, scale_percent_gauge.value());
 }
 
 TEST_F(OverloadManagerImplTest, FailedUpdates) {
@@ -428,7 +426,8 @@ TEST_F(OverloadManagerImplTest, RangeTriggerMaxLessThanMin) {
     }
   )EOF";
 
-  EXPECT_THROW_WITH_REGEX(createOverloadManager(config), EnvoyException, "min_value.*max_value.*");
+  EXPECT_THROW_WITH_REGEX(createOverloadManager(config), EnvoyException,
+                          "min_value must be less than max_value.*");
 }
 
 TEST_F(OverloadManagerImplTest, RangeTriggerMaxEqualsMin) {
@@ -448,7 +447,8 @@ TEST_F(OverloadManagerImplTest, RangeTriggerMaxEqualsMin) {
     }
   )EOF";
 
-  EXPECT_THROW_WITH_REGEX(createOverloadManager(config), EnvoyException, "min_value.*max_value.*");
+  EXPECT_THROW_WITH_REGEX(createOverloadManager(config), EnvoyException,
+                          "min_value must be less than max_value.*");
 }
 
 TEST_F(OverloadManagerImplTest, UnknownTrigger) {

From 4f9426efecb6f3ae3028307cb4c8683fd5258dee Mon Sep 17 00:00:00 2001
From: Alex Konradi <akonradi@google.com>
Date: Wed, 5 Aug 2020 10:29:10 -0400
Subject: [PATCH 10/17] Restore missing build rule dependency

Signed-off-by: Alex Konradi <akonradi@google.com>
---
 api/BUILD | 1 +
 1 file changed, 1 insertion(+)

diff --git a/api/BUILD b/api/BUILD
index 8ec38a529521..3ac2738ebc3e 100644
--- a/api/BUILD
+++ b/api/BUILD
@@ -249,6 +249,7 @@ proto_library(
         "//envoy/service/discovery/v3:pkg",
         "//envoy/service/endpoint/v3:pkg",
         "//envoy/service/event_reporting/v3:pkg",
+        "//envoy/service/extension/v3:pkg",
         "//envoy/service/health/v3:pkg",
         "//envoy/service/listener/v3:pkg",
         "//envoy/service/load_stats/v3:pkg",

From 6d6fcf9643712ef04334a09a82381b127ad9874d Mon Sep 17 00:00:00 2001
From: Alex Konradi <akonradi@google.com>
Date: Wed, 5 Aug 2020 11:43:55 -0400
Subject: [PATCH 11/17] Make action state documentation less opaque

Explicitly document the meaning of the scaling and saturated ranges in
the overload manager overview, and list the two types of triggers
available and the meaning of each.

Signed-off-by: Alex Konradi <akonradi@google.com>
---
 .../overload_manager/overload_manager.rst      | 18 ++++++++++++++++++
 .../operations/overload_manager.rst            | 10 +++++-----
 2 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/docs/root/configuration/operations/overload_manager/overload_manager.rst b/docs/root/configuration/operations/overload_manager/overload_manager.rst
index 6f2286225db4..294dfb19926f 100644
--- a/docs/root/configuration/operations/overload_manager/overload_manager.rst
+++ b/docs/root/configuration/operations/overload_manager/overload_manager.rst
@@ -40,6 +40,24 @@ The overload manager uses Envoy's :ref:`extension <extending>` framework for def
 resource monitors. Envoy's builtin resource monitors are listed
 :ref:`here <config_resource_monitors>`.
 
+Triggers
+--------
+
+Triggers connect resource monitors to actions. There are two types of triggers supported:
+
+.. list-table::
+  :header-rows: 1
+  :widths: 1, 2
+
+  * - Type
+    - Description
+  * - :ref:`threshold <envoy_v3_api_msg_config.overload.v3.ThresholdTrigger>`
+    - Sets the action state to 1 (= *saturated*) when the resource pressure is above a threshold, and to 0 otherwise.
+  * - :ref:`range <envoy_v3_api_msg_config.overload.v3.RangeTrigger>`
+    - Sets the action state to 0 when the resource pressure is below the min threshold,
+      `(pressure - min)/(max - min)` when `min < pressure < max`,
+      and to 1 (*saturated*) when the pressure is above the max."
+
 Overload actions
 ----------------
 
diff --git a/docs/root/intro/arch_overview/operations/overload_manager.rst b/docs/root/intro/arch_overview/operations/overload_manager.rst
index ee1f20197884..256052bde184 100644
--- a/docs/root/intro/arch_overview/operations/overload_manager.rst
+++ b/docs/root/intro/arch_overview/operations/overload_manager.rst
@@ -32,16 +32,16 @@ Triggers
 ~~~~~~~~
 
 Triggers are evaluated on each resource pressure update, and convert a resource pressure value
-into an action state in one of two ranges:
+into an action state. An action state has a value in the range [0, 1], and is categorized into one of two groups:
 
 .. _arch_overview_overload_manager-triggers-state:
 
 .. csv-table::
-  :header: state, description
-  :widths: 1, 2
+  :header: action state, value, description
+  :widths: 1, 1, 2
 
-  saturated, the resource pressure is at or above the critical point; drastic action should be taken
-  scaling,   the resource pressure is below the critical point; action may be taken
+  scaling,   "[0, 1)", the resource pressure is below the configured saturation point; action may be taken
+  saturated, 1, the resource pressure is at or above the configured saturation point; drastic action should be taken
 
 When a resource pressure value is updated, the relevant triggers are reevaluated. For each action
 with at least one trigger, the resulting action state is the maximum value over the configured

From ad0b0abc9c9b88a7fc5411ca291644999bdab803 Mon Sep 17 00:00:00 2001
From: Alex Konradi <akonradi@google.com>
Date: Wed, 5 Aug 2020 13:44:52 -0400
Subject: [PATCH 12/17] Use insert_or_assign instead of spelling out

Signed-off-by: Alex Konradi <akonradi@google.com>
---
 source/server/overload_manager_impl.cc | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/source/server/overload_manager_impl.cc b/source/server/overload_manager_impl.cc
index e87f0b932fd5..9ff006913119 100644
--- a/source/server/overload_manager_impl.cc
+++ b/source/server/overload_manager_impl.cc
@@ -85,12 +85,7 @@ class ThreadLocalOverloadStateImpl : public ThreadLocalOverloadState {
   }
 
   void setState(const std::string& action, OverloadActionState state) {
-    auto it = actions_.find(action);
-    if (it == actions_.end()) {
-      actions_.emplace(action, state);
-    } else {
-      it->second = state;
-    }
+    actions_.insert_or_assign(action, state);
   }
 
 private:

From 7a4578ca82c339b916095627491768e511a555a2 Mon Sep 17 00:00:00 2001
From: Alex Konradi <akonradi@google.com>
Date: Thu, 6 Aug 2020 13:54:39 -0400
Subject: [PATCH 13/17] Rename RangeTrigger -> ScaledTrigger

Also address feedback on C++ code and add to the release notes

Signed-off-by: Alex Konradi <akonradi@google.com>
---
 api/envoy/config/overload/v3/overload.proto   | 10 ++---
 .../overload_manager/overload_manager.rst     |  2 +-
 docs/root/version_history/current.rst         |  1 +
 .../envoy/config/overload/v3/overload.proto   | 10 ++---
 source/server/overload_manager_impl.cc        | 39 +++++++++----------
 5 files changed, 31 insertions(+), 31 deletions(-)

diff --git a/api/envoy/config/overload/v3/overload.proto b/api/envoy/config/overload/v3/overload.proto
index 395aaa294fc9..061783a04b77 100644
--- a/api/envoy/config/overload/v3/overload.proto
+++ b/api/envoy/config/overload/v3/overload.proto
@@ -54,14 +54,14 @@ message ThresholdTrigger {
   double value = 1 [(validate.rules).double = {lte: 1.0 gte: 0.0}];
 }
 
-message RangeTrigger {
+message ScaledTrigger {
   // If the resource pressure is greater than this value, the trigger will be in the
   // :ref:`scaling <arch_overview_overload_manager-triggers-state>` state with value
-  // `(pressure - min_value) / (max_value - min_value)`.
-  double min_value = 1 [(validate.rules).double = {lte: 1.0 gte: 0.0}];
+  // `(pressure - scaling_threshold) / (saturation_threshold - scaling_threshold)`.
+  double scaling_threshold = 1 [(validate.rules).double = {lte: 1.0 gte: 0.0}];
 
   // If the resource pressure is greater than this value, the trigger will enter saturation.
-  double max_value = 2 [(validate.rules).double = {lte: 1.0 gte: 0.0}];
+  double saturation_threshold = 2 [(validate.rules).double = {lte: 1.0 gte: 0.0}];
 }
 
 message Trigger {
@@ -76,7 +76,7 @@ message Trigger {
 
     ThresholdTrigger threshold = 2;
 
-    RangeTrigger range = 3;
+    ScaledTrigger scaled = 3;
   }
 }
 
diff --git a/docs/root/configuration/operations/overload_manager/overload_manager.rst b/docs/root/configuration/operations/overload_manager/overload_manager.rst
index 294dfb19926f..17baa9b0e65e 100644
--- a/docs/root/configuration/operations/overload_manager/overload_manager.rst
+++ b/docs/root/configuration/operations/overload_manager/overload_manager.rst
@@ -53,7 +53,7 @@ Triggers connect resource monitors to actions. There are two types of triggers s
     - Description
   * - :ref:`threshold <envoy_v3_api_msg_config.overload.v3.ThresholdTrigger>`
     - Sets the action state to 1 (= *saturated*) when the resource pressure is above a threshold, and to 0 otherwise.
-  * - :ref:`range <envoy_v3_api_msg_config.overload.v3.RangeTrigger>`
+  * - :ref:`scaled <envoy_v3_api_msg_config.overload.v3.ScaledTrigger>`
     - Sets the action state to 0 when the resource pressure is below the min threshold,
       `(pressure - min)/(max - min)` when `min < pressure < max`,
       and to 1 (*saturated*) when the pressure is above the max."
diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst
index f06045af222a..c56d2ffd1139 100644
--- a/docs/root/version_history/current.rst
+++ b/docs/root/version_history/current.rst
@@ -53,6 +53,7 @@ New Features
 * http: introduced new HTTP/1 and HTTP/2 codec implementations that will remove the use of exceptions for control flow due to high risk factors and instead use error statuses. The old behavior is used by default, but the new codecs can be enabled for testing by setting the runtime feature `envoy.reloadable_features.new_codec_behavior` to true. The new codecs will be in development for one month, and then enabled by default while the old codecs are deprecated.
 * load balancer: added a :ref:`configuration<envoy_v3_api_msg_config.cluster.v3.Cluster.LeastRequestLbConfig>` option to specify the active request bias used by the least request load balancer.
 * lua: added Lua APIs to access :ref:`SSL connection info <config_http_filters_lua_ssl_socket_info>` object.
+* overload management: add :ref:`scaling <envoy_v3_api_field_config.overload.v3.Trigger.scaled>` trigger for OverloadManager actions.
 * postgres network filter: :ref:`metadata <config_network_filters_postgres_proxy_dynamic_metadata>` is produced based on SQL query.
 * router: added new
   :ref:`envoy-ratelimited<config_http_filters_router_retry_policy-envoy-ratelimited>`
diff --git a/generated_api_shadow/envoy/config/overload/v3/overload.proto b/generated_api_shadow/envoy/config/overload/v3/overload.proto
index c7cb2be58016..7c32906d142b 100644
--- a/generated_api_shadow/envoy/config/overload/v3/overload.proto
+++ b/generated_api_shadow/envoy/config/overload/v3/overload.proto
@@ -52,14 +52,14 @@ message ThresholdTrigger {
   double value = 1 [(validate.rules).double = {lte: 1.0 gte: 0.0}];
 }
 
-message RangeTrigger {
+message ScaledTrigger {
   // If the resource pressure is greater than this value, the trigger will be in the
   // :ref:`scaling <arch_overview_overload_manager-triggers-state>` state with value
-  // `(pressure - min_value) / (max_value - min_value)`.
-  double min_value = 1 [(validate.rules).double = {lte: 1.0 gte: 0.0}];
+  // `(pressure - scaling_threshold) / (saturation_threshold - scaling_threshold)`.
+  double scaling_threshold = 1 [(validate.rules).double = {lte: 1.0 gte: 0.0}];
 
   // If the resource pressure is greater than this value, the trigger will enter saturation.
-  double max_value = 2 [(validate.rules).double = {lte: 1.0 gte: 0.0}];
+  double saturation_threshold = 2 [(validate.rules).double = {lte: 1.0 gte: 0.0}];
 }
 
 message Trigger {
@@ -74,7 +74,7 @@ message Trigger {
 
     ThresholdTrigger threshold = 2;
 
-    RangeTrigger range = 3;
+    ScaledTrigger scaled = 3;
   }
 }
 
diff --git a/source/server/overload_manager_impl.cc b/source/server/overload_manager_impl.cc
index 9ff006913119..c464fe2662c2 100644
--- a/source/server/overload_manager_impl.cc
+++ b/source/server/overload_manager_impl.cc
@@ -19,10 +19,10 @@ namespace Server {
 
 namespace {
 
-class ThresholdTriggerImpl : public OverloadAction::Trigger {
+class ThresholdTriggerImpl final : public OverloadAction::Trigger {
 public:
   ThresholdTriggerImpl(const envoy::config::overload::v3::ThresholdTrigger& config)
-      : threshold_(config.value()) {}
+      : threshold_(config.value()), state_(OverloadActionState::inactive()) {}
 
   bool updateValue(double value) override {
     const OverloadActionState state = actionState();
@@ -31,44 +31,42 @@ class ThresholdTriggerImpl : public OverloadAction::Trigger {
     return state.value() != actionState().value();
   }
 
-  OverloadActionState actionState() const override {
-    return state_.value_or(OverloadActionState::inactive());
-  }
+  OverloadActionState actionState() const override { return state_; }
 
 private:
   const double threshold_;
-  absl::optional<OverloadActionState> state_;
+  OverloadActionState state_;
 };
 
 class RangeTriggerImpl final : public OverloadAction::Trigger {
 public:
-  RangeTriggerImpl(const envoy::config::overload::v3::RangeTrigger& config)
-      : minimum_(config.min_value()), maximum_(config.max_value()) {
-    if (minimum_ >= maximum_) {
+  RangeTriggerImpl(const envoy::config::overload::v3::ScaledTrigger& config)
+      : scaling_threshold_(config.min_value()), saturated_threshold_(config.max_value()),
+        state_(OverloadActionState::inactive()) {
+    if (scaling_threshold_ >= saturated_threshold_) {
       throw EnvoyException("min_value must be less than max_value");
     }
   }
 
   bool updateValue(double value) override {
     const OverloadActionState old_state = actionState();
-    if (value <= minimum_) {
+    if (value <= scaling_threshold_) {
       state_ = OverloadActionState::inactive();
-    } else if (value >= maximum_) {
+    } else if (value >= saturated_threshold_) {
       state_ = OverloadActionState::saturated();
     } else {
-      state_ = OverloadActionState((value - minimum_) / (maximum_ - minimum_));
+      state_ = OverloadActionState((value - scaling_threshold_) /
+                                   (saturated_threshold_ - scaling_threshold_));
     }
     return state_->value() != old_state.value();
   }
 
-  OverloadActionState actionState() const override {
-    return state_.value_or(OverloadActionState::inactive());
-  }
+  OverloadActionState actionState() const override { return state_; }
 
 private:
-  const double minimum_;
-  const double maximum_;
-  absl::optional<OverloadActionState> state_;
+  const double scaling_threshold_;
+  const double saturated_threshold_;
+  OverloadActionState state_;
 };
 
 /**
@@ -121,8 +119,8 @@ OverloadAction::OverloadAction(const envoy::config::overload::v3::OverloadAction
     case envoy::config::overload::v3::Trigger::TriggerOneofCase::kThreshold:
       trigger = std::make_unique<ThresholdTriggerImpl>(trigger_config.threshold());
       break;
-    case envoy::config::overload::v3::Trigger::TriggerOneofCase::kRange:
-      trigger = std::make_unique<RangeTriggerImpl>(trigger_config.range());
+    case envoy::config::overload::v3::Trigger::TriggerOneofCase::kScaled:
+      trigger = std::make_unique<RangeTriggerImpl>(trigger_config.scaled());
       break;
     default:
       NOT_REACHED_GCOVR_EXCL_LINE;
@@ -151,6 +149,7 @@ bool OverloadAction::updateResourcePressure(const std::string& name, double pres
   scale_percent_gauge_.set(trigger_new_state.value() * 100);
 
   {
+    // Compute the new state as the maximum over all trigger states.
     OverloadActionState new_state = OverloadActionState::inactive();
     for (auto& trigger : triggers_) {
       const auto trigger_state = trigger.second->actionState();

From 4de8f833f7a26c24d5c9955533a6d120ece967cb Mon Sep 17 00:00:00 2001
From: Alex Konradi <akonradi@google.com>
Date: Fri, 7 Aug 2020 10:53:50 -0400
Subject: [PATCH 14/17] Fix compilation errors

Signed-off-by: Alex Konradi <akonradi@google.com>
---
 source/server/overload_manager_impl.cc | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/source/server/overload_manager_impl.cc b/source/server/overload_manager_impl.cc
index c464fe2662c2..06ababf0edb5 100644
--- a/source/server/overload_manager_impl.cc
+++ b/source/server/overload_manager_impl.cc
@@ -26,8 +26,8 @@ class ThresholdTriggerImpl final : public OverloadAction::Trigger {
 
   bool updateValue(double value) override {
     const OverloadActionState state = actionState();
-    state_.emplace(value >= threshold_ ? OverloadActionState::saturated()
-                                       : OverloadActionState::inactive());
+    state_ =
+        value >= threshold_ ? OverloadActionState::saturated() : OverloadActionState::inactive();
     return state.value() != actionState().value();
   }
 
@@ -41,7 +41,7 @@ class ThresholdTriggerImpl final : public OverloadAction::Trigger {
 class RangeTriggerImpl final : public OverloadAction::Trigger {
 public:
   RangeTriggerImpl(const envoy::config::overload::v3::ScaledTrigger& config)
-      : scaling_threshold_(config.min_value()), saturated_threshold_(config.max_value()),
+      : scaling_threshold_(config.scaling_threshold()), saturated_threshold_(config.saturation_threshold()),
         state_(OverloadActionState::inactive()) {
     if (scaling_threshold_ >= saturated_threshold_) {
       throw EnvoyException("min_value must be less than max_value");
@@ -58,7 +58,7 @@ class RangeTriggerImpl final : public OverloadAction::Trigger {
       state_ = OverloadActionState((value - scaling_threshold_) /
                                    (saturated_threshold_ - scaling_threshold_));
     }
-    return state_->value() != old_state.value();
+    return state_.value() != old_state.value();
   }
 
   OverloadActionState actionState() const override { return state_; }

From 264a7538b1ef97ae9ff3e4782d35bfae0988d1a7 Mon Sep 17 00:00:00 2001
From: Alex Konradi <akonradi@google.com>
Date: Fri, 7 Aug 2020 11:24:24 -0400
Subject: [PATCH 15/17] Fix more name errors

Signed-off-by: Alex Konradi <akonradi@google.com>
---
 source/server/overload_manager_impl.cc    |  9 ++++----
 test/server/overload_manager_impl_test.cc | 26 +++++++++++------------
 2 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/source/server/overload_manager_impl.cc b/source/server/overload_manager_impl.cc
index 06ababf0edb5..5055fba6e1cf 100644
--- a/source/server/overload_manager_impl.cc
+++ b/source/server/overload_manager_impl.cc
@@ -38,10 +38,11 @@ class ThresholdTriggerImpl final : public OverloadAction::Trigger {
   OverloadActionState state_;
 };
 
-class RangeTriggerImpl final : public OverloadAction::Trigger {
+class ScaledTriggerImpl final : public OverloadAction::Trigger {
 public:
-  RangeTriggerImpl(const envoy::config::overload::v3::ScaledTrigger& config)
-      : scaling_threshold_(config.scaling_threshold()), saturated_threshold_(config.saturation_threshold()),
+  ScaledTriggerImpl(const envoy::config::overload::v3::ScaledTrigger& config)
+      : scaling_threshold_(config.scaling_threshold()),
+        saturated_threshold_(config.saturation_threshold()),
         state_(OverloadActionState::inactive()) {
     if (scaling_threshold_ >= saturated_threshold_) {
       throw EnvoyException("min_value must be less than max_value");
@@ -120,7 +121,7 @@ OverloadAction::OverloadAction(const envoy::config::overload::v3::OverloadAction
       trigger = std::make_unique<ThresholdTriggerImpl>(trigger_config.threshold());
       break;
     case envoy::config::overload::v3::Trigger::TriggerOneofCase::kScaled:
-      trigger = std::make_unique<RangeTriggerImpl>(trigger_config.scaled());
+      trigger = std::make_unique<ScaledTriggerImpl>(trigger_config.scaled());
       break;
     default:
       NOT_REACHED_GCOVR_EXCL_LINE;
diff --git a/test/server/overload_manager_impl_test.cc b/test/server/overload_manager_impl_test.cc
index d63de1c7aebe..3610fc89250f 100644
--- a/test/server/overload_manager_impl_test.cc
+++ b/test/server/overload_manager_impl_test.cc
@@ -136,9 +136,9 @@ class OverloadManagerImplTest : public testing::Test {
         }
         triggers {
           name: "envoy.resource_monitors.fake_resource3"
-          range {
-            min_value: 0.5
-            max_value: 0.8
+          scaled {
+            scaling_threshold: 0.5
+            saturation_threshold: 0.8
           }
         }
       }
@@ -293,7 +293,7 @@ TEST_F(OverloadManagerImplTest, CallbackOnlyFiresWhenStateChanges) {
   manager->stop();
 }
 
-TEST_F(OverloadManagerImplTest, RangeTrigger) {
+TEST_F(OverloadManagerImplTest, ScaledTrigger) {
   setDispatcherExpectation();
 
   auto manager(createOverloadManager(getConfig()));
@@ -314,7 +314,7 @@ TEST_F(OverloadManagerImplTest, RangeTrigger) {
   EXPECT_EQ(0, active_gauge.value());
   EXPECT_EQ(0, scale_percent_gauge.value());
 
-  // The trigger for fake_resource3 is a range trigger with a min of 0.5 and a max of 0.8. Set the
+  // The trigger for fake_resource3 is a scaled trigger with a min of 0.5 and a max of 0.8. Set the
   // current pressure value to halfway in that range.
   factory3_.monitor_->setPressure(0.65);
   timer_cb_();
@@ -409,7 +409,7 @@ TEST_F(OverloadManagerImplTest, DuplicateOverloadAction) {
                           "Duplicate overload action .*");
 }
 
-TEST_F(OverloadManagerImplTest, RangeTriggerMaxLessThanMin) {
+TEST_F(OverloadManagerImplTest, ScaledTriggerMaxLessThanMin) {
   const std::string config = R"EOF(
     resource_monitors {
       name: "envoy.resource_monitors.fake_resource1"
@@ -418,9 +418,9 @@ TEST_F(OverloadManagerImplTest, RangeTriggerMaxLessThanMin) {
       name: "envoy.overload_actions.dummy_action"
       triggers {
         name: "envoy.resource_monitors.fake_resource1"
-        range {
-          min_value: 0.9
-          max_value: 0.8
+        scaled {
+          scaling_threshold: 0.9
+          saturation_threshold: 0.8
         }
       }
     }
@@ -430,7 +430,7 @@ TEST_F(OverloadManagerImplTest, RangeTriggerMaxLessThanMin) {
                           "min_value must be less than max_value.*");
 }
 
-TEST_F(OverloadManagerImplTest, RangeTriggerMaxEqualsMin) {
+TEST_F(OverloadManagerImplTest, ScaledTriggerMaxEqualsMin) {
   const std::string config = R"EOF(
     resource_monitors {
       name: "envoy.resource_monitors.fake_resource1"
@@ -439,9 +439,9 @@ TEST_F(OverloadManagerImplTest, RangeTriggerMaxEqualsMin) {
       name: "envoy.overload_actions.dummy_action"
       triggers {
         name: "envoy.resource_monitors.fake_resource1"
-        range {
-          min_value: 0.9
-          max_value: 0.9
+        scaled {
+          scaling_threshold: 0.9
+          saturation_threshold: 0.9
         }
       }
     }

From d0acf25833d46f39eb5d042349db8c3d8e76a577 Mon Sep 17 00:00:00 2001
From: Alex Konradi <akonradi@google.com>
Date: Mon, 10 Aug 2020 16:55:52 -0400
Subject: [PATCH 16/17] Fix min/max refs in docs

Gotta get the bonus points.

Signed-off-by: Alex Konradi <akonradi@google.com>
---
 .../operations/overload_manager/overload_manager.rst     | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/docs/root/configuration/operations/overload_manager/overload_manager.rst b/docs/root/configuration/operations/overload_manager/overload_manager.rst
index 17baa9b0e65e..8497f5292f32 100644
--- a/docs/root/configuration/operations/overload_manager/overload_manager.rst
+++ b/docs/root/configuration/operations/overload_manager/overload_manager.rst
@@ -54,9 +54,12 @@ Triggers connect resource monitors to actions. There are two types of triggers s
   * - :ref:`threshold <envoy_v3_api_msg_config.overload.v3.ThresholdTrigger>`
     - Sets the action state to 1 (= *saturated*) when the resource pressure is above a threshold, and to 0 otherwise.
   * - :ref:`scaled <envoy_v3_api_msg_config.overload.v3.ScaledTrigger>`
-    - Sets the action state to 0 when the resource pressure is below the min threshold,
-      `(pressure - min)/(max - min)` when `min < pressure < max`,
-      and to 1 (*saturated*) when the pressure is above the max."
+    - Sets the action state to 0 when the resource pressure is below the
+      :ref:`scaling_threshold <envoy_v3_api_field_config.overload.v3.ScaledTrigger.scaling_threshold>`,
+      `(pressure - scaling_threshold)/(saturation_threshold - scaling_threshold)` when
+      `scaling_threshold < pressure < saturation_threshold`, and to 1 (*saturated*) when the
+      pressure is above the
+      :ref:`saturation_threshold <envoy_v3_api_field_config.overload.v3.ScaledTrigger.saturation_threshold>`."
 
 Overload actions
 ----------------

From 111f31d4d74e5c6c94e8453e8f66b8b5632f3a88 Mon Sep 17 00:00:00 2001
From: Alex Konradi <akonradi@google.com>
Date: Tue, 11 Aug 2020 09:49:49 -0400
Subject: [PATCH 17/17] Address feedback

Signed-off-by: Alex Konradi <akonradi@google.com>
---
 source/server/overload_manager_impl.cc    |  4 ++--
 test/server/overload_manager_impl_test.cc | 10 ++++++----
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/source/server/overload_manager_impl.cc b/source/server/overload_manager_impl.cc
index 5055fba6e1cf..529bef09a162 100644
--- a/source/server/overload_manager_impl.cc
+++ b/source/server/overload_manager_impl.cc
@@ -45,7 +45,7 @@ class ScaledTriggerImpl final : public OverloadAction::Trigger {
         saturated_threshold_(config.saturation_threshold()),
         state_(OverloadActionState::inactive()) {
     if (scaling_threshold_ >= saturated_threshold_) {
-      throw EnvoyException("min_value must be less than max_value");
+      throw EnvoyException("scaling_threshold must be less than saturation_threshold");
     }
   }
 
@@ -278,7 +278,7 @@ void OverloadManagerImpl::updateResourcePressure(const std::string& resource, do
                     const auto state = action_it->second.getState();
 
                     if (old_state.isSaturated() != state.isSaturated()) {
-                      ENVOY_LOG(info, "Overload action {} became {}", action,
+                      ENVOY_LOG(debug, "Overload action {} became {}", action,
                                 (state.isSaturated() ? "saturated" : "scaling"));
                     }
                     tls_->runOnAllThreads([this, action, state] {
diff --git a/test/server/overload_manager_impl_test.cc b/test/server/overload_manager_impl_test.cc
index 3610fc89250f..4ccb043e1d8d 100644
--- a/test/server/overload_manager_impl_test.cc
+++ b/test/server/overload_manager_impl_test.cc
@@ -409,7 +409,8 @@ TEST_F(OverloadManagerImplTest, DuplicateOverloadAction) {
                           "Duplicate overload action .*");
 }
 
-TEST_F(OverloadManagerImplTest, ScaledTriggerMaxLessThanMin) {
+// A scaled trigger action's thresholds must conform to scaling < saturation.
+TEST_F(OverloadManagerImplTest, ScaledTriggerSaturationLessThanScalingThreshold) {
   const std::string config = R"EOF(
     resource_monitors {
       name: "envoy.resource_monitors.fake_resource1"
@@ -427,10 +428,11 @@ TEST_F(OverloadManagerImplTest, ScaledTriggerMaxLessThanMin) {
   )EOF";
 
   EXPECT_THROW_WITH_REGEX(createOverloadManager(config), EnvoyException,
-                          "min_value must be less than max_value.*");
+                          "scaling_threshold must be less than saturation_threshold.*");
 }
 
-TEST_F(OverloadManagerImplTest, ScaledTriggerMaxEqualsMin) {
+// A scaled trigger action can't have threshold values that are equal.
+TEST_F(OverloadManagerImplTest, ScaledTriggerThresholdsEqual) {
   const std::string config = R"EOF(
     resource_monitors {
       name: "envoy.resource_monitors.fake_resource1"
@@ -448,7 +450,7 @@ TEST_F(OverloadManagerImplTest, ScaledTriggerMaxEqualsMin) {
   )EOF";
 
   EXPECT_THROW_WITH_REGEX(createOverloadManager(config), EnvoyException,
-                          "min_value must be less than max_value.*");
+                          "scaling_threshold must be less than saturation_threshold.*");
 }
 
 TEST_F(OverloadManagerImplTest, UnknownTrigger) {