reviewer comments

2 files changed, 15 insertions, 6 deletions

diff --git a/src/core/ext/filters/client_channel/client_channel.cc b/src/core/ext/filters/client_channel/client_channel.cc
index 64e206ec63..daf1b89b09 100644
--- a/src/core/ext/filters/client_channel/client_channel.cc
+++ b/src/core/ext/filters/client_channel/client_channel.cc
@@ -511,7 +511,7 @@ get_service_config_from_resolver_result_locked(channel_data* chand) {
   return grpc_core::UniquePtr<char>(gpr_strdup(service_config_json));
 }
 
-static void check_for_important_resolution_change(
+static void maybe_add_trace_message_for_address_changes_locked(
     channel_data* chand, TraceStringVector* trace_strings) {
   int resolution_contains_addresses = false;
   const grpc_arg* channel_arg =
@@ -534,7 +534,7 @@ static void check_for_important_resolution_change(
       resolution_contains_addresses;
 }
 
-static void concatenate_and_add_channel_trace(
+static void concatenate_and_add_channel_trace_locked(
     channel_data* chand, TraceStringVector* trace_strings) {
   if (!trace_strings->empty()) {
     gpr_strvec v;
@@ -641,8 +641,8 @@ static void on_resolver_result_changed_locked(void* arg, grpc_error* error) {
         // config in the trace, at the risk of bloating the trace logs.
         trace_strings.push_back(gpr_strdup("Service config changed"));
       }
-      check_for_important_resolution_change(chand, &trace_strings);
-      concatenate_and_add_channel_trace(chand, &trace_strings);
+      maybe_add_trace_message_for_address_changes_locked(chand, &trace_strings);
+      concatenate_and_add_channel_trace_locked(chand, &trace_strings);
     }
     // Swap out the data used by cc_get_channel_info().
     gpr_mu_lock(&chand->info_mu);
diff --git a/test/core/end2end/tests/retry_streaming.cc b/test/core/end2end/tests/retry_streaming.cc
index d52574fc58..94a27faf7b 100644
--- a/test/core/end2end/tests/retry_streaming.cc
+++ b/test/core/end2end/tests/retry_streaming.cc
@@ -136,7 +136,7 @@ static void test_retry_streaming(grpc_end2end_test_config config) {
   int was_cancelled = 2;
   char* peer;
 
-  grpc_arg arg[] = {
+  grpc_arg args[] = {
       grpc_channel_arg_integer_create(
           const_cast<char*>(GRPC_ARG_MAX_CHANNEL_TRACE_EVENT_MEMORY_PER_NODE),
           1024 * 8),
@@ -159,7 +159,7 @@ static void test_retry_streaming(grpc_end2end_test_config config) {
               "    }\n"
               "  } ]\n"
               "}"))};
-  grpc_channel_args client_args = {GPR_ARRAY_SIZE(arg), arg};
+  grpc_channel_args client_args = {GPR_ARRAY_SIZE(args), args};
   grpc_end2end_test_fixture f =
       begin_test(config, "retry_streaming", &client_args, nullptr);
 
@@ -399,6 +399,15 @@ static void test_retry_streaming(grpc_end2end_test_config config) {
   char* json = channelz_channel->RenderJsonString();
   GPR_ASSERT(json != nullptr);
   gpr_log(GPR_INFO, "%s", json);
+  GPR_ASSERT(nullptr != strstr(json, "\"trace\""));
+  GPR_ASSERT(nullptr != strstr(json, "\"description\":\"Channel created\""));
+  GPR_ASSERT(nullptr != strstr(json, "\"severity\":\"CT_INFO\""));
+  GPR_ASSERT(nullptr != strstr(json, "Resolution event"));
+  GPR_ASSERT(nullptr != strstr(json, "Created new LB policy"));
+  GPR_ASSERT(nullptr != strstr(json, "Service config changed"));
+  GPR_ASSERT(nullptr != strstr(json, "Address list became non-empty"));
+  GPR_ASSERT(nullptr != strstr(json, "Channel state change to CONNECTING"));
+  gpr_free(json);
 
   grpc_slice_unref(details);
   grpc_metadata_array_destroy(&initial_metadata_recv);