reviewer comments

5 files changed, 18 insertions, 13 deletions

diff --git a/src/core/ext/filters/client_channel/client_channel.cc b/src/core/ext/filters/client_channel/client_channel.cc
index 888a5efda3..b3c9268bd8 100644
--- a/src/core/ext/filters/client_channel/client_channel.cc
+++ b/src/core/ext/filters/client_channel/client_channel.cc
@@ -1942,11 +1942,14 @@ static void recv_trailing_metadata_ready(void* arg, grpc_error* error) {
   grpc_mdelem* server_pushback_md = nullptr;
   get_call_status(batch_data, GRPC_ERROR_REF(error), &status,
                   &server_pushback_md);
-  if (status == GRPC_STATUS_OK) {
-    calld->pick.connected_subchannel->channelz_subchannel()
-        ->RecordCallSucceeded();
-  } else {
-    calld->pick.connected_subchannel->channelz_subchannel()->RecordCallFailed();
+  grpc_core::channelz::SubchannelNode* channelz_subchannel =
+      calld->pick.connected_subchannel->channelz_subchannel();
+  if (channelz_subchannel != nullptr) {
+    if (status == GRPC_STATUS_OK) {
+      channelz_subchannel->RecordCallSucceeded();
+    } else {
+      channelz_subchannel->RecordCallFailed();
+    }
   }
   if (grpc_client_channel_trace.enabled()) {
     gpr_log(GPR_INFO, "chand=%p calld=%p: call finished, status=%s", chand,
@@ -2581,6 +2584,11 @@ static void create_subchannel_call(grpc_call_element* elem, grpc_error* error) {
     new_error = grpc_error_add_child(new_error, error);
     pending_batches_fail(elem, new_error, true /* yield_call_combiner */);
   } else {
+    grpc_core::channelz::SubchannelNode* channelz_subchannel =
+        calld->pick.connected_subchannel->channelz_subchannel();
+    if (channelz_subchannel != nullptr) {
+      channelz_subchannel->RecordCallStarted();
+    }
     if (parent_data_size > 0) {
       subchannel_call_retry_state* retry_state =
           static_cast<subchannel_call_retry_state*>(
diff --git a/src/core/ext/filters/client_channel/client_channel_channelz.cc b/src/core/ext/filters/client_channel/client_channel_channelz.cc
index d904fc28b2..06b4d2ab95 100644
--- a/src/core/ext/filters/client_channel/client_channel_channelz.cc
+++ b/src/core/ext/filters/client_channel/client_channel_channelz.cc
@@ -130,7 +130,7 @@ grpc_json* ClientChannelNode::RenderJson() {
     grpc_json_link_child(json, trace_json, nullptr);
   }
   // ask CallCountingHelper to populate trace and call count data.
-  call_counter()->PopulateCallData(json);
+  call_counter()->PopulateCallCounts(json);
   // reset to the top level
   json = top_level_json;
   PopulateChildRefs(json);
@@ -204,7 +204,7 @@ grpc_json* SubchannelNode::RenderJson() {
     grpc_json_link_child(json, trace_json, nullptr);
   }
   // ask CallCountingHelper to populate trace and call count data.
-  call_counter_.PopulateCallData(json);
+  call_counter_.PopulateCallCounts(json);
   return top_level_json;
 }
 
diff --git a/src/core/ext/filters/client_channel/subchannel.cc b/src/core/ext/filters/client_channel/subchannel.cc
index 639a5fe9db..a7df2ee6cc 100644
--- a/src/core/ext/filters/client_channel/subchannel.cc
+++ b/src/core/ext/filters/client_channel/subchannel.cc
@@ -865,9 +865,6 @@ grpc_error* ConnectedSubchannel::CreateCall(const CallArgs& args,
     gpr_log(GPR_ERROR, "error: %s", error_string);
     return error;
   }
-  if (channelz_subchannel_ != nullptr) {
-    channelz_subchannel_->RecordCallStarted();
-  }
   grpc_call_stack_set_pollset_or_pollset_set(callstk, args.pollent);
   return GRPC_ERROR_NONE;
 }
diff --git a/src/core/lib/channel/channelz.cc b/src/core/lib/channel/channelz.cc
index b908363b1b..1ac7bd3976 100644
--- a/src/core/lib/channel/channelz.cc
+++ b/src/core/lib/channel/channelz.cc
@@ -66,7 +66,7 @@ void CallCountingHelper::RecordCallStarted() {
                            (gpr_atm)ExecCtx::Get()->Now());
 }
 
-void CallCountingHelper::PopulateCallData(grpc_json* json) {
+void CallCountingHelper::PopulateCallCounts(grpc_json* json) {
   grpc_json* json_iterator = nullptr;
   if (calls_started_ != 0) {
     json_iterator = grpc_json_add_number_string_child(
@@ -128,7 +128,7 @@ grpc_json* ChannelNode::RenderJson() {
     grpc_json_link_child(json, trace_json, nullptr);
   }
   // ask CallCountingHelper to populate trace and call count data.
-  call_counter_.PopulateCallData(json);
+  call_counter_.PopulateCallCounts(json);
   return top_level_json;
 }
 
diff --git a/src/core/lib/channel/channelz.h b/src/core/lib/channel/channelz.h
index 66753c96ee..14edc75123 100644
--- a/src/core/lib/channel/channelz.h
+++ b/src/core/lib/channel/channelz.h
@@ -99,7 +99,7 @@ class CallCountingHelper {
   }
 
   // Common rendering of the call count data and last_call_started_timestamp.
-  void PopulateCallData(grpc_json* json);
+  void PopulateCallCounts(grpc_json* json);
 
  private:
   // testing peer friend.