diff --git a/api/envoy/config/endpoint/v3/endpoint_components.proto b/api/envoy/config/endpoint/v3/endpoint_components.proto index eacc555df7373..a4cd059db8bb3 100644 --- a/api/envoy/config/endpoint/v3/endpoint_components.proto +++ b/api/envoy/config/endpoint/v3/endpoint_components.proto @@ -25,6 +25,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // [#protodoc-title: Endpoints] // Upstream host identifier. +// [#next-free-field: 6] message Endpoint { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.endpoint.Endpoint"; @@ -97,6 +98,19 @@ message Endpoint { // sorted by preference order of the addresses. This will only be supported // for STATIC and EDS clusters. repeated AdditionalAddress additional_addresses = 4; + + // Optional alternative stat name for this endpoint. If not specified, the main address will be used + // as the stat name and be extracted as ``envoy.endpoint_address`` tag value in generated stats. + // If specified, the ``stat_name`` here will be used to replace the main address. + // + // .. note:: + // + // This field make no sense for logic DNS host implementation for now. + // + // This is useful when there are duplicate addresses in the cluster, for example when multiple + // endpoints share the same address but have different hostnames or metadata. + // In this case, the stat name can be used to differentiate between these endpoints in stats and logs. + string stat_name = 5; } // An Endpoint that Envoy can route traffic to. diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 0fc3636aa24fb..d5dbc3bb467e6 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -166,6 +166,12 @@ new_features: change: | Added ``upstream.server_name`` CEL attribute returning the SNI from the established upstream TLS connection. +- area: upstream + change: | + Added :ref:`stat_name ` to + :ref:`Endpoint ` so endpoints can override the + observability name used in per-endpoint stats and related logs. This allows clusters with + duplicate endpoint addresses to expose distinct per-endpoint observability output. - area: tls change: | diff --git a/envoy/upstream/host_description.h b/envoy/upstream/host_description.h index 434f326505d45..78d59e4026dd0 100644 --- a/envoy/upstream/host_description.h +++ b/envoy/upstream/host_description.h @@ -207,6 +207,15 @@ class HostDescription { */ virtual const std::string& hostname() const PURE; + /** + * @return the observability name associated with the host. Used in per-endpoint stats and other + * observability surfaces. This is configured with + * :ref:`stat_name `. If this method + * returns an empty string view, then the host's address should be used as fallback for the + * observability name. + */ + virtual absl::string_view observabilityName() const PURE; + /** * @return the transport socket factory responsible for this host. */ diff --git a/source/common/upstream/health_discovery_service.cc b/source/common/upstream/health_discovery_service.cc index 2b3bcc062510c..7477592d76169 100644 --- a/source/common/upstream/health_discovery_service.cc +++ b/source/common/upstream/health_discovery_service.cc @@ -393,7 +393,7 @@ HdsCluster::HdsCluster(Server::Configuration::ServerFactoryContext& server_conte HostImpl::create(info_, "", std::move(address_or_error.value()), nullptr, nullptr, 1, const_locality_shared_pool->getObject(locality_endpoints.locality()), host.endpoint().health_check_config(), 0, - envoy::config::core::v3::UNKNOWN), + envoy::config::core::v3::UNKNOWN, {}, host.endpoint().stat_name()), std::unique_ptr)); // Add this host/endpoint pointer to our flat list of endpoints for health checking. hosts_->push_back(endpoint); @@ -512,7 +512,7 @@ void HdsCluster::updateHosts( HostImpl::create(info_, "", std::move(address_or_error.value()), nullptr, nullptr, 1, const_locality_shared_pool->getObject(endpoints.locality()), endpoint.endpoint().health_check_config(), 0, - envoy::config::core::v3::UNKNOWN), + envoy::config::core::v3::UNKNOWN, {}, endpoint.endpoint().stat_name()), std::unique_ptr)); // Set the initial health status as in HdsCluster::initialize. diff --git a/source/common/upstream/host_utility.cc b/source/common/upstream/host_utility.cc index 2d5005bd29d41..c6869e23cc045 100644 --- a/source/common/upstream/host_utility.cc +++ b/source/common/upstream/host_utility.cc @@ -186,12 +186,20 @@ void HostUtility::forEachHostMetric( for (auto& host_set : cluster.prioritySet().hostSetsPerPriority()) { for (auto& host : host_set->hosts()) { + absl::string_view endpoint_observability_name = host->observabilityName(); + Network::Address::InstanceConstSharedPtr address; + if (endpoint_observability_name.empty()) { + // Only logic host will have empty observability name for now. + address = host->address(); + endpoint_observability_name = address->asStringView(); + } Stats::TagVector tags; tags.reserve(fixed_tags.size() + 3); tags.insert(tags.end(), fixed_tags.begin(), fixed_tags.end()); tags.emplace_back(Stats::Tag{Envoy::Config::TagNames::get().CLUSTER_NAME, cluster_name}); - tags.emplace_back(Stats::Tag{"envoy.endpoint_address", host->address()->asString()}); + tags.emplace_back( + Stats::Tag{"envoy.endpoint_address", std::string(endpoint_observability_name)}); const auto& hostname = host->hostname(); if (!hostname.empty()) { @@ -200,10 +208,9 @@ void HostUtility::forEachHostMetric( auto set_metric_metadata = [&](absl::string_view metric_name, Stats::PrimitiveMetricMetadata& metric) { - metric.setName( - absl::StrCat("cluster.", cluster_name, ".endpoint.", - Stats::Utility::sanitizeStatsName(host->address()->asStringView()), - ".", metric_name)); + metric.setName(absl::StrCat( + "cluster.", cluster_name, ".endpoint.", + Stats::Utility::sanitizeStatsName(endpoint_observability_name), ".", metric_name)); metric.setTagExtractedName(absl::StrCat("cluster.endpoint.", metric_name)); metric.setTags(tags); diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc index bc8bf43f55927..38a62669e5836 100644 --- a/source/common/upstream/upstream_impl.cc +++ b/source/common/upstream/upstream_impl.cc @@ -474,11 +474,11 @@ absl::StatusOr> HostDescriptionImpl::create MetadataConstSharedPtr locality_metadata, std::shared_ptr locality, const envoy::config::endpoint::v3::Endpoint::HealthCheckConfig& health_check_config, - uint32_t priority, const AddressVector& address_list) { + uint32_t priority, const AddressVector& address_list, absl::string_view stat_name) { absl::Status creation_status = absl::OkStatus(); auto ret = std::unique_ptr(new HostDescriptionImpl( creation_status, cluster, hostname, dest_address, endpoint_metadata, locality_metadata, - locality, health_check_config, priority, address_list)); + locality, health_check_config, priority, address_list, stat_name)); RETURN_IF_NOT_OK(creation_status); return ret; } @@ -489,12 +489,13 @@ HostDescriptionImpl::HostDescriptionImpl( MetadataConstSharedPtr locality_metadata, std::shared_ptr locality, const envoy::config::endpoint::v3::Endpoint::HealthCheckConfig& health_check_config, - uint32_t priority, const AddressVector& address_list) + uint32_t priority, const AddressVector& address_list, absl::string_view stat_name) : HostDescriptionImplBase(cluster, hostname, dest_address, endpoint_metadata, locality_metadata, locality, health_check_config, priority, creation_status), address_(dest_address), address_list_or_null_(makeAddressListOrNull(dest_address, address_list)), - health_check_address_(resolveHealthCheckAddress(health_check_config, dest_address)) {} + health_check_address_(resolveHealthCheckAddress(health_check_config, dest_address)), + observability_name_(stat_name) {} HostDescriptionImplBase::HostDescriptionImplBase( ClusterInfoConstSharedPtr cluster, const std::string& hostname, @@ -724,11 +725,12 @@ absl::StatusOr> HostImpl::create( std::shared_ptr locality, const envoy::config::endpoint::v3::Endpoint::HealthCheckConfig& health_check_config, uint32_t priority, const envoy::config::core::v3::HealthStatus health_status, - const AddressVector& address_list) { + const AddressVector& address_list, absl::string_view stat_name) { absl::Status creation_status = absl::OkStatus(); - auto ret = std::unique_ptr(new HostImpl( - creation_status, cluster, hostname, address, endpoint_metadata, locality_metadata, - initial_weight, locality, health_check_config, priority, health_status, address_list)); + auto ret = std::unique_ptr( + new HostImpl(creation_status, cluster, hostname, address, endpoint_metadata, + locality_metadata, initial_weight, locality, health_check_config, priority, + health_status, address_list, stat_name)); RETURN_IF_NOT_OK(creation_status); return ret; } @@ -2207,7 +2209,7 @@ void PriorityStateManager::registerHostForPriority( lb_endpoint.load_balancing_weight().value(), parent_.constLocalitySharedPool()->getObject(locality_lb_endpoint.locality()), lb_endpoint.endpoint().health_check_config(), locality_lb_endpoint.priority(), - lb_endpoint.health_status(), address_list), + lb_endpoint.health_status(), address_list, lb_endpoint.endpoint().stat_name()), std::unique_ptr)); registerHostForPriority(host, locality_lb_endpoint); } diff --git a/source/common/upstream/upstream_impl.h b/source/common/upstream/upstream_impl.h index 3572b1cf32033..bc10641cf2d8e 100644 --- a/source/common/upstream/upstream_impl.h +++ b/source/common/upstream/upstream_impl.h @@ -317,7 +317,8 @@ class HostDescriptionImpl : public HostDescriptionImplBase { MetadataConstSharedPtr endpoint_metadata, MetadataConstSharedPtr locality_metadata, std::shared_ptr locality, const envoy::config::endpoint::v3::Endpoint::HealthCheckConfig& health_check_config, - uint32_t priority, const AddressVector& address_list = {}); + uint32_t priority, const AddressVector& address_list = {}, + absl::string_view stat_name = {}); // HostDescription Network::Address::InstanceConstSharedPtr address() const override { return address_; } @@ -325,6 +326,13 @@ class HostDescriptionImpl : public HostDescriptionImplBase { return health_check_address_; } SharedConstAddressVector addressListOrNull() const override { return address_list_or_null_; } + absl::string_view observabilityName() const override { + if (observability_name_.empty()) { + ASSERT(address_ != nullptr); + return address_->asStringView(); + } + return observability_name_; + } protected: HostDescriptionImpl( @@ -333,7 +341,7 @@ class HostDescriptionImpl : public HostDescriptionImplBase { MetadataConstSharedPtr endpoint_metadata, MetadataConstSharedPtr locality_metadata, std::shared_ptr locality, const envoy::config::endpoint::v3::Endpoint::HealthCheckConfig& health_check_config, - uint32_t priority, const AddressVector& address_list = {}); + uint32_t priority, const AddressVector& address_list = {}, absl::string_view stat_name = {}); private: // No locks are required in this implementation: all address-related member @@ -343,6 +351,7 @@ class HostDescriptionImpl : public HostDescriptionImplBase { const Network::Address::InstanceConstSharedPtr address_; const SharedConstAddressVector address_list_or_null_; const Network::Address::InstanceConstSharedPtr health_check_address_; + const std::string observability_name_; }; /** @@ -514,7 +523,7 @@ class HostImpl : public HostImplBase, public HostDescriptionImpl { std::shared_ptr locality, const envoy::config::endpoint::v3::Endpoint::HealthCheckConfig& health_check_config, uint32_t priority, const envoy::config::core::v3::HealthStatus health_status, - const AddressVector& address_list = {}); + const AddressVector& address_list = {}, absl::string_view stat_name = {}); protected: HostImpl(absl::Status& creation_status, ClusterInfoConstSharedPtr cluster, @@ -524,11 +533,11 @@ class HostImpl : public HostImplBase, public HostDescriptionImpl { std::shared_ptr locality, const envoy::config::endpoint::v3::Endpoint::HealthCheckConfig& health_check_config, uint32_t priority, const envoy::config::core::v3::HealthStatus health_status, - const AddressVector& address_list = {}) + const AddressVector& address_list = {}, absl::string_view stat_name = {}) : HostImplBase(initial_weight, health_check_config, health_status), HostDescriptionImpl(creation_status, cluster, hostname, address, endpoint_metadata, locality_metadata, locality, health_check_config, priority, - address_list) {} + address_list, stat_name) {} }; class HostsPerLocalityImpl : public HostsPerLocality { diff --git a/source/extensions/clusters/common/logical_host.h b/source/extensions/clusters/common/logical_host.h index f50a8db560494..91b5c69dacbc0 100644 --- a/source/extensions/clusters/common/logical_host.h +++ b/source/extensions/clusters/common/logical_host.h @@ -51,6 +51,7 @@ class LogicalHost : public HostImplBase, public HostDescriptionImplBase { SharedConstAddressVector addressListOrNull() const override; Network::Address::InstanceConstSharedPtr address() const override; Network::Address::InstanceConstSharedPtr healthCheckAddress() const override; + absl::string_view observabilityName() const override { return {}; } protected: LogicalHost( @@ -107,6 +108,7 @@ class RealHostDescription : public HostDescription { return logical_host_->hostnameForHealthChecks(); } const std::string& hostname() const override { return logical_host_->hostname(); } + absl::string_view observabilityName() const override { return {}; } Network::Address::InstanceConstSharedPtr address() const override { return address_; } SharedConstAddressVector addressListOrNull() const override { return logical_host_->addressListOrNull(); diff --git a/test/common/upstream/host_utility_test.cc b/test/common/upstream/host_utility_test.cc index 60dcd29b46572..0b8eb968461d8 100644 --- a/test/common/upstream/host_utility_test.cc +++ b/test/common/upstream/host_utility_test.cc @@ -441,6 +441,32 @@ TEST_F(PerEndpointMetricsTest, Tags) { })); } +TEST_F(PerEndpointMetricsTest, CustomEndpointStatName) { + auto& cluster = makeCluster("cluster1", 0); + auto& host = addHost(cluster); + host.observability_name_ = "shared.backend/a"; + + auto [counters, gauges] = run(); + + EXPECT_THAT(metricNamesAndValues(counters), + UnorderedElementsAreArray({ + std::make_pair("cluster.cluster1.endpoint.shared.backend/a.c1", 11), + std::make_pair("cluster.cluster1.endpoint.shared.backend/a.c2", 12), + })); + EXPECT_THAT(metricNamesAndValues(gauges), + UnorderedElementsAreArray({ + std::make_pair("cluster.cluster1.endpoint.shared.backend/a.g1", 13), + std::make_pair("cluster.cluster1.endpoint.shared.backend/a.g2", 14), + std::make_pair("cluster.cluster1.endpoint.shared.backend/a.healthy", 1), + })); + + EXPECT_THAT(getMetric("cluster.cluster1.endpoint.shared.backend/a.c1", counters).tags(), + UnorderedElementsAreArray({ + Stats::Tag{"envoy.cluster_name", "cluster1"}, + Stats::Tag{"envoy.endpoint_address", "shared.backend/a"}, + })); +} + TEST_F(PerEndpointMetricsTest, FixedTags) { auto& cluster = makeCluster("cluster1", 1); Stats::TagVector fixed_tags{{"fixed1", "value1"}, {"fixed2", "value2"}}; diff --git a/test/common/upstream/upstream_impl_test.cc b/test/common/upstream/upstream_impl_test.cc index d3898794c0084..8927f9db52f2e 100644 --- a/test/common/upstream/upstream_impl_test.cc +++ b/test/common/upstream/upstream_impl_test.cc @@ -2513,6 +2513,36 @@ TEST_F(StaticClusterImplTest, LoadAssignmentNonEmptyHostnameWithHealthChecks) { EXPECT_FALSE(cluster->info()->addedViaApi()); } +TEST_F(StaticClusterImplTest, LoadAssignmentEndpointStatName) { + const std::string yaml = R"EOF( + name: staticcluster + connect_timeout: 0.25s + type: STATIC + lb_policy: ROUND_ROBIN + load_assignment: + endpoints: + - lb_endpoints: + - endpoint: + stat_name: shared.backend/a + address: + socket_address: + address: 10.0.0.1 + port_value: 443 + )EOF"; + + envoy::config::cluster::v3::Cluster cluster_config = parseClusterFromV3Yaml(yaml); + + Envoy::Upstream::ClusterFactoryContextImpl factory_context(server_context_, nullptr, nullptr, + false); + std::shared_ptr cluster = createCluster(cluster_config, factory_context); + + cluster->initialize([] { return absl::OkStatus(); }); + + ASSERT_EQ(1UL, cluster->prioritySet().hostSetsPerPriority()[0]->healthyHosts().size()); + EXPECT_EQ("shared.backend/a", + cluster->prioritySet().hostSetsPerPriority()[0]->hosts()[0]->observabilityName()); +} + TEST_F(StaticClusterImplTest, LoadAssignmentMultiplePriorities) { const std::string yaml = R"EOF( name: staticcluster diff --git a/test/mocks/upstream/host.cc b/test/mocks/upstream/host.cc index 0a2512b5f24ee..94cae5817b09f 100644 --- a/test/mocks/upstream/host.cc +++ b/test/mocks/upstream/host.cc @@ -33,7 +33,11 @@ MockHealthCheckHostMonitor::~MockHealthCheckHostMonitor() = default; MockHostDescription::MockHostDescription() : address_(*Network::Utility::resolveUrl("tcp://10.0.0.1:443")), socket_factory_(new testing::NiceMock) { + observability_name_ = address_->asString(); ON_CALL(*this, hostname()).WillByDefault(ReturnRef(hostname_)); + ON_CALL(*this, observabilityName()).WillByDefault(Invoke([this]() { + return absl::string_view(observability_name_); + })); ON_CALL(*this, address()).WillByDefault(Return(address_)); ON_CALL(*this, outlierDetector()).WillByDefault(ReturnRef(outlier_detector_)); ON_CALL(*this, stats()).WillByDefault(ReturnRef(stats_)); @@ -65,10 +69,17 @@ MockHostDescription::MockHostDescription() MockHostDescription::~MockHostDescription() = default; -MockHostLight::MockHostLight() = default; +MockHostLight::MockHostLight() { + ON_CALL(*this, observabilityName()).WillByDefault(Invoke([this]() { + return absl::string_view(observability_name_); + })); +} MockHostLight::~MockHostLight() = default; MockHost::MockHost() : socket_factory_(new testing::NiceMock) { + ON_CALL(*this, observabilityName()).WillByDefault(Invoke([this]() { + return absl::string_view(observability_name_); + })); ON_CALL(*this, cluster()).WillByDefault(ReturnRef(cluster_)); ON_CALL(*this, outlierDetector()).WillByDefault(ReturnRef(outlier_detector_)); ON_CALL(*this, stats()).WillByDefault(ReturnRef(stats_)); diff --git a/test/mocks/upstream/host.h b/test/mocks/upstream/host.h index aed10c5d7b4ec..5e5815212a057 100644 --- a/test/mocks/upstream/host.h +++ b/test/mocks/upstream/host.h @@ -99,6 +99,7 @@ class MockHostDescription : public HostDescription { MOCK_METHOD(void, setHealthChecker, (HealthCheckHostMonitorPtr && health_checker)); MOCK_METHOD(const std::string&, hostnameForHealthChecks, (), (const)); MOCK_METHOD(const std::string&, hostname, (), (const)); + MOCK_METHOD(absl::string_view, observabilityName, (), (const)); MOCK_METHOD(Network::UpstreamTransportSocketFactory&, transportSocketFactory, (), (const)); MOCK_METHOD(HostStats&, stats, (), (const)); MOCK_METHOD(LoadMetricStats&, loadMetricStats, (), (const)); @@ -122,6 +123,7 @@ class MockHostDescription : public HostDescription { MOCK_METHOD(OptRef, lbPolicyDataAt, (size_t index), (const)); std::string hostname_; + std::string observability_name_; Network::Address::InstanceConstSharedPtr address_; testing::NiceMock outlier_detector_; testing::NiceMock health_checker_; @@ -200,6 +202,7 @@ class MockHostLight : public Host { MOCK_METHOD(const std::string&, hostnameForHealthChecks, (), (const)); MOCK_METHOD(const std::string&, hostname, (), (const)); + MOCK_METHOD(absl::string_view, observabilityName, (), (const)); MOCK_METHOD(Network::UpstreamTransportSocketFactory&, transportSocketFactory, (), (const)); MOCK_METHOD(Outlier::DetectorHostMonitor&, outlierDetector, (), (const)); MOCK_METHOD(void, setHealthChecker, (HealthCheckHostMonitorPtr && health_checker)); @@ -225,6 +228,7 @@ class MockHostLight : public Host { MOCK_METHOD(absl::optional, lastHealthCheckHttpStatus, (), (const)); bool disable_active_health_check_ = false; + std::string observability_name_; }; class MockHost : public MockHostLight { diff --git a/test/server/admin/stats_handler_speed_test.cc b/test/server/admin/stats_handler_speed_test.cc index b21f8c3d11e0c..5881aa062d906 100644 --- a/test/server/admin/stats_handler_speed_test.cc +++ b/test/server/admin/stats_handler_speed_test.cc @@ -77,6 +77,7 @@ class FastMockHost : public testing::StrictMock { public: Network::Address::InstanceConstSharedPtr address() const override { return address_; } const std::string& hostname() const override { return hostname_; } + absl::string_view observabilityName() const override { return address_->asStringView(); } std::vector> counters() const override { diff --git a/test/test_common/stats_utility.h b/test/test_common/stats_utility.h index 08cdcecdd7279..3d25fdb0bb5a0 100644 --- a/test/test_common/stats_utility.h +++ b/test/test_common/stats_utility.h @@ -46,6 +46,7 @@ class PerEndpointMetricsTestHelper { host_count_++; MockHostSet* host_set = cluster.priority_set_.getMockHostSet(priority); auto host = std::make_shared>(); + host->observability_name_ = fmt::format("127.0.0.{}:80", host_count_); ON_CALL(*host, address()) .WillByDefault(Return(Network::Utility::parseInternetAddressAndPortNoThrow( fmt::format("127.0.0.{}:80", host_count_)))); @@ -81,6 +82,7 @@ class PerEndpointMetricsTestHelper { host_count_++; MockHostSet* host_set = cluster.priority_set_.getMockHostSet(priority); auto host = std::make_shared>(); + host->observability_name_ = fmt::format("127.0.0.{}:80", host_count_); ON_CALL(*host, address()) .WillByDefault(Return(Network::Utility::parseInternetAddressAndPortNoThrow( fmt::format("127.0.0.{}:80", host_count_)))); @@ -106,6 +108,7 @@ class PerEndpointMetricsTestHelper { host_count_++; MockHostSet* host_set = cluster.priority_set_.getMockHostSet(priority); auto host = std::make_shared>(); + host->observability_name_ = fmt::format("127.0.0.{}:80", host_count_); ON_CALL(*host, address()) .WillByDefault(Return(Network::Utility::parseInternetAddressAndPortNoThrow( fmt::format("127.0.0.{}:80", host_count_))));