Reviewer feedback

3 files changed, 35 insertions, 27 deletions

diff --git a/src/core/lib/channel/channel_tracer.cc b/src/core/lib/channel/channel_tracer.cc
index fdb96570cd..38a5b1ee20 100644
--- a/src/core/lib/channel/channel_tracer.cc
+++ b/src/core/lib/channel/channel_tracer.cc
@@ -40,6 +40,27 @@
 
 namespace grpc_core {
 
+ChannelTrace::TraceEvent::TraceEvent(
+    grpc_slice data, grpc_error* error,
+    grpc_connectivity_state connectivity_state,
+    RefCountedPtr<ChannelTrace> referenced_tracer)
+    : data_(data),
+      error_(error),
+      timestamp_(grpc_millis_to_timespec(grpc_core::ExecCtx::Get()->Now(),
+                                         GPR_CLOCK_REALTIME)),
+      connectivity_state_(connectivity_state),
+      next_(nullptr),
+      referenced_tracer_(std::move(referenced_tracer)) {}
+
+ChannelTrace::TraceEvent::TraceEvent(grpc_slice data, grpc_error* error,
+                                     grpc_connectivity_state connectivity_state)
+    : data_(data),
+      error_(error),
+      timestamp_(grpc_millis_to_timespec(grpc_core::ExecCtx::Get()->Now(),
+                                         GPR_CLOCK_REALTIME)),
+      connectivity_state_(connectivity_state),
+      next_(nullptr) {}
+
 ChannelTrace::TraceEvent::~TraceEvent() {
   GRPC_ERROR_UNREF(error_);
   grpc_slice_unref_internal(data_);
@@ -72,7 +93,7 @@ ChannelTrace::~ChannelTrace() {
   gpr_mu_destroy(&tracer_mu_);
 }
 
-intptr_t ChannelTrace::GetUuid() { return channel_uuid_; }
+intptr_t ChannelTrace::GetUuid() const { return channel_uuid_; }
 
 void ChannelTrace::AddTraceEventHelper(TraceEvent* new_trace_event) {
   ++num_events_logged_;
@@ -125,7 +146,7 @@ char* fmt_time(gpr_timespec tm) {
 
 }  // anonymous namespace
 
-void ChannelTrace::TraceEvent::RenderTraceEvent(grpc_json* json) {
+void ChannelTrace::TraceEvent::RenderTraceEvent(grpc_json* json) const {
   grpc_json* json_iterator = nullptr;
   json_iterator = grpc_json_create_child(json_iterator, json, "description",
                                          grpc_slice_to_c_string(data_),
@@ -150,7 +171,7 @@ void ChannelTrace::TraceEvent::RenderTraceEvent(grpc_json* json) {
   }
   json_iterator =
       grpc_json_create_child(json_iterator, json, "timestamp",
-                             fmt_time(time_created_), GRPC_JSON_STRING, true);
+                             fmt_time(timestamp_), GRPC_JSON_STRING, true);
   json_iterator =
       grpc_json_create_child(json_iterator, json, "state",
                              grpc_connectivity_state_name(connectivity_state_),
@@ -163,7 +184,7 @@ void ChannelTrace::TraceEvent::RenderTraceEvent(grpc_json* json) {
   }
 }
 
-char* ChannelTrace::RenderTrace() {
+char* ChannelTrace::RenderTrace() const {
   if (!max_list_size_)
     return nullptr;  // tracing is disabled if max_events == 0
   grpc_json* json = grpc_json_create(GRPC_JSON_OBJECT);
diff --git a/src/core/lib/channel/channel_tracer.h b/src/core/lib/channel/channel_tracer.h
index 976e66c49e..3339be85e3 100644
--- a/src/core/lib/channel/channel_tracer.h
+++ b/src/core/lib/channel/channel_tracer.h
@@ -38,7 +38,7 @@ class ChannelTrace : public RefCounted<ChannelTrace> {
   ~ChannelTrace();
 
   // returns the tracer's uuid
-  intptr_t GetUuid();
+  intptr_t GetUuid() const;
 
   // Adds a new trace event to the tracing object
   void AddTraceEvent(grpc_slice data, grpc_error* error,
@@ -57,7 +57,7 @@ class ChannelTrace : public RefCounted<ChannelTrace> {
 
   // Returns the tracing data rendered as a grpc json string.
   // The string is owned by the caller and must be freed.
-  char* RenderTrace();
+  char* RenderTrace() const;
 
  private:
   // Private class to encapsulate all the data and bookkeeping needed for a
@@ -69,41 +69,27 @@ class ChannelTrace : public RefCounted<ChannelTrace> {
     // overall channelz object, not just the ChannelTrace object
     TraceEvent(grpc_slice data, grpc_error* error,
                grpc_connectivity_state connectivity_state,
-               RefCountedPtr<ChannelTrace> referenced_tracer)
-        : data_(data),
-          error_(error),
-          connectivity_state_(connectivity_state),
-          next_(nullptr),
-          referenced_tracer_(std::move(referenced_tracer)) {
-      time_created_ = gpr_now(GPR_CLOCK_REALTIME);
-    }
+               RefCountedPtr<ChannelTrace> referenced_tracer);
 
     // Constructor for a TraceEvent that does not reverence a different
     // channel.
     TraceEvent(grpc_slice data, grpc_error* error,
-               grpc_connectivity_state connectivity_state)
-        : data_(data),
-          error_(error),
-          connectivity_state_(connectivity_state),
-          next_(nullptr),
-          referenced_tracer_(nullptr) {
-      time_created_ = gpr_now(GPR_CLOCK_REALTIME);
-    }
+               grpc_connectivity_state connectivity_state);
 
     ~TraceEvent();
 
     // Renders the data inside of this TraceEvent into a json object. This is
     // used by the ChannelTrace, when it is rendering itself.
-    void RenderTraceEvent(grpc_json* json);
+    void RenderTraceEvent(grpc_json* json) const;
 
     // set and get for the next_ pointer.
-    TraceEvent* next() { return next_; }
+    TraceEvent* next() const { return next_; }
     void set_next(TraceEvent* next) { next_ = next; }
 
    private:
     grpc_slice data_;
     grpc_error* error_;
-    gpr_timespec time_created_;
+    gpr_timespec timestamp_;
     grpc_connectivity_state connectivity_state_;
     TraceEvent* next_;
     // the tracer object for the (sub)channel that this trace event refers to.
diff --git a/src/core/lib/surface/channel.cc b/src/core/lib/surface/channel.cc
index 1488736c49..fb435b9881 100644
--- a/src/core/lib/surface/channel.cc
+++ b/src/core/lib/surface/channel.cc
@@ -21,6 +21,7 @@
 #include "src/core/lib/surface/channel.h"
 
 #include <inttypes.h>
+#include <limits.h>
 #include <stdlib.h>
 #include <string.h>
 
@@ -173,8 +174,8 @@ grpc_channel* grpc_channel_create_with_builder(
     } else if (0 == strcmp(args->args[i].key,
                            GRPC_ARG_MAX_CHANNEL_TRACE_EVENTS_PER_NODE)) {
       GPR_ASSERT(channel_tracer_max_nodes == 0);
-      // max_nodes defaults to 0 (which is off), clamped between 0 and 100.
-      const grpc_integer_options options = {0, 0, 100};
+      // max_nodes defaults to 0 (which is off), clamped between 0 and INT_MAX
+      const grpc_integer_options options = {0, 0, INT_MAX};
       channel_tracer_max_nodes =
           (size_t)grpc_channel_arg_get_integer(&args->args[i], options);
     }