subchannel list: pass in string instead of tracer (#29569)

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

@@ -81,11 +81,14 @@ class PickFirst : public LoadBalancingPolicy {
       : public SubchannelList<PickFirstSubchannelList,
                               PickFirstSubchannelData> {
    public:
-    PickFirstSubchannelList(PickFirst* policy, TraceFlag* tracer,
-                            ServerAddressList addresses,
+    PickFirstSubchannelList(PickFirst* policy, ServerAddressList addresses,
                             const grpc_channel_args& args)
-        : SubchannelList(policy, tracer, std::move(addresses),
-                         policy->channel_control_helper(), args) {
+        : SubchannelList(policy,
+                         (GRPC_TRACE_FLAG_ENABLED(grpc_lb_pick_first_trace)
+                              ? "PickFirstSubchannelList"
+                              : nullptr),
+                         std::move(addresses), policy->channel_control_helper(),
+                         args) {
       // Need to maintain a ref to the LB policy as long as we maintain
       // any references to subchannels, since the subchannels'
       // pollset_sets will include the LB policy's pollset_set.
@@ -185,8 +188,7 @@ void PickFirst::AttemptToConnectUsingLatestUpdateArgsLocked() {
     addresses = *latest_update_args_.addresses;
   }
   auto subchannel_list = MakeOrphanable<PickFirstSubchannelList>(
-      this, &grpc_lb_pick_first_trace, std::move(addresses),
-      *latest_update_args_.args);
+      this, std::move(addresses), *latest_update_args_.args);
   // Empty update or no valid subchannels.
   if (subchannel_list->num_subchannels() == 0) {
     // Unsubscribe from all current subchannels.

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

@@ -173,11 +173,14 @@ class RingHash : public LoadBalancingPolicy {
   class RingHashSubchannelList
       : public SubchannelList<RingHashSubchannelList, RingHashSubchannelData> {
    public:
-    RingHashSubchannelList(RingHash* policy, TraceFlag* tracer,
-                           ServerAddressList addresses,
+    RingHashSubchannelList(RingHash* policy, ServerAddressList addresses,
                            const grpc_channel_args& args)
-        : SubchannelList(policy, tracer, std::move(addresses),
-                         policy->channel_control_helper(), args) {
+        : SubchannelList(policy,
+                         (GRPC_TRACE_FLAG_ENABLED(grpc_lb_ring_hash_trace)
+                              ? "RingHashSubchannelList"
+                              : nullptr),
+                         std::move(addresses), policy->channel_control_helper(),
+                         args) {
       // Need to maintain a ref to the LB policy as long as we maintain
       // any references to subchannels, since the subchannels'
       // pollset_sets will include the LB policy's pollset_set.
@@ -776,7 +779,7 @@ void RingHash::UpdateLocked(UpdateArgs args) {
     if (subchannel_list_ != nullptr) return;
   }
   subchannel_list_ = MakeOrphanable<RingHashSubchannelList>(
-      this, &grpc_lb_ring_hash_trace, std::move(addresses), *args.args);
+      this, std::move(addresses), *args.args);
   if (subchannel_list_->num_subchannels() == 0) {
     // If the new list is empty, immediately transition to TRANSIENT_FAILURE.
     absl::Status status =

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

@@ -100,11 +100,14 @@ class RoundRobin : public LoadBalancingPolicy {
       : public SubchannelList<RoundRobinSubchannelList,
                               RoundRobinSubchannelData> {
    public:
-    RoundRobinSubchannelList(RoundRobin* policy, TraceFlag* tracer,
-                             ServerAddressList addresses,
+    RoundRobinSubchannelList(RoundRobin* policy, ServerAddressList addresses,
                              const grpc_channel_args& args)
-        : SubchannelList(policy, tracer, std::move(addresses),
-                         policy->channel_control_helper(), args) {
+        : SubchannelList(policy,
+                         (GRPC_TRACE_FLAG_ENABLED(grpc_lb_round_robin_trace)
+                              ? "RoundRobinSubchannelList"
+                              : nullptr),
+                         std::move(addresses), policy->channel_control_helper(),
+                         args) {
       // Need to maintain a ref to the LB policy as long as we maintain
       // any references to subchannels, since the subchannels'
       // pollset_sets will include the LB policy's pollset_set.
@@ -266,7 +269,7 @@ void RoundRobin::UpdateLocked(UpdateArgs args) {
             this, latest_pending_subchannel_list_.get());
   }
   latest_pending_subchannel_list_ = MakeOrphanable<RoundRobinSubchannelList>(
-      this, &grpc_lb_round_robin_trace, std::move(addresses), *args.args);
+      this, std::move(addresses), *args.args);
   // Start watching the new list.  If appropriate, this will cause it to be
   // immediately promoted to subchannel_list_ and to generate a new picker.
   latest_pending_subchannel_list_->StartWatchingLocked(

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

@@ -31,7 +31,6 @@
 #include "src/core/ext/filters/client_channel/subchannel_interface.h"
 #include "src/core/lib/address_utils/sockaddr_utils.h"
 #include "src/core/lib/channel/channel_args.h"
-#include "src/core/lib/debug/trace.h"
 #include "src/core/lib/gprpp/manual_constructor.h"
 #include "src/core/lib/gprpp/orphanable.h"
 #include "src/core/lib/gprpp/ref_counted.h"
@@ -191,7 +190,7 @@ class SubchannelList : public InternallyRefCounted<SubchannelListType> {
 
   // Accessors.
   LoadBalancingPolicy* policy() const { return policy_; }
-  TraceFlag* tracer() const { return tracer_; }
+  const char* tracer() const { return tracer_; }
 
   // Resets connection backoff of all subchannels.
   // TODO(roth): We will probably need to rethink this as part of moving
@@ -204,7 +203,7 @@ class SubchannelList : public InternallyRefCounted<SubchannelListType> {
   }
 
  protected:
-  SubchannelList(LoadBalancingPolicy* policy, TraceFlag* tracer,
+  SubchannelList(LoadBalancingPolicy* policy, const char* tracer,
                  ServerAddressList addresses,
                  LoadBalancingPolicy::ChannelControlHelper* helper,
                  const grpc_channel_args& args);
@@ -220,7 +219,7 @@ class SubchannelList : public InternallyRefCounted<SubchannelListType> {
   // Backpointer to owning policy.
   LoadBalancingPolicy* policy_;
 
-  TraceFlag* tracer_;
+  const char* tracer_;
 
   // The list of subchannels.
   SubchannelVector subchannels_;
@@ -242,12 +241,12 @@ class SubchannelList : public InternallyRefCounted<SubchannelListType> {
 template <typename SubchannelListType, typename SubchannelDataType>
 void SubchannelData<SubchannelListType, SubchannelDataType>::Watcher::
     OnConnectivityStateChange(grpc_connectivity_state new_state) {
-  if (GRPC_TRACE_FLAG_ENABLED(*subchannel_list_->tracer())) {
+  if (GPR_UNLIKELY(subchannel_list_->tracer() != nullptr)) {
     gpr_log(GPR_INFO,
             "[%s %p] subchannel list %p index %" PRIuPTR " of %" PRIuPTR
             " (subchannel %p): connectivity changed: state=%s, "
             "shutting_down=%d, pending_watcher=%p",
-            subchannel_list_->tracer()->name(), subchannel_list_->policy(),
+            subchannel_list_->tracer(), subchannel_list_->policy(),
             subchannel_list_.get(), subchannel_data_->Index(),
             subchannel_list_->num_subchannels(),
             subchannel_data_->subchannel_.get(),
@@ -286,11 +285,11 @@ template <typename SubchannelListType, typename SubchannelDataType>
 void SubchannelData<SubchannelListType, SubchannelDataType>::
     UnrefSubchannelLocked(const char* reason) {
   if (subchannel_ != nullptr) {
-    if (GRPC_TRACE_FLAG_ENABLED(*subchannel_list_->tracer())) {
+    if (GPR_UNLIKELY(subchannel_list_->tracer() != nullptr)) {
       gpr_log(GPR_INFO,
               "[%s %p] subchannel list %p index %" PRIuPTR " of %" PRIuPTR
               " (subchannel %p): unreffing subchannel (%s)",
-              subchannel_list_->tracer()->name(), subchannel_list_->policy(),
+              subchannel_list_->tracer(), subchannel_list_->policy(),
               subchannel_list_, Index(), subchannel_list_->num_subchannels(),
               subchannel_.get(), reason);
     }
@@ -309,11 +308,11 @@ void SubchannelData<SubchannelListType,
 template <typename SubchannelListType, typename SubchannelDataType>
 void SubchannelData<SubchannelListType,
                     SubchannelDataType>::StartConnectivityWatchLocked() {
-  if (GRPC_TRACE_FLAG_ENABLED(*subchannel_list_->tracer())) {
+  if (GPR_UNLIKELY(subchannel_list_->tracer() != nullptr)) {
     gpr_log(GPR_INFO,
             "[%s %p] subchannel list %p index %" PRIuPTR " of %" PRIuPTR
             " (subchannel %p): starting watch (from %s)",
-            subchannel_list_->tracer()->name(), subchannel_list_->policy(),
+            subchannel_list_->tracer(), subchannel_list_->policy(),
             subchannel_list_, Index(), subchannel_list_->num_subchannels(),
             subchannel_.get(), ConnectivityStateName(connectivity_state_));
   }
@@ -329,11 +328,11 @@ void SubchannelData<SubchannelListType,
 template <typename SubchannelListType, typename SubchannelDataType>
 void SubchannelData<SubchannelListType, SubchannelDataType>::
     CancelConnectivityWatchLocked(const char* reason) {
-  if (GRPC_TRACE_FLAG_ENABLED(*subchannel_list_->tracer())) {
+  if (GPR_UNLIKELY(subchannel_list_->tracer() != nullptr)) {
     gpr_log(GPR_INFO,
             "[%s %p] subchannel list %p index %" PRIuPTR " of %" PRIuPTR
             " (subchannel %p): canceling connectivity watch (%s)",
-            subchannel_list_->tracer()->name(), subchannel_list_->policy(),
+            subchannel_list_->tracer(), subchannel_list_->policy(),
             subchannel_list_, Index(), subchannel_list_->num_subchannels(),
             subchannel_.get(), reason);
   }
@@ -355,17 +354,17 @@ void SubchannelData<SubchannelListType, SubchannelDataType>::ShutdownLocked() {
 
 template <typename SubchannelListType, typename SubchannelDataType>
 SubchannelList<SubchannelListType, SubchannelDataType>::SubchannelList(
-    LoadBalancingPolicy* policy, TraceFlag* tracer, ServerAddressList addresses,
+    LoadBalancingPolicy* policy, const char* tracer,
+    ServerAddressList addresses,
     LoadBalancingPolicy::ChannelControlHelper* helper,
     const grpc_channel_args& args)
-    : InternallyRefCounted<SubchannelListType>(
-          GRPC_TRACE_FLAG_ENABLED(*tracer) ? "SubchannelList" : nullptr),
+    : InternallyRefCounted<SubchannelListType>(tracer),
       policy_(policy),
       tracer_(tracer) {
-  if (GRPC_TRACE_FLAG_ENABLED(*tracer_)) {
+  if (GPR_UNLIKELY(tracer_ != nullptr)) {
     gpr_log(GPR_INFO,
             "[%s %p] Creating subchannel list %p for %" PRIuPTR " subchannels",
-            tracer_->name(), policy, this, addresses.size());
+            tracer_, policy, this, addresses.size());
   }
   subchannels_.reserve(addresses.size());
   // Create a subchannel for each address.
@@ -374,20 +373,19 @@ SubchannelList<SubchannelListType, SubchannelDataType>::SubchannelList(
         helper->CreateSubchannel(address, args);
     if (subchannel == nullptr) {
       // Subchannel could not be created.
-      if (GRPC_TRACE_FLAG_ENABLED(*tracer_)) {
+      if (GPR_UNLIKELY(tracer_ != nullptr)) {
         gpr_log(GPR_INFO,
-                "[%s %p] could not create subchannel for address %s, "
-                "ignoring",
-                tracer_->name(), policy_, address.ToString().c_str());
+                "[%s %p] could not create subchannel for address %s, ignoring",
+                tracer_, policy_, address.ToString().c_str());
       }
       continue;
     }
-    if (GRPC_TRACE_FLAG_ENABLED(*tracer_)) {
+    if (GPR_UNLIKELY(tracer_ != nullptr)) {
       gpr_log(GPR_INFO,
               "[%s %p] subchannel list %p index %" PRIuPTR
               ": Created subchannel %p for address %s",
-              tracer_->name(), policy_, this, subchannels_.size(),
-              subchannel.get(), address.ToString().c_str());
+              tracer_, policy_, this, subchannels_.size(), subchannel.get(),
+              address.ToString().c_str());
     }
     subchannels_.emplace_back();
     subchannels_.back().Init(this, std::move(address), std::move(subchannel));
@@ -396,9 +394,9 @@ SubchannelList<SubchannelListType, SubchannelDataType>::SubchannelList(
 
 template <typename SubchannelListType, typename SubchannelDataType>
 SubchannelList<SubchannelListType, SubchannelDataType>::~SubchannelList() {
-  if (GRPC_TRACE_FLAG_ENABLED(*tracer_)) {
-    gpr_log(GPR_INFO, "[%s %p] Destroying subchannel_list %p", tracer_->name(),
-            policy_, this);
+  if (GPR_UNLIKELY(tracer_ != nullptr)) {
+    gpr_log(GPR_INFO, "[%s %p] Destroying subchannel_list %p", tracer_, policy_,
+            this);
   }
   for (auto& sd : subchannels_) {
     sd.Destroy();
@@ -407,9 +405,9 @@ SubchannelList<SubchannelListType, SubchannelDataType>::~SubchannelList() {
 
 template <typename SubchannelListType, typename SubchannelDataType>
 void SubchannelList<SubchannelListType, SubchannelDataType>::ShutdownLocked() {
-  if (GRPC_TRACE_FLAG_ENABLED(*tracer_)) {
-    gpr_log(GPR_INFO, "[%s %p] Shutting down subchannel_list %p",
-            tracer_->name(), policy_, this);
+  if (GPR_UNLIKELY(tracer_ != nullptr)) {
+    gpr_log(GPR_INFO, "[%s %p] Shutting down subchannel_list %p", tracer_,
+            policy_, this);
   }
   GPR_ASSERT(!shutting_down_);
   shutting_down_ = true;