reviewer comments

2 files changed, 35 insertions, 43 deletions

diff --git a/test/core/channel/channel_trace_test.cc b/test/core/channel/channel_trace_test.cc
index 9bc9dde8d6..3f5aa85c8c 100644
--- a/test/core/channel/channel_trace_test.cc
+++ b/test/core/channel/channel_trace_test.cc
@@ -89,7 +89,6 @@ void ValidateChannelTrace(ChannelTrace* tracer,
   grpc_json* json = tracer->RenderJson();
   EXPECT_NE(json, nullptr);
   char* json_str = grpc_json_dump_to_string(json, 0);
-  gpr_log(GPR_ERROR, "%s", json_str);
   grpc_json_destroy(json);
   grpc::testing::ValidateChannelTraceProtoJsonTranslation(json_str);
   grpc_json* parsed_json = grpc_json_parse_string(json_str);
@@ -161,14 +160,14 @@ TEST_P(ChannelTracerTest, ComplexTest) {
       ChannelTrace::Severity::Info,
       grpc_slice_from_static_string("subchannel one created"), sc1);
   ValidateChannelTrace(&tracer, 3, GetParam());
-  AddSimpleTrace(sc1->counter_and_tracer()->trace());
-  AddSimpleTrace(sc1->counter_and_tracer()->trace());
-  AddSimpleTrace(sc1->counter_and_tracer()->trace());
-  ValidateChannelTrace(sc1->counter_and_tracer()->trace(), 3, GetParam());
-  AddSimpleTrace(sc1->counter_and_tracer()->trace());
-  AddSimpleTrace(sc1->counter_and_tracer()->trace());
-  AddSimpleTrace(sc1->counter_and_tracer()->trace());
-  ValidateChannelTrace(sc1->counter_and_tracer()->trace(), 6, GetParam());
+  AddSimpleTrace(sc1->trace());
+  AddSimpleTrace(sc1->trace());
+  AddSimpleTrace(sc1->trace());
+  ValidateChannelTrace(sc1->trace(), 3, GetParam());
+  AddSimpleTrace(sc1->trace());
+  AddSimpleTrace(sc1->trace());
+  AddSimpleTrace(sc1->trace());
+  ValidateChannelTrace(sc1->trace(), 6, GetParam());
   AddSimpleTrace(&tracer);
   AddSimpleTrace(&tracer);
   ValidateChannelTrace(&tracer, 5, GetParam());
@@ -208,20 +207,20 @@ TEST_P(ChannelTracerTest, TestNesting) {
       ChannelTrace::Severity::Info,
       grpc_slice_from_static_string("subchannel one created"), sc1);
   ValidateChannelTrace(&tracer, 3, GetParam());
-  AddSimpleTrace(sc1->counter_and_tracer()->trace());
+  AddSimpleTrace(sc1->trace());
   ChannelFixture channel2(GetParam());
   RefCountedPtr<ChannelNode> conn1 =
       MakeRefCounted<ChannelNode>(channel2.channel(), GetParam(), true);
   // nesting one level deeper.
-  sc1->counter_and_tracer()->trace()->AddTraceEventWithReference(
+  sc1->trace()->AddTraceEventWithReference(
       ChannelTrace::Severity::Info,
       grpc_slice_from_static_string("connection one created"), conn1);
   ValidateChannelTrace(&tracer, 3, GetParam());
-  AddSimpleTrace(conn1->counter_and_tracer()->trace());
+  AddSimpleTrace(conn1->trace());
   AddSimpleTrace(&tracer);
   AddSimpleTrace(&tracer);
   ValidateChannelTrace(&tracer, 5, GetParam());
-  ValidateChannelTrace(conn1->counter_and_tracer()->trace(), 1, GetParam());
+  ValidateChannelTrace(conn1->trace(), 1, GetParam());
   ChannelFixture channel3(GetParam());
   RefCountedPtr<ChannelNode> sc2 =
       MakeRefCounted<ChannelNode>(channel3.channel(), GetParam(), true);
diff --git a/test/core/channel/channelz_test.cc b/test/core/channel/channelz_test.cc
index 5e9a3f89a2..8fa46a18da 100644
--- a/test/core/channel/channelz_test.cc
+++ b/test/core/channel/channelz_test.cc
@@ -44,17 +44,16 @@ namespace channelz {
 namespace testing {
 
 // testing peer to access channel internals
-class CallCountingAndTracingNodePeer {
+class CallCountingHelperPeer {
  public:
-  CallCountingAndTracingNodePeer(CallCountingAndTracingNode* node)
-      : node_(node) {}
+  CallCountingHelperPeer(CallCountingHelper* node) : node_(node) {}
   grpc_millis last_call_started_millis() {
     return (grpc_millis)gpr_atm_no_barrier_load(
         &node_->last_call_started_millis_);
   }
 
  private:
-  CallCountingAndTracingNode* node_;
+  CallCountingHelper* node_;
 };
 
 namespace {
@@ -164,8 +163,8 @@ void ValidateChannel(ChannelNode* channel, validate_channel_data_args args) {
   gpr_free(core_api_json_str);
 }
 
-grpc_millis GetLastCallStartedMillis(CallCountingAndTracingNode* channel) {
-  CallCountingAndTracingNodePeer peer(channel);
+grpc_millis GetLastCallStartedMillis(CallCountingHelper* channel) {
+  CallCountingHelperPeer peer(channel);
   return peer.last_call_started_millis();
 }
 
@@ -201,46 +200,40 @@ TEST_P(ChannelzChannelTest, BasicChannelAPIFunctionality) {
   ChannelFixture channel(GetParam());
   ChannelNode* channelz_channel =
       grpc_channel_get_channelz_node(channel.channel());
-  channelz_channel->counter_and_tracer()->RecordCallStarted();
-  channelz_channel->counter_and_tracer()->RecordCallFailed();
-  channelz_channel->counter_and_tracer()->RecordCallSucceeded();
+  channelz_channel->RecordCallStarted();
+  channelz_channel->RecordCallFailed();
+  channelz_channel->RecordCallSucceeded();
   ValidateChannel(channelz_channel, {1, 1, 1});
-  channelz_channel->counter_and_tracer()->RecordCallStarted();
-  channelz_channel->counter_and_tracer()->RecordCallFailed();
-  channelz_channel->counter_and_tracer()->RecordCallSucceeded();
-  channelz_channel->counter_and_tracer()->RecordCallStarted();
-  channelz_channel->counter_and_tracer()->RecordCallFailed();
-  channelz_channel->counter_and_tracer()->RecordCallSucceeded();
+  channelz_channel->RecordCallStarted();
+  channelz_channel->RecordCallFailed();
+  channelz_channel->RecordCallSucceeded();
+  channelz_channel->RecordCallStarted();
+  channelz_channel->RecordCallFailed();
+  channelz_channel->RecordCallSucceeded();
   ValidateChannel(channelz_channel, {3, 3, 3});
 }
 
 TEST_P(ChannelzChannelTest, LastCallStartedMillis) {
   grpc_core::ExecCtx exec_ctx;
-  ChannelFixture channel(GetParam());
-  ChannelNode* channelz_channel =
-      grpc_channel_get_channelz_node(channel.channel());
+  CallCountingHelper counter;
   // start a call to set the last call started timestamp
-  channelz_channel->counter_and_tracer()->RecordCallStarted();
-  grpc_millis millis1 =
-      GetLastCallStartedMillis(channelz_channel->counter_and_tracer());
+  counter.RecordCallStarted();
+  grpc_millis millis1 = GetLastCallStartedMillis(&counter);
   // time gone by should not affect the timestamp
   ChannelzSleep(100);
-  grpc_millis millis2 =
-      GetLastCallStartedMillis(channelz_channel->counter_and_tracer());
+  grpc_millis millis2 = GetLastCallStartedMillis(&counter);
   EXPECT_EQ(millis1, millis2);
   // calls succeeded or failed should not affect the timestamp
   ChannelzSleep(100);
-  channelz_channel->counter_and_tracer()->RecordCallFailed();
-  channelz_channel->counter_and_tracer()->RecordCallSucceeded();
-  grpc_millis millis3 =
-      GetLastCallStartedMillis(channelz_channel->counter_and_tracer());
+  counter.RecordCallFailed();
+  counter.RecordCallSucceeded();
+  grpc_millis millis3 = GetLastCallStartedMillis(&counter);
   EXPECT_EQ(millis1, millis3);
   // another call started should affect the timestamp
   // sleep for extra long to avoid flakes (since we cache Now())
   ChannelzSleep(5000);
-  channelz_channel->counter_and_tracer()->RecordCallStarted();
-  grpc_millis millis4 =
-      GetLastCallStartedMillis(channelz_channel->counter_and_tracer());
+  counter.RecordCallStarted();
+  grpc_millis millis4 = GetLastCallStartedMillis(&counter);
   EXPECT_NE(millis1, millis4);
 }