Fix channel trace polymorphism

grpc.def

@@ -71,6 +71,7 @@ EXPORTS
     grpc_resource_quota_arg_vtable
     grpc_channelz_get_top_channels
     grpc_channelz_get_channel
+    grpc_channelz_get_subchannel
     grpc_insecure_channel_create_from_fd
     grpc_server_add_insecure_channel_from_fd
     grpc_use_signal

src/core/lib/channel/channel_trace.cc

@@ -41,25 +41,14 @@
 namespace grpc_core {
 namespace channelz {
 
-ChannelTrace::TraceEvent::TraceEvent(
+ChannelTrace::TraceEvent::TraceEvent(Severity severity, grpc_slice data,
-    Severity severity, grpc_slice data,
+                                     RefCountedPtr<BaseNode> referenced_entity)
-    RefCountedPtr<ChannelNode> referenced_channel)
     : severity_(severity),
       data_(data),
       timestamp_(grpc_millis_to_timespec(grpc_core::ExecCtx::Get()->Now(),
                                          GPR_CLOCK_REALTIME)),
       next_(nullptr),
-      referenced_channel_(std::move(referenced_channel)) {}
+      referenced_entity_(std::move(referenced_entity)) {}
-
-// ChannelTrace::TraceEvent::TraceEvent(Severity severity, grpc_slice data,
-//                                      RefCountedPtr<SubchannelNode>
-//                                      referenced_subchannel)
-//     : severity_(severity),
-//       data_(data),
-//       timestamp_(grpc_millis_to_timespec(grpc_core::ExecCtx::Get()->Now(),
-//                                          GPR_CLOCK_REALTIME)),
-//       next_(nullptr),
-//       referenced_subchannel_(std::move(referenced_subchannel)) {}
 
 ChannelTrace::TraceEvent::TraceEvent(Severity severity, grpc_slice data)
     : severity_(severity),
@@ -119,24 +108,15 @@ void ChannelTrace::AddTraceEvent(Severity severity, grpc_slice data) {
   AddTraceEventHelper(New<TraceEvent>(severity, data));
 }
 
-void ChannelTrace::AddTraceEventReferencingChannel(
+void ChannelTrace::AddTraceEventWithReference(
     Severity severity, grpc_slice data,
-    RefCountedPtr<ChannelNode> referenced_channel) {
+    RefCountedPtr<BaseNode> referenced_entity) {
   if (max_list_size_ == 0) return;  // tracing is disabled if max_events == 0
   // create and fill up the new event
   AddTraceEventHelper(
-      New<TraceEvent>(severity, data, std::move(referenced_channel)));
+      New<TraceEvent>(severity, data, std::move(referenced_entity)));
 }
 
-// void ChannelTrace::AddTraceEventReferencingSubchannel(
-//     Severity severity, grpc_slice data,
-//     RefCountedPtr<SubchannelNode> referenced_subchannel) {
-//   if (max_list_size_ == 0) return;  // tracing is disabled if max_events == 0
-//   // create and fill up the new event
-//   AddTraceEventHelper(
-//       New<TraceEvent>(severity, data, std::move(referenced_subchannel)));
-// }
-
 namespace {
 
 const char* severity_string(ChannelTrace::Severity severity) {
@@ -165,26 +145,20 @@ void ChannelTrace::TraceEvent::RenderTraceEvent(grpc_json* json) const {
   json_iterator = grpc_json_create_child(json_iterator, json, "timestamp",
                                          gpr_format_timespec(timestamp_),
                                          GRPC_JSON_STRING, true);
-  if (referenced_channel_ != nullptr) {
+  if (referenced_entity_ != nullptr) {
+    const bool is_channel =
+        (referenced_entity_->type() == BaseNode::EntityType::kTopLevelChannel ||
+         referenced_entity_->type() == BaseNode::EntityType::kInternalChannel);
     char* uuid_str;
-    gpr_asprintf(&uuid_str, "%" PRIdPTR, referenced_channel_->uuid());
+    gpr_asprintf(&uuid_str, "%" PRIdPTR, referenced_entity_->uuid());
     grpc_json* child_ref = grpc_json_create_child(
-        json_iterator, json, "channelRef", nullptr, GRPC_JSON_OBJECT, false);
+        json_iterator, json, is_channel ? "channelRef" : "subchannelRef",
-    json_iterator = grpc_json_create_child(nullptr, child_ref, "channelId",
+        nullptr, GRPC_JSON_OBJECT, false);
-                                           uuid_str, GRPC_JSON_STRING, true);
+    json_iterator = grpc_json_create_child(
+        nullptr, child_ref, is_channel ? "channelId" : "subchannelId", uuid_str,
+        GRPC_JSON_STRING, true);
     json_iterator = child_ref;
   }
-  // else {
-  //   char* uuid_str;
-  //   gpr_asprintf(&uuid_str, "%" PRIdPTR, referenced_subchannel_->uuid());
-  //   grpc_json* child_ref = grpc_json_create_child(
-  //       json_iterator, json, "subchannelRef",
-  //       nullptr, GRPC_JSON_OBJECT, false);
-  //   json_iterator = grpc_json_create_child(
-  //       nullptr, child_ref, "subchannelId",
-  //       uuid_str, GRPC_JSON_STRING, true);
-  //   json_iterator = child_ref;
-  // }
 }
 
 grpc_json* ChannelTrace::RenderJson() const {

src/core/lib/channel/channel_trace.h

@@ -30,8 +30,7 @@
 namespace grpc_core {
 namespace channelz {
 
-class ChannelNode;
+class BaseNode;
-class SubchannelNode;
 
 // Object used to hold live data for a channel. This data is exposed via the
 // channelz service:
@@ -63,12 +62,8 @@ class ChannelTrace {
   // TODO(ncteisen): as this call is used more and more throughout the gRPC
   // stack, determine if it makes more sense to accept a char* instead of a
   // slice.
-  void AddTraceEventReferencingChannel(
+  void AddTraceEventWithReference(Severity severity, grpc_slice data,
-      Severity severity, grpc_slice data,
+                                  RefCountedPtr<BaseNode> referenced_channel);
-      RefCountedPtr<ChannelNode> referenced_channel);
-  // void AddTraceEventReferencingSubchannel(
-  //     Severity severity, grpc_slice data,
-  //     RefCountedPtr<SubchannelNode> referenced_subchannel);
 
   // Creates and returns the raw grpc_json object, so a parent channelz
   // object may incorporate the json before rendering.
@@ -81,11 +76,7 @@ class ChannelTrace {
    public:
     // Constructor for a TraceEvent that references a channel.
     TraceEvent(Severity severity, grpc_slice data,
-               RefCountedPtr<ChannelNode> referenced_channel);
+               RefCountedPtr<BaseNode> referenced_entity_);
-
-    // Constructor for a TraceEvent that references a subchannel.
-    TraceEvent(Severity severity, grpc_slice data,
-               RefCountedPtr<SubchannelNode> referenced_subchannel);
 
     // Constructor for a TraceEvent that does not reverence a different
     // channel.
@@ -107,8 +98,7 @@ class ChannelTrace {
     gpr_timespec timestamp_;
     TraceEvent* next_;
     // the tracer object for the (sub)channel that this trace event refers to.
-    RefCountedPtr<ChannelNode> referenced_channel_;
+    RefCountedPtr<BaseNode> referenced_entity_;
-    // RefCountedPtr<SubchannelNode> referenced_subchannel_;
   };  // TraceEvent
 
   // Internal helper to add and link in a trace event

src/ruby/ext/grpc/rb_grpc_imports.generated.c

@@ -94,6 +94,7 @@ grpc_resource_quota_resize_type grpc_resource_quota_resize_import;
 grpc_resource_quota_arg_vtable_type grpc_resource_quota_arg_vtable_import;
 grpc_channelz_get_top_channels_type grpc_channelz_get_top_channels_import;
 grpc_channelz_get_channel_type grpc_channelz_get_channel_import;
+grpc_channelz_get_subchannel_type grpc_channelz_get_subchannel_import;
 grpc_insecure_channel_create_from_fd_type grpc_insecure_channel_create_from_fd_import;
 grpc_server_add_insecure_channel_from_fd_type grpc_server_add_insecure_channel_from_fd_import;
 grpc_use_signal_type grpc_use_signal_import;
@@ -344,6 +345,7 @@ void grpc_rb_load_imports(HMODULE library) {
   grpc_resource_quota_arg_vtable_import = (grpc_resource_quota_arg_vtable_type) GetProcAddress(library, "grpc_resource_quota_arg_vtable");
   grpc_channelz_get_top_channels_import = (grpc_channelz_get_top_channels_type) GetProcAddress(library, "grpc_channelz_get_top_channels");
   grpc_channelz_get_channel_import = (grpc_channelz_get_channel_type) GetProcAddress(library, "grpc_channelz_get_channel");
+  grpc_channelz_get_subchannel_import = (grpc_channelz_get_subchannel_type) GetProcAddress(library, "grpc_channelz_get_subchannel");
   grpc_insecure_channel_create_from_fd_import = (grpc_insecure_channel_create_from_fd_type) GetProcAddress(library, "grpc_insecure_channel_create_from_fd");
   grpc_server_add_insecure_channel_from_fd_import = (grpc_server_add_insecure_channel_from_fd_type) GetProcAddress(library, "grpc_server_add_insecure_channel_from_fd");
   grpc_use_signal_import = (grpc_use_signal_type) GetProcAddress(library, "grpc_use_signal");

src/ruby/ext/grpc/rb_grpc_imports.generated.h

@@ -257,6 +257,9 @@ extern grpc_channelz_get_top_channels_type grpc_channelz_get_top_channels_import
 typedef char*(*grpc_channelz_get_channel_type)(intptr_t channel_id);
 extern grpc_channelz_get_channel_type grpc_channelz_get_channel_import;
 #define grpc_channelz_get_channel grpc_channelz_get_channel_import
+typedef char*(*grpc_channelz_get_subchannel_type)(intptr_t subchannel_id);
+extern grpc_channelz_get_subchannel_type grpc_channelz_get_subchannel_import;
+#define grpc_channelz_get_subchannel grpc_channelz_get_subchannel_import
 typedef grpc_channel*(*grpc_insecure_channel_create_from_fd_type)(const char* target, int fd, const grpc_channel_args* args);
 extern grpc_insecure_channel_create_from_fd_type grpc_insecure_channel_create_from_fd_import;
 #define grpc_insecure_channel_create_from_fd grpc_insecure_channel_create_from_fd_import

test/core/channel/channel_trace_test.cc

@@ -89,6 +89,7 @@ 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);
@@ -156,7 +157,7 @@ TEST_P(ChannelTracerTest, ComplexTest) {
   ChannelFixture channel1(GetParam());
   RefCountedPtr<ChannelNode> sc1 =
       MakeRefCounted<ChannelNode>(channel1.channel(), GetParam(), true);
-  tracer.AddTraceEventReferencingChannel(
+  tracer.AddTraceEventWithReference(
       ChannelTrace::Severity::Info,
       grpc_slice_from_static_string("subchannel one created"), sc1);
   ValidateChannelTrace(&tracer, 3, GetParam());
@@ -174,10 +175,10 @@ TEST_P(ChannelTracerTest, ComplexTest) {
   ChannelFixture channel2(GetParam());
   RefCountedPtr<ChannelNode> sc2 =
       MakeRefCounted<ChannelNode>(channel2.channel(), GetParam(), true);
-  tracer.AddTraceEventReferencingChannel(
+  tracer.AddTraceEventWithReference(
       ChannelTrace::Severity::Info,
       grpc_slice_from_static_string("LB channel two created"), sc2);
-  tracer.AddTraceEventReferencingChannel(
+  tracer.AddTraceEventWithReference(
       ChannelTrace::Severity::Warning,
       grpc_slice_from_static_string("subchannel one inactive"), sc1);
   ValidateChannelTrace(&tracer, 7, GetParam());
@@ -203,7 +204,7 @@ TEST_P(ChannelTracerTest, TestNesting) {
   ChannelFixture channel1(GetParam());
   RefCountedPtr<ChannelNode> sc1 =
       MakeRefCounted<ChannelNode>(channel1.channel(), GetParam(), true);
-  tracer.AddTraceEventReferencingChannel(
+  tracer.AddTraceEventWithReference(
       ChannelTrace::Severity::Info,
       grpc_slice_from_static_string("subchannel one created"), sc1);
   ValidateChannelTrace(&tracer, 3, GetParam());
@@ -212,7 +213,7 @@ TEST_P(ChannelTracerTest, TestNesting) {
   RefCountedPtr<ChannelNode> conn1 =
       MakeRefCounted<ChannelNode>(channel2.channel(), GetParam(), true);
   // nesting one level deeper.
-  sc1->counter_and_tracer()->trace()->AddTraceEventReferencingChannel(
+  sc1->counter_and_tracer()->trace()->AddTraceEventWithReference(
       ChannelTrace::Severity::Info,
       grpc_slice_from_static_string("connection one created"), conn1);
   ValidateChannelTrace(&tracer, 3, GetParam());
@@ -224,12 +225,12 @@ TEST_P(ChannelTracerTest, TestNesting) {
   ChannelFixture channel3(GetParam());
   RefCountedPtr<ChannelNode> sc2 =
       MakeRefCounted<ChannelNode>(channel3.channel(), GetParam(), true);
-  tracer.AddTraceEventReferencingChannel(
+  tracer.AddTraceEventWithReference(
       ChannelTrace::Severity::Info,
       grpc_slice_from_static_string("subchannel two created"), sc2);
   // this trace should not get added to the parents children since it is already
   // present in the tracer.
-  tracer.AddTraceEventReferencingChannel(
+  tracer.AddTraceEventWithReference(
       ChannelTrace::Severity::Warning,
       grpc_slice_from_static_string("subchannel one inactive"), sc1);
   AddSimpleTrace(&tracer);

test/core/surface/public_headers_must_be_c89.c

@@ -133,6 +133,7 @@ int main(int argc, char **argv) {
   printf("%lx", (unsigned long) grpc_resource_quota_arg_vtable);
   printf("%lx", (unsigned long) grpc_channelz_get_top_channels);
   printf("%lx", (unsigned long) grpc_channelz_get_channel);
+  printf("%lx", (unsigned long) grpc_channelz_get_subchannel);
   printf("%lx", (unsigned long) grpc_auth_property_iterator_next);
   printf("%lx", (unsigned long) grpc_auth_context_property_iterator);
   printf("%lx", (unsigned long) grpc_auth_context_peer_identity);