runtime: envoy.reloadable_features.use_observable_cluster_name removal (

envoyproxy#19475) See envoyproxy#15139 ([cluster] Use alt_stat_name for general observability purposes (access log, tracing, admin)), which introduced a runtime guarded feature, which has been enabled by default for 6 months, so remove the old code path. Risk Level: Low Testing: n/a Docs Changes: updated Release Notes: Deprecate envoy.reloadable_features.use_observable_cluster_name. Platform Specific Features: n/a Signed-off-by: Loong <[email protected]> Signed-off-by: Josh Perry <[email protected]>
joshperry · Feb 13, 2022 · 9060f75 · 9060f75
1 parent 33cd39a
commit 9060f75
Show file tree

Hide file tree

Showing 7 changed files with 7 additions and 45 deletions.
diff --git a/api/envoy/config/cluster/v3/cluster.proto b/api/envoy/config/cluster/v3/cluster.proto
@@ -735,11 +735,9 @@ message Cluster {
   // emitting stats for the cluster and access logging the cluster name. This will appear as
   // additional information in configuration dumps of a cluster's current status as
   // :ref:`observability_name <envoy_v3_api_field_admin.v3.ClusterStatus.observability_name>`
-  // and as an additional tag "upstream_cluster.name" while tracing. Note: access logging using
-  // this field is presently enabled with runtime feature
-  // `envoy.reloadable_features.use_observable_cluster_name`. Any ``:`` in the name will be
-  // converted to ``_`` when emitting statistics. This should not be confused with :ref:`Router
-  // Filter Header <config_http_filters_router_x-envoy-upstream-alt-stat-name>`.
+  // and as an additional tag "upstream_cluster.name" while tracing. Note: Any ``:`` in the name
+  // will be converted to ``_`` when emitting statistics. This should not be confused with
+  // :ref:`Router Filter Header <config_http_filters_router_x-envoy-upstream-alt-stat-name>`.
   string alt_stat_name = 28 [(udpa.annotations.field_migrate).rename = "observability_name"];
 
   oneof cluster_discovery_type {

diff --git a/docs/root/configuration/observability/access_log/usage.rst b/docs/root/configuration/observability/access_log/usage.rst
@@ -406,8 +406,7 @@ The following command operators are supported:
   Upstream host URL (e.g., tcp://ip:port for TCP connections).
 
 %UPSTREAM_CLUSTER%
-  Upstream cluster to which the upstream host belongs to. If runtime feature
-  ``envoy.reloadable_features.use_observable_cluster_name`` is enabled, then :ref:`alt_stat_name
+  Upstream cluster to which the upstream host belongs to. :ref:`alt_stat_name
   <envoy_v3_api_field_config.cluster.v3.Cluster.alt_stat_name>` will be used if provided.
 
 %UPSTREAM_LOCAL_ADDRESS%

diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst
@@ -27,6 +27,7 @@ Removed Config or Runtime
 * http: removed ``envoy.reloadable_features.hash_multiple_header_values`` and legacy code paths.
 * http: removed ``envoy.reloadable_features.require_strict_1xx_and_204_response_headers`` and ``envoy.reloadable_features.send_strict_1xx_and_204_response_headers`` and legacy code paths.
 * http: removed ``envoy.reloadable_features.strip_port_from_connect`` and legacy code paths.
+* http: removed ``envoy.reloadable_features.use_observable_cluster_name`` and legacy code paths.
 
 
 New Features

diff --git a/source/common/formatter/substitution_formatter.cc b/source/common/formatter/substitution_formatter.cc
@@ -856,13 +856,8 @@ const StreamInfoFormatter::FieldExtractorLookupTbl& StreamInfoFormatter::getKnow
                 std::string upstream_cluster_name;
                 if (stream_info.upstreamClusterInfo().has_value() &&
                     stream_info.upstreamClusterInfo().value() != nullptr) {
-                  if (Runtime::runtimeFeatureEnabled(
-                          "envoy.reloadable_features.use_observable_cluster_name")) {
-                    upstream_cluster_name =
-                        stream_info.upstreamClusterInfo().value()->observabilityName();
-                  } else {
-                    upstream_cluster_name = stream_info.upstreamClusterInfo().value()->name();
-                  }
+                  upstream_cluster_name =
+                      stream_info.upstreamClusterInfo().value()->observabilityName();
                 }
 
                 return upstream_cluster_name.empty()

diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc
@@ -87,7 +87,6 @@ constexpr const char* runtime_features[] = {
     "envoy.reloadable_features.unquote_log_string_values",
     "envoy.reloadable_features.update_expected_rq_timeout_on_retry",
     "envoy.reloadable_features.use_dns_ttl",
-    "envoy.reloadable_features.use_observable_cluster_name",
     "envoy.reloadable_features.validate_connect",
     "envoy.reloadable_features.vhds_heartbeats",
     "envoy.restart_features.explicit_wildcard_resource",

diff --git a/test/common/formatter/substitution_formatter_test.cc b/test/common/formatter/substitution_formatter_test.cc
@@ -539,23 +539,6 @@ TEST(SubstitutionFormatterTest, streamInfoFormatter) {
                 ProtoEq(ValueUtil::stringValue("observability_name")));
   }
 
-  {
-    TestScopedRuntime scoped_runtime;
-    Runtime::LoaderSingleton::getExisting()->mergeValues(
-        {{"envoy.reloadable_features.use_observable_cluster_name", "false"}});
-    StreamInfoFormatter upstream_format("UPSTREAM_CLUSTER");
-    const std::string upstream_cluster_name = "cluster_name";
-    auto cluster_info_mock = std::make_shared<Upstream::MockClusterInfo>();
-    absl::optional<Upstream::ClusterInfoConstSharedPtr> cluster_info = cluster_info_mock;
-    EXPECT_CALL(stream_info, upstreamClusterInfo()).WillRepeatedly(Return(cluster_info));
-    EXPECT_CALL(*cluster_info_mock, name()).WillRepeatedly(ReturnRef(upstream_cluster_name));
-    EXPECT_EQ("cluster_name", upstream_format.format(request_headers, response_headers,
-                                                     response_trailers, stream_info, body));
-    EXPECT_THAT(upstream_format.formatValue(request_headers, response_headers, response_trailers,
-                                            stream_info, body),
-                ProtoEq(ValueUtil::stringValue("cluster_name")));
-  }
-
   {
     StreamInfoFormatter upstream_format("UPSTREAM_CLUSTER");
     absl::optional<Upstream::ClusterInfoConstSharedPtr> cluster_info = nullptr;

diff --git a/test/common/tcp_proxy/tcp_proxy_test.cc b/test/common/tcp_proxy/tcp_proxy_test.cc
@@ -852,19 +852,6 @@ TEST_F(TcpProxyTest, AccessLogUpstreamHost) {
   EXPECT_EQ(access_log_data_, "127.0.0.1:80 observability_name");
 }
 
-// Test that access log fields %UPSTREAM_HOST% and %UPSTREAM_CLUSTER% are correctly logged with the
-// cluster name.
-TEST_F(TcpProxyTest, AccessLogUpstreamHostLegacyName) {
-  TestScopedRuntime scoped_runtime;
-  Runtime::LoaderSingleton::getExisting()->mergeValues(
-      {{"envoy.reloadable_features.use_observable_cluster_name", "false"}});
-  setup(1, accessLogConfig("%UPSTREAM_HOST% %UPSTREAM_CLUSTER%"));
-  raiseEventUpstreamConnected(0);
-  filter_callbacks_.connection_.raiseEvent(Network::ConnectionEvent::RemoteClose);
-  filter_.reset();
-  EXPECT_EQ(access_log_data_, "127.0.0.1:80 fake_cluster");
-}
-
 // Test that access log field %UPSTREAM_LOCAL_ADDRESS% is correctly logged.
 TEST_F(TcpProxyTest, AccessLogUpstreamLocalAddress) {
   setup(1, accessLogConfig("%UPSTREAM_LOCAL_ADDRESS%"));