Implements subchannel refs for pick_first

18 files changed, 232 insertions, 95 deletions

diff --git a/src/core/ext/filters/client_channel/client_channel.cc b/src/core/ext/filters/client_channel/client_channel.cc
index 520431e63b..8c3e9f2b30 100644
--- a/src/core/ext/filters/client_channel/client_channel.cc
+++ b/src/core/ext/filters/client_channel/client_channel.cc
@@ -3159,6 +3159,16 @@ static void try_to_connect_locked(void* arg, grpc_error* error_ignored) {
   GRPC_CHANNEL_STACK_UNREF(chand->owning_stack, "try_to_connect");
 }
 
+void grpc_client_channel_populate_child_refs(
+    grpc_channel_element* elem, grpc_core::ChildRefsList* child_subchannels,
+    grpc_core::ChildRefsList* child_channels) {
+  channel_data* chand = static_cast<channel_data*>(elem->channel_data);
+  if (chand->lb_policy) {
+    chand->lb_policy->FillChildRefsForChannelz(child_subchannels,
+                                               child_channels);
+  }
+}
+
 grpc_connectivity_state grpc_client_channel_check_connectivity_state(
     grpc_channel_element* elem, int try_to_connect) {
   channel_data* chand = static_cast<channel_data*>(elem->channel_data);
diff --git a/src/core/ext/filters/client_channel/client_channel.h b/src/core/ext/filters/client_channel/client_channel.h
index a21e5623a7..0f95f6670a 100644
--- a/src/core/ext/filters/client_channel/client_channel.h
+++ b/src/core/ext/filters/client_channel/client_channel.h
@@ -21,9 +21,11 @@
 
 #include <grpc/support/port_platform.h>
 
+#include "src/core/ext/filters/client_channel/client_channel_channelz.h"
 #include "src/core/ext/filters/client_channel/client_channel_factory.h"
 #include "src/core/ext/filters/client_channel/resolver.h"
 #include "src/core/lib/channel/channel_stack.h"
+#include "src/core/lib/json/json.h"
 
 extern grpc_core::TraceFlag grpc_client_channel_trace;
 
@@ -39,6 +41,10 @@ extern grpc_core::TraceFlag grpc_client_channel_trace;
 
 extern const grpc_channel_filter grpc_client_channel_filter;
 
+void grpc_client_channel_populate_child_refs(
+    grpc_channel_element* elem, grpc_core::ChildRefsList* child_subchannels,
+    grpc_core::ChildRefsList* child_channels);
+
 grpc_connectivity_state grpc_client_channel_check_connectivity_state(
     grpc_channel_element* elem, int try_to_connect);
 
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 08ceb2dd05..235b8f3207 100644
--- a/src/core/ext/filters/client_channel/client_channel_channelz.cc
+++ b/src/core/ext/filters/client_channel/client_channel_channelz.cc
@@ -63,6 +63,37 @@ void ClientChannelNode::PopulateConnectivityState(grpc_json* json) {
                          false);
 }
 
