diff --git a/api/envoy/admin/v3/clusters.proto b/api/envoy/admin/v3/clusters.proto index 7a5e83c9aac4..b526fa31460f 100644 --- a/api/envoy/admin/v3/clusters.proto +++ b/api/envoy/admin/v3/clusters.proto @@ -29,7 +29,7 @@ message Clusters { } // Details an individual cluster's current status. -// [#next-free-field: 7] +// [#next-free-field: 8] message ClusterStatus { option (udpa.annotations.versioning).previous_message_type = "envoy.admin.v2alpha.ClusterStatus"; @@ -80,6 +80,9 @@ message ClusterStatus { // :ref:`Circuit breaking ` settings of the cluster. config.cluster.v3.CircuitBreakers circuit_breakers = 6; + + // Observability name of the cluster. + string observability_name = 7; } // Current state of a particular host. diff --git a/api/envoy/admin/v4alpha/clusters.proto b/api/envoy/admin/v4alpha/clusters.proto index cc4525576fb1..116dc226c25e 100644 --- a/api/envoy/admin/v4alpha/clusters.proto +++ b/api/envoy/admin/v4alpha/clusters.proto @@ -29,7 +29,7 @@ message Clusters { } // Details an individual cluster's current status. -// [#next-free-field: 7] +// [#next-free-field: 8] message ClusterStatus { option (udpa.annotations.versioning).previous_message_type = "envoy.admin.v3.ClusterStatus"; @@ -80,6 +80,9 @@ message ClusterStatus { // :ref:`Circuit breaking ` settings of the cluster. config.cluster.v4alpha.CircuitBreakers circuit_breakers = 6; + + // Observability name of the cluster. + string observability_name = 7; } // Current state of a particular host. diff --git a/api/envoy/config/cluster/v3/cluster.proto b/api/envoy/config/cluster/v3/cluster.proto index 6b89257cb23a..efc08ae0700a 100644 --- a/api/envoy/config/cluster/v3/cluster.proto +++ b/api/envoy/config/cluster/v3/cluster.proto @@ -21,6 +21,7 @@ import "google/protobuf/wrappers.proto"; import "xds/core/v3/collection_entry.proto"; +import "udpa/annotations/migrate.proto"; import "udpa/annotations/security.proto"; import "udpa/annotations/status.proto"; import "udpa/annotations/versioning.proto"; @@ -683,11 +684,16 @@ message Cluster { // Any ``:`` in the cluster name will be converted to ``_`` when emitting statistics. string name = 1 [(validate.rules).string = {min_len: 1}]; - // An optional alternative to the cluster name to be used while emitting stats. - // Any ``:`` in the name will be converted to ``_`` when emitting statistics. This should not be - // confused with :ref:`Router Filter Header - // `. - string alt_stat_name = 28; + // An optional alternative to the cluster name to be used for observability. This name is used + // 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 ` + // 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 `. + string alt_stat_name = 28 [(udpa.annotations.field_migrate).rename = "observability_name"]; oneof cluster_discovery_type { // The :ref:`service discovery type ` diff --git a/api/envoy/config/cluster/v4alpha/cluster.proto b/api/envoy/config/cluster/v4alpha/cluster.proto index 062123fc4206..927bbddaba5e 100644 --- a/api/envoy/config/cluster/v4alpha/cluster.proto +++ b/api/envoy/config/cluster/v4alpha/cluster.proto @@ -694,11 +694,16 @@ message Cluster { // Any ``:`` in the cluster name will be converted to ``_`` when emitting statistics. string name = 1 [(validate.rules).string = {min_len: 1}]; - // An optional alternative to the cluster name to be used while emitting stats. - // Any ``:`` in the name will be converted to ``_`` when emitting statistics. This should not be - // confused with :ref:`Router Filter Header - // `. - string alt_stat_name = 28; + // An optional alternative to the cluster name to be used for observability. This name is used + // 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 ` + // 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 `. + string observability_name = 28; oneof cluster_discovery_type { // The :ref:`service discovery type ` diff --git a/docs/root/configuration/observability/access_log/usage.rst b/docs/root/configuration/observability/access_log/usage.rst index 2005e054a5fa..a4227a7ce429 100644 --- a/docs/root/configuration/observability/access_log/usage.rst +++ b/docs/root/configuration/observability/access_log/usage.rst @@ -318,8 +318,9 @@ The following command operators are supported: %UPSTREAM_HOST% Upstream host URL (e.g., tcp://ip:port for TCP connections). -%UPSTREAM_CLUSTER% - Upstream cluster to which the upstream host belongs to. +%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 + ` will be used if provided. %UPSTREAM_LOCAL_ADDRESS% Local address of the upstream connection. If the address is an IP address it includes both diff --git a/docs/root/intro/arch_overview/observability/tracing.rst b/docs/root/intro/arch_overview/observability/tracing.rst index bfddeb924a02..b29fec56b345 100644 --- a/docs/root/intro/arch_overview/observability/tracing.rst +++ b/docs/root/intro/arch_overview/observability/tracing.rst @@ -100,7 +100,7 @@ associated with it. Each span generated by Envoy contains the following data: * HTTP request URL, method, protocol and user-agent. * Additional custom tags set via :ref:`custom_tags `. -* Upstream cluster name and address. +* Upstream cluster name, observability name, and address. * HTTP response status code. * GRPC response status and message (if available). * An error tag when HTTP status is 5xx or GRPC status is not "OK". diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index 6aa9e13dfd5b..b22784f194d9 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -12,6 +12,8 @@ Minor Behavior Changes ---------------------- *Changes that may cause incompatibilities for some users, but should not for most* +* access_logs: change command operator %UPSTREAM_CLUSTER% to resolve to :ref:`alt_stat_name ` if provided. This behavior can be reverted by disabling the runtime feature `envoy.reloadable_features.use_observable_cluster_name`. +* admin: added :ref:`observability_name ` information to GET /clusters?format=json :ref:`cluster status `. * dns: both the :ref:`strict DNS ` and :ref:`logical DNS ` cluster types now honor the :ref:`hostname ` field if not empty. @@ -47,6 +49,7 @@ Minor Behavior Changes * oauth filter: added the optional parameter :ref:`auth_scopes ` with default value of 'user' if not provided. Enables this value to be overridden in the Authorization request to the OAuth provider. * perf: allow reading more bytes per operation from raw sockets to improve performance. * router: extended custom date formatting to DOWNSTREAM_PEER_CERT_V_START and DOWNSTREAM_PEER_CERT_V_END when using :ref:`custom request/response header formats `. +* tracing: added `upstream_cluster.name` tag that resolves to resolve to :ref:`alt_stat_name ` if provided (and otherwise the cluster name). * upstream: host weight changes now cause a full load balancer rebuild as opposed to happening atomically inline. This change has been made to support load balancer pre-computation of data structures based on host weight, but may have performance implications if host weight changes diff --git a/generated_api_shadow/envoy/admin/v3/clusters.proto b/generated_api_shadow/envoy/admin/v3/clusters.proto index 7a5e83c9aac4..b526fa31460f 100644 --- a/generated_api_shadow/envoy/admin/v3/clusters.proto +++ b/generated_api_shadow/envoy/admin/v3/clusters.proto @@ -29,7 +29,7 @@ message Clusters { } // Details an individual cluster's current status. -// [#next-free-field: 7] +// [#next-free-field: 8] message ClusterStatus { option (udpa.annotations.versioning).previous_message_type = "envoy.admin.v2alpha.ClusterStatus"; @@ -80,6 +80,9 @@ message ClusterStatus { // :ref:`Circuit breaking ` settings of the cluster. config.cluster.v3.CircuitBreakers circuit_breakers = 6; + + // Observability name of the cluster. + string observability_name = 7; } // Current state of a particular host. diff --git a/generated_api_shadow/envoy/admin/v4alpha/clusters.proto b/generated_api_shadow/envoy/admin/v4alpha/clusters.proto index cc4525576fb1..116dc226c25e 100644 --- a/generated_api_shadow/envoy/admin/v4alpha/clusters.proto +++ b/generated_api_shadow/envoy/admin/v4alpha/clusters.proto @@ -29,7 +29,7 @@ message Clusters { } // Details an individual cluster's current status. -// [#next-free-field: 7] +// [#next-free-field: 8] message ClusterStatus { option (udpa.annotations.versioning).previous_message_type = "envoy.admin.v3.ClusterStatus"; @@ -80,6 +80,9 @@ message ClusterStatus { // :ref:`Circuit breaking ` settings of the cluster. config.cluster.v4alpha.CircuitBreakers circuit_breakers = 6; + + // Observability name of the cluster. + string observability_name = 7; } // Current state of a particular host. diff --git a/generated_api_shadow/envoy/config/cluster/v3/cluster.proto b/generated_api_shadow/envoy/config/cluster/v3/cluster.proto index e47d3bf00e11..ef6801a300d9 100644 --- a/generated_api_shadow/envoy/config/cluster/v3/cluster.proto +++ b/generated_api_shadow/envoy/config/cluster/v3/cluster.proto @@ -23,6 +23,7 @@ import "google/protobuf/wrappers.proto"; import "xds/core/v3/collection_entry.proto"; import "envoy/annotations/deprecation.proto"; +import "udpa/annotations/migrate.proto"; import "udpa/annotations/security.proto"; import "udpa/annotations/status.proto"; import "udpa/annotations/versioning.proto"; @@ -682,11 +683,16 @@ message Cluster { // Any ``:`` in the cluster name will be converted to ``_`` when emitting statistics. string name = 1 [(validate.rules).string = {min_len: 1}]; - // An optional alternative to the cluster name to be used while emitting stats. - // Any ``:`` in the name will be converted to ``_`` when emitting statistics. This should not be - // confused with :ref:`Router Filter Header - // `. - string alt_stat_name = 28; + // An optional alternative to the cluster name to be used for observability. This name is used + // 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 ` + // 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 `. + string alt_stat_name = 28 [(udpa.annotations.field_migrate).rename = "observability_name"]; oneof cluster_discovery_type { // The :ref:`service discovery type ` diff --git a/generated_api_shadow/envoy/config/cluster/v4alpha/cluster.proto b/generated_api_shadow/envoy/config/cluster/v4alpha/cluster.proto index ddac03683118..4cd136d07dd6 100644 --- a/generated_api_shadow/envoy/config/cluster/v4alpha/cluster.proto +++ b/generated_api_shadow/envoy/config/cluster/v4alpha/cluster.proto @@ -693,11 +693,16 @@ message Cluster { // Any ``:`` in the cluster name will be converted to ``_`` when emitting statistics. string name = 1 [(validate.rules).string = {min_len: 1}]; - // An optional alternative to the cluster name to be used while emitting stats. - // Any ``:`` in the name will be converted to ``_`` when emitting statistics. This should not be - // confused with :ref:`Router Filter Header - // `. - string alt_stat_name = 28; + // An optional alternative to the cluster name to be used for observability. This name is used + // 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 ` + // 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 `. + string observability_name = 28; oneof cluster_discovery_type { // The :ref:`service discovery type ` diff --git a/include/envoy/upstream/upstream.h b/include/envoy/upstream/upstream.h index 1315920465c7..17be6d1b5a0b 100644 --- a/include/envoy/upstream/upstream.h +++ b/include/envoy/upstream/upstream.h @@ -861,6 +861,14 @@ class ClusterInfo { */ virtual const std::string& name() const PURE; + /** + * @return the observability name associated to the cluster. Used in stats, tracing, logging, and + * config dumps. The observability name is configured with :ref:`alt_stat_name + * `. If unprovided, the default value is + * the cluster name. + */ + virtual const std::string& observabilityName() const PURE; + /** * @return ResourceManager& the resource manager to use by proxy agents for this cluster (at * a particular priority). diff --git a/source/common/formatter/BUILD b/source/common/formatter/BUILD index c333858b22e1..3acb8213a463 100644 --- a/source/common/formatter/BUILD +++ b/source/common/formatter/BUILD @@ -16,6 +16,7 @@ envoy_cc_library( deps = [ "//include/envoy/api:api_interface", "//include/envoy/formatter:substitution_formatter_interface", + "//include/envoy/runtime:runtime_interface", "//include/envoy/stream_info:stream_info_interface", "//include/envoy/upstream:upstream_interface", "//source/common/common:assert_lib", @@ -25,6 +26,7 @@ envoy_cc_library( "//source/common/grpc:common_lib", "//source/common/http:utility_lib", "//source/common/protobuf:message_validator_lib", + "//source/common/runtime:runtime_features_lib", "//source/common/stream_info:utility_lib", "@envoy_api//envoy/config/core/v3:pkg_cc_proto", ], diff --git a/source/common/formatter/substitution_formatter.cc b/source/common/formatter/substitution_formatter.cc index c264c23aec83..1d7b4138b573 100644 --- a/source/common/formatter/substitution_formatter.cc +++ b/source/common/formatter/substitution_formatter.cc @@ -20,6 +20,7 @@ #include "common/http/utility.h" #include "common/protobuf/message_validator_impl.h" #include "common/protobuf/utility.h" +#include "common/runtime/runtime_features.h" #include "common/stream_info/utility.h" #include "absl/strings/str_split.h" @@ -756,7 +757,13 @@ StreamInfoFormatter::StreamInfoFormatter(const std::string& field_name) { std::string upstream_cluster_name; if (stream_info.upstreamClusterInfo().has_value() && stream_info.upstreamClusterInfo().value() != nullptr) { - upstream_cluster_name = stream_info.upstreamClusterInfo().value()->name(); + 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(); + } } return upstream_cluster_name.empty() diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index 758980ca1c56..4d7d0ac79369 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -86,6 +86,7 @@ constexpr const char* runtime_features[] = { "envoy.reloadable_features.treat_host_like_authority", "envoy.reloadable_features.treat_upstream_connect_timeout_as_connect_failure", "envoy.reloadable_features.upstream_host_weight_change_causes_rebuild", + "envoy.reloadable_features.use_observable_cluster_name", "envoy.reloadable_features.vhds_heartbeats", "envoy.reloadable_features.unify_grpc_handling", "envoy.reloadable_features.upstream_http2_flood_checks", diff --git a/source/common/tracing/http_tracer_impl.cc b/source/common/tracing/http_tracer_impl.cc index 8d2127a33544..cb721b4d98fa 100644 --- a/source/common/tracing/http_tracer_impl.cc +++ b/source/common/tracing/http_tracer_impl.cc @@ -235,6 +235,8 @@ void HttpTracerUtility::setCommonTags(Span& span, const Http::ResponseHeaderMap* if (nullptr != stream_info.upstreamHost()) { span.setTag(Tracing::Tags::get().UpstreamCluster, stream_info.upstreamHost()->cluster().name()); + span.setTag(Tracing::Tags::get().UpstreamClusterName, + stream_info.upstreamHost()->cluster().observabilityName()); } // Post response data. diff --git a/source/common/tracing/http_tracer_impl.h b/source/common/tracing/http_tracer_impl.h index 7ebe45655d41..8495a991b9fa 100644 --- a/source/common/tracing/http_tracer_impl.h +++ b/source/common/tracing/http_tracer_impl.h @@ -65,6 +65,7 @@ class TracingTagValues { const std::string Status = "status"; const std::string UpstreamAddress = "upstream_address"; const std::string UpstreamCluster = "upstream_cluster"; + const std::string UpstreamClusterName = "upstream_cluster.name"; const std::string UserAgent = "user_agent"; const std::string Zone = "zone"; diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc index 38456f70b15f..33f3043fa000 100644 --- a/source/common/upstream/upstream_impl.cc +++ b/source/common/upstream/upstream_impl.cc @@ -703,7 +703,9 @@ ClusterInfoImpl::ClusterInfoImpl( const envoy::config::core::v3::BindConfig& bind_config, Runtime::Loader& runtime, TransportSocketMatcherPtr&& socket_matcher, Stats::ScopePtr&& stats_scope, bool added_via_api, Server::Configuration::TransportSocketFactoryContext& factory_context) - : runtime_(runtime), name_(config.name()), type_(config.type()), + : runtime_(runtime), name_(config.name()), + observability_name_(PROTOBUF_GET_STRING_OR_DEFAULT(config, alt_stat_name, name_)), + type_(config.type()), extension_protocol_options_(parseExtensionProtocolOptions(config, factory_context)), http_protocol_options_( createOptions(config, extensionProtocolOptionsTyped( diff --git a/source/common/upstream/upstream_impl.h b/source/common/upstream/upstream_impl.h index afad91dc8ec4..3510313adc7d 100644 --- a/source/common/upstream/upstream_impl.h +++ b/source/common/upstream/upstream_impl.h @@ -596,6 +596,7 @@ class ClusterInfoImpl : public ClusterInfo, uint64_t maxRequestsPerConnection() const override { return max_requests_per_connection_; } uint32_t maxResponseHeadersCount() const override { return max_response_headers_count_; } const std::string& name() const override { return name_; } + const std::string& observabilityName() const override { return observability_name_; } ResourceManager& resourceManager(ResourcePriority priority) const override; TransportSocketMatcher& transportSocketMatcher() const override { return *socket_matcher_; } ClusterStats& stats() const override { return stats_; } @@ -682,6 +683,7 @@ class ClusterInfoImpl : public ClusterInfo, Runtime::Loader& runtime_; const std::string name_; + const std::string observability_name_; const envoy::config::cluster::v3::Cluster::DiscoveryType type_; const absl::flat_hash_map extension_protocol_options_; diff --git a/source/server/admin/clusters_handler.cc b/source/server/admin/clusters_handler.cc index 0e2149c53d31..77523012e0dc 100644 --- a/source/server/admin/clusters_handler.cc +++ b/source/server/admin/clusters_handler.cc @@ -117,6 +117,7 @@ void ClustersHandler::writeClustersAsJson(Buffer::Instance& response) { envoy::admin::v3::ClusterStatus& cluster_status = *clusters.add_cluster_statuses(); cluster_status.set_name(cluster_info->name()); + cluster_status.set_observability_name(cluster_info->observabilityName()); addCircuitBreakerSettingsAsJson( envoy::config::core::v3::RoutingPriority::DEFAULT, @@ -201,6 +202,8 @@ void ClustersHandler::writeClustersAsText(Buffer::Instance& response) { UNREFERENCED_PARAMETER(name); const Upstream::Cluster& cluster = cluster_ref.get(); const std::string& cluster_name = cluster.info()->name(); + response.add(fmt::format("{}::observability_name::{}\n", cluster_name, + cluster.info()->observabilityName())); addOutlierInfo(cluster_name, cluster.outlierDetector(), response); addCircuitBreakerSettingsAsText( diff --git a/test/common/formatter/BUILD b/test/common/formatter/BUILD index 316c58e02fa0..d9b213e1eee0 100644 --- a/test/common/formatter/BUILD +++ b/test/common/formatter/BUILD @@ -60,6 +60,7 @@ envoy_cc_test( "//test/mocks/ssl:ssl_mocks", "//test/mocks/stream_info:stream_info_mocks", "//test/mocks/upstream:cluster_info_mocks", + "//test/test_common:test_runtime_lib", "//test/test_common:threadsafe_singleton_injector_lib", "//test/test_common:utility_lib", "@envoy_api//envoy/config/core/v3:pkg_cc_proto", diff --git a/test/common/formatter/substitution_formatter_test.cc b/test/common/formatter/substitution_formatter_test.cc index 64584ca98086..9811ab4d2a00 100644 --- a/test/common/formatter/substitution_formatter_test.cc +++ b/test/common/formatter/substitution_formatter_test.cc @@ -22,6 +22,7 @@ #include "test/mocks/stream_info/mocks.h" #include "test/mocks/upstream/cluster_info.h" #include "test/test_common/printers.h" +#include "test/test_common/test_runtime.h" #include "test/test_common/threadsafe_singleton_injector.h" #include "test/test_common/utility.h" @@ -345,6 +346,26 @@ TEST(SubstitutionFormatterTest, streamInfoFormatter) { { StreamInfoFormatter upstream_format("UPSTREAM_CLUSTER"); + const std::string observable_cluster_name = "observability_name"; + auto cluster_info_mock = std::make_shared(); + absl::optional cluster_info = cluster_info_mock; + // Make sure that cluster info is obtained without calling upstreamHost. + EXPECT_CALL(stream_info, upstreamHost()).Times(0); + EXPECT_CALL(stream_info, upstreamClusterInfo()).WillRepeatedly(Return(cluster_info)); + EXPECT_CALL(*cluster_info_mock, observabilityName()) + .WillRepeatedly(ReturnRef(observable_cluster_name)); + EXPECT_EQ("observability_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("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(); absl::optional cluster_info = cluster_info_mock; diff --git a/test/common/http/async_client_impl_test.cc b/test/common/http/async_client_impl_test.cc index c9fbc8abf617..b76107d395e2 100644 --- a/test/common/http/async_client_impl_test.cc +++ b/test/common/http/async_client_impl_test.cc @@ -234,6 +234,8 @@ TEST_F(AsyncClientImplTracingTest, Basic) { EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().HttpProtocol), Eq("HTTP/1.1"))); EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().UpstreamAddress), Eq("10.0.0.1:443"))); EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().UpstreamCluster), Eq("fake_cluster"))); + EXPECT_CALL(*child_span, + setTag(Eq(Tracing::Tags::get().UpstreamClusterName), Eq("observability_name"))); EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().HttpStatusCode), Eq("200"))); EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().ResponseFlags), Eq("-"))); EXPECT_CALL(*child_span, finishSpan()); @@ -284,6 +286,8 @@ TEST_F(AsyncClientImplTracingTest, BasicNamedChildSpan) { EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().HttpProtocol), Eq("HTTP/1.1"))); EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().UpstreamAddress), Eq("10.0.0.1:443"))); EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().UpstreamCluster), Eq("fake_cluster"))); + EXPECT_CALL(*child_span, + setTag(Eq(Tracing::Tags::get().UpstreamClusterName), Eq("observability_name"))); EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().HttpStatusCode), Eq("200"))); EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().ResponseFlags), Eq("-"))); EXPECT_CALL(*child_span, finishSpan()); @@ -1018,6 +1022,8 @@ TEST_F(AsyncClientImplTracingTest, CancelRequest) { EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().HttpProtocol), Eq("HTTP/1.1"))); EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().UpstreamAddress), Eq("10.0.0.1:443"))); EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().UpstreamCluster), Eq("fake_cluster"))); + EXPECT_CALL(*child_span, + setTag(Eq(Tracing::Tags::get().UpstreamClusterName), Eq("observability_name"))); EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().HttpStatusCode), Eq("0"))); EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().ResponseFlags), Eq("-"))); EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().Error), Eq(Tracing::Tags::get().True))); @@ -1103,6 +1109,8 @@ TEST_F(AsyncClientImplTracingTest, DestroyWithActiveRequest) { EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().HttpProtocol), Eq("HTTP/1.1"))); EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().UpstreamAddress), Eq("10.0.0.1:443"))); EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().UpstreamCluster), Eq("fake_cluster"))); + EXPECT_CALL(*child_span, + setTag(Eq(Tracing::Tags::get().UpstreamClusterName), Eq("observability_name"))); EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().HttpStatusCode), Eq("0"))); EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().ResponseFlags), Eq("-"))); EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().Error), Eq(Tracing::Tags::get().True))) @@ -1288,6 +1296,8 @@ TEST_F(AsyncClientImplTracingTest, RequestTimeout) { EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().HttpProtocol), Eq("HTTP/1.1"))); EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().UpstreamAddress), Eq("10.0.0.1:443"))); EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().UpstreamCluster), Eq("fake_cluster"))); + EXPECT_CALL(*child_span, + setTag(Eq(Tracing::Tags::get().UpstreamClusterName), Eq("observability_name"))); EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().HttpStatusCode), Eq("504"))); EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().ResponseFlags), Eq("UT"))); EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().Error), Eq(Tracing::Tags::get().True))) diff --git a/test/common/router/router_2_test.cc b/test/common/router/router_2_test.cc index 34ecfdbdda52..450683b9410a 100644 --- a/test/common/router/router_2_test.cc +++ b/test/common/router/router_2_test.cc @@ -346,6 +346,8 @@ TEST_F(RouterTestChildSpan, BasicFlow) { EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().HttpProtocol), Eq("HTTP/1.0"))); EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().UpstreamAddress), Eq("10.0.0.5:9211"))); EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().UpstreamCluster), Eq("fake_cluster"))); + EXPECT_CALL(*child_span, + setTag(Eq(Tracing::Tags::get().UpstreamClusterName), Eq("observability_name"))); EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().HttpStatusCode), Eq("200"))); EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().ResponseFlags), Eq("-"))); EXPECT_CALL(*child_span, finishSpan()); @@ -395,6 +397,8 @@ TEST_F(RouterTestChildSpan, ResetFlow) { EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().HttpProtocol), Eq("HTTP/1.0"))); EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().UpstreamAddress), Eq("10.0.0.5:9211"))); EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().UpstreamCluster), Eq("fake_cluster"))); + EXPECT_CALL(*child_span, + setTag(Eq(Tracing::Tags::get().UpstreamClusterName), Eq("observability_name"))); EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().HttpStatusCode), Eq("200"))); EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().ResponseFlags), Eq("UR"))); EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().Error), Eq(Tracing::Tags::get().True))); @@ -438,6 +442,8 @@ TEST_F(RouterTestChildSpan, CancelFlow) { EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().HttpProtocol), Eq("HTTP/1.0"))); EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().UpstreamAddress), Eq("10.0.0.5:9211"))); EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().UpstreamCluster), Eq("fake_cluster"))); + EXPECT_CALL(*child_span, + setTag(Eq(Tracing::Tags::get().UpstreamClusterName), Eq("observability_name"))); EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().HttpStatusCode), Eq("0"))); EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().ResponseFlags), Eq("-"))); EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().Error), Eq(Tracing::Tags::get().True))); @@ -482,6 +488,8 @@ TEST_F(RouterTestChildSpan, ResetRetryFlow) { EXPECT_CALL(*child_span_1, setTag(Eq(Tracing::Tags::get().HttpProtocol), Eq("HTTP/1.0"))); EXPECT_CALL(*child_span_1, setTag(Eq(Tracing::Tags::get().UpstreamAddress), Eq("10.0.0.5:9211"))); EXPECT_CALL(*child_span_1, setTag(Eq(Tracing::Tags::get().UpstreamCluster), Eq("fake_cluster"))); + EXPECT_CALL(*child_span_1, + setTag(Eq(Tracing::Tags::get().UpstreamClusterName), Eq("observability_name"))); EXPECT_CALL(*child_span_1, setTag(Eq(Tracing::Tags::get().HttpStatusCode), Eq("0"))); EXPECT_CALL(*child_span_1, setTag(Eq(Tracing::Tags::get().ResponseFlags), Eq("UR"))); EXPECT_CALL(*child_span_1, setTag(Eq(Tracing::Tags::get().Error), Eq(Tracing::Tags::get().True))) @@ -524,6 +532,8 @@ TEST_F(RouterTestChildSpan, ResetRetryFlow) { EXPECT_CALL(*child_span_2, setTag(Eq(Tracing::Tags::get().HttpProtocol), Eq("HTTP/1.0"))); EXPECT_CALL(*child_span_2, setTag(Eq(Tracing::Tags::get().UpstreamAddress), Eq("10.0.0.5:9211"))); EXPECT_CALL(*child_span_2, setTag(Eq(Tracing::Tags::get().UpstreamCluster), Eq("fake_cluster"))); + EXPECT_CALL(*child_span_2, + setTag(Eq(Tracing::Tags::get().UpstreamClusterName), Eq("observability_name"))); EXPECT_CALL(*child_span_2, setTag(Eq(Tracing::Tags::get().HttpStatusCode), Eq("200"))); EXPECT_CALL(*child_span_2, setTag(Eq(Tracing::Tags::get().ResponseFlags), Eq("-"))); EXPECT_CALL(*child_span_2, finishSpan()); diff --git a/test/common/tcp_proxy/tcp_proxy_test.cc b/test/common/tcp_proxy/tcp_proxy_test.cc index c84be573a861..7f41807e5dfd 100644 --- a/test/common/tcp_proxy/tcp_proxy_test.cc +++ b/test/common/tcp_proxy/tcp_proxy_test.cc @@ -1756,12 +1756,26 @@ TEST_F(TcpProxyTest, IdleTimeoutWithOutstandingDataFlushed) { idle_timer->invokeCallback(); } -// Test that access log fields %UPSTREAM_HOST% and %UPSTREAM_CLUSTER% are correctly logged. +// Test that access log fields %UPSTREAM_HOST% and %UPSTREAM_CLUSTER% are correctly logged with the +// observability name. TEST_F(TcpProxyTest, AccessLogUpstreamHost) { 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 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"); } diff --git a/test/common/tracing/http_tracer_impl_test.cc b/test/common/tracing/http_tracer_impl_test.cc index b5a8a0df4970..8097069bbd77 100644 --- a/test/common/tracing/http_tracer_impl_test.cc +++ b/test/common/tracing/http_tracer_impl_test.cc @@ -261,6 +261,7 @@ TEST_F(HttpConnManFinalizerImplTest, NullRequestHeadersAndNullRouteEntry) { EXPECT_CALL(span, setTag(Eq(Tracing::Tags::get().Component), Eq(Tracing::Tags::get().Proxy))); EXPECT_CALL(span, setTag(Eq(Tracing::Tags::get().UpstreamAddress), _)).Times(0); EXPECT_CALL(span, setTag(Eq(Tracing::Tags::get().UpstreamCluster), _)).Times(0); + EXPECT_CALL(span, setTag(Eq(Tracing::Tags::get().UpstreamClusterName), _)).Times(0); expectSetCustomTags({{"{ tag: a, request_header: { name: X-Ax } }", false, ""}, {R"EOF( @@ -293,7 +294,7 @@ TEST_F(HttpConnManFinalizerImplTest, StreamInfoLogs) { EXPECT_CALL(stream_info, bytesSent()).WillOnce(Return(11)); absl::optional response_code; EXPECT_CALL(stream_info, responseCode()).WillRepeatedly(ReturnPointee(&response_code)); - EXPECT_CALL(stream_info, upstreamHost()).Times(2); + EXPECT_CALL(stream_info, upstreamHost()).Times(3); const auto start_timestamp = SystemTime{std::chrono::duration_cast(std::chrono::hours{123})}; EXPECT_CALL(stream_info, startTime()).WillRepeatedly(Return(start_timestamp)); @@ -323,15 +324,18 @@ TEST_F(HttpConnManFinalizerImplTest, StreamInfoLogs) { TEST_F(HttpConnManFinalizerImplTest, UpstreamClusterTagSet) { stream_info.host_->cluster_.name_ = "my_upstream_cluster"; + stream_info.host_->cluster_.observability_name_ = "my_upstream_cluster_observable"; EXPECT_CALL(stream_info, bytesReceived()).WillOnce(Return(10)); EXPECT_CALL(stream_info, bytesSent()).WillOnce(Return(11)); absl::optional response_code; EXPECT_CALL(stream_info, responseCode()).WillRepeatedly(ReturnPointee(&response_code)); - EXPECT_CALL(stream_info, upstreamHost()).Times(2); + EXPECT_CALL(stream_info, upstreamHost()).Times(3); EXPECT_CALL(span, setTag(Eq(Tracing::Tags::get().Component), Eq(Tracing::Tags::get().Proxy))); EXPECT_CALL(span, setTag(Eq(Tracing::Tags::get().UpstreamCluster), Eq("my_upstream_cluster"))); + EXPECT_CALL(span, setTag(Eq(Tracing::Tags::get().UpstreamClusterName), + Eq("my_upstream_cluster_observable"))); EXPECT_CALL(span, setTag(Eq(Tracing::Tags::get().HttpStatusCode), Eq("0"))); EXPECT_CALL(span, setTag(Eq(Tracing::Tags::get().Error), Eq(Tracing::Tags::get().True))); EXPECT_CALL(span, setTag(Eq(Tracing::Tags::get().ResponseSize), Eq("11"))); @@ -379,6 +383,7 @@ TEST_F(HttpConnManFinalizerImplTest, SpanOptionalHeaders) { EXPECT_CALL(span, setTag(Eq(Tracing::Tags::get().Component), Eq(Tracing::Tags::get().Proxy))); EXPECT_CALL(span, setTag(Eq(Tracing::Tags::get().UpstreamAddress), _)).Times(0); EXPECT_CALL(span, setTag(Eq(Tracing::Tags::get().UpstreamCluster), _)).Times(0); + EXPECT_CALL(span, setTag(Eq(Tracing::Tags::get().UpstreamClusterName), _)).Times(0); HttpTracerUtility::finalizeDownstreamSpan(span, &request_headers, &response_headers, &response_trailers, stream_info, config); @@ -573,6 +578,7 @@ TEST_F(HttpConnManFinalizerImplTest, SpanPopulatedFailureResponse) { EXPECT_CALL(span, setTag(Eq(Tracing::Tags::get().Component), Eq(Tracing::Tags::get().Proxy))); EXPECT_CALL(span, setTag(Eq(Tracing::Tags::get().UpstreamAddress), _)).Times(0); EXPECT_CALL(span, setTag(Eq(Tracing::Tags::get().UpstreamCluster), _)).Times(0); + EXPECT_CALL(span, setTag(Eq(Tracing::Tags::get().UpstreamClusterName), _)).Times(0); HttpTracerUtility::finalizeDownstreamSpan(span, &request_headers, &response_headers, &response_trailers, stream_info, config); @@ -607,6 +613,7 @@ TEST_F(HttpConnManFinalizerImplTest, GrpcOkStatus) { EXPECT_CALL(span, setTag(Eq(Tracing::Tags::get().Component), Eq(Tracing::Tags::get().Proxy))); EXPECT_CALL(span, setTag(Eq(Tracing::Tags::get().DownstreamCluster), Eq("-"))); EXPECT_CALL(span, setTag(Eq(Tracing::Tags::get().UpstreamCluster), Eq("fake_cluster"))); + EXPECT_CALL(span, setTag(Eq(Tracing::Tags::get().UpstreamClusterName), Eq("observability_name"))); EXPECT_CALL(span, setTag(Eq(Tracing::Tags::get().HttpUrl), Eq("http://example.com:80/pb.Foo/Bar"))); EXPECT_CALL(span, setTag(Eq(Tracing::Tags::get().UserAgent), Eq("-"))); diff --git a/test/mocks/upstream/cluster_info.cc b/test/mocks/upstream/cluster_info.cc index fd46bb958846..0b986eda49f4 100644 --- a/test/mocks/upstream/cluster_info.cc +++ b/test/mocks/upstream/cluster_info.cc @@ -64,6 +64,7 @@ MockClusterInfo::MockClusterInfo() ON_CALL(*this, idleTimeout()).WillByDefault(Return(absl::optional())); ON_CALL(*this, perUpstreamPreconnectRatio()).WillByDefault(Return(1.0)); ON_CALL(*this, name()).WillByDefault(ReturnRef(name_)); + ON_CALL(*this, observabilityName()).WillByDefault(ReturnRef(observability_name_)); ON_CALL(*this, edsServiceName()).WillByDefault(ReturnPointee(&eds_service_name_)); ON_CALL(*this, http1Settings()).WillByDefault(ReturnRef(http1_settings_)); ON_CALL(*this, http2Options()).WillByDefault(ReturnRef(http2_options_)); diff --git a/test/mocks/upstream/cluster_info.h b/test/mocks/upstream/cluster_info.h index 35b810b515ed..ca9bb74024fb 100644 --- a/test/mocks/upstream/cluster_info.h +++ b/test/mocks/upstream/cluster_info.h @@ -122,6 +122,7 @@ class MockClusterInfo : public ClusterInfo { MOCK_METHOD(uint32_t, maxResponseHeadersCount, (), (const)); MOCK_METHOD(uint64_t, maxRequestsPerConnection, (), (const)); MOCK_METHOD(const std::string&, name, (), (const)); + MOCK_METHOD(const std::string&, observabilityName, (), (const)); MOCK_METHOD(ResourceManager&, resourceManager, (ResourcePriority priority), (const)); MOCK_METHOD(TransportSocketMatcher&, transportSocketMatcher, (), (const)); MOCK_METHOD(ClusterStats&, stats, (), (const)); @@ -149,6 +150,7 @@ class MockClusterInfo : public ClusterInfo { Http::Http2::CodecStats& http2CodecStats() const override; std::string name_{"fake_cluster"}; + std::string observability_name_{"observability_name"}; absl::optional eds_service_name_; Http::Http1Settings http1_settings_; envoy::config::core::v3::Http2ProtocolOptions http2_options_; diff --git a/test/server/admin/clusters_handler_test.cc b/test/server/admin/clusters_handler_test.cc index 4d41b6c24969..484bc9509a0e 100644 --- a/test/server/admin/clusters_handler_test.cc +++ b/test/server/admin/clusters_handler_test.cc @@ -106,6 +106,7 @@ TEST_P(AdminInstanceTest, ClustersJson) { "cluster_statuses": [ { "name": "fake_cluster", + "observability_name": "observability_name", "success_rate_ejection_threshold": { "value": 6 }, @@ -203,7 +204,8 @@ TEST_P(AdminInstanceTest, ClustersJson) { // Ensure that the normal text format is used by default. Buffer::OwnedImpl response2; EXPECT_EQ(Http::Code::OK, getCallback("/clusters", header_map, response2)); - const std::string expected_text = R"EOF(fake_cluster::outlier::success_rate_average::0 + const std::string expected_text = R"EOF(fake_cluster::observability_name::observability_name +fake_cluster::outlier::success_rate_average::0 fake_cluster::outlier::success_rate_ejection_threshold::6 fake_cluster::outlier::local_origin_success_rate_average::0 fake_cluster::outlier::local_origin_success_rate_ejection_threshold::9