LB policy API: use UniqueTypeName for call attributes (grpc#29747)

* implement UniqueTypeName API

* convert security code to use UniqueTypeName

* change subchannel data producer API to use UniqueTypeName

* sanitize

* add missing build dep

* fix credentials_test

* fix certificate_provider_store_test

* fix tls_security_connector_test

* attempt to fix windows build

* avoid unnecessary allocation

* work around MSVC 2017 bug

* sanity

* change factory to not be templated

* fix sanity

* fix bug in chttp2 connector that used server creds instead of channel creds

* add missing build dep

* LB policy API: use UniqueTypeName for call attributes

* Automated change: Fix sanity tests

* add missing build deps

* simplify API

* update to use new API

* add missing BUILD dep

* add comment

Co-authored-by: markdroth <[email protected]>

Author: markdroth

BUILD

@@ -2807,6 +2807,7 @@ grpc_cc_library(
         "ref_counted_ptr",
         "service_config_parser",
         "slice_refcount",
+        "unique_type_name",
     ],
 )
 
@@ -4153,6 +4154,7 @@ grpc_cc_library(
         "ref_counted_ptr",
         "server_address",
         "sockaddr_utils",
+        "unique_type_name",
     ],
 )
 
@@ -4704,6 +4706,9 @@ grpc_cc_library(
     hdrs = [
         "src/core/ext/filters/client_channel/resolver/xds/xds_resolver.h",
     ],
+    external_deps = [
+        "absl/strings",
+    ],
     language = "c++",
 )
 
@@ -4749,6 +4754,7 @@ grpc_cc_library(
         "ref_counted_ptr",
         "server_address",
         "time",
+        "unique_type_name",
         "uri_parser",
         "useful",
     ],

src/core/ext/filters/client_channel/client_channel.cc

@@ -2540,25 +2540,16 @@ class ClientChannel::LoadBalancedCall::Metadata
 // ClientChannel::LoadBalancedCall::LbCallState
 //
 
-class ClientChannel::LoadBalancedCall::LbCallState
-    : public LoadBalancingPolicy::CallState {
- public:
-  explicit LbCallState(LoadBalancedCall* lb_call) : lb_call_(lb_call) {}
-
-  void* Alloc(size_t size) override { return lb_call_->arena_->Alloc(size); }
-
-  absl::string_view ExperimentalGetCallAttribute(const char* key) override {
-    auto* service_config_call_data = static_cast<ServiceConfigCallData*>(
-        lb_call_->call_context_[GRPC_CONTEXT_SERVICE_CONFIG_CALL_DATA].value);
-    auto& call_attributes = service_config_call_data->call_attributes();
-    auto it = call_attributes.find(key);
-    if (it == call_attributes.end()) return absl::string_view();
-    return it->second;
-  }
-
- private:
-  LoadBalancedCall* lb_call_;
-};
+absl::string_view
+ClientChannel::LoadBalancedCall::LbCallState::GetCallAttribute(
+    UniqueTypeName type) {
+  auto* service_config_call_data = static_cast<ServiceConfigCallData*>(
+      lb_call_->call_context_[GRPC_CONTEXT_SERVICE_CONFIG_CALL_DATA].value);
+  auto& call_attributes = service_config_call_data->call_attributes();
+  auto it = call_attributes.find(type);
+  if (it == call_attributes.end()) return absl::string_view();
+  return it->second;
+}
 
 //
 // ClientChannel::LoadBalancedCall::BackendMetricAccessor

src/core/ext/filters/client_channel/client_channel.h

@@ -30,6 +30,7 @@
 
 #include "absl/base/thread_annotations.h"
 #include "absl/status/status.h"
+#include "absl/strings/string_view.h"
 #include "absl/types/optional.h"
 
 #include <grpc/impl/codegen/connectivity_state.h>
@@ -54,6 +55,7 @@
 #include "src/core/lib/gprpp/ref_counted_ptr.h"
 #include "src/core/lib/gprpp/sync.h"
 #include "src/core/lib/gprpp/time.h"
+#include "src/core/lib/gprpp/unique_type_name.h"
 #include "src/core/lib/iomgr/call_combiner.h"
 #include "src/core/lib/iomgr/closure.h"
 #include "src/core/lib/iomgr/error.h"
