diff --git a/BUILD b/BUILD index 614aa9db96b..945acd00260 100644 --- a/BUILD +++ b/BUILD @@ -2892,7 +2892,6 @@ grpc_cc_library( "absl/status", "absl/status:statusor", "absl/strings", - "absl/strings:str_format", ], language = "c++", visibility = ["@grpc:client_channel"], diff --git a/CMakeLists.txt b/CMakeLists.txt index 79256f992b8..3f5b7a2193f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1707,7 +1707,6 @@ add_library(grpc src/core/ext/filters/client_channel/lb_policy/weighted_round_robin/weighted_round_robin.cc src/core/ext/filters/client_channel/lb_policy/weighted_target/weighted_target.cc src/core/ext/filters/client_channel/lb_policy/xds/cds.cc - src/core/ext/filters/client_channel/lb_policy/xds/xds_attributes.cc src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_impl.cc src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_manager.cc src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_resolver.cc diff --git a/Makefile b/Makefile index 4c1a32175c8..fe3213dee24 100644 --- a/Makefile +++ b/Makefile @@ -997,7 +997,6 @@ LIBGRPC_SRC = \ src/core/ext/filters/client_channel/lb_policy/weighted_round_robin/weighted_round_robin.cc \ src/core/ext/filters/client_channel/lb_policy/weighted_target/weighted_target.cc \ src/core/ext/filters/client_channel/lb_policy/xds/cds.cc \ - src/core/ext/filters/client_channel/lb_policy/xds/xds_attributes.cc \ src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_impl.cc \ src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_manager.cc \ src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_resolver.cc \ @@ -3050,7 +3049,6 @@ ifneq ($(OPENSSL_DEP),) # otherwise parallel compilation will fail if a source is compiled first. src/core/ext/filters/client_channel/lb_policy/ring_hash/ring_hash.cc: $(OPENSSL_DEP) src/core/ext/filters/client_channel/lb_policy/xds/cds.cc: $(OPENSSL_DEP) -src/core/ext/filters/client_channel/lb_policy/xds/xds_attributes.cc: $(OPENSSL_DEP) src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_impl.cc: $(OPENSSL_DEP) src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_manager.cc: $(OPENSSL_DEP) src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_resolver.cc: $(OPENSSL_DEP) diff --git a/Package.swift b/Package.swift index c3e971d3d12..19057f6fe4a 100644 --- a/Package.swift +++ b/Package.swift @@ -178,8 +178,6 @@ let package = Package( "src/core/ext/filters/client_channel/lb_policy/weighted_round_robin/weighted_round_robin.cc", "src/core/ext/filters/client_channel/lb_policy/weighted_target/weighted_target.cc", "src/core/ext/filters/client_channel/lb_policy/xds/cds.cc", - "src/core/ext/filters/client_channel/lb_policy/xds/xds_attributes.cc", - "src/core/ext/filters/client_channel/lb_policy/xds/xds_attributes.h", "src/core/ext/filters/client_channel/lb_policy/xds/xds_channel_args.h", "src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_impl.cc", "src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_manager.cc", diff --git a/build_autogenerated.yaml b/build_autogenerated.yaml index 23eaeed3ed8..d96032cc360 100644 --- a/build_autogenerated.yaml +++ b/build_autogenerated.yaml @@ -244,7 +244,6 @@ libs: - src/core/ext/filters/client_channel/lb_policy/ring_hash/ring_hash.h - src/core/ext/filters/client_channel/lb_policy/subchannel_list.h - src/core/ext/filters/client_channel/lb_policy/weighted_round_robin/static_stride_scheduler.h - - src/core/ext/filters/client_channel/lb_policy/xds/xds_attributes.h - src/core/ext/filters/client_channel/lb_policy/xds/xds_channel_args.h - src/core/ext/filters/client_channel/lb_policy/xds/xds_override_host.h - src/core/ext/filters/client_channel/local_subchannel_pool.h @@ -1052,7 +1051,6 @@ libs: - src/core/ext/filters/client_channel/lb_policy/weighted_round_robin/weighted_round_robin.cc - src/core/ext/filters/client_channel/lb_policy/weighted_target/weighted_target.cc - src/core/ext/filters/client_channel/lb_policy/xds/cds.cc - - src/core/ext/filters/client_channel/lb_policy/xds/xds_attributes.cc - src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_impl.cc - src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_manager.cc - src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_resolver.cc diff --git a/config.m4 b/config.m4 index 22bc192d4b0..f9ee388bae4 100644 --- a/config.m4 +++ b/config.m4 @@ -76,7 +76,6 @@ if test "$PHP_GRPC" != "no"; then src/core/ext/filters/client_channel/lb_policy/weighted_round_robin/weighted_round_robin.cc \ src/core/ext/filters/client_channel/lb_policy/weighted_target/weighted_target.cc \ src/core/ext/filters/client_channel/lb_policy/xds/cds.cc \ - src/core/ext/filters/client_channel/lb_policy/xds/xds_attributes.cc \ src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_impl.cc \ src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_manager.cc \ src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_resolver.cc \ diff --git a/config.w32 b/config.w32 index a21cb6d4237..f8756ae7c58 100644 --- a/config.w32 +++ b/config.w32 @@ -41,7 +41,6 @@ if (PHP_GRPC != "no") { "src\\core\\ext\\filters\\client_channel\\lb_policy\\weighted_round_robin\\weighted_round_robin.cc " + "src\\core\\ext\\filters\\client_channel\\lb_policy\\weighted_target\\weighted_target.cc " + "src\\core\\ext\\filters\\client_channel\\lb_policy\\xds\\cds.cc " + - "src\\core\\ext\\filters\\client_channel\\lb_policy\\xds\\xds_attributes.cc " + "src\\core\\ext\\filters\\client_channel\\lb_policy\\xds\\xds_cluster_impl.cc " + "src\\core\\ext\\filters\\client_channel\\lb_policy\\xds\\xds_cluster_manager.cc " + "src\\core\\ext\\filters\\client_channel\\lb_policy\\xds\\xds_cluster_resolver.cc " + diff --git a/gRPC-C++.podspec b/gRPC-C++.podspec index 1ba1875284a..d342da2858a 100644 --- a/gRPC-C++.podspec +++ b/gRPC-C++.podspec @@ -276,7 +276,6 @@ Pod::Spec.new do |s| 'src/core/ext/filters/client_channel/lb_policy/ring_hash/ring_hash.h', 'src/core/ext/filters/client_channel/lb_policy/subchannel_list.h', 'src/core/ext/filters/client_channel/lb_policy/weighted_round_robin/static_stride_scheduler.h', - 'src/core/ext/filters/client_channel/lb_policy/xds/xds_attributes.h', 'src/core/ext/filters/client_channel/lb_policy/xds/xds_channel_args.h', 'src/core/ext/filters/client_channel/lb_policy/xds/xds_override_host.h', 'src/core/ext/filters/client_channel/local_subchannel_pool.h', @@ -1338,7 +1337,6 @@ Pod::Spec.new do |s| 'src/core/ext/filters/client_channel/lb_policy/ring_hash/ring_hash.h', 'src/core/ext/filters/client_channel/lb_policy/subchannel_list.h', 'src/core/ext/filters/client_channel/lb_policy/weighted_round_robin/static_stride_scheduler.h', - 'src/core/ext/filters/client_channel/lb_policy/xds/xds_attributes.h', 'src/core/ext/filters/client_channel/lb_policy/xds/xds_channel_args.h', 'src/core/ext/filters/client_channel/lb_policy/xds/xds_override_host.h', 'src/core/ext/filters/client_channel/local_subchannel_pool.h', diff --git a/gRPC-Core.podspec b/gRPC-Core.podspec index 5fe96406928..1e24a14310c 100644 --- a/gRPC-Core.podspec +++ b/gRPC-Core.podspec @@ -279,8 +279,6 @@ Pod::Spec.new do |s| 'src/core/ext/filters/client_channel/lb_policy/weighted_round_robin/weighted_round_robin.cc', 'src/core/ext/filters/client_channel/lb_policy/weighted_target/weighted_target.cc', 'src/core/ext/filters/client_channel/lb_policy/xds/cds.cc', - 'src/core/ext/filters/client_channel/lb_policy/xds/xds_attributes.cc', - 'src/core/ext/filters/client_channel/lb_policy/xds/xds_attributes.h', 'src/core/ext/filters/client_channel/lb_policy/xds/xds_channel_args.h', 'src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_impl.cc', 'src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_manager.cc', @@ -2088,7 +2086,6 @@ Pod::Spec.new do |s| 'src/core/ext/filters/client_channel/lb_policy/ring_hash/ring_hash.h', 'src/core/ext/filters/client_channel/lb_policy/subchannel_list.h', 'src/core/ext/filters/client_channel/lb_policy/weighted_round_robin/static_stride_scheduler.h', - 'src/core/ext/filters/client_channel/lb_policy/xds/xds_attributes.h', 'src/core/ext/filters/client_channel/lb_policy/xds/xds_channel_args.h', 'src/core/ext/filters/client_channel/lb_policy/xds/xds_override_host.h', 'src/core/ext/filters/client_channel/local_subchannel_pool.h', diff --git a/grpc.gemspec b/grpc.gemspec index 1cb1cef491a..3c30d689e33 100644 --- a/grpc.gemspec +++ b/grpc.gemspec @@ -184,8 +184,6 @@ Gem::Specification.new do |s| s.files += %w( src/core/ext/filters/client_channel/lb_policy/weighted_round_robin/weighted_round_robin.cc ) s.files += %w( src/core/ext/filters/client_channel/lb_policy/weighted_target/weighted_target.cc ) s.files += %w( src/core/ext/filters/client_channel/lb_policy/xds/cds.cc ) - s.files += %w( src/core/ext/filters/client_channel/lb_policy/xds/xds_attributes.cc ) - s.files += %w( src/core/ext/filters/client_channel/lb_policy/xds/xds_attributes.h ) s.files += %w( src/core/ext/filters/client_channel/lb_policy/xds/xds_channel_args.h ) s.files += %w( src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_impl.cc ) s.files += %w( src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_manager.cc ) diff --git a/grpc.gyp b/grpc.gyp index d6791097df6..1a056667251 100644 --- a/grpc.gyp +++ b/grpc.gyp @@ -301,7 +301,6 @@ 'src/core/ext/filters/client_channel/lb_policy/weighted_round_robin/weighted_round_robin.cc', 'src/core/ext/filters/client_channel/lb_policy/weighted_target/weighted_target.cc', 'src/core/ext/filters/client_channel/lb_policy/xds/cds.cc', - 'src/core/ext/filters/client_channel/lb_policy/xds/xds_attributes.cc', 'src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_impl.cc', 'src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_manager.cc', 'src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_resolver.cc', diff --git a/package.xml b/package.xml index 3cfcec2c46e..ace800d863f 100644 --- a/package.xml +++ b/package.xml @@ -166,8 +166,6 @@ - - diff --git a/src/core/BUILD b/src/core/BUILD index 530241727de..5167903f3d2 100644 --- a/src/core/BUILD +++ b/src/core/BUILD @@ -4349,24 +4349,9 @@ grpc_cc_library( "ext/filters/client_channel/lb_policy/xds/xds_channel_args.h", ], language = "c++", -) - -grpc_cc_library( - name = "grpc_lb_xds_attributes", - srcs = [ - "ext/filters/client_channel/lb_policy/xds/xds_attributes.cc", - ], - hdrs = [ - "ext/filters/client_channel/lb_policy/xds/xds_attributes.h", - ], - external_deps = ["absl/strings"], - language = "c++", deps = [ - "useful", "//:gpr_platform", - "//:ref_counted_ptr", "//:server_address", - "//:xds_client", ], ) @@ -4386,7 +4371,6 @@ grpc_cc_library( "channel_args", "delegating_helper", "grpc_lb_address_filtering", - "grpc_lb_xds_attributes", "grpc_lb_xds_channel_args", "grpc_xds_client", "json", @@ -4432,7 +4416,6 @@ grpc_cc_library( "channel_args", "delegating_helper", "grpc_backend_metric_data", - "grpc_lb_xds_attributes", "grpc_lb_xds_channel_args", "grpc_xds_client", "json", @@ -4507,12 +4490,13 @@ grpc_cc_library( "absl/status", "absl/status:statusor", "absl/strings", + "absl/types:optional", ], language = "c++", deps = [ "channel_args", "delegating_helper", - "grpc_lb_xds_attributes", + "grpc_lb_xds_channel_args", "json", "json_args", "json_object_loader", @@ -4548,7 +4532,10 @@ grpc_cc_library( ], language = "c++", deps = [ + "channel_args", + "ref_counted", "//:gpr_platform", + "//:ref_counted_ptr", "//:server_address", ], ) diff --git a/src/core/ext/filters/client_channel/client_channel.cc b/src/core/ext/filters/client_channel/client_channel.cc index 2e712e91187..afbc704c117 100644 --- a/src/core/ext/filters/client_channel/client_channel.cc +++ b/src/core/ext/filters/client_channel/client_channel.cc @@ -1163,7 +1163,9 @@ ChannelArgs ClientChannel::MakeSubchannelArgs( // uniqueness. .Remove(GRPC_ARG_HEALTH_CHECK_SERVICE_NAME) .Remove(GRPC_ARG_INHIBIT_HEALTH_CHECKING) - .Remove(GRPC_ARG_CHANNELZ_CHANNEL_NODE); + .Remove(GRPC_ARG_CHANNELZ_CHANNEL_NODE) + // Remove all keys with the no-subchannel prefix. + .RemoveAllKeysWithPrefix(GRPC_ARG_NO_SUBCHANNEL_PREFIX); } void ClientChannel::ReprocessQueuedResolverCalls() { diff --git a/src/core/ext/filters/client_channel/lb_policy/address_filtering.cc b/src/core/ext/filters/client_channel/lb_policy/address_filtering.cc index e7893d6ff56..0ddaaf0f523 100644 --- a/src/core/ext/filters/client_channel/lb_policy/address_filtering.cc +++ b/src/core/ext/filters/client_channel/lb_policy/address_filtering.cc @@ -23,53 +23,24 @@ #include #include -#include "absl/strings/str_cat.h" -#include "absl/strings/str_join.h" - -#define GRPC_ARG_HIERARCHICAL_PATH "grpc.internal.address.hierarchical_path" +#include "src/core/lib/channel/channel_args.h" +#include "src/core/lib/gprpp/ref_counted_ptr.h" namespace grpc_core { -const char* kHierarchicalPathAttributeKey = "hierarchical_path"; - -namespace { - -class HierarchicalPathAttribute : public ServerAddress::AttributeInterface { - public: - explicit HierarchicalPathAttribute(std::vector path) - : path_(std::move(path)) {} - - std::unique_ptr Copy() const override { - return std::make_unique(path_); - } - - int Cmp(const AttributeInterface* other) const override { - const std::vector& other_path = - static_cast(other)->path_; - for (size_t i = 0; i < path_.size(); ++i) { - if (other_path.size() == i) return 1; - int r = path_[i].compare(other_path[i]); - if (r != 0) return r; - } - if (other_path.size() > path_.size()) return -1; - return 0; - } +absl::string_view HierarchicalPathArg::ChannelArgName() { + return GRPC_ARG_NO_SUBCHANNEL_PREFIX "address.hierarchical_path"; +} - std::string ToString() const override { - return absl::StrCat("[", absl::StrJoin(path_, ", "), "]"); +int HierarchicalPathArg::ChannelArgsCompare(const HierarchicalPathArg* a, + const HierarchicalPathArg* b) { + for (size_t i = 0; i < a->path_.size(); ++i) { + if (b->path_.size() == i) return 1; + int r = a->path_[i].compare(b->path_[i]); + if (r != 0) return r; } - - const std::vector& path() const { return path_; } - - private: - std::vector path_; -}; - -} // namespace - -std::unique_ptr -MakeHierarchicalPathAttribute(std::vector path) { - return std::make_unique(std::move(path)); + if (b->path_.size() > a->path_.size()) return -1; + return 0; } absl::StatusOr MakeHierarchicalAddressMap( @@ -77,22 +48,20 @@ absl::StatusOr MakeHierarchicalAddressMap( if (!addresses.ok()) return addresses.status(); HierarchicalAddressMap result; for (const ServerAddress& address : *addresses) { - const HierarchicalPathAttribute* path_attribute = - static_cast( - address.GetAttribute(kHierarchicalPathAttributeKey)); - if (path_attribute == nullptr) continue; - const std::vector& path = path_attribute->path(); + const auto* path_arg = address.args().GetObject(); + if (path_arg == nullptr) continue; + const std::vector& path = path_arg->path(); auto it = path.begin(); + if (it == path.end()) continue; ServerAddressList& target_list = result[*it]; - std::unique_ptr new_attribute; + ChannelArgs args = address.args(); ++it; if (it != path.end()) { std::vector remaining_path(it, path.end()); - new_attribute = std::make_unique( - std::move(remaining_path)); + args = args.SetObject( + MakeRefCounted(std::move(remaining_path))); } - target_list.emplace_back(address.WithAttribute( - kHierarchicalPathAttributeKey, std::move(new_attribute))); + target_list.emplace_back(address.address(), args); } return result; } diff --git a/src/core/ext/filters/client_channel/lb_policy/address_filtering.h b/src/core/ext/filters/client_channel/lb_policy/address_filtering.h index 771ef6fc2f2..2e4aa2017d4 100644 --- a/src/core/ext/filters/client_channel/lb_policy/address_filtering.h +++ b/src/core/ext/filters/client_channel/lb_policy/address_filtering.h @@ -20,12 +20,14 @@ #include #include -#include #include +#include #include #include "absl/status/statusor.h" +#include "absl/strings/string_view.h" +#include "src/core/lib/gprpp/ref_counted.h" #include "src/core/lib/resolver/server_address.h" // The resolver returns a flat list of addresses. When a hierarchy of @@ -82,13 +84,23 @@ namespace grpc_core { -// The attribute key to be used for hierarchical paths in ServerAddress. -extern const char* kHierarchicalPathAttributeKey; - -// Constructs an address attribute containing the hierarchical path +// An address channel arg containing the hierarchical path // to be associated with the address. -std::unique_ptr -MakeHierarchicalPathAttribute(std::vector path); +class HierarchicalPathArg : public RefCounted { + public: + explicit HierarchicalPathArg(std::vector path) + : path_(std::move(path)) {} + + // Channel arg traits methods. + static absl::string_view ChannelArgName(); + static int ChannelArgsCompare(const HierarchicalPathArg* a, + const HierarchicalPathArg* b); + + const std::vector& path() const { return path_; } + + private: + std::vector path_; +}; // A map from the next path element to the addresses that fall under // that path element. diff --git a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc index 3dfb92007c5..182bb9f1187 100644 --- a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc +++ b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc @@ -161,8 +161,6 @@ namespace grpc_core { TraceFlag grpc_lb_glb_trace(false, "glb"); -const char kGrpcLbAddressAttributeKey[] = "grpclb"; - namespace { using ::grpc_event_engine::experimental::EventEngine; @@ -336,30 +334,22 @@ class GrpcLb : public LoadBalancingPolicy { RefCountedPtr client_stats_; }; - class TokenAndClientStatsAttribute - : public ServerAddress::AttributeInterface { + class TokenAndClientStatsArg : public RefCounted { public: - TokenAndClientStatsAttribute(std::string lb_token, - RefCountedPtr client_stats) + TokenAndClientStatsArg(std::string lb_token, + RefCountedPtr client_stats) : lb_token_(std::move(lb_token)), client_stats_(std::move(client_stats)) {} - std::unique_ptr Copy() const override { - return std::make_unique(lb_token_, - client_stats_); + static absl::string_view ChannelArgName() { + return GRPC_ARG_NO_SUBCHANNEL_PREFIX "grpclb_token_and_client_stats"; } - int Cmp(const AttributeInterface* other_base) const override { - const TokenAndClientStatsAttribute* other = - static_cast(other_base); - int r = lb_token_.compare(other->lb_token_); + static int ChannelArgsCompare(const TokenAndClientStatsArg* a, + const TokenAndClientStatsArg* b) { + int r = a->lb_token_.compare(b->lb_token_); if (r != 0) return r; - return QsortCompare(client_stats_.get(), other->client_stats_.get()); - } - - std::string ToString() const override { - return absl::StrFormat("lb_token=\"%s\" client_stats=%p", lb_token_, - client_stats_.get()); + return QsortCompare(a->client_stats_.get(), b->client_stats_.get()); } const std::string& lb_token() const { return lb_token_; } @@ -691,14 +681,10 @@ ServerAddressList GrpcLb::Serverlist::GetServerAddressList( addr_uri.ok() ? addr_uri->c_str() : addr_uri.status().ToString().c_str()); } - // Attach attribute to address containing LB token and stats object. - std::map> - attributes; - attributes[kGrpcLbAddressAttributeKey] = - std::make_unique(std::move(lb_token), - stats); - // Add address. - addresses.emplace_back(addr, ChannelArgs(), std::move(attributes)); + // Add address with a channel arg containing LB token and stats object. + addresses.emplace_back( + addr, ChannelArgs().SetObject(MakeRefCounted( + std::move(lb_token), stats))); } return addresses; } @@ -785,16 +771,14 @@ GrpcLb::PickResult GrpcLb::Picker::Pick(PickArgs args) { RefCountedPtr GrpcLb::Helper::CreateSubchannel( ServerAddress address, const ChannelArgs& args) { if (parent()->shutting_down_) return nullptr; - const TokenAndClientStatsAttribute* attribute = - static_cast( - address.GetAttribute(kGrpcLbAddressAttributeKey)); - if (attribute == nullptr) { - Crash(absl::StrFormat( - "[grpclb %p] no TokenAndClientStatsAttribute for address %p", parent(), - address.ToString().c_str())); + const auto* arg = address.args().GetObject(); + if (arg == nullptr) { + Crash( + absl::StrFormat("[grpclb %p] no TokenAndClientStatsArg for address %p", + parent(), address.ToString().c_str())); } - std::string lb_token = attribute->lb_token(); - RefCountedPtr client_stats = attribute->client_stats(); + std::string lb_token = arg->lb_token(); + RefCountedPtr client_stats = arg->client_stats(); return MakeRefCounted( parent()->channel_control_helper()->CreateSubchannel(std::move(address), args), @@ -1534,9 +1518,10 @@ absl::Status GrpcLb::UpdateLocked(UpdateArgs args) { if (fallback_backend_addresses_.ok()) { // Add null LB token attributes. for (ServerAddress& address : *fallback_backend_addresses_) { - address = address.WithAttribute( - kGrpcLbAddressAttributeKey, - std::make_unique("", nullptr)); + address = ServerAddress( + address.address(), + address.args().SetObject( + MakeRefCounted("", nullptr))); } } resolution_note_ = std::move(args.resolution_note); diff --git a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.cc b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.cc index eccfacc3112..c45be05a6a3 100644 --- a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.cc +++ b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.cc @@ -26,7 +26,8 @@ #include "src/core/lib/gpr/useful.h" // Channel arg key for the list of balancer addresses. -#define GRPC_ARG_GRPCLB_BALANCER_ADDRESSES "grpc.grpclb_balancer_addresses" +#define GRPC_ARG_GRPCLB_BALANCER_ADDRESSES \ + GRPC_ARG_NO_SUBCHANNEL_PREFIX "grpc.grpclb_balancer_addresses" namespace grpc_core { diff --git a/src/core/ext/filters/client_channel/lb_policy/outlier_detection/outlier_detection.cc b/src/core/ext/filters/client_channel/lb_policy/outlier_detection/outlier_detection.cc index 98755f0e07a..4ebdca2c93c 100644 --- a/src/core/ext/filters/client_channel/lb_policy/outlier_detection/outlier_detection.cc +++ b/src/core/ext/filters/client_channel/lb_policy/outlier_detection/outlier_detection.cc @@ -69,9 +69,6 @@ namespace grpc_core { TraceFlag grpc_outlier_detection_lb_trace(false, "outlier_detection_lb"); -const char* DisableOutlierDetectionAttribute::kName = - "disable_outlier_detection"; - namespace { using ::grpc_event_engine::experimental::EventEngine; @@ -542,14 +539,12 @@ OutlierDetectionLb::~OutlierDetectionLb() { std::string OutlierDetectionLb::MakeKeyForAddress( const ServerAddress& address) { - // If the address has the DisableOutlierDetectionAttribute attribute, - // ignore it. + // If the address has outlier detection disabled, ignore it. // TODO(roth): This is a hack to prevent outlier_detection from // working with pick_first, as per discussion in // https://github.com/grpc/grpc/issues/32967. Remove this as part of // implementing dualstack backend support. - if (address.GetAttribute(DisableOutlierDetectionAttribute::kName) != - nullptr) { + if (address.args().GetInt(GRPC_ARG_OUTLIER_DETECTION_DISABLE) == 1) { return ""; } // Use only the address, not the attributes. diff --git a/src/core/ext/filters/client_channel/lb_policy/outlier_detection/outlier_detection.h b/src/core/ext/filters/client_channel/lb_policy/outlier_detection/outlier_detection.h index ca1dc3a3c71..c8c7f52afd3 100644 --- a/src/core/ext/filters/client_channel/lb_policy/outlier_detection/outlier_detection.h +++ b/src/core/ext/filters/client_channel/lb_policy/outlier_detection/outlier_detection.h @@ -21,9 +21,6 @@ #include // for uint32_t -#include -#include - #include "absl/types/optional.h" #include "src/core/lib/gprpp/time.h" @@ -96,19 +93,8 @@ struct OutlierDetectionConfig { // TODO(roth): This is a horrible hack used to disable outlier detection // when used with the pick_first policy. Remove this as part of // implementing the dualstack backend design. -class DisableOutlierDetectionAttribute - : public ServerAddress::AttributeInterface { - public: - static const char* kName; - - std::unique_ptr Copy() const override { - return std::make_unique(); - } - - int Cmp(const AttributeInterface*) const override { return true; } - - std::string ToString() const override { return "true"; } -}; +#define GRPC_ARG_OUTLIER_DETECTION_DISABLE \ + GRPC_ARG_NO_SUBCHANNEL_PREFIX "outlier_detection_disable" } // namespace grpc_core diff --git a/src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc b/src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc index 8cc2b158e73..076ae6bad6b 100644 --- a/src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc +++ b/src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc @@ -330,9 +330,9 @@ absl::Status PickFirst::UpdateLocked(UpdateArgs args) { if (args.addresses.ok()) { ServerAddressList addresses; for (const auto& address : *args.addresses) { - addresses.emplace_back(address.WithAttribute( - DisableOutlierDetectionAttribute::kName, - std::make_unique())); + addresses.emplace_back( + address.address(), + address.args().Set(GRPC_ARG_OUTLIER_DETECTION_DISABLE, 1)); } args.addresses = std::move(addresses); } diff --git a/src/core/ext/filters/client_channel/lb_policy/ring_hash/ring_hash.cc b/src/core/ext/filters/client_channel/lb_policy/ring_hash/ring_hash.cc index d104dbe520e..5eb92861448 100644 --- a/src/core/ext/filters/client_channel/lb_policy/ring_hash/ring_hash.cc +++ b/src/core/ext/filters/client_channel/lb_policy/ring_hash/ring_hash.cc @@ -470,16 +470,14 @@ RingHash::RingHashSubchannelList::Ring::Ring( address_weights.reserve(subchannel_list->num_subchannels()); for (size_t i = 0; i < subchannel_list->num_subchannels(); ++i) { RingHashSubchannelData* sd = subchannel_list->subchannel(i); - const ServerAddressWeightAttribute* weight_attribute = static_cast< - const ServerAddressWeightAttribute*>(sd->address().GetAttribute( - ServerAddressWeightAttribute::kServerAddressWeightAttributeKey)); + auto weight_arg = sd->address().args().GetInt(GRPC_ARG_ADDRESS_WEIGHT); AddressWeight address_weight; address_weight.address = grpc_sockaddr_to_string(&sd->address().address(), false).value(); // Weight should never be zero, but ignore it just in case, since // that value would screw up the ring-building algorithm. - if (weight_attribute != nullptr && weight_attribute->weight() > 0) { - address_weight.weight = weight_attribute->weight(); + if (weight_arg.value_or(0) > 0) { + address_weight.weight = *weight_arg; } sum += address_weight.weight; address_weights.push_back(std::move(address_weight)); diff --git a/src/core/ext/filters/client_channel/lb_policy/xds/xds_attributes.cc b/src/core/ext/filters/client_channel/lb_policy/xds/xds_attributes.cc deleted file mode 100644 index e6918fa4d4e..00000000000 --- a/src/core/ext/filters/client_channel/lb_policy/xds/xds_attributes.cc +++ /dev/null @@ -1,42 +0,0 @@ -// -// Copyright 2018 gRPC authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// - -#include - -#include "src/core/ext/filters/client_channel/lb_policy/xds/xds_attributes.h" - -#include "absl/strings/str_cat.h" - -#include "src/core/lib/gpr/useful.h" - -namespace grpc_core { - -const char* kXdsLocalityNameAttributeKey = "xds_locality_name"; - -int XdsLocalityAttribute::Cmp(const AttributeInterface* other) const { - const auto* other_locality_attr = - static_cast(other); - int r = locality_name_->Compare(*other_locality_attr->locality_name_); - if (r != 0) return r; - return QsortCompare(weight_, other_locality_attr->weight_); -} - -std::string XdsLocalityAttribute::ToString() const { - return absl::StrCat("{name=", locality_name_->AsHumanReadableString(), - ", weight=", weight_, "}"); -} - -} // namespace grpc_core diff --git a/src/core/ext/filters/client_channel/lb_policy/xds/xds_attributes.h b/src/core/ext/filters/client_channel/lb_policy/xds/xds_attributes.h deleted file mode 100644 index bb8d52273fe..00000000000 --- a/src/core/ext/filters/client_channel/lb_policy/xds/xds_attributes.h +++ /dev/null @@ -1,64 +0,0 @@ -// -// Copyright 2018 gRPC authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// - -#ifndef GRPC_SRC_CORE_EXT_FILTERS_CLIENT_CHANNEL_LB_POLICY_XDS_XDS_ATTRIBUTES_H -#define GRPC_SRC_CORE_EXT_FILTERS_CLIENT_CHANNEL_LB_POLICY_XDS_XDS_ATTRIBUTES_H - -#include - -#include - -#include -#include -#include - -#include "src/core/ext/xds/xds_client_stats.h" -#include "src/core/lib/gprpp/ref_counted_ptr.h" -#include "src/core/lib/resolver/server_address.h" - -namespace grpc_core { - -extern const char* kXdsLocalityNameAttributeKey; - -class XdsLocalityAttribute : public ServerAddress::AttributeInterface { - public: - XdsLocalityAttribute(RefCountedPtr locality_name, - uint32_t weight) - : locality_name_(std::move(locality_name)), weight_(weight) {} - - RefCountedPtr locality_name() const { - return locality_name_; - } - - uint32_t weight() const { return weight_; } - - std::unique_ptr Copy() const override { - return std::make_unique(locality_name_->Ref(), - weight_); - } - - int Cmp(const AttributeInterface* other) const override; - - std::string ToString() const override; - - private: - RefCountedPtr locality_name_; - uint32_t weight_; -}; - -} // namespace grpc_core - -#endif // GRPC_SRC_CORE_EXT_FILTERS_CLIENT_CHANNEL_LB_POLICY_XDS_XDS_ATTRIBUTES_H diff --git a/src/core/ext/filters/client_channel/lb_policy/xds/xds_channel_args.h b/src/core/ext/filters/client_channel/lb_policy/xds/xds_channel_args.h index 290e67ca116..c7bbf197da0 100644 --- a/src/core/ext/filters/client_channel/lb_policy/xds/xds_channel_args.h +++ b/src/core/ext/filters/client_channel/lb_policy/xds/xds_channel_args.h @@ -17,6 +17,10 @@ #ifndef GRPC_SRC_CORE_EXT_FILTERS_CLIENT_CHANNEL_LB_POLICY_XDS_XDS_CHANNEL_ARGS_H #define GRPC_SRC_CORE_EXT_FILTERS_CLIENT_CHANNEL_LB_POLICY_XDS_XDS_CHANNEL_ARGS_H +#include + +#include "src/core/lib/resolver/server_address.h" + // Channel arg indicating the xDS cluster name. // Set by xds_cluster_impl LB policy and used by GoogleDefaultCredentials. #define GRPC_ARG_XDS_CLUSTER_NAME "grpc.internal.xds_cluster_name" @@ -26,4 +30,8 @@ #define GRPC_ARG_XDS_LOGICAL_DNS_CLUSTER_FAKE_RESOLVER_RESPONSE_GENERATOR \ "grpc.TEST_ONLY.xds_logical_dns_cluster_fake_resolver_response_generator" +// Channel arg for encoding xDS locality weight. +#define GRPC_ARG_XDS_LOCALITY_WEIGHT \ + GRPC_ARG_NO_SUBCHANNEL_PREFIX "xds_locality_weight" + #endif // GRPC_SRC_CORE_EXT_FILTERS_CLIENT_CHANNEL_LB_POLICY_XDS_XDS_CHANNEL_ARGS_H diff --git a/src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_impl.cc b/src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_impl.cc index 566e53ae0e5..158428996f7 100644 --- a/src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_impl.cc +++ b/src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_impl.cc @@ -39,7 +39,6 @@ #include "src/core/ext/filters/client_channel/lb_policy/backend_metric_data.h" #include "src/core/ext/filters/client_channel/lb_policy/child_policy_handler.h" -#include "src/core/ext/filters/client_channel/lb_policy/xds/xds_attributes.h" #include "src/core/ext/filters/client_channel/lb_policy/xds/xds_channel_args.h" #include "src/core/ext/xds/xds_bootstrap.h" #include "src/core/ext/xds/xds_bootstrap_grpc.h" @@ -600,13 +599,7 @@ RefCountedPtr XdsClusterImplLb::Helper::CreateSubchannel( // If load reporting is enabled, wrap the subchannel such that it // includes the locality stats object, which will be used by the Picker. if (parent()->config_->lrs_load_reporting_server().has_value()) { - RefCountedPtr locality_name; - auto* attribute = address.GetAttribute(kXdsLocalityNameAttributeKey); - if (attribute != nullptr) { - const auto* locality_attr = - static_cast(attribute); - locality_name = locality_attr->locality_name(); - } + auto locality_name = address.args().GetObjectRef(); RefCountedPtr locality_stats = parent()->xds_client_->AddClusterLocalityStats( parent()->config_->lrs_load_reporting_server().value(), diff --git a/src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_resolver.cc b/src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_resolver.cc index 27193add42e..b4db0dbec14 100644 --- a/src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_resolver.cc +++ b/src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_resolver.cc @@ -42,7 +42,6 @@ #include "src/core/ext/filters/client_channel/lb_policy/address_filtering.h" #include "src/core/ext/filters/client_channel/lb_policy/child_policy_handler.h" -#include "src/core/ext/filters/client_channel/lb_policy/xds/xds_attributes.h" #include "src/core/ext/filters/client_channel/lb_policy/xds/xds_channel_args.h" #include "src/core/ext/filters/client_channel/resolver/fake/fake_resolver.h" #include "src/core/ext/xds/xds_bootstrap.h" @@ -768,25 +767,17 @@ ServerAddressList XdsClusterResolverLb::CreateChildPolicyAddressesLocked() { std::vector hierarchical_path = { priority_child_name, locality_name->AsHumanReadableString()}; for (const auto& endpoint : locality.endpoints) { - const ServerAddressWeightAttribute* weight_attribute = static_cast< - const ServerAddressWeightAttribute*>(endpoint.GetAttribute( - ServerAddressWeightAttribute::kServerAddressWeightAttributeKey)); - uint32_t weight = locality.lb_weight; - if (weight_attribute != nullptr) { - weight = locality.lb_weight * weight_attribute->weight(); - } + uint32_t endpoint_weight = + locality.lb_weight * + endpoint.args().GetInt(GRPC_ARG_ADDRESS_WEIGHT).value_or(1); addresses.emplace_back( - endpoint - .WithAttribute( - kHierarchicalPathAttributeKey, - MakeHierarchicalPathAttribute(hierarchical_path)) - .WithAttribute(kXdsLocalityNameAttributeKey, - std::make_unique( - locality_name->Ref(), locality.lb_weight)) - .WithAttribute( - ServerAddressWeightAttribute:: - kServerAddressWeightAttributeKey, - std::make_unique(weight))); + endpoint.address(), + endpoint.args() + .SetObject( + MakeRefCounted(hierarchical_path)) + .Set(GRPC_ARG_ADDRESS_WEIGHT, endpoint_weight) + .SetObject(locality_name->Ref()) + .Set(GRPC_ARG_XDS_LOCALITY_WEIGHT, locality.lb_weight)); } } } diff --git a/src/core/ext/filters/client_channel/lb_policy/xds/xds_override_host.cc b/src/core/ext/filters/client_channel/lb_policy/xds/xds_override_host.cc index 96f5fee13b3..c5445c3dcf2 100644 --- a/src/core/ext/filters/client_channel/lb_policy/xds/xds_override_host.cc +++ b/src/core/ext/filters/client_channel/lb_policy/xds/xds_override_host.cc @@ -95,12 +95,10 @@ struct PtrLessThan { }; XdsHealthStatus GetAddressHealthStatus(const ServerAddress& address) { - auto attribute = address.GetAttribute(XdsEndpointHealthStatusAttribute::kKey); - if (attribute == nullptr) { - return XdsHealthStatus(XdsHealthStatus::HealthStatus::kUnknown); - } - return static_cast(attribute) - ->status(); + return XdsHealthStatus(static_cast( + address.args() + .GetInt(GRPC_ARG_XDS_HEALTH_STATUS) + .value_or(XdsHealthStatus::HealthStatus::kUnknown))); } // diff --git a/src/core/ext/filters/client_channel/lb_policy/xds/xds_wrr_locality.cc b/src/core/ext/filters/client_channel/lb_policy/xds/xds_wrr_locality.cc index fc709f4100d..fa4918633c9 100644 --- a/src/core/ext/filters/client_channel/lb_policy/xds/xds_wrr_locality.cc +++ b/src/core/ext/filters/client_channel/lb_policy/xds/xds_wrr_locality.cc @@ -27,12 +27,13 @@ #include "absl/status/statusor.h" #include "absl/strings/str_cat.h" #include "absl/strings/string_view.h" +#include "absl/types/optional.h" #include #include #include -#include "src/core/ext/filters/client_channel/lb_policy/xds/xds_attributes.h" +#include "src/core/ext/filters/client_channel/lb_policy/xds/xds_channel_args.h" #include "src/core/ext/xds/xds_client_stats.h" #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/config/core_configuration.h" @@ -170,17 +171,17 @@ absl::Status XdsWrrLocalityLb::UpdateLocked(UpdateArgs args) { std::map locality_weights; if (args.addresses.ok()) { for (const auto& address : *args.addresses) { - auto* attribute = static_cast( - address.GetAttribute(kXdsLocalityNameAttributeKey)); - if (attribute != nullptr) { + auto* locality_name = address.args().GetObject(); + uint32_t weight = + address.args().GetInt(GRPC_ARG_XDS_LOCALITY_WEIGHT).value_or(0); + if (locality_name != nullptr && weight > 0) { auto p = locality_weights.emplace( - attribute->locality_name()->AsHumanReadableString(), - attribute->weight()); - if (!p.second && p.first->second != attribute->weight()) { + locality_name->AsHumanReadableString(), weight); + if (!p.second && p.first->second != weight) { gpr_log(GPR_ERROR, "INTERNAL ERROR: xds_wrr_locality found different weights " - "for locality %s (%d vs %d); using first value", - p.first->first.c_str(), p.first->second, attribute->weight()); + "for locality %s (%u vs %u); using first value", + p.first->first.c_str(), p.first->second, weight); } } } diff --git a/src/core/ext/xds/xds_client_grpc.h b/src/core/ext/xds/xds_client_grpc.h index a258b2a3118..7ecb96cb52a 100644 --- a/src/core/ext/xds/xds_client_grpc.h +++ b/src/core/ext/xds/xds_client_grpc.h @@ -35,6 +35,7 @@ #include "src/core/lib/gprpp/orphanable.h" #include "src/core/lib/gprpp/ref_counted_ptr.h" #include "src/core/lib/iomgr/iomgr_fwd.h" +#include "src/core/lib/resolver/server_address.h" namespace grpc_core { @@ -63,7 +64,7 @@ class GrpcXdsClient : public XdsClient { // Helpers for encoding the XdsClient object in channel args. static absl::string_view ChannelArgName() { - return "grpc.internal.xds_client"; + return GRPC_ARG_NO_SUBCHANNEL_PREFIX "xds_client"; } static int ChannelArgsCompare(const XdsClient* a, const XdsClient* b) { return QsortCompare(a, b); diff --git a/src/core/ext/xds/xds_client_stats.h b/src/core/ext/xds/xds_client_stats.h index 3f3a776f613..0a5a293ba68 100644 --- a/src/core/ext/xds/xds_client_stats.h +++ b/src/core/ext/xds/xds_client_stats.h @@ -38,6 +38,7 @@ #include "src/core/lib/gprpp/ref_counted.h" #include "src/core/lib/gprpp/ref_counted_ptr.h" #include "src/core/lib/gprpp/sync.h" +#include "src/core/lib/resolver/server_address.h" namespace grpc_core { @@ -95,6 +96,15 @@ class XdsLocalityName : public RefCounted { return human_readable_string_; } + // Channel args traits. + static absl::string_view ChannelArgName() { + return GRPC_ARG_NO_SUBCHANNEL_PREFIX "xds_locality_name"; + } + static int ChannelArgsCompare(const XdsLocalityName* a, + const XdsLocalityName* b) { + return a->Compare(*b); + } + private: std::string region_; std::string zone_; diff --git a/src/core/ext/xds/xds_endpoint.cc b/src/core/ext/xds/xds_endpoint.cc index 2638423d1c1..df6475b9740 100644 --- a/src/core/ext/xds/xds_endpoint.cc +++ b/src/core/ext/xds/xds_endpoint.cc @@ -220,13 +220,10 @@ absl::optional ServerAddressParse( } } // Convert to ServerAddress. - std::map> - attributes; - attributes[ServerAddressWeightAttribute::kServerAddressWeightAttributeKey] = - std::make_unique(weight); - attributes[XdsEndpointHealthStatusAttribute::kKey] = - std::make_unique(*status); - return ServerAddress(grpc_address, ChannelArgs(), std::move(attributes)); + return ServerAddress(grpc_address, + ChannelArgs() + .Set(GRPC_ARG_ADDRESS_WEIGHT, weight) + .Set(GRPC_ARG_XDS_HEALTH_STATUS, status->status())); } struct ParsedLocality { diff --git a/src/core/ext/xds/xds_health_status.cc b/src/core/ext/xds/xds_health_status.cc index e5c3a924802..8626a6f46f6 100644 --- a/src/core/ext/xds/xds_health_status.cc +++ b/src/core/ext/xds/xds_health_status.cc @@ -18,11 +18,8 @@ #include "src/core/ext/xds/xds_health_status.h" -#include "absl/strings/str_cat.h" #include "envoy/config/core/v3/health_check.upb.h" -#include "src/core/lib/gpr/useful.h" - namespace grpc_core { absl::optional XdsHealthStatus::FromUpb(uint32_t status) { @@ -63,18 +60,4 @@ bool operator<(const XdsHealthStatus& hs1, const XdsHealthStatus& hs2) { return hs1.status() < hs2.status(); } -const char* XdsEndpointHealthStatusAttribute::kKey = - "xds_endpoint_health_status"; - -int XdsEndpointHealthStatusAttribute::Cmp( - const AttributeInterface* other) const { - const auto* other_attr = - static_cast(other); - return QsortCompare(status_, other_attr->status_); -} - -std::string XdsEndpointHealthStatusAttribute::ToString() const { - return absl::StrCat("{status_=", status_.ToString(), "}"); -} - } // namespace grpc_core diff --git a/src/core/ext/xds/xds_health_status.h b/src/core/ext/xds/xds_health_status.h index c9c27315953..c680a03aa6a 100644 --- a/src/core/ext/xds/xds_health_status.h +++ b/src/core/ext/xds/xds_health_status.h @@ -21,15 +21,17 @@ #include -#include -#include - #include "absl/strings/string_view.h" #include "absl/types/optional.h" #include "absl/types/span.h" #include "src/core/lib/resolver/server_address.h" +// Channel arg key for xDS health status. +// Value is an XdsHealthStatus::HealthStatus enum. +#define GRPC_ARG_XDS_HEALTH_STATUS \ + GRPC_ARG_NO_SUBCHANNEL_PREFIX "xds_health_status" + namespace grpc_core { class XdsHealthStatus { @@ -82,28 +84,6 @@ class XdsHealthStatusSet { bool operator<(const XdsHealthStatus& hs1, const XdsHealthStatus& hs2); -class XdsEndpointHealthStatusAttribute - : public ServerAddress::AttributeInterface { - public: - static const char* kKey; - - explicit XdsEndpointHealthStatusAttribute(XdsHealthStatus status) - : status_(status) {} - - XdsHealthStatus status() const { return status_; } - - std::unique_ptr Copy() const override { - return std::make_unique(status_); - } - - int Cmp(const AttributeInterface* other) const override; - - std::string ToString() const override; - - private: - XdsHealthStatus status_; -}; - } // namespace grpc_core #endif // GRPC_SRC_CORE_EXT_XDS_XDS_HEALTH_STATUS_H diff --git a/src/core/lib/channel/channel_args.cc b/src/core/lib/channel/channel_args.cc index 9fced0de4a1..52bcb1d087e 100644 --- a/src/core/lib/channel/channel_args.cc +++ b/src/core/lib/channel/channel_args.cc @@ -210,6 +210,17 @@ ChannelArgs ChannelArgs::Remove(absl::string_view key) const { return ChannelArgs(args_.Remove(key)); } +ChannelArgs ChannelArgs::RemoveAllKeysWithPrefix( + absl::string_view prefix) const { + ChannelArgs result; + args_.ForEach([&](const std::string& key, const Value& value) { + if (!absl::StartsWith(key, prefix)) { + result.args_ = result.args_.Add(key, value); + } + }); + return result; +} + absl::optional ChannelArgs::GetInt(absl::string_view name) const { auto* v = Get(name); if (v == nullptr) return absl::nullopt; diff --git a/src/core/lib/channel/channel_args.h b/src/core/lib/channel/channel_args.h index 8bf1a9c84c7..57d6f35e335 100644 --- a/src/core/lib/channel/channel_args.h +++ b/src/core/lib/channel/channel_args.h @@ -395,6 +395,9 @@ class ChannelArgs { GRPC_MUST_USE_RESULT ChannelArgs Remove(absl::string_view name) const; bool Contains(absl::string_view name) const; + GRPC_MUST_USE_RESULT ChannelArgs + RemoveAllKeysWithPrefix(absl::string_view prefix) const; + template bool ContainsObject() const { return Get(ChannelArgNameTraits::ChannelArgName()) != nullptr; diff --git a/src/core/lib/resolver/server_address.cc b/src/core/lib/resolver/server_address.cc index 140d6ef7155..96a9f7ddff6 100644 --- a/src/core/lib/resolver/server_address.cc +++ b/src/core/lib/resolver/server_address.cc @@ -23,8 +23,6 @@ #include #include -#include -#include #include #include #include @@ -32,119 +30,49 @@ #include "absl/status/status.h" #include "absl/status/statusor.h" #include "absl/strings/str_cat.h" -#include "absl/strings/str_format.h" #include "absl/strings/str_join.h" #include "src/core/lib/address_utils/sockaddr_utils.h" #include "src/core/lib/channel/channel_args.h" +#include "src/core/lib/gpr/useful.h" // IWYU pragma: no_include namespace grpc_core { -// -// ServerAddressWeightAttribute -// -const char* ServerAddressWeightAttribute::kServerAddressWeightAttributeKey = - "server_address_weight"; - // // ServerAddress // -ServerAddress::ServerAddress( - const grpc_resolved_address& address, const ChannelArgs& args, - std::map> attributes) - : address_(address), args_(args), attributes_(std::move(attributes)) {} +ServerAddress::ServerAddress(const grpc_resolved_address& address, + const ChannelArgs& args) + : address_(address), args_(args) {} ServerAddress::ServerAddress(const ServerAddress& other) - : address_(other.address_), args_(other.args_) { - for (const auto& p : other.attributes_) { - attributes_[p.first] = p.second->Copy(); - } -} + : address_(other.address_), args_(other.args_) {} + ServerAddress& ServerAddress::operator=(const ServerAddress& other) { - if (&other == this) { - return *this; - } + if (&other == this) return *this; address_ = other.address_; args_ = other.args_; - attributes_.clear(); - for (const auto& p : other.attributes_) { - attributes_[p.first] = p.second->Copy(); - } return *this; } ServerAddress::ServerAddress(ServerAddress&& other) noexcept - : address_(other.address_), - args_(std::move(other.args_)), - attributes_(std::move(other.attributes_)) {} + : address_(other.address_), args_(std::move(other.args_)) {} ServerAddress& ServerAddress::operator=(ServerAddress&& other) noexcept { address_ = other.address_; args_ = std::move(other.args_); - attributes_ = std::move(other.attributes_); return *this; } -namespace { - -int CompareAttributes( - const std::map>& - attributes1, - const std::map>& - attributes2) { - auto it2 = attributes2.begin(); - for (auto it1 = attributes1.begin(); it1 != attributes1.end(); ++it1) { - // attributes2 has fewer elements than attributes1 - if (it2 == attributes2.end()) return -1; - // compare keys - int retval = strcmp(it1->first, it2->first); - if (retval != 0) return retval; - // compare values - retval = it1->second->Cmp(it2->second.get()); - if (retval != 0) return retval; - ++it2; - } - // attributes1 has fewer elements than attributes2 - if (it2 != attributes2.end()) return 1; - // equal - return 0; -} - -} // namespace - int ServerAddress::Cmp(const ServerAddress& other) const { if (address_.len > other.address_.len) return 1; if (address_.len < other.address_.len) return -1; int retval = memcmp(address_.addr, other.address_.addr, address_.len); if (retval != 0) return retval; - retval = QsortCompare(args_, other.args_); - if (retval != 0) return retval; - return CompareAttributes(attributes_, other.attributes_); -} - -const ServerAddress::AttributeInterface* ServerAddress::GetAttribute( - const char* key) const { - auto it = attributes_.find(key); - if (it == attributes_.end()) return nullptr; - return it->second.get(); -} - -// Returns a copy of the address with a modified attribute. -// If the new value is null, the attribute is removed. -ServerAddress ServerAddress::WithAttribute( - const char* key, std::unique_ptr value) const { - ServerAddress address = *this; - if (value == nullptr) { - address.attributes_.erase(key); - } else { - address.attributes_[key] = std::move(value); - } - return address; + return QsortCompare(args_, other.args_); } std::string ServerAddress::ToString() const { @@ -155,21 +83,7 @@ std::string ServerAddress::ToString() const { if (args_ != ChannelArgs()) { parts.emplace_back(absl::StrCat("args=", args_.ToString())); } - if (!attributes_.empty()) { - std::vector attrs; - attrs.reserve(attributes_.size()); - for (const auto& p : attributes_) { - attrs.emplace_back(absl::StrCat(p.first, "=", p.second->ToString())); - } - std::sort(attrs.begin(), attrs.end()); - parts.emplace_back( - absl::StrCat("attributes={", absl::StrJoin(attrs, ", "), "}")); - } return absl::StrJoin(parts, " "); } -std::string ServerAddressWeightAttribute::ToString() const { - return absl::StrFormat("%d", weight_); -} - } // namespace grpc_core diff --git a/src/core/lib/resolver/server_address.h b/src/core/lib/resolver/server_address.h index 05d8dedc4b6..d42a70f9d74 100644 --- a/src/core/lib/resolver/server_address.h +++ b/src/core/lib/resolver/server_address.h @@ -21,17 +21,21 @@ #include -#include - -#include -#include #include #include #include "src/core/lib/channel/channel_args.h" -#include "src/core/lib/gpr/useful.h" #include "src/core/lib/iomgr/resolved_address.h" +// A channel arg key prefix used for args that are intended to be used +// only internally to resolvers and LB policies and should not be part +// of the subchannel key. The channel will automatically filter out any +// args with this prefix from the subchannel's args. +#define GRPC_ARG_NO_SUBCHANNEL_PREFIX "grpc.internal.no_subchannel." + +// A channel arg indicating the weight of an address. +#define GRPC_ARG_ADDRESS_WEIGHT GRPC_ARG_NO_SUBCHANNEL_PREFIX "address.weight" + namespace grpc_core { // @@ -43,30 +47,7 @@ namespace grpc_core { // args when a subchannel is created for this address. class ServerAddress { public: - // Base class for resolver-supplied attributes. - // Unlike channel args, these attributes don't affect subchannel - // uniqueness or behavior. They are for use by LB policies only. - // - // Attributes are keyed by a C string that is unique by address, not - // by value. All attributes added with the same key must be of the - // same type. - class AttributeInterface { - public: - virtual ~AttributeInterface() = default; - - // Creates a copy of the attribute. - virtual std::unique_ptr Copy() const = 0; - - // Compares this attribute with another. - virtual int Cmp(const AttributeInterface* other) const = 0; - - // Returns a human-readable representation of the attribute. - virtual std::string ToString() const = 0; - }; - - ServerAddress(const grpc_resolved_address& address, const ChannelArgs& args, - std::map> - attributes = {}); + ServerAddress(const grpc_resolved_address& address, const ChannelArgs& args); // Copyable. ServerAddress(const ServerAddress& other); @@ -83,13 +64,6 @@ class ServerAddress { const grpc_resolved_address& address() const { return address_; } const ChannelArgs& args() const { return args_; } - const AttributeInterface* GetAttribute(const char* key) const; - - // Returns a copy of the address with a modified attribute. - // If the new value is null, the attribute is removed. - ServerAddress WithAttribute(const char* key, - std::unique_ptr value) const; - // TODO(ctiller): Prior to making this a public API we should ensure that the // channel args are not part of the generated string, lest we make that debug // format load-bearing via Hyrum's law. @@ -98,7 +72,6 @@ class ServerAddress { private: grpc_resolved_address address_; ChannelArgs args_; - std::map> attributes_; }; // @@ -107,33 +80,6 @@ class ServerAddress { using ServerAddressList = std::vector; -// -// ServerAddressWeightAttribute -// -class ServerAddressWeightAttribute : public ServerAddress::AttributeInterface { - public: - static const char* kServerAddressWeightAttributeKey; - - explicit ServerAddressWeightAttribute(uint32_t weight) : weight_(weight) {} - - uint32_t weight() const { return weight_; } - - std::unique_ptr Copy() const override { - return std::make_unique(weight_); - } - - int Cmp(const AttributeInterface* other) const override { - const auto* other_locality_attr = - static_cast(other); - return QsortCompare(weight_, other_locality_attr->weight_); - } - - std::string ToString() const override; - - private: - uint32_t weight_; -}; - } // namespace grpc_core #endif // GRPC_SRC_CORE_LIB_RESOLVER_SERVER_ADDRESS_H diff --git a/src/python/grpcio/grpc_core_dependencies.py b/src/python/grpcio/grpc_core_dependencies.py index 69ca1aac096..411e16f38cb 100644 --- a/src/python/grpcio/grpc_core_dependencies.py +++ b/src/python/grpcio/grpc_core_dependencies.py @@ -50,7 +50,6 @@ CORE_SOURCE_FILES = [ 'src/core/ext/filters/client_channel/lb_policy/weighted_round_robin/weighted_round_robin.cc', 'src/core/ext/filters/client_channel/lb_policy/weighted_target/weighted_target.cc', 'src/core/ext/filters/client_channel/lb_policy/xds/cds.cc', - 'src/core/ext/filters/client_channel/lb_policy/xds/xds_attributes.cc', 'src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_impl.cc', 'src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_manager.cc', 'src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_resolver.cc', diff --git a/test/core/channel/channel_args_test.cc b/test/core/channel/channel_args_test.cc index ead8ecd4310..a7e55f9724a 100644 --- a/test/core/channel/channel_args_test.cc +++ b/test/core/channel/channel_args_test.cc @@ -73,6 +73,19 @@ TEST(ChannelArgsTest, SetGetRemove) { gpr_free(ptr); } +TEST(ChannelArgsTest, RemoveAllKeysWithPrefix) { + ChannelArgs args; + args = args.Set("foo", 1); + args = args.Set("foo.bar", 2); + args = args.Set("foo.baz", 3); + args = args.Set("bar", 4); + ChannelArgs modified = args.RemoveAllKeysWithPrefix("foo."); + EXPECT_EQ(modified.GetInt("foo"), 1); + EXPECT_EQ(modified.GetInt("foo.bar"), absl::nullopt); + EXPECT_EQ(modified.GetInt("foo.baz"), absl::nullopt); + EXPECT_EQ(modified.GetInt("bar"), 4); +} + TEST(ChannelArgsTest, StoreRefCountedPtr) { struct Test : public RefCounted { explicit Test(int n) : n(n) {} diff --git a/test/core/client_channel/client_channel_test.cc b/test/core/client_channel/client_channel_test.cc index 3508cfa1515..58f16e2166e 100644 --- a/test/core/client_channel/client_channel_test.cc +++ b/test/core/client_channel/client_channel_test.cc @@ -18,12 +18,14 @@ #include "src/core/ext/filters/client_channel/client_channel.h" +#include "absl/types/optional.h" #include "gtest/gtest.h" #include #include "src/core/ext/filters/client_channel/subchannel_pool_interface.h" #include "src/core/lib/channel/channel_args.h" +#include "src/core/lib/resolver/server_address.h" #include "test/core/util/test_config.h" namespace grpc_core { @@ -60,6 +62,22 @@ TEST(MakeSubchannelArgs, EXPECT_EQ(args.GetString(GRPC_ARG_DEFAULT_AUTHORITY), "bar.example.com"); } +TEST(MakeSubchannelArgs, ArgsFromChannelTrumpPerAddressArgs) { + ChannelArgs args = ClientChannel::MakeSubchannelArgs( + ChannelArgs().Set("foo", 1), ChannelArgs().Set("foo", 2), nullptr, + "foo.example.com"); + EXPECT_EQ(args.GetInt("foo"), 1); +} + +TEST(MakeSubchannelArgs, StripsOutNoSubchannelArgs) { + ChannelArgs args = ClientChannel::MakeSubchannelArgs( + ChannelArgs().Set(GRPC_ARG_NO_SUBCHANNEL_PREFIX "foo", 1), + ChannelArgs().Set(GRPC_ARG_NO_SUBCHANNEL_PREFIX "bar", 1), nullptr, + "foo.example.com"); + EXPECT_EQ(args.GetString(GRPC_ARG_NO_SUBCHANNEL_PREFIX "foo"), absl::nullopt); + EXPECT_EQ(args.GetString(GRPC_ARG_NO_SUBCHANNEL_PREFIX "bar"), absl::nullopt); +} + } // namespace } // namespace testing } // namespace grpc_core diff --git a/test/core/client_channel/lb_policy/xds_override_host_test.cc b/test/core/client_channel/lb_policy/xds_override_host_test.cc index 541682aff6d..26cfc7a60d4 100644 --- a/test/core/client_channel/lb_policy/xds_override_host_test.cc +++ b/test/core/client_channel/lb_policy/xds_override_host_test.cc @@ -16,7 +16,6 @@ #include #include -#include #include #include #include @@ -33,6 +32,7 @@ #include "src/core/ext/filters/stateful_session/stateful_session_filter.h" #include "src/core/ext/xds/xds_health_status.h" +#include "src/core/lib/channel/channel_args.h" #include "src/core/lib/gprpp/orphanable.h" #include "src/core/lib/gprpp/ref_counted_ptr.h" #include "src/core/lib/json/json.h" @@ -78,12 +78,8 @@ class XdsOverrideHostTest : public LoadBalancingPolicyTest { ServerAddress MakeAddressWithHealthStatus( absl::string_view address, XdsHealthStatus::HealthStatus status) { - std::map> - attrs; - attrs.emplace(XdsEndpointHealthStatusAttribute::kKey, - std::make_unique( - XdsHealthStatus(status))); - return {MakeAddress(address), {}, std::move(attrs)}; + return ServerAddress(MakeAddress(address), + ChannelArgs().Set(GRPC_ARG_XDS_HEALTH_STATUS, status)); } void ApplyUpdateWithHealthStatuses( diff --git a/test/core/xds/xds_endpoint_resource_type_test.cc b/test/core/xds/xds_endpoint_resource_type_test.cc index 24294f93f98..65a4afd0d2d 100644 --- a/test/core/xds/xds_endpoint_resource_type_test.cc +++ b/test/core/xds/xds_endpoint_resource_type_test.cc @@ -161,12 +161,10 @@ TEST_F(XdsEndpointTest, MinimumValidConfig) { auto addr = grpc_sockaddr_to_string(&address.address(), /*normalize=*/false); ASSERT_TRUE(addr.ok()) << addr.status(); EXPECT_EQ(*addr, "127.0.0.1:443"); - EXPECT_EQ(address.args(), ChannelArgs()); - const auto* attribute = - static_cast(address.GetAttribute( - ServerAddressWeightAttribute::kServerAddressWeightAttributeKey)); - ASSERT_NE(attribute, nullptr); - EXPECT_EQ(attribute->weight(), 1); + EXPECT_EQ(address.args(), ChannelArgs() + .Set(GRPC_ARG_ADDRESS_WEIGHT, 1) + .Set(GRPC_ARG_XDS_HEALTH_STATUS, + XdsHealthStatus::HealthStatus::kUnknown)); ASSERT_NE(resource.drop_config, nullptr); EXPECT_TRUE(resource.drop_config->drop_category_list().empty()); } @@ -209,12 +207,10 @@ TEST_F(XdsEndpointTest, EndpointWeight) { auto addr = grpc_sockaddr_to_string(&address.address(), /*normalize=*/false); ASSERT_TRUE(addr.ok()) << addr.status(); EXPECT_EQ(*addr, "127.0.0.1:443"); - EXPECT_EQ(address.args(), ChannelArgs()); - const auto* attribute = - static_cast(address.GetAttribute( - ServerAddressWeightAttribute::kServerAddressWeightAttributeKey)); - ASSERT_NE(attribute, nullptr); - EXPECT_EQ(attribute->weight(), 3); + EXPECT_EQ(address.args(), ChannelArgs() + .Set(GRPC_ARG_ADDRESS_WEIGHT, 3) + .Set(GRPC_ARG_XDS_HEALTH_STATUS, + XdsHealthStatus::HealthStatus::kUnknown)); ASSERT_NE(resource.drop_config, nullptr); EXPECT_TRUE(resource.drop_config->drop_category_list().empty()); } @@ -259,12 +255,10 @@ TEST_F(XdsEndpointTest, IgnoresLocalityWithNoWeight) { auto addr = grpc_sockaddr_to_string(&address.address(), /*normalize=*/false); ASSERT_TRUE(addr.ok()) << addr.status(); EXPECT_EQ(*addr, "127.0.0.1:443"); - EXPECT_EQ(address.args(), ChannelArgs()); - const auto* attribute = - static_cast(address.GetAttribute( - ServerAddressWeightAttribute::kServerAddressWeightAttributeKey)); - ASSERT_NE(attribute, nullptr); - EXPECT_EQ(attribute->weight(), 1); + EXPECT_EQ(address.args(), ChannelArgs() + .Set(GRPC_ARG_ADDRESS_WEIGHT, 1) + .Set(GRPC_ARG_XDS_HEALTH_STATUS, + XdsHealthStatus::HealthStatus::kUnknown)); ASSERT_NE(resource.drop_config, nullptr); EXPECT_TRUE(resource.drop_config->drop_category_list().empty()); } @@ -310,12 +304,10 @@ TEST_F(XdsEndpointTest, IgnoresLocalityWithZeroWeight) { auto addr = grpc_sockaddr_to_string(&address.address(), /*normalize=*/false); ASSERT_TRUE(addr.ok()) << addr.status(); EXPECT_EQ(*addr, "127.0.0.1:443"); - EXPECT_EQ(address.args(), ChannelArgs()); - const auto* attribute = - static_cast(address.GetAttribute( - ServerAddressWeightAttribute::kServerAddressWeightAttributeKey)); - ASSERT_NE(attribute, nullptr); - EXPECT_EQ(attribute->weight(), 1); + EXPECT_EQ(address.args(), ChannelArgs() + .Set(GRPC_ARG_ADDRESS_WEIGHT, 1) + .Set(GRPC_ARG_XDS_HEALTH_STATUS, + XdsHealthStatus::HealthStatus::kUnknown)); ASSERT_NE(resource.drop_config, nullptr); EXPECT_TRUE(resource.drop_config->drop_category_list().empty()); } @@ -1011,19 +1003,14 @@ TEST_F(XdsEndpointTest, EndpointHealthStatus) { auto addr = grpc_sockaddr_to_string(&address->address(), /*normalize=*/false); ASSERT_TRUE(addr.ok()) << addr.status(); EXPECT_EQ(*addr, "127.0.0.1:443"); - const auto* health_attribute = - static_cast( - address->GetAttribute(XdsEndpointHealthStatusAttribute::kKey)); - ASSERT_NE(health_attribute, nullptr); - EXPECT_EQ(health_attribute->status().status(), XdsHealthStatus::kUnknown); + EXPECT_EQ(address->args().GetInt(GRPC_ARG_XDS_HEALTH_STATUS), + XdsHealthStatus::kUnknown); address = &p.second.endpoints[1]; addr = grpc_sockaddr_to_string(&address->address(), /*normalize=*/false); ASSERT_TRUE(addr.ok()) << addr.status(); EXPECT_EQ(*addr, "127.0.0.2:443"); - health_attribute = static_cast( - address->GetAttribute(XdsEndpointHealthStatusAttribute::kKey)); - ASSERT_NE(health_attribute, nullptr); - EXPECT_EQ(health_attribute->status().status(), XdsHealthStatus::kDraining); + EXPECT_EQ(address->args().GetInt(GRPC_ARG_XDS_HEALTH_STATUS), + XdsHealthStatus::kDraining); } } // namespace diff --git a/test/cpp/end2end/client_lb_end2end_test.cc b/test/cpp/end2end/client_lb_end2end_test.cc index e062cc28d80..bfda0c63588 100644 --- a/test/cpp/end2end/client_lb_end2end_test.cc +++ b/test/cpp/end2end/client_lb_end2end_test.cc @@ -212,17 +212,13 @@ class FakeResolverResponseGeneratorWrapper { response_generator_ = std::move(other.response_generator_); } - void SetNextResolution( - const std::vector& ports, const char* service_config_json = nullptr, - const char* attribute_key = nullptr, - std::unique_ptr attribute = - nullptr, - const grpc_core::ChannelArgs& per_address_args = - grpc_core::ChannelArgs()) { + void SetNextResolution(const std::vector& ports, + const char* service_config_json = nullptr, + const grpc_core::ChannelArgs& per_address_args = + grpc_core::ChannelArgs()) { grpc_core::ExecCtx exec_ctx; - response_generator_->SetResponse( - BuildFakeResults(ipv6_only_, ports, service_config_json, attribute_key, - std::move(attribute), per_address_args)); + response_generator_->SetResponse(BuildFakeResults( + ipv6_only_, ports, service_config_json, per_address_args)); } void SetNextResolutionUponError(const std::vector& ports) { @@ -249,9 +245,6 @@ class FakeResolverResponseGeneratorWrapper { static grpc_core::Resolver::Result BuildFakeResults( bool ipv6_only, const std::vector& ports, const char* service_config_json = nullptr, - const char* attribute_key = nullptr, - std::unique_ptr attribute = - nullptr, const grpc_core::ChannelArgs& per_address_args = grpc_core::ChannelArgs()) { grpc_core::Resolver::Result result; @@ -262,14 +255,7 @@ class FakeResolverResponseGeneratorWrapper { GPR_ASSERT(lb_uri.ok()); grpc_resolved_address address; GPR_ASSERT(grpc_parse_uri(*lb_uri, &address)); - std::map> - attributes; - if (attribute != nullptr) { - attributes[attribute_key] = attribute->Copy(); - } - result.addresses->emplace_back(address, per_address_args, - std::move(attributes)); + result.addresses->emplace_back(address, per_address_args); } if (result.addresses->empty()) { result.resolution_note = "fake resolver empty address list"; @@ -717,7 +703,6 @@ TEST_F(ClientLbEnd2endTest, AuthorityOverrideFromResolver) { // different value. response_generator.SetNextResolution( GetServersPorts(), /*service_config_json=*/nullptr, - /*attribute_key=*/nullptr, /*attribute=*/nullptr, grpc_core::ChannelArgs().Set(GRPC_ARG_DEFAULT_AUTHORITY, "foo.example.com")); // Send an RPC. @@ -744,7 +729,6 @@ TEST_F(ClientLbEnd2endTest, AuthorityOverridePrecedence) { // different value. response_generator.SetNextResolution( GetServersPorts(), /*service_config_json=*/nullptr, - /*attribute_key=*/nullptr, /*attribute=*/nullptr, grpc_core::ChannelArgs().Set(GRPC_ARG_DEFAULT_AUTHORITY, "bar.example.com")); // Send an RPC. @@ -2847,31 +2831,11 @@ TEST_F(ClientLbInterceptTrailingMetadataTest, BackendMetricDataMerge) { } // -// tests that address attributes from the resolver are visible to the LB policy +// tests that address args from the resolver are visible to the LB policy // class ClientLbAddressTest : public ClientLbEnd2endTest { protected: - static const char* kAttributeKey; - - class Attribute : public grpc_core::ServerAddress::AttributeInterface { - public: - explicit Attribute(const std::string& str) : str_(str) {} - - std::unique_ptr Copy() const override { - return std::make_unique(str_); - } - - int Cmp(const AttributeInterface* other) const override { - return str_.compare(static_cast(other)->str_); - } - - std::string ToString() const override { return str_; } - - private: - std::string str_; - }; - void SetUp() override { ClientLbEnd2endTest::SetUp(); current_test_instance_ = this; @@ -2909,8 +2873,6 @@ class ClientLbAddressTest : public ClientLbEnd2endTest { std::vector addresses_seen_; }; -const char* ClientLbAddressTest::kAttributeKey = "attribute_key"; - ClientLbAddressTest* ClientLbAddressTest::current_test_instance_ = nullptr; TEST_F(ClientLbAddressTest, Basic) { @@ -2919,10 +2881,10 @@ TEST_F(ClientLbAddressTest, Basic) { auto response_generator = BuildResolverResponseGenerator(); auto channel = BuildChannel("address_test_lb", response_generator); auto stub = BuildStub(channel); - // Addresses returned by the resolver will have attached attributes. - response_generator.SetNextResolution(GetServersPorts(), nullptr, - kAttributeKey, - std::make_unique("foo")); + // Addresses returned by the resolver will have attached args. + response_generator.SetNextResolution( + GetServersPorts(), nullptr, + grpc_core::ChannelArgs().Set("test_key", "test_value")); CheckRpcSendOk(DEBUG_LOCATION, stub); // Check LB policy name for the channel. EXPECT_EQ("address_test_lb", channel->GetLoadBalancingPolicyName()); @@ -2930,8 +2892,9 @@ TEST_F(ClientLbAddressTest, Basic) { std::vector expected; for (const int port : GetServersPorts()) { expected.emplace_back(absl::StrCat( - ipv6_only_ ? "[::1]:" : "127.0.0.1:", port, " attributes={", - kAttributeKey, "=foo, disable_outlier_detection=true}")); + ipv6_only_ ? "[::1]:" : "127.0.0.1:", port, + " args={grpc.internal.no_subchannel.outlier_detection_disable=1, " + "test_key=test_value}")); } EXPECT_EQ(addresses_seen(), expected); } diff --git a/tools/doxygen/Doxyfile.c++.internal b/tools/doxygen/Doxyfile.c++.internal index 71f7e1f2ab1..8c6debdc00f 100644 --- a/tools/doxygen/Doxyfile.c++.internal +++ b/tools/doxygen/Doxyfile.c++.internal @@ -1142,8 +1142,6 @@ src/core/ext/filters/client_channel/lb_policy/weighted_round_robin/static_stride src/core/ext/filters/client_channel/lb_policy/weighted_round_robin/weighted_round_robin.cc \ src/core/ext/filters/client_channel/lb_policy/weighted_target/weighted_target.cc \ src/core/ext/filters/client_channel/lb_policy/xds/cds.cc \ -src/core/ext/filters/client_channel/lb_policy/xds/xds_attributes.cc \ -src/core/ext/filters/client_channel/lb_policy/xds/xds_attributes.h \ src/core/ext/filters/client_channel/lb_policy/xds/xds_channel_args.h \ src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_impl.cc \ src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_manager.cc \ diff --git a/tools/doxygen/Doxyfile.core.internal b/tools/doxygen/Doxyfile.core.internal index 234e3f6c9e5..206860b57cb 100644 --- a/tools/doxygen/Doxyfile.core.internal +++ b/tools/doxygen/Doxyfile.core.internal @@ -948,8 +948,6 @@ src/core/ext/filters/client_channel/lb_policy/weighted_round_robin/static_stride src/core/ext/filters/client_channel/lb_policy/weighted_round_robin/weighted_round_robin.cc \ src/core/ext/filters/client_channel/lb_policy/weighted_target/weighted_target.cc \ src/core/ext/filters/client_channel/lb_policy/xds/cds.cc \ -src/core/ext/filters/client_channel/lb_policy/xds/xds_attributes.cc \ -src/core/ext/filters/client_channel/lb_policy/xds/xds_attributes.h \ src/core/ext/filters/client_channel/lb_policy/xds/xds_channel_args.h \ src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_impl.cc \ src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_manager.cc \