+void ClientChannelNode::PopulateChildRefs(grpc_json* json) {
+  ChildRefsList child_subchannels;
+  ChildRefsList child_channels;
+  grpc_json* json_iterator = nullptr;
+  grpc_client_channel_populate_child_refs(client_channel_, &child_subchannels,
+                                          &child_channels);
+  if (child_subchannels.size() > 0) {
+    grpc_json* array_parent = grpc_json_create_child(
+        nullptr, json, "subchannelRef", nullptr, GRPC_JSON_ARRAY, false);
+    for (size_t i = 0; i < child_subchannels.size(); ++i) {
+      json_iterator =
+          grpc_json_create_child(json_iterator, array_parent, nullptr, nullptr,
+                                 GRPC_JSON_OBJECT, false);
+      grpc_json_add_number_string_child(json_iterator, nullptr, "subchannelId",
+                                        child_subchannels[i]);
+    }
+  }
+  if (child_channels.size() > 0) {
+    grpc_json* array_parent = grpc_json_create_child(
+        nullptr, json, "channelRef", nullptr, GRPC_JSON_ARRAY, false);
+    json_iterator = nullptr;
+    for (size_t i = 0; i < child_subchannels.size(); ++i) {
+      json_iterator =
+          grpc_json_create_child(json_iterator, array_parent, nullptr, nullptr,
+                                 GRPC_JSON_OBJECT, false);
+      grpc_json_add_number_string_child(json_iterator, nullptr, "channelId",
+                                        child_subchannels[i]);
+    }
+  }
+}
+
 grpc_arg ClientChannelNode::CreateChannelArg() {
   return grpc_channel_arg_pointer_create(
       const_cast<char*>(GRPC_ARG_CHANNELZ_CHANNEL_NODE_CREATION_FUNC),
diff --git a/src/core/ext/filters/client_channel/client_channel_channelz.h b/src/core/ext/filters/client_channel/client_channel_channelz.h
index cf3ef7b6f2..e5be52e778 100644
--- a/src/core/ext/filters/client_channel/client_channel_channelz.h
+++ b/src/core/ext/filters/client_channel/client_channel_channelz.h
@@ -22,9 +22,14 @@
 #include <grpc/support/port_platform.h>
 
 #include "src/core/lib/channel/channel_args.h"
+#include "src/core/lib/channel/channel_stack.h"
 #include "src/core/lib/channel/channelz.h"
+#include "src/core/lib/gprpp/inlined_vector.h"
 
 namespace grpc_core {
+
+typedef InlinedVector<intptr_t, 10> ChildRefsList;
+
 namespace channelz {
 
 // Subtype of ChannelNode that overrides and provides client_channel specific
@@ -38,6 +43,9 @@ class ClientChannelNode : public ChannelNode {
   // channel connectivity.
   void PopulateConnectivityState(grpc_json* json) override;
 
+  // Override this functionality since client_channels have subchannels
+  void PopulateChildRefs(grpc_json* json) override;
+
   // Helper to create a channel arg to ensure this type of ChannelNode is
   // created.
   static grpc_arg CreateChannelArg();
diff --git a/src/core/ext/filters/client_channel/lb_policy.cc b/src/core/ext/filters/client_channel/lb_policy.cc
index e065f45639..a0d7758f23 100644
--- a/src/core/ext/filters/client_channel/lb_policy.cc
+++ b/src/core/ext/filters/client_channel/lb_policy.cc
@@ -31,13 +31,28 @@ LoadBalancingPolicy::LoadBalancingPolicy(const Args& args)
       combiner_(GRPC_COMBINER_REF(args.combiner, "lb_policy")),
       client_channel_factory_(args.client_channel_factory),
       interested_parties_(grpc_pollset_set_create()),
-      request_reresolution_(nullptr) {}
+      request_reresolution_(nullptr) {
+  gpr_mu_init(&child_refs_mu_);
+}
 
 LoadBalancingPolicy::~LoadBalancingPolicy() {
   grpc_pollset_set_destroy(interested_parties_);
+  gpr_mu_destroy(&child_refs_mu_);
   GRPC_COMBINER_UNREF(combiner_, "lb_policy");
 }
 
+void LoadBalancingPolicy::FillChildRefsForChannelz(
+    ChildRefsList* child_subchannels, ChildRefsList* child_channels) {
+  mu_guard guard(&child_refs_mu_);
+  // TODO, de dup these.
+  for (size_t i = 0; i < child_subchannels_.size(); ++i) {
+    child_subchannels->push_back(child_subchannels_[i]);
+  }
+  for (size_t i = 0; i < child_channels_.size(); ++i) {
+    child_channels->push_back(child_channels_[i]);
+  }
+}
+
 void LoadBalancingPolicy::TryReresolutionLocked(
     grpc_core::TraceFlag* grpc_lb_trace, grpc_error* error) {
   if (request_reresolution_ != nullptr) {
diff --git a/src/core/ext/filters/client_channel/lb_policy.h b/src/core/ext/filters/client_channel/lb_policy.h
index dab4466b21..0c5fd2ad9c 100644
--- a/src/core/ext/filters/client_channel/lb_policy.h
+++ b/src/core/ext/filters/client_channel/lb_policy.h
@@ -21,6 +21,7 @@
 
 #include <grpc/support/port_platform.h>
 
+#include "src/core/ext/filters/client_channel/client_channel_channelz.h"
 #include "src/core/ext/filters/client_channel/client_channel_factory.h"
 #include "src/core/ext/filters/client_channel/subchannel.h"
 #include "src/core/lib/gprpp/abstract.h"
@@ -28,6 +29,7 @@
 #include "src/core/lib/gprpp/ref_counted_ptr.h"
 #include "src/core/lib/iomgr/combiner.h"
 #include "src/core/lib/iomgr/polling_entity.h"
+#include "src/core/lib/json/json.h"
 #include "src/core/lib/transport/connectivity_state.h"
 
 extern grpc_core::DebugOnlyTraceFlag grpc_trace_lb_policy_refcount;
@@ -157,6 +159,13 @@ class LoadBalancingPolicy
     request_reresolution_ = request_reresolution;
   }
 
+  /// populates child_subchannels and child_channels with the uuids of this
+  /// LB policies referenced children. This is not invoked from the
+  /// client_channel's combiner. It has its own synchronization. This is
+  /// not abstract, since the behavior is the same for all LB policies.
+  void FillChildRefsForChannelz(ChildRefsList* child_subchannels,
+                                ChildRefsList* child_channels);
+
   grpc_pollset_set* interested_parties() const { return interested_parties_; }
 
   GRPC_ABSTRACT_BASE_CLASS
@@ -171,6 +180,9 @@ class LoadBalancingPolicy
   grpc_client_channel_factory* client_channel_factory() const {
     return client_channel_factory_;
   }
+  gpr_mu* child_refs_mu() { return &child_refs_mu_; }
+  ChildRefsList* child_subchannels() { return &child_subchannels_; }
+  ChildRefsList* child_channels() { return &child_channels_; }
 
   /// Shuts down the policy.  Any pending picks that have not been
   /// handed off to a new policy via HandOffPendingPicksLocked() will be
@@ -190,6 +202,11 @@ class LoadBalancingPolicy
 
   /// Combiner under which LB policy actions take place.
   grpc_combiner* combiner_;
+  /// Lock and data used to capture snapshots of this channels child
+  /// channels and subchannels. This data is consumed by channelz.
+  gpr_mu child_refs_mu_;
+  ChildRefsList child_subchannels_;
+  ChildRefsList child_channels_;
   /// Client channel factory, used to create channels and subchannels.
   grpc_client_channel_factory* client_channel_factory_;
   /// Owned pointer to interested parties in load balancing decisions.
diff --git a/src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc b/src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc
index ff2140e628..e43d1c2ab4 100644
--- a/src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc
+++ b/src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc
@@ -37,6 +37,15 @@ TraceFlag grpc_lb_pick_first_trace(false, "pick_first");
 
 namespace {
 
+class LockGuard {
+ public:
+  LockGuard(gpr_mu* mu) : mu_(mu) { gpr_mu_lock(mu_); }
+  ~LockGuard() { gpr_mu_unlock(mu_); }
+
+ private:
+  gpr_mu* mu_;
+};
+
 //
 // pick_first LB policy
 //
@@ -103,10 +112,20 @@ class PickFirst : public LoadBalancingPolicy {
     }
   };
 
+  class UpdateGuard {
+   public:
+    UpdateGuard(PickFirst* pf) : pf_(pf) {}
+    ~UpdateGuard() { pf_->UpdateChildRefsLocked(); }
+
+   private:
+    PickFirst* pf_;
+  };
+
   void ShutdownLocked() override;
 
   void StartPickingLocked();
   void DestroyUnselectedSubchannelsLocked();
+  void UpdateChildRefsLocked();
 
   // All our subchannels.
   OrphanablePtr<PickFirstSubchannelList> subchannel_list_;
@@ -158,6 +177,7 @@ void PickFirst::HandOffPendingPicksLocked(LoadBalancingPolicy* new_policy) {
 }
 
 void PickFirst::ShutdownLocked() {
+  UpdateGuard(this);
   grpc_error* error = GRPC_ERROR_CREATE_FROM_STATIC_STRING("Channel shutdown");
   if (grpc_lb_pick_first_trace.enabled()) {
     gpr_log(GPR_INFO, "Pick First %p Shutting down", this);
@@ -280,7 +300,37 @@ void PickFirst::PingOneLocked(grpc_closure* on_initiate, grpc_closure* on_ack) {
   }
 }
 
+void PickFirst::UpdateChildRefsLocked() {
+  mu_guard guard(child_refs_mu());
+  // reset both lists
+  child_subchannels()->clear();
+  // this will stay empty, because pick_first channels have no children
+  // channels.
+  child_channels()->clear();
+  // populate the subchannels with boths subchannels lists, they will be
+  // deduped when the actual channelz query comes in.
+  if (subchannel_list_ != nullptr) {
+    for (size_t i = 0; i < subchannel_list_->num_subchannels(); ++i) {
+      if (subchannel_list_->subchannel(i)->subchannel() != nullptr) {
+        child_subchannels()->push_back(grpc_subchannel_get_uuid(
+            subchannel_list_->subchannel(i)->subchannel()));
+      }
+    }
+  }
+  if (latest_pending_subchannel_list_ != nullptr) {
+    for (size_t i = 0; i < latest_pending_subchannel_list_->num_subchannels();
+         ++i) {
+      if (latest_pending_subchannel_list_->subchannel(i)->subchannel() !=
+          nullptr) {
+        child_subchannels()->push_back(grpc_subchannel_get_uuid(
+            latest_pending_subchannel_list_->subchannel(i)->subchannel()));
+      }
+    }
+  }
+}
+
 void PickFirst::UpdateLocked(const grpc_channel_args& args) {
+  UpdateGuard guard(this);
   const grpc_arg* arg = grpc_channel_args_find(&args, GRPC_ARG_LB_ADDRESSES);
   if (arg == nullptr || arg->type != GRPC_ARG_POINTER) {
     if (subchannel_list_ == nullptr) {
@@ -388,6 +438,7 @@ void PickFirst::UpdateLocked(const grpc_channel_args& args) {
 void PickFirst::PickFirstSubchannelData::ProcessConnectivityChangeLocked(
     grpc_connectivity_state connectivity_state, grpc_error* error) {
   PickFirst* p = static_cast<PickFirst*>(subchannel_list()->policy());
+  UpdateGuard guard(p);
   // The notification must be for a subchannel in either the current or
   // latest pending subchannel lists.
   GPR_ASSERT(subchannel_list() == p->subchannel_list_.get() ||
diff --git a/src/core/ext/filters/client_channel/subchannel.cc b/src/core/ext/filters/client_channel/subchannel.cc
index 8ab3fe40f5..6c4f1869ea 100644
--- a/src/core/ext/filters/client_channel/subchannel.cc
+++ b/src/core/ext/filters/client_channel/subchannel.cc
@@ -134,6 +134,11 @@ struct grpc_subchannel {
   bool backoff_begun;
   /** our alarm */
   grpc_timer alarm;
+
+  /* the global uuid for this subchannel */
+  // TODO(ncteisen): move this into SubchannelNode while implementing
+  //  GetSubchannel.
+  intptr_t subchannel_uuid;
 };
 
 struct grpc_subchannel_call {
@@ -374,9 +379,16 @@ grpc_subchannel* grpc_subchannel_create(grpc_connector* connector,
   c->backoff.Init(backoff_options);
   gpr_mu_init(&c->mu);
 
+  // This is just a placeholder for now
+  c->subchannel_uuid = 42;
+
   return grpc_subchannel_index_register(key, c);
 }
 
+intptr_t grpc_subchannel_get_uuid(grpc_subchannel* s) {
+  return s->subchannel_uuid;
+}
+
 static void continue_connect_locked(grpc_subchannel* c) {
   grpc_connect_in_args args;
   args.interested_parties = c->pollset_set;
diff --git a/src/core/ext/filters/client_channel/subchannel.h b/src/core/ext/filters/client_channel/subchannel.h
index e23aec12df..590e80f507 100644
--- a/src/core/ext/filters/client_channel/subchannel.h
+++ b/src/core/ext/filters/client_channel/subchannel.h
@@ -115,6 +115,8 @@ grpc_subchannel_call* grpc_subchannel_call_ref(
 void grpc_subchannel_call_unref(
     grpc_subchannel_call* call GRPC_SUBCHANNEL_REF_EXTRA_ARGS);
 
+intptr_t grpc_subchannel_get_uuid(grpc_subchannel* subchannel);
+
 /** Returns a pointer to the parent data associated with \a subchannel_call.
     The data will be of the size specified in \a parent_data_size
     field of the args passed to \a grpc_connected_subchannel_create_call(). */
diff --git a/src/core/lib/channel/channel_trace.cc b/src/core/lib/channel/channel_trace.cc
index 0f655d8716..c17885a713 100644
--- a/src/core/lib/channel/channel_trace.cc
+++ b/src/core/lib/channel/channel_trace.cc
@@ -131,38 +131,6 @@ void ChannelTrace::AddTraceEventReferencingSubchannel(
 
 namespace {
 
-// returns an allocated string that represents tm according to RFC-3339, and,
-// more specifically, follows:
-// https://developers.google.com/protocol-buffers/docs/proto3#json
-//
-// "Uses RFC 3339, where generated output will always be Z-normalized and uses
-// 0, 3, 6 or 9 fractional digits."
-char* fmt_time(gpr_timespec tm) {
-  char time_buffer[35];
-  char ns_buffer[11];  // '.' + 9 digits of precision
-  struct tm* tm_info = localtime((const time_t*)&tm.tv_sec);
-  strftime(time_buffer, sizeof(time_buffer), "%Y-%m-%dT%H:%M:%S", tm_info);
-  snprintf(ns_buffer, 11, ".%09d", tm.tv_nsec);
-  // This loop trims off trailing zeros by inserting a null character that the
-  // right point. We iterate in chunks of three because we want 0, 3, 6, or 9
-  // fractional digits.
-  for (int i = 7; i >= 1; i -= 3) {
-    if (ns_buffer[i] == '0' && ns_buffer[i + 1] == '0' &&
-        ns_buffer[i + 2] == '0') {
-      ns_buffer[i] = '\0';
-      // Edge case in which all fractional digits were 0.
-      if (i == 1) {
-        ns_buffer[0] = '\0';
-      }
-    } else {
-      break;
-    }
-  }
-  char* full_time_str;
-  gpr_asprintf(&full_time_str, "%s%sZ", time_buffer, ns_buffer);
-  return full_time_str;
-}
-
 const char* severity_string(ChannelTrace::Severity severity) {
   switch (severity) {
     case ChannelTrace::Severity::Info:
@@ -186,9 +154,9 @@ void ChannelTrace::TraceEvent::RenderTraceEvent(grpc_json* json) const {
   json_iterator = grpc_json_create_child(json_iterator, json, "severity",
                                          severity_string(severity_),
                                          GRPC_JSON_STRING, false);
-  json_iterator =
-      grpc_json_create_child(json_iterator, json, "timestamp",
-                             fmt_time(timestamp_), GRPC_JSON_STRING, true);
+  json_iterator = grpc_json_create_child(json_iterator, json, "timestamp",
+                                         gpr_format_timespec(timestamp_),
+                                         GRPC_JSON_STRING, true);
   if (referenced_channel_ != nullptr) {
     char* uuid_str;
     gpr_asprintf(&uuid_str, "%" PRIdPTR, referenced_channel_->channel_uuid());
@@ -216,9 +184,9 @@ grpc_json* ChannelTrace::RenderJSON() const {
   json_iterator =
       grpc_json_create_child(json_iterator, json, "numEventsLogged",
                              num_events_logged_str, GRPC_JSON_STRING, true);
-  json_iterator =
-      grpc_json_create_child(json_iterator, json, "creationTimestamp",
-                             fmt_time(time_created_), GRPC_JSON_STRING, true);
+  json_iterator = grpc_json_create_child(
+      json_iterator, json, "creationTimestamp",
+      gpr_format_timespec(time_created_), GRPC_JSON_STRING, true);
   grpc_json* events = grpc_json_create_child(json_iterator, json, "events",
                                              nullptr, GRPC_JSON_ARRAY, false);
   json_iterator = nullptr;
diff --git a/src/core/lib/channel/channelz.cc b/src/core/lib/channel/channelz.cc
index 2074cb0cc5..ffc7ff884c 100644
--- a/src/core/lib/channel/channelz.cc
+++ b/src/core/lib/channel/channelz.cc
@@ -41,53 +41,6 @@
 namespace grpc_core {
 namespace channelz {
 
-namespace {
-
-// TODO(ncteisen): move this function to a common helper location.
-//
-// returns an allocated string that represents tm according to RFC-3339, and,
-// more specifically, follows:
-// https://developers.google.com/protocol-buffers/docs/proto3#json
-//
-// "Uses RFC 3339, where generated output will always be Z-normalized and uses
-// 0, 3, 6 or 9 fractional digits."
-char* fmt_time(gpr_timespec tm) {
-  char time_buffer[35];
-  char ns_buffer[11];  // '.' + 9 digits of precision
-  struct tm* tm_info = localtime((const time_t*)&tm.tv_sec);
-  strftime(time_buffer, sizeof(time_buffer), "%Y-%m-%dT%H:%M:%S", tm_info);
-  snprintf(ns_buffer, 11, ".%09d", tm.tv_nsec);
-  // This loop trims off trailing zeros by inserting a null character that the
-  // right point. We iterate in chunks of three because we want 0, 3, 6, or 9
-  // fractional digits.
-  for (int i = 7; i >= 1; i -= 3) {
-    if (ns_buffer[i] == '0' && ns_buffer[i + 1] == '0' &&
-        ns_buffer[i + 2] == '0') {
-      ns_buffer[i] = '\0';
-      // Edge case in which all fractional digits were 0.
-      if (i == 1) {
-        ns_buffer[0] = '\0';
-      }
-    } else {
-      break;
-    }
-  }
-  char* full_time_str;
-  gpr_asprintf(&full_time_str, "%s%sZ", time_buffer, ns_buffer);
-  return full_time_str;
-}
-
-// TODO(ncteisen); move this to json library
-grpc_json* add_num_str(grpc_json* parent, grpc_json* it, const char* name,
-                       int64_t num) {
-  char* num_str;
-  gpr_asprintf(&num_str, "%" PRId64, num);
-  return grpc_json_create_child(it, parent, name, num_str, GRPC_JSON_STRING,
-                                true);
-}
-
-}  // namespace
-
 ChannelNode::ChannelNode(grpc_channel* channel, size_t channel_tracer_max_nodes)
     : channel_(channel), target_(nullptr), channel_uuid_(-1) {
   trace_.Init(channel_tracer_max_nodes);
@@ -110,6 +63,8 @@ void ChannelNode::RecordCallStarted() {
 
 void ChannelNode::PopulateConnectivityState(grpc_json* json) {}
 
+void ChannelNode::PopulateChildRefs(grpc_json* json) {}
+
 char* ChannelNode::RenderJSON() {
   // We need to track these three json objects to build our object
   grpc_json* top_level_json = grpc_json_create(GRPC_JSON_OBJECT);
@@ -120,7 +75,8 @@ char* ChannelNode::RenderJSON() {
                                          GRPC_JSON_OBJECT, false);
   json = json_iterator;
   json_iterator = nullptr;
-  json_iterator = add_num_str(json, json_iterator, "channelId", channel_uuid_);
+  json_iterator = grpc_json_add_number_string_child(json, json_iterator,
+                                                    "channelId", channel_uuid_);
   // reset json iterators to top level object
   json = top_level_json;
   json_iterator = nullptr;
@@ -148,17 +104,22 @@ char* ChannelNode::RenderJSON() {
   json_iterator = nullptr;
   // We use -1 as sentinel values since proto default value for integers is
   // zero, and the confuses the parser into thinking the value weren't present
-  json_iterator =
-      add_num_str(json, json_iterator, "callsStarted", calls_started_);
-  json_iterator =
-      add_num_str(json, json_iterator, "callsSucceeded", calls_succeeded_);
-  json_iterator =
-      add_num_str(json, json_iterator, "callsFailed", calls_failed_);
+  json_iterator = grpc_json_add_number_string_child(
+      json, json_iterator, "callsStarted", calls_started_);
+  json_iterator = grpc_json_add_number_string_child(
+      json, json_iterator, "callsSucceeded", calls_succeeded_);
+  json_iterator = grpc_json_add_number_string_child(
+      json, json_iterator, "callsFailed", calls_failed_);
   gpr_timespec ts =
       grpc_millis_to_timespec(last_call_started_millis_, GPR_CLOCK_REALTIME);
   json_iterator =
       grpc_json_create_child(json_iterator, json, "lastCallStartedTimestamp",
-                             fmt_time(ts), GRPC_JSON_STRING, true);
+                             gpr_format_timespec(ts), GRPC_JSON_STRING, true);
+
+  json = top_level_json;
+  json_iterator = nullptr;
+  PopulateChildRefs(json);
+
   // render and return the over json object
   char* json_str = grpc_json_dump_to_string(top_level_json, 0);
   grpc_json_destroy(top_level_json);
diff --git a/src/core/lib/channel/channelz.h b/src/core/lib/channel/channelz.h
index 9bd01ece50..e84c187f11 100644
--- a/src/core/lib/channel/channelz.h
+++ b/src/core/lib/channel/channelz.h
@@ -62,6 +62,8 @@ class ChannelNode : public RefCounted<ChannelNode> {
   // instead of lib/
   virtual void PopulateConnectivityState(grpc_json* json);
 
+  virtual void PopulateChildRefs(grpc_json* json);
+
   ChannelTrace* trace() { return trace_.get(); }
 
   void MarkChannelDestroyed() {
diff --git a/src/core/lib/gpr/string.cc b/src/core/lib/gpr/string.cc
index ef2a6900b4..9bcb6714a3 100644
--- a/src/core/lib/gpr/string.cc
+++ b/src/core/lib/gpr/string.cc
@@ -23,8 +23,10 @@
 #include <ctype.h>
 #include <limits.h>
 #include <stddef.h>
+#include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <time.h>
 
 #include <grpc/support/alloc.h>
 #include <grpc/support/log.h>
@@ -54,6 +56,38 @@ typedef struct {
   char* data;
 } dump_out;
 
+// Returns an allocated string that represents tm according to RFC-3339, and,
+// more specifically, follows:
+// https://developers.google.com/protocol-buffers/docs/proto3#json
+//
+// "Uses RFC 3339, where generated output will always be Z-normalized and uses
+// 0, 3, 6 or 9 fractional digits."
+char* gpr_format_timespec(gpr_timespec tm) {
+  char time_buffer[35];
+  char ns_buffer[11];  // '.' + 9 digits of precision
+  struct tm* tm_info = localtime((const time_t*)&tm.tv_sec);
+  strftime(time_buffer, sizeof(time_buffer), "%Y-%m-%dT%H:%M:%S", tm_info);
+  snprintf(ns_buffer, 11, ".%09d", tm.tv_nsec);
+  // This loop trims off trailing zeros by inserting a null character that the
+  // right point. We iterate in chunks of three because we want 0, 3, 6, or 9
+  // fractional digits.
+  for (int i = 7; i >= 1; i -= 3) {
+    if (ns_buffer[i] == '0' && ns_buffer[i + 1] == '0' &&
+        ns_buffer[i + 2] == '0') {
+      ns_buffer[i] = '\0';
+      // Edge case in which all fractional digits were 0.
+      if (i == 1) {
+        ns_buffer[0] = '\0';
+      }
+    } else {
+      break;
+    }
+  }
+  char* full_time_str;
+  gpr_asprintf(&full_time_str, "%s%sZ", time_buffer, ns_buffer);
+  return full_time_str;
+}
+
 static dump_out dump_out_create(void) {
   dump_out r = {0, 0, nullptr};
   return r;
diff --git a/src/core/lib/iomgr/error.cc b/src/core/lib/iomgr/error.cc
index 90ed34da11..8e7205f558 100644
--- a/src/core/lib/iomgr/error.cc
+++ b/src/core/lib/iomgr/error.cc
@@ -670,7 +670,8 @@ static void collect_times_kvs(grpc_error* err, kv_pairs* kvs) {
     uint8_t slot = err->times[which];
     if (slot != UINT8_MAX) {
       append_kv(kvs, key_time(static_cast<grpc_error_times>(which)),
-                fmt_time(*reinterpret_cast<gpr_timespec*>(err->arena + slot)));
+                gpr_format_timespec(
+                    *reinterpret_cast<gpr_timespec*>(err->arena + slot)));
     }
   }
 }
diff --git a/src/core/lib/json/json.cc b/src/core/lib/json/json.cc
index 816241bbf0..6d359573f8 100644
--- a/src/core/lib/json/json.cc
+++ b/src/core/lib/json/json.cc
@@ -18,10 +18,12 @@
 
 #include <grpc/support/port_platform.h>
 
+#include <inttypes.h>
 #include <string.h>
 
 #include <grpc/support/alloc.h>
 #include <grpc/support/log.h>
+#include <grpc/support/string_util.h>
 
 #include "src/core/lib/json/json.h"
 
@@ -84,3 +86,11 @@ grpc_json* grpc_json_create_child(grpc_json* sibling, grpc_json* parent,
   child->key = key;
   return child;
 }
+
+grpc_json* grpc_json_add_number_string_child(grpc_json* parent, grpc_json* it,
+                                             const char* name, int64_t num) {
+  char* num_str;
+  gpr_asprintf(&num_str, "%" PRId64, num);
+  return grpc_json_create_child(it, parent, name, num_str, GRPC_JSON_STRING,
+                                true);
+}
diff --git a/src/core/lib/json/json.h b/src/core/lib/json/json.h
index f93b43048b..8742774b5f 100644
--- a/src/core/lib/json/json.h
+++ b/src/core/lib/json/json.h
@@ -91,4 +91,8 @@ grpc_json* grpc_json_create_child(grpc_json* sibling, grpc_json* parent,
                                   const char* key, const char* value,
                                   grpc_json_type type, bool owns_value);
 
+/* TODO */
+grpc_json* grpc_json_add_number_string_child(grpc_json* parent, grpc_json* it,
+                                             const char* name, int64_t num);
+
 #endif /* GRPC_CORE_LIB_JSON_JSON_H */
diff --git a/src/ruby/ext/grpc/rb_grpc_imports.generated.c b/src/ruby/ext/grpc/rb_grpc_imports.generated.c
index 4e235121e2..1086b7dfcb 100644
--- a/src/ruby/ext/grpc/rb_grpc_imports.generated.c
+++ b/src/ruby/ext/grpc/rb_grpc_imports.generated.c
@@ -222,6 +222,7 @@ gpr_cpu_current_cpu_type gpr_cpu_current_cpu_import;
 gpr_format_message_type gpr_format_message_import;
 gpr_strdup_type gpr_strdup_import;
 gpr_asprintf_type gpr_asprintf_import;
+gpr_format_timespec_type gpr_format_timespec_import;
 gpr_mu_init_type gpr_mu_init_import;
 gpr_mu_destroy_type gpr_mu_destroy_import;
 gpr_mu_lock_type gpr_mu_lock_import;
@@ -470,6 +471,7 @@ void grpc_rb_load_imports(HMODULE library) {
   gpr_format_message_import = (gpr_format_message_type) GetProcAddress(library, "gpr_format_message");
   gpr_strdup_import = (gpr_strdup_type) GetProcAddress(library, "gpr_strdup");
   gpr_asprintf_import = (gpr_asprintf_type) GetProcAddress(library, "gpr_asprintf");
+  gpr_format_timespec_import = (gpr_format_timespec_type) GetProcAddress(library, "gpr_format_timespec");
   gpr_mu_init_import = (gpr_mu_init_type) GetProcAddress(library, "gpr_mu_init");
   gpr_mu_destroy_import = (gpr_mu_destroy_type) GetProcAddress(library, "gpr_mu_destroy");
   gpr_mu_lock_import = (gpr_mu_lock_type) GetProcAddress(library, "gpr_mu_lock");
diff --git a/src/ruby/ext/grpc/rb_grpc_imports.generated.h b/src/ruby/ext/grpc/rb_grpc_imports.generated.h
index f01c9c8248..b4107641a4 100644
--- a/src/ruby/ext/grpc/rb_grpc_imports.generated.h
+++ b/src/ruby/ext/grpc/rb_grpc_imports.generated.h
@@ -641,6 +641,9 @@ extern gpr_strdup_type gpr_strdup_import;
 typedef int(*gpr_asprintf_type)(char** strp, const char* format, ...) GPR_PRINT_FORMAT_CHECK(2, 3);
 extern gpr_asprintf_type gpr_asprintf_import;
 #define gpr_asprintf gpr_asprintf_import
+typedef char*(*gpr_format_timespec_type)(gpr_timespec);
+extern gpr_format_timespec_type gpr_format_timespec_import;
+#define gpr_format_timespec gpr_format_timespec_import
 typedef void(*gpr_mu_init_type)(gpr_mu* mu);
 extern gpr_mu_init_type gpr_mu_init_import;
 #define gpr_mu_init gpr_mu_init_import