Reviewer comments

reviewable/pr13883/r2
ncteisen 7 years ago
parent 95cf05794a
commit 6ea7f3330e
  1. 3
      include/grpc/impl/codegen/grpc_types.h
  2. 2
      src/core/ext/filters/client_channel/subchannel.cc
  3. 90
      src/core/lib/channel/channel_tracer.cc
  4. 27
      src/core/lib/channel/channel_tracer.h
  5. 1
      src/core/lib/json/json.h
  6. 13
      src/core/lib/surface/channel.cc

@ -283,7 +283,8 @@ typedef struct {
#define GRPC_ARG_SOCKET_FACTORY "grpc.socket_factory" #define GRPC_ARG_SOCKET_FACTORY "grpc.socket_factory"
/** The maximum number of trace events to keep in the tracer for each channel or /** The maximum number of trace events to keep in the tracer for each channel or
* subchannel. The default is 10. If set to 0, channel tracing is disabled. */ * subchannel. The default is 10. If set to 0, channel tracing is disabled. */
#define GRPC_ARG_MAX_CHANNEL_TRACE_EVENTS_PER_NODE "grpc.max_channel_trace_events_per_node" #define GRPC_ARG_MAX_CHANNEL_TRACE_EVENTS_PER_NODE \
"grpc.max_channel_trace_events_per_node"
/** If non-zero, Cronet transport will coalesce packets to fewer frames /** If non-zero, Cronet transport will coalesce packets to fewer frames
* when possible. */ * when possible. */
#define GRPC_ARG_USE_CRONET_PACKET_COALESCING \ #define GRPC_ARG_USE_CRONET_PACKET_COALESCING \

@ -136,7 +136,7 @@ struct grpc_subchannel {
/** our alarm */ /** our alarm */
grpc_timer alarm; grpc_timer alarm;
grpc_core::RefCountedPtr<grpc_core::ChannelTracer> tracer; grpc_core::RefCountedPtr<grpc_core::ChannelTrace> tracer;
}; };
struct grpc_subchannel_call { struct grpc_subchannel_call {

@ -40,7 +40,7 @@ class TraceEvent {
public: public:
TraceEvent(grpc_slice data, grpc_error* error, TraceEvent(grpc_slice data, grpc_error* error,
grpc_connectivity_state connectivity_state, grpc_connectivity_state connectivity_state,
RefCountedPtr<ChannelTracer> referenced_tracer) RefCountedPtr<ChannelTrace> referenced_tracer)
: data_(data), : data_(data),
error_(error), error_(error),
connectivity_state_(connectivity_state), connectivity_state_(connectivity_state),
@ -66,34 +66,34 @@ class TraceEvent {
} }
private: private:
friend class ChannelTracer; friend class ChannelTrace;
friend class ChannelTracerRenderer; friend class ChannelTraceRenderer;
grpc_slice data_; grpc_slice data_;
grpc_error* error_; grpc_error* error_;
gpr_timespec time_created_; gpr_timespec time_created_;
grpc_connectivity_state connectivity_state_; grpc_connectivity_state connectivity_state_;
TraceEvent* next_; TraceEvent* next_;
// the tracer object for the (sub)channel that this trace node refers to. // the tracer object for the (sub)channel that this trace event refers to.
RefCountedPtr<ChannelTracer> referenced_tracer_; RefCountedPtr<ChannelTrace> referenced_tracer_;
}; };
ChannelTracer::ChannelTracer(size_t max_nodes) ChannelTrace::ChannelTrace(size_t max_events)
: channel_uuid_(-1), : channel_uuid_(-1),
num_nodes_logged_(0), num_events_logged_(0),
num_children_seen_(0), num_children_seen_(0),
list_size_(0), list_size_(0),
max_list_size_(max_nodes), max_list_size_(max_events),
head_trace_(nullptr), head_trace_(nullptr),
tail_trace_(nullptr) { tail_trace_(nullptr) {
if (max_list_size_ == 0) return; // tracing is disabled if max_nodes == 0 if (max_list_size_ == 0) return; // tracing is disabled if max_events == 0
gpr_mu_init(&tracer_mu_); gpr_mu_init(&tracer_mu_);
channel_uuid_ = grpc_object_registry_register_object( channel_uuid_ = grpc_object_registry_register_object(
this, GRPC_OBJECT_REGISTRY_CHANNEL_TRACER); this, GRPC_OBJECT_REGISTRY_CHANNEL_TRACER);
time_created_ = gpr_now(GPR_CLOCK_REALTIME); time_created_ = gpr_now(GPR_CLOCK_REALTIME);
} }
ChannelTracer::~ChannelTracer() { ChannelTrace::~ChannelTrace() {
if (max_list_size_ == 0) return; // tracing is disabled if max_nodes == 0 if (max_list_size_ == 0) return; // tracing is disabled if max_events == 0
TraceEvent* it = head_trace_; TraceEvent* it = head_trace_;
while (it != nullptr) { while (it != nullptr) {
TraceEvent* to_free = it; TraceEvent* to_free = it;
@ -103,17 +103,17 @@ ChannelTracer::~ChannelTracer() {
gpr_mu_destroy(&tracer_mu_); gpr_mu_destroy(&tracer_mu_);
} }
intptr_t ChannelTracer::GetUuid() { return channel_uuid_; } intptr_t ChannelTrace::GetUuid() { return channel_uuid_; }
void ChannelTracer::AddTraceEventNode(TraceEvent* new_trace_node) { void ChannelTrace::AddTraceEventHelper(TraceEvent* new_trace_event) {
++num_nodes_logged_; ++num_events_logged_;
// first node case // first event case
if (head_trace_ == nullptr) { if (head_trace_ == nullptr) {
head_trace_ = tail_trace_ = new_trace_node; head_trace_ = tail_trace_ = new_trace_event;
} }
// regular node add case // regular event add case
else { else {
tail_trace_->next_ = new_trace_node; tail_trace_->next_ = new_trace_event;
tail_trace_ = tail_trace_->next_; tail_trace_ = tail_trace_->next_;
} }
++list_size_; ++list_size_;
@ -126,19 +126,20 @@ void ChannelTracer::AddTraceEventNode(TraceEvent* new_trace_node) {
} }
} }
void ChannelTracer::AddTrace(grpc_slice data, grpc_error* error, void ChannelTrace::AddTraceEvent(
grpc_slice data, grpc_error* error,
grpc_connectivity_state connectivity_state, grpc_connectivity_state connectivity_state,
RefCountedPtr<ChannelTracer> referenced_tracer) { RefCountedPtr<ChannelTrace> referenced_tracer) {
if (max_list_size_ == 0) return; // tracing is disabled if max_nodes == 0 if (max_list_size_ == 0) return; // tracing is disabled if max_events == 0
++num_children_seen_; ++num_children_seen_;
// create and fill up the new node // create and fill up the new event
AddTraceEventNode( AddTraceEventHelper(New<TraceEvent>(data, error, connectivity_state,
New<TraceEvent>(data, error, connectivity_state, std::move(referenced_tracer))); std::move(referenced_tracer)));
} }
void ChannelTracer::AddTrace(grpc_slice data, grpc_error* error, void ChannelTrace::AddTraceEvent(grpc_slice data, grpc_error* error,
grpc_connectivity_state connectivity_state) { grpc_connectivity_state connectivity_state) {
AddTraceEventNode(New<TraceEvent>(data, error, connectivity_state)); AddTraceEventHelper(New<TraceEvent>(data, error, connectivity_state));
} }
namespace { namespace {
@ -155,11 +156,11 @@ char* fmt_time(gpr_timespec tm) {
} // anonymous namespace } // anonymous namespace
class ChannelTracerRenderer { class ChannelTraceRenderer {
public: public:
// If recursive==true, then the entire tree of trace will be rendered. // If recursive==true, then the entire tree of trace will be rendered.
// If not, then only the top level data will be. // If not, then only the top level data will be.
ChannelTracerRenderer(ChannelTracer* tracer, bool recursive) ChannelTraceRenderer(ChannelTrace* tracer, bool recursive)
: current_tracer_(tracer), : current_tracer_(tracer),
recursive_(recursive), recursive_(recursive),
seen_tracers_(nullptr), seen_tracers_(nullptr),
@ -180,16 +181,16 @@ class ChannelTracerRenderer {
private: private:
// tracks that a tracer has already been rendered to avoid infinite // tracks that a tracer has already been rendered to avoid infinite
// recursion. // recursion.
void AddSeenTracer(ChannelTracer* newly_seen) { void AddSeenTracer(ChannelTrace* newly_seen) {
if (size_ >= cap_) { if (size_ >= cap_) {
cap_ = GPR_MAX(5 * sizeof(newly_seen), 3 * cap_ / 2); cap_ = GPR_MAX(5 * sizeof(newly_seen), 3 * cap_ / 2);
seen_tracers_ = (ChannelTracer**)gpr_realloc(seen_tracers_, cap_); seen_tracers_ = (ChannelTrace**)gpr_realloc(seen_tracers_, cap_);
} }
seen_tracers_[size_++] = newly_seen; seen_tracers_[size_++] = newly_seen;
} }
// Checks if a tracer has already been seen. // Checks if a tracer has already been seen.
bool TracerAlreadySeen(ChannelTracer* tracer) { bool TracerAlreadySeen(ChannelTrace* tracer) {
for (size_t i = 0; i < size_; ++i) { for (size_t i = 0; i < size_; ++i) {
if (seen_tracers_[i] == tracer) return true; if (seen_tracers_[i] == tracer) return true;
} }
@ -211,19 +212,19 @@ class ChannelTracerRenderer {
} }
// Fills up the channelData object. If children is not null, it will // Fills up the channelData object. If children is not null, it will
// recursively populate each referenced child as it passes that node. // recursively populate each referenced child as it passes that event.
void PopulateChannelData(grpc_json* channel_data, grpc_json* children) { void PopulateChannelData(grpc_json* channel_data, grpc_json* children) {
grpc_json* child = nullptr; grpc_json* child = nullptr;
char* uuid_str; char* uuid_str;
gpr_asprintf(&uuid_str, "%" PRIdPTR, current_tracer_->channel_uuid_); gpr_asprintf(&uuid_str, "%" PRIdPTR, current_tracer_->channel_uuid_);
child = grpc_json_create_child(child, channel_data, "uuid", uuid_str, child = grpc_json_create_child(child, channel_data, "uuid", uuid_str,
GRPC_JSON_NUMBER, true); GRPC_JSON_NUMBER, true);
char* num_nodes_logged_str; char* num_events_logged_str;
gpr_asprintf(&num_nodes_logged_str, "%" PRId64, gpr_asprintf(&num_events_logged_str, "%" PRId64,
current_tracer_->num_nodes_logged_); current_tracer_->num_events_logged_);
child = child =
grpc_json_create_child(child, channel_data, "numNodesLogged", grpc_json_create_child(child, channel_data, "numNodesLogged",
num_nodes_logged_str, GRPC_JSON_NUMBER, true); num_events_logged_str, GRPC_JSON_NUMBER, true);
child = grpc_json_create_child(child, channel_data, "startTime", child = grpc_json_create_child(child, channel_data, "startTime",
fmt_time(current_tracer_->time_created_), fmt_time(current_tracer_->time_created_),
GRPC_JSON_STRING, true); GRPC_JSON_STRING, true);
@ -278,7 +279,7 @@ class ChannelTracerRenderer {
grpc_json* referenced_tracer = grpc_json_create_child( grpc_json* referenced_tracer = grpc_json_create_child(
nullptr, children, nullptr, nullptr, GRPC_JSON_OBJECT, false); nullptr, children, nullptr, nullptr, GRPC_JSON_OBJECT, false);
AddSeenTracer(node->referenced_tracer_.get()); AddSeenTracer(node->referenced_tracer_.get());
ChannelTracer* saved = current_tracer_; ChannelTrace* saved = current_tracer_;
current_tracer_ = node->referenced_tracer_.get(); current_tracer_ = node->referenced_tracer_.get();
RecursivelyPopulateJson(referenced_tracer); RecursivelyPopulateJson(referenced_tracer);
current_tracer_ = saved; current_tracer_ = saved;
@ -287,30 +288,31 @@ class ChannelTracerRenderer {
} }
// Tracks the current tracer we are rendering as we walk the tree of tracers. // Tracks the current tracer we are rendering as we walk the tree of tracers.
ChannelTracer* current_tracer_; ChannelTrace* current_tracer_;
// If true, we will render the data of all of this tracer's children. // If true, we will render the data of all of this tracer's children.
bool recursive_; bool recursive_;
// These members are used to track tracers we have already rendered. This is // These members are used to track tracers we have already rendered. This is
// a dynamically growing array that is deallocated when the rendering is done. // a dynamically growing array that is deallocated when the rendering is done.
ChannelTracer** seen_tracers_; ChannelTrace** seen_tracers_;
size_t size_; size_t size_;
size_t cap_; size_t cap_;
}; };
char* ChannelTracer::RenderTrace(bool recursive) { char* ChannelTrace::RenderTrace(bool recursive) {
if (!max_list_size_) return nullptr; // tracing is disabled if max_nodes == 0 if (!max_list_size_)
ChannelTracerRenderer renderer(this, recursive); return nullptr; // tracing is disabled if max_events == 0
ChannelTraceRenderer renderer(this, recursive);
return renderer.Run(); return renderer.Run();
} }
char* ChannelTracer::GetChannelTraceFromUuid(intptr_t uuid, bool recursive) { char* ChannelTrace::GetChannelTraceFromUuid(intptr_t uuid, bool recursive) {
void* object; void* object;
grpc_object_registry_type type = grpc_object_registry_type type =
grpc_object_registry_get_object(uuid, &object); grpc_object_registry_get_object(uuid, &object);
GPR_ASSERT(type == GRPC_OBJECT_REGISTRY_CHANNEL_TRACER); GPR_ASSERT(type == GRPC_OBJECT_REGISTRY_CHANNEL_TRACER);
return static_cast<ChannelTracer*>(object)->RenderTrace(recursive); return static_cast<ChannelTrace*>(object)->RenderTrace(recursive);
} }
} // namespace grpc_core } // namespace grpc_core

@ -29,24 +29,24 @@ namespace grpc_core {
class TraceEvent; class TraceEvent;
class ChannelTracer : public RefCounted<ChannelTracer> { class ChannelTrace : public RefCounted<ChannelTrace> {
public: public:
ChannelTracer(size_t max_nodes); ChannelTrace(size_t max_events);
~ChannelTracer(); ~ChannelTrace();
/* returns the tracers uuid */ /* returns the tracer's uuid */
intptr_t GetUuid(); intptr_t GetUuid();
/* Adds a new trace node to the tracing object */ /* Adds a new trace event to the tracing object */
void AddTrace(grpc_slice data, grpc_error* error, void AddTraceEvent(grpc_slice data, grpc_error* error,
grpc_connectivity_state connectivity_state); grpc_connectivity_state connectivity_state);
/* Adds a new trace node to the tracing object. This trace node refers to a /* Adds a new trace event to the tracing object. This trace event refers to a
an event on a child of the channel. For example this could log when a an event on a child of the channel. For example this could log when a
particular subchannel becomes connected */ particular subchannel becomes connected */
void AddTrace(grpc_slice data, grpc_error* error, void AddTraceEvent(grpc_slice data, grpc_error* error,
grpc_connectivity_state connectivity_state, grpc_connectivity_state connectivity_state,
RefCountedPtr<ChannelTracer> referenced_tracer); RefCountedPtr<ChannelTrace> referenced_tracer);
/* Returns the tracing data rendered as a grpc json string. /* Returns the tracing data rendered as a grpc json string.
The string is owned by the caller and must be freed. If recursive The string is owned by the caller and must be freed. If recursive
@ -59,14 +59,13 @@ class ChannelTracer : public RefCounted<ChannelTracer> {
static char* GetChannelTraceFromUuid(intptr_t uuid, bool recursive); static char* GetChannelTraceFromUuid(intptr_t uuid, bool recursive);
private: private:
// Internal helper to add and link in a trace event
void AddTraceEventHelper(TraceEvent* new_trace_event);
// Internal helper to add and link in a tracenode friend class ChannelTraceRenderer;
void AddTraceEventNode(TraceEvent* new_trace_node);
friend class ChannelTracerRenderer;
gpr_mu tracer_mu_; gpr_mu tracer_mu_;
intptr_t channel_uuid_; intptr_t channel_uuid_;
uint64_t num_nodes_logged_; uint64_t num_events_logged_;
uint64_t num_children_seen_; uint64_t num_children_seen_;
size_t list_size_; size_t list_size_;
size_t max_list_size_; size_t max_list_size_;

@ -19,7 +19,6 @@
#ifndef GRPC_CORE_LIB_JSON_JSON_H #ifndef GRPC_CORE_LIB_JSON_JSON_H
#define GRPC_CORE_LIB_JSON_JSON_H #define GRPC_CORE_LIB_JSON_JSON_H
#include <grpc/support/port_platform.h> #include <grpc/support/port_platform.h>
#include <stdbool.h> #include <stdbool.h>

@ -67,7 +67,7 @@ struct grpc_channel {
gpr_mu registered_call_mu; gpr_mu registered_call_mu;
registered_call* registered_calls; registered_call* registered_calls;
grpc_core::RefCountedPtr<grpc_core::ChannelTracer> tracer; grpc_core::RefCountedPtr<grpc_core::ChannelTrace> tracer;
char* target; char* target;
}; };
@ -170,8 +170,8 @@ grpc_channel* grpc_channel_create_with_builder(
channel->compression_options.enabled_algorithms_bitset = channel->compression_options.enabled_algorithms_bitset =
static_cast<uint32_t>(args->args[i].value.integer) | static_cast<uint32_t>(args->args[i].value.integer) |
0x1; /* always support no compression */ 0x1; /* always support no compression */
} else if (0 == } else if (0 == strcmp(args->args[i].key,
strcmp(args->args[i].key, GRPC_ARG_MAX_CHANNEL_TRACE_EVENTS_PER_NODE)) { GRPC_ARG_MAX_CHANNEL_TRACE_EVENTS_PER_NODE)) {
GPR_ASSERT(channel_tracer_max_nodes == 0); GPR_ASSERT(channel_tracer_max_nodes == 0);
// max_nodes defaults to 10, clamped between 0 and 100. // max_nodes defaults to 10, clamped between 0 and 100.
const grpc_integer_options options = {10, 0, 100}; const grpc_integer_options options = {10, 0, 100};
@ -181,10 +181,11 @@ grpc_channel* grpc_channel_create_with_builder(
} }
grpc_channel_args_destroy(args); grpc_channel_args_destroy(args);
channel->tracer = grpc_core::MakeRefCounted<grpc_core::ChannelTracer>( channel->tracer = grpc_core::MakeRefCounted<grpc_core::ChannelTrace>(
channel_tracer_max_nodes); channel_tracer_max_nodes);
channel->tracer->AddTrace(grpc_slice_from_static_string("Channel created"), channel->tracer->AddTraceEvent(
GRPC_ERROR_NONE, GRPC_CHANNEL_IDLE); grpc_slice_from_static_string("Channel created"), GRPC_ERROR_NONE,
GRPC_CHANNEL_IDLE);
return channel; return channel;
} }

Loading…
Cancel
Save