runtime: debug log that condition is always true when fractionalPerce…

…nt numerator > denominator (#12068) Signed-off-by: Konstantin Belyalov <[email protected]>
envoyproxy · Aug 13, 2020 · 466245b · 466245b
1 parent 8c312f2
commit 466245b
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 0 deletions.
diff --git a/source/common/runtime/runtime_impl.cc b/source/common/runtime/runtime_impl.cc
@@ -122,6 +122,18 @@ bool SnapshotImpl::featureEnabled(absl::string_view key,
     percent = default_value;
   }
 
+  // When numerator > denominator condition is always evaluates to TRUE
+  // It becomes hard to debug why configuration does not work in case of wrong numerator.
+  // Log debug message that numerator is invalid.
+  uint64_t denominator_value =
+      ProtobufPercentHelper::fractionalPercentDenominatorToInt(percent.denominator());
+  if (percent.numerator() > denominator_value) {
+    ENVOY_LOG(debug,
+              "WARNING runtime key '{}': numerator ({}) > denominator ({}), condition always "
+              "evaluates to true",
+              key, percent.numerator(), denominator_value);
+  }
+
   return ProtobufPercentHelper::evaluateFractionalPercent(percent, random_value);
 }
 

diff --git a/test/common/runtime/BUILD b/test/common/runtime/BUILD
@@ -57,6 +57,7 @@ envoy_cc_test(
         "//test/mocks/thread_local:thread_local_mocks",
         "//test/mocks/upstream:upstream_mocks",
         "//test/test_common:environment_lib",
+        "//test/test_common:logging_lib",
         "@envoy_api//envoy/config/bootstrap/v3:pkg_cc_proto",
         "@envoy_api//envoy/config/core/v3:pkg_cc_proto",
         "@envoy_api//envoy/service/discovery/v3:pkg_cc_proto",

diff --git a/test/common/runtime/runtime_impl_test.cc b/test/common/runtime/runtime_impl_test.cc
@@ -23,6 +23,7 @@
 #include "test/mocks/thread_local/mocks.h"
 #include "test/mocks/upstream/mocks.h"
 #include "test/test_common/environment.h"
+#include "test/test_common/logging.h"
 
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
@@ -684,6 +685,25 @@ TEST_F(StaticLoaderImplTest, ProtoParsing) {
   EXPECT_EQ(2, store_.gauge("runtime.num_layers", Stats::Gauge::ImportMode::NeverImport).value());
 }
 
+TEST_F(StaticLoaderImplTest, InvalidNumerator) {
+  base_ = TestUtility::parseYaml<ProtobufWkt::Struct>(R"EOF(
+    invalid_numerator:
+      numerator: 111
+      denominator: HUNDRED
+  )EOF");
+  setup();
+
+  envoy::type::v3::FractionalPercent fractional_percent;
+
+  // There is no assertion here - when numerator is invalid
+  // featureEnabled() will just drop debug log line.
+  EXPECT_CALL(generator_, random()).WillOnce(Return(500000));
+  EXPECT_LOG_CONTAINS("debug",
+                      "runtime key 'invalid_numerator': numerator (111) > denominator (100), "
+                      "condition always evaluates to true",
+                      loader_->snapshot().featureEnabled("invalid_numerator", fractional_percent));
+}
+
 TEST_F(StaticLoaderImplTest, RuntimeFromNonWorkerThreads) {
   // Force the thread to be considered a non-worker thread.
   tls_.registered_ = false;