access_logs: removing disallow_unbounded_access_logs (#14677)

Signed-off-by: Alyssa Wilk <[email protected]>
envoyproxy · Jan 13, 2021 · 1d01c75 · 1d01c75
1 parent c5718c3
commit 1d01c75
Show file tree

Hide file tree

Showing 5 changed files with 4 additions and 46 deletions.
diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst
@@ -26,6 +26,8 @@ Removed Config or Runtime
 -------------------------
 *Normally occurs at the end of the* :ref:`deprecation period <deprecated>`
 
+* access_logs: removed legacy unbounded access logs and runtime guard `envoy.reloadable_features.disallow_unbounded_access_logs`.
+
 New Features
 ------------
 * access log: added the :ref:`formatters <envoy_v3_api_field_config.core.v3.SubstitutionFormatString.formatters>` extension point for custom formatters (command operators).

diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc
@@ -64,7 +64,6 @@ constexpr const char* runtime_features[] = {
     "envoy.reloadable_features.consume_all_retry_headers",
     "envoy.reloadable_features.check_ocsp_policy",
     "envoy.reloadable_features.disable_tls_inspector_injection",
-    "envoy.reloadable_features.disallow_unbounded_access_logs",
     "envoy.reloadable_features.early_errors_via_hcm",
     "envoy.reloadable_features.enable_dns_cache_circuit_breakers",
     "envoy.reloadable_features.fix_upgrade_response",

diff --git a/source/extensions/access_loggers/common/BUILD b/source/extensions/access_loggers/common/BUILD
@@ -34,7 +34,6 @@ envoy_cc_library(
         "//source/common/common:assert_lib",
         "//source/common/grpc:typed_async_client_lib",
         "//source/common/protobuf:utility_lib",
-        "//source/common/runtime:runtime_features_lib",
         "@com_google_absl//absl/types:optional",
         "@envoy_api//envoy/config/core/v3:pkg_cc_proto",
     ],

diff --git a/source/extensions/access_loggers/common/grpc_access_logger.h b/source/extensions/access_loggers/common/grpc_access_logger.h
@@ -12,7 +12,6 @@
 #include "common/common/assert.h"
 #include "common/grpc/typed_async_client.h"
 #include "common/protobuf/utility.h"
-#include "common/runtime/runtime_features.h"
 
 #include "absl/container/flat_hash_map.h"
 #include "absl/types/optional.h"
@@ -248,13 +247,8 @@ class GrpcAccessLogger : public Detail::GrpcAccessLogger<HttpLogProto, TcpLogPro
       stats_.logs_written_.inc();
       return true;
     }
-    if (Runtime::runtimeFeatureEnabled(
-            "envoy.reloadable_features.disallow_unbounded_access_logs")) {
-      stats_.logs_dropped_.inc();
-      return false;
-    }
-    stats_.logs_written_.inc();
-    return true;
+    stats_.logs_dropped_.inc();
+    return false;
   }
 
   const std::chrono::milliseconds buffer_flush_interval_msec_;

diff --git a/test/extensions/access_loggers/common/grpc_access_logger_test.cc b/test/extensions/access_loggers/common/grpc_access_logger_test.cc
@@ -230,42 +230,6 @@ TEST_F(GrpcAccessLogTest, WatermarksOverrun) {
             TestUtility::findCounter(stats_store_, "mock_access_log_prefix.logs_dropped")->value());
 }
 
-// Test legacy behavior of unbounded access logs.
-TEST_F(GrpcAccessLogTest, WatermarksLegacy) {
-  TestScopedRuntime scoped_runtime;
-  Runtime::LoaderSingleton::getExisting()->mergeValues(
-      {{"envoy.reloadable_features.disallow_unbounded_access_logs", "false"}});
-
-  initLogger(FlushInterval, 1);
-
-  // Start a stream for the first log.
-  MockAccessLogStream stream;
-  AccessLogCallbacks* callbacks;
-  expectStreamStart(stream, &callbacks);
-
-  EXPECT_CALL(stream, isAboveWriteBufferHighWatermark())
-      .Times(AnyNumber())
-      .WillRepeatedly(Return(true));
-
-  // Fail to flush, so the log stays buffered up.
-  EXPECT_CALL(stream, sendMessageRaw_(_, false)).Times(0);
-  logger_->log(mockHttpEntry());
-  // Message should still be initialized.
-  EXPECT_EQ(1, logger_->numInits());
-  EXPECT_EQ(1,
-            TestUtility::findCounter(stats_store_, "mock_access_log_prefix.logs_written")->value());
-  EXPECT_EQ(0,
-            TestUtility::findCounter(stats_store_, "mock_access_log_prefix.logs_dropped")->value());
-
-  // As with the above test, try to log more. The log will not be dropped.
-  EXPECT_CALL(stream, sendMessageRaw_(_, _)).Times(0);
-  logger_->log(mockHttpEntry());
-  EXPECT_EQ(2,
-            TestUtility::findCounter(stats_store_, "mock_access_log_prefix.logs_written")->value());
-  EXPECT_EQ(0,
-            TestUtility::findCounter(stats_store_, "mock_access_log_prefix.logs_dropped")->value());
-}
-
 // Test that stream failure is handled correctly.
 TEST_F(GrpcAccessLogTest, StreamFailure) {
   initLogger(FlushInterval, 0);