@@ -382,6 +384,20 @@ class ClientChannel {
 class ClientChannel::LoadBalancedCall
     : public InternallyRefCounted<LoadBalancedCall, kUnrefCallDtor> {
  public:
+  class LbCallState : public LoadBalancingPolicy::CallState {
+   public:
+    explicit LbCallState(LoadBalancedCall* lb_call) : lb_call_(lb_call) {}
+
+    void* Alloc(size_t size) override { return lb_call_->arena_->Alloc(size); }
+
+    // Internal API to allow first-party LB policies to access per-call
+    // attributes set by the ConfigSelector.
+    absl::string_view GetCallAttribute(UniqueTypeName type);
+
+   private:
+    LoadBalancedCall* lb_call_;
+  };
+
   // If on_call_destruction_complete is non-null, then it will be
   // invoked once the LoadBalancedCall is completely destroyed.
   // If it is null, then the caller is responsible for checking whether
@@ -417,7 +433,6 @@ class ClientChannel::LoadBalancedCall
  private:
   class LbQueuedCallCanceller;
   class Metadata;
-  class LbCallState;
   class BackendMetricAccessor;
 
   // Returns the index into pending_batches_ to be used for batch.

src/core/ext/filters/client_channel/lb_policy.h

@@ -106,13 +106,6 @@ class LoadBalancingPolicy : public InternallyRefCounted<LoadBalancingPolicy> {
     /// It is more efficient to use this than to allocate memory directly
     /// for allocations that need to be made on a per-call basis.
     virtual void* Alloc(size_t size) = 0;
-
-    /// EXPERIMENTAL API.
-    /// Returns the value of the call attribute \a key.
-    /// Keys are static strings, so an attribute can be accessed by an LB
-    /// policy implementation only if it knows about the internal key.
-    /// Returns a null string_view if key not found.
-    virtual absl::string_view ExperimentalGetCallAttribute(const char* key) = 0;
   };
 
   /// Interface for accessing metadata.

src/core/ext/filters/client_channel/lb_policy/ring_hash/ring_hash.cc

@@ -38,13 +38,16 @@
 #include "absl/strings/string_view.h"
 #include "absl/types/optional.h"
 
+#include "src/core/lib/gprpp/unique_type_name.h"
+
 #define XXH_INLINE_ALL
 #include "xxhash.h"
 
 #include <grpc/impl/codegen/connectivity_state.h>
 #include <grpc/impl/codegen/grpc_types.h>
 #include <grpc/support/log.h>
 
+#include "src/core/ext/filters/client_channel/client_channel.h"
 #include "src/core/ext/filters/client_channel/lb_policy.h"
 #include "src/core/ext/filters/client_channel/lb_policy/subchannel_list.h"
 #include "src/core/ext/filters/client_channel/lb_policy_factory.h"
@@ -67,9 +70,13 @@
 
 namespace grpc_core {
 
-const char* kRequestRingHashAttribute = "request_ring_hash";
 TraceFlag grpc_lb_ring_hash_trace(false, "ring_hash_lb");
 
+UniqueTypeName RequestHashAttributeName() {
+  static UniqueTypeName::Factory kFactory("request_hash");
+  return kFactory.Create();
+}
+
 // Helper Parser method
 void ParseRingHashLbConfig(const Json& json, size_t* min_ring_size,
                            size_t* max_ring_size,
@@ -443,8 +450,9 @@ RingHash::Ring::Ring(RingHash* parent,
 //
 
 RingHash::PickResult RingHash::Picker::Pick(PickArgs args) {
-  auto hash =
-      args.call_state->ExperimentalGetCallAttribute(kRequestRingHashAttribute);
+  auto* call_state = static_cast<ClientChannel::LoadBalancedCall::LbCallState*>(
+      args.call_state);
+  auto hash = call_state->GetCallAttribute(RequestHashAttributeName());
   uint64_t h;
   if (!absl::SimpleAtoi(hash, &h)) {
     return PickResult::Fail(

src/core/ext/filters/client_channel/lb_policy/ring_hash/ring_hash.h

@@ -23,17 +23,20 @@
 
 #include <vector>
 
+#include "src/core/lib/gprpp/unique_type_name.h"
 #include "src/core/lib/iomgr/error.h"
 #include "src/core/lib/json/json.h"
 
 namespace grpc_core {
-extern const char* kRequestRingHashAttribute;
+
+UniqueTypeName RequestHashAttributeName();
 
 // Helper Parsing method to parse ring hash policy configs; for example, ring
 // hash size validity.
 void ParseRingHashLbConfig(const Json& json, size_t* min_ring_size,
                            size_t* max_ring_size,
                            std::vector<grpc_error_handle>* error_list);
+
 }  // namespace grpc_core
 
 #endif  // GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_LB_POLICY_RING_HASH_RING_HASH_H

src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_manager.cc

@@ -36,6 +36,7 @@
 #include <grpc/impl/codegen/grpc_types.h>
 #include <grpc/support/log.h>
 
+#include "src/core/ext/filters/client_channel/client_channel.h"
 #include "src/core/ext/filters/client_channel/lb_policy.h"
 #include "src/core/ext/filters/client_channel/lb_policy/child_policy_handler.h"
 #include "src/core/ext/filters/client_channel/lb_policy_factory.h"
@@ -228,8 +229,10 @@ class XdsClusterManagerLb : public LoadBalancingPolicy {
 
 XdsClusterManagerLb::PickResult XdsClusterManagerLb::ClusterPicker::Pick(
     PickArgs args) {
+  auto* call_state = static_cast<ClientChannel::LoadBalancedCall::LbCallState*>(
+      args.call_state);
   auto cluster_name =
-      args.call_state->ExperimentalGetCallAttribute(kXdsClusterAttribute);
+      call_state->GetCallAttribute(XdsClusterAttributeTypeName());
   auto it = cluster_map_.find(cluster_name);
   if (it != cluster_map_.end()) {
     return it->second->Pick(args);

src/core/ext/filters/client_channel/resolver/xds/xds_resolver.cc

@@ -45,6 +45,8 @@
 #include "absl/types/variant.h"
 #include "re2/re2.h"
 
+#include "src/core/lib/gprpp/unique_type_name.h"
+
 #define XXH_INLINE_ALL
 #include "xxhash.h"
 
@@ -94,7 +96,10 @@ namespace grpc_core {
 
 TraceFlag grpc_xds_resolver_trace(false, "xds_resolver");
 
-const char* kXdsClusterAttribute = "xds_cluster_name";
+UniqueTypeName XdsClusterAttributeTypeName() {
+  static UniqueTypeName::Factory kFactory("xds_cluster_name");
+  return kFactory.Create();
+}
 
 namespace {
 
@@ -764,13 +769,13 @@ ConfigSelector::CallConfig XdsResolver::XdsConfigSelector::GetCallConfig(
         method_config->GetMethodParsedConfigVector(grpc_empty_slice());
     call_config.service_config = std::move(method_config);
   }
-  call_config.call_attributes[kXdsClusterAttribute] = it->first;
+  call_config.call_attributes[XdsClusterAttributeTypeName()] = it->first;
   std::string hash_string = absl::StrCat(hash.value());
   char* hash_value =
       static_cast<char*>(args.arena->Alloc(hash_string.size() + 1));
   memcpy(hash_value, hash_string.c_str(), hash_string.size());
   hash_value[hash_string.size()] = '\0';
-  call_config.call_attributes[kRequestRingHashAttribute] = hash_value;
+  call_config.call_attributes[RequestHashAttributeName()] = hash_value;
   call_config.call_dispatch_controller =
       args.arena->New<XdsCallDispatchController>(it->second->Ref());
   return call_config;

src/core/ext/filters/client_channel/resolver/xds/xds_resolver.h

@@ -19,9 +19,11 @@
 
 #include <grpc/support/port_platform.h>
 
+#include "src/core/lib/gprpp/unique_type_name.h"
+
 namespace grpc_core {
 
-extern const char* kXdsClusterAttribute;
+UniqueTypeName XdsClusterAttributeTypeName();
 
 }  // namespace grpc_core
 

src/core/lib/service_config/service_config_call_data.h

@@ -28,6 +28,7 @@
 #include "absl/strings/string_view.h"
 
 #include "src/core/lib/gprpp/ref_counted_ptr.h"
+#include "src/core/lib/gprpp/unique_type_name.h"
 #include "src/core/lib/service_config/service_config.h"
 #include "src/core/lib/service_config/service_config_parser.h"
 
@@ -39,7 +40,7 @@ namespace grpc_core {
 /// easily access method and global parameters for the call.
 class ServiceConfigCallData {
  public:
-  using CallAttributes = std::map<const char*, absl::string_view>;
+  using CallAttributes = std::map<UniqueTypeName, absl::string_view>;
 
   ServiceConfigCallData() : method_configs_(nullptr) {}