Incorperate channel trace into channelz

src/core/lib/channel/channel_trace.cc

@@ -40,16 +40,17 @@
 #include "src/core/lib/transport/error_utils.h"
 
 namespace grpc_core {
+namespace channelz {
 
-ChannelTrace::TraceEvent::TraceEvent(
+ChannelTrace::TraceEvent::TraceEvent(Severity severity, grpc_slice data,
-    Severity severity, grpc_slice data,
+                                     RefCountedPtr<Channel> referenced_channel,
-    RefCountedPtr<ChannelTrace> referenced_tracer, ReferencedType type)
+                                     ReferencedType type)
     : severity_(severity),
       data_(data),
       timestamp_(grpc_millis_to_timespec(grpc_core::ExecCtx::Get()->Now(),
                                          GPR_CLOCK_REALTIME)),
       next_(nullptr),
-      referenced_tracer_(std::move(referenced_tracer)),
+      referenced_channel_(std::move(referenced_channel)),
       referenced_type_(type) {}
 
 ChannelTrace::TraceEvent::TraceEvent(Severity severity, grpc_slice data)
@@ -117,20 +118,21 @@ void ChannelTrace::AddTraceEvent(Severity severity, grpc_slice data) {
 
 void ChannelTrace::AddTraceEventReferencingChannel(
     Severity severity, grpc_slice data,
-    RefCountedPtr<ChannelTrace> referenced_tracer) {
+    RefCountedPtr<Channel> referenced_channel) {
   if (max_list_size_ == 0) return;  // tracing is disabled if max_events == 0
   // create and fill up the new event
-  AddTraceEventHelper(
+  AddTraceEventHelper(New<TraceEvent>(
-      New<TraceEvent>(severity, data, std::move(referenced_tracer), Channel));
+      severity, data, std::move(referenced_channel), ReferencedType::Channel));
 }
 
 void ChannelTrace::AddTraceEventReferencingSubchannel(
     Severity severity, grpc_slice data,
-    RefCountedPtr<ChannelTrace> referenced_tracer) {
+    RefCountedPtr<Channel> referenced_channel) {
   if (max_list_size_ == 0) return;  // tracing is disabled if max_events == 0
   // create and fill up the new event
-  AddTraceEventHelper(New<TraceEvent>(
+  AddTraceEventHelper(New<TraceEvent>(severity, data,
-      severity, data, std::move(referenced_tracer), Subchannel));
+                                      std::move(referenced_channel),
+                                      ReferencedType::Subchannel));
 }
 
 namespace {
@@ -193,17 +195,19 @@ void ChannelTrace::TraceEvent::RenderTraceEvent(grpc_json* json) const {
   json_iterator =
       grpc_json_create_child(json_iterator, json, "timestamp",
                              fmt_time(timestamp_), GRPC_JSON_STRING, true);
-  if (referenced_tracer_ != nullptr) {
+  if (referenced_channel_ != nullptr) {
     char* uuid_str;
-    gpr_asprintf(&uuid_str, "%" PRIdPTR, referenced_tracer_->channel_uuid_);
+    gpr_asprintf(&uuid_str, "%" PRIdPTR, referenced_channel_->channel_uuid());
     grpc_json* child_ref = grpc_json_create_child(
         json_iterator, json,
-        (referenced_type_ == Channel) ? "channelRef" : "subchannelRef", nullptr,
+        (referenced_type_ == ReferencedType::Channel) ? "channelRef"
-        GRPC_JSON_OBJECT, false);
+                                                      : "subchannelRef",
+        nullptr, GRPC_JSON_OBJECT, false);
     json_iterator = grpc_json_create_child(
         nullptr, child_ref,
-        (referenced_type_ == Channel) ? "channelId" : "subchannelId", uuid_str,
+        (referenced_type_ == ReferencedType::Channel) ? "channelId"
-        GRPC_JSON_STRING, true);
+                                                      : "subchannelId",
+        uuid_str, GRPC_JSON_STRING, true);
     json_iterator = child_ref;
   }
 }
@@ -236,4 +240,5 @@ char* ChannelTrace::RenderTrace() const {
   return json_str;
 }
 
+}  // namespace channelz
 }  // namespace grpc_core

src/core/lib/channel/channel_trace.h

@@ -28,11 +28,14 @@
 #include "src/core/lib/json/json.h"
 
 namespace grpc_core {
+namespace channelz {
+
+class Channel;
 
 // Object used to hold live data for a channel. This data is exposed via the
 // channelz service:
 // https://github.com/grpc/proposal/blob/master/A14-channelz.md
-class ChannelTrace : public RefCounted<ChannelTrace> {
+class ChannelTrace {
  public:
   ChannelTrace(size_t max_events);
   ~ChannelTrace();
@@ -59,17 +62,15 @@ class ChannelTrace : public RefCounted<ChannelTrace> {
   // created a new subchannel, then it would record that with a TraceEvent
   // referencing the new subchannel.
   //
-  // TODO(ncteisen): Once channelz is implemented, the events should reference
-  // the overall channelz object, not just the ChannelTrace object.
   // 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(
       Severity severity, grpc_slice data,
-      RefCountedPtr<ChannelTrace> referenced_tracer);
+      RefCountedPtr<Channel> referenced_tracer);
   void AddTraceEventReferencingSubchannel(
       Severity severity, grpc_slice data,
-      RefCountedPtr<ChannelTrace> referenced_tracer);
+      RefCountedPtr<Channel> referenced_tracer);
 
   // Returns the tracing data rendered as a grpc json string.
   // The string is owned by the caller and must be freed.
@@ -77,17 +78,14 @@ class ChannelTrace : public RefCounted<ChannelTrace> {
 
  private:
   // Types of objects that can be references by trace events.
-  enum ReferencedType { Channel, Subchannel };
+  enum class ReferencedType { Channel, Subchannel };
   // Private class to encapsulate all the data and bookkeeping needed for a
   // a trace event.
   class TraceEvent {
    public:
     // Constructor for a TraceEvent that references a different channel.
-    // TODO(ncteisen): once channelz is implemented, this should reference the
-    // overall channelz object, not just the ChannelTrace object
     TraceEvent(Severity severity, grpc_slice data,
-               RefCountedPtr<ChannelTrace> referenced_tracer,
+               RefCountedPtr<Channel> referenced_tracer, ReferencedType type);
-               ReferencedType type);
 
     // Constructor for a TraceEvent that does not reverence a different
     // channel.
@@ -109,7 +107,7 @@ class ChannelTrace : public RefCounted<ChannelTrace> {
     gpr_timespec timestamp_;
     TraceEvent* next_;
     // the tracer object for the (sub)channel that this trace event refers to.
-    RefCountedPtr<ChannelTrace> referenced_tracer_;
+    RefCountedPtr<Channel> referenced_channel_;
     // the type that the referenced tracer points to. Unused if this trace
     // does not point to any channel or subchannel
     ReferencedType referenced_type_;
@@ -128,6 +126,7 @@ class ChannelTrace : public RefCounted<ChannelTrace> {
   gpr_timespec time_created_;
 };
 
+}  // namespace channelz
 }  // namespace grpc_core
 
 #endif /* GRPC_CORE_LIB_CHANNEL_CHANNEL_TRACE_H */

src/core/lib/channel/channelz.cc

@@ -91,7 +91,9 @@ grpc_json* add_num_str(grpc_json* parent, grpc_json* it, const char* name,
 
 }  // namespace
 
-Channel::Channel(grpc_channel* channel) : channel_(channel) {
+Channel::Channel(grpc_channel* channel, size_t channel_tracer_max_nodes)
+    : channel_(channel) {
+  trace_.Init(channel_tracer_max_nodes);
   target_ = grpc_channel_get_target(channel_);
   channel_uuid_ = ChannelzRegistry::Register(this);
 }
@@ -103,8 +105,8 @@ Channel::~Channel() {
 
 void Channel::CallStarted() {
   calls_started_++;
-  last_call_started_timestamp_ = grpc_millis_to_timespec(
+  last_call_started_timestamp_ =
-      grpc_core::ExecCtx::Get()->Now(), GPR_CLOCK_REALTIME);
+      grpc_millis_to_timespec(ExecCtx::Get()->Now(), GPR_CLOCK_REALTIME);
 }
 
 grpc_connectivity_state Channel::GetConnectivityState() {
@@ -153,6 +155,11 @@ char* Channel::RenderJSON() {
       grpc_json_create_child(json_iterator, json, "state",
                              grpc_connectivity_state_name(connectivity_state),
                              GRPC_JSON_STRING, false);
+  char* trace = trace_->RenderTrace();
+  if (trace != nullptr) {
+    json_iterator = grpc_json_create_child(json_iterator, json, "trace", trace,
+                                           GRPC_JSON_STRING, true);
+  }
 
   // render and return the over json object
   char* json_str = grpc_json_dump_to_string(top_level_json, 0);

src/core/lib/channel/channelz.h

@@ -24,6 +24,8 @@
 #include <grpc/grpc.h>
 
 #include "src/core/ext/filters/client_channel/client_channel.h"
+#include "src/core/lib/channel/channel_trace.h"
+#include "src/core/lib/gprpp/manual_constructor.h"
 #include "src/core/lib/gprpp/ref_counted.h"
 #include "src/core/lib/gprpp/ref_counted_ptr.h"
 #include "src/core/lib/iomgr/error.h"
@@ -35,7 +37,7 @@ namespace channelz {
 // owned by the client_channel that it points to and tracks
 class Channel : public RefCounted<Channel> {
  public:
-  Channel(grpc_channel* channel);
+  Channel(grpc_channel* channel, size_t channel_tracer_max_nodes);
   ~Channel();
 
   void CallStarted();
@@ -44,11 +46,15 @@ class Channel : public RefCounted<Channel> {
 
   char* RenderJSON();
 
+  ChannelTrace* Trace() { return trace_.get(); }
+
   void set_channel_destroyed() {
     GPR_ASSERT(!channel_destroyed_);
     channel_destroyed_ = true;
   }
 
+  intptr_t channel_uuid() { return channel_uuid_; }
+
  private:
   bool channel_destroyed_ = false;
   grpc_channel* channel_;
@@ -58,6 +64,7 @@ class Channel : public RefCounted<Channel> {
   uint64_t calls_failed_ = 0;
   gpr_timespec last_call_started_timestamp_;
   intptr_t channel_uuid_;
+  ManualConstructor<ChannelTrace> trace_;
 
   grpc_connectivity_state GetConnectivityState();
 };

src/core/lib/surface/channel.cc

@@ -67,7 +67,6 @@ struct grpc_channel {
   gpr_mu registered_call_mu;
   registered_call* registered_calls;
 
-  grpc_core::RefCountedPtr<grpc_core::ChannelTrace> tracer;
   grpc_core::RefCountedPtr<grpc_core::channelz::Channel> channelz_channel;
 
   char* target;
@@ -147,13 +146,13 @@ grpc_channel* grpc_channel_create_with_builder(
   }
 
   grpc_channel_args_destroy(args);
-  channel->tracer = grpc_core::MakeRefCounted<grpc_core::ChannelTrace>(
+  channel_tracer_max_nodes = 10;
-      channel_tracer_max_nodes);
-  channel->tracer->AddTraceEvent(
-      grpc_core::ChannelTrace::Severity::Info,
-      grpc_slice_from_static_string("Channel created"));
   channel->channelz_channel =
-      grpc_core::MakeRefCounted<grpc_core::channelz::Channel>(channel);
+      grpc_core::MakeRefCounted<grpc_core::channelz::Channel>(
+          channel, channel_tracer_max_nodes);
+  channel->channelz_channel->Trace()->AddTraceEvent(
+      grpc_core::channelz::ChannelTrace::Severity::Info,
+      grpc_slice_from_static_string("Channel created"));
   return channel;
 }
 
@@ -189,7 +188,7 @@ static grpc_channel_args* build_channel_args(
 }
 
 char* grpc_channel_get_trace(grpc_channel* channel) {
-  return channel->tracer->RenderTrace();
+  return channel->channelz_channel->Trace()->RenderTrace();
 }
 
 char* grpc_channel_render_channelz(grpc_channel* channel) {
@@ -202,7 +201,7 @@ grpc_core::channelz::Channel* grpc_channel_get_channelz_channel(
 }
 
 intptr_t grpc_channel_get_uuid(grpc_channel* channel) {
-  return channel->tracer->GetUuid();
+  return channel->channelz_channel->Trace()->GetUuid();
 }
 
 grpc_channel* grpc_channel_create(const char* target,
@@ -416,7 +415,7 @@ static void destroy_channel(void* arg, grpc_error* error) {
     GRPC_MDELEM_UNREF(rc->authority);
     gpr_free(rc);
   }
-  channel->tracer.reset();
+  channel->channelz_channel.reset();
   gpr_mu_destroy(&channel->registered_call_mu);
   gpr_free(channel->target);
   gpr_free(channel);