From b69f1f6aacad86bdc72e25085d74e64f17f32195 Mon Sep 17 00:00:00 2001 From: ncteisen Date: Wed, 9 May 2018 10:38:44 -0700 Subject: Update channelz proto --- src/core/lib/channel/channel_trace.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/core/lib/channel/channel_trace.cc') diff --git a/src/core/lib/channel/channel_trace.cc b/src/core/lib/channel/channel_trace.cc index eb7214b355..84f7f000f7 100644 --- a/src/core/lib/channel/channel_trace.cc +++ b/src/core/lib/channel/channel_trace.cc @@ -219,7 +219,7 @@ char* ChannelTrace::RenderTrace() const { 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, "creationTime", + grpc_json_create_child(json_iterator, json, "creationTimestamp", fmt_time(time_created_), GRPC_JSON_STRING, true); grpc_json* events = grpc_json_create_child(json_iterator, json, "events", nullptr, GRPC_JSON_ARRAY, false); -- cgit v1.2.3 From 23c50fda51a38d0484f4b1537492fd93eeae33ac Mon Sep 17 00:00:00 2001 From: ncteisen Date: Fri, 18 May 2018 16:38:29 -0700 Subject: Incorperate channel trace into channelz --- src/core/lib/channel/channel_trace.cc | 37 ++++++++++++++++++++--------------- src/core/lib/channel/channel_trace.h | 21 ++++++++++---------- src/core/lib/channel/channelz.cc | 13 +++++++++--- src/core/lib/channel/channelz.h | 9 ++++++++- src/core/lib/surface/channel.cc | 19 +++++++++--------- 5 files changed, 58 insertions(+), 41 deletions(-) (limited to 'src/core/lib/channel/channel_trace.cc') diff --git a/src/core/lib/channel/channel_trace.cc b/src/core/lib/channel/channel_trace.cc index 84f7f000f7..efa034a188 100644 --- a/src/core/lib/channel/channel_trace.cc +++ b/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( - Severity severity, grpc_slice data, - RefCountedPtr referenced_tracer, ReferencedType type) +ChannelTrace::TraceEvent::TraceEvent(Severity severity, grpc_slice data, + RefCountedPtr referenced_channel, + 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 referenced_tracer) { + RefCountedPtr referenced_channel) { if (max_list_size_ == 0) return; // tracing is disabled if max_events == 0 // create and fill up the new event - AddTraceEventHelper( - New(severity, data, std::move(referenced_tracer), Channel)); + AddTraceEventHelper(New( + severity, data, std::move(referenced_channel), ReferencedType::Channel)); } void ChannelTrace::AddTraceEventReferencingSubchannel( Severity severity, grpc_slice data, - RefCountedPtr referenced_tracer) { + RefCountedPtr referenced_channel) { if (max_list_size_ == 0) return; // tracing is disabled if max_events == 0 // create and fill up the new event - AddTraceEventHelper(New( - severity, data, std::move(referenced_tracer), Subchannel)); + AddTraceEventHelper(New(severity, data, + 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, - GRPC_JSON_OBJECT, false); + (referenced_type_ == ReferencedType::Channel) ? "channelRef" + : "subchannelRef", + nullptr, GRPC_JSON_OBJECT, false); json_iterator = grpc_json_create_child( nullptr, child_ref, - (referenced_type_ == Channel) ? "channelId" : "subchannelId", uuid_str, - GRPC_JSON_STRING, true); + (referenced_type_ == ReferencedType::Channel) ? "channelId" + : "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 diff --git a/src/core/lib/channel/channel_trace.h b/src/core/lib/channel/channel_trace.h index 1df1e585f2..687c6bc063 100644 --- a/src/core/lib/channel/channel_trace.h +++ b/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 { +class ChannelTrace { public: ChannelTrace(size_t max_events); ~ChannelTrace(); @@ -59,17 +62,15 @@ class ChannelTrace : public RefCounted { // 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 referenced_tracer); + RefCountedPtr referenced_tracer); void AddTraceEventReferencingSubchannel( Severity severity, grpc_slice data, - RefCountedPtr referenced_tracer); + RefCountedPtr 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 { 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 referenced_tracer, - ReferencedType type); + RefCountedPtr referenced_tracer, ReferencedType type); // Constructor for a TraceEvent that does not reverence a different // channel. @@ -109,7 +107,7 @@ class ChannelTrace : public RefCounted { gpr_timespec timestamp_; TraceEvent* next_; // the tracer object for the (sub)channel that this trace event refers to. - RefCountedPtr referenced_tracer_; + RefCountedPtr 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 { gpr_timespec time_created_; }; +} // namespace channelz } // namespace grpc_core #endif /* GRPC_CORE_LIB_CHANNEL_CHANNEL_TRACE_H */ diff --git a/src/core/lib/channel/channelz.cc b/src/core/lib/channel/channelz.cc index aabc941dcd..cf7545621e 100644 --- a/src/core/lib/channel/channelz.cc +++ b/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( - grpc_core::ExecCtx::Get()->Now(), GPR_CLOCK_REALTIME); + last_call_started_timestamp_ = + 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); diff --git a/src/core/lib/channel/channelz.h b/src/core/lib/channel/channelz.h index 661228cf44..7cea00392f 100644 --- a/src/core/lib/channel/channelz.h +++ b/src/core/lib/channel/channelz.h @@ -24,6 +24,8 @@ #include #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 { 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 { 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 { uint64_t calls_failed_ = 0; gpr_timespec last_call_started_timestamp_; intptr_t channel_uuid_; + ManualConstructor trace_; grpc_connectivity_state GetConnectivityState(); }; diff --git a/src/core/lib/surface/channel.cc b/src/core/lib/surface/channel.cc index da66a120d5..22760f4ea9 100644 --- a/src/core/lib/surface/channel.cc +++ b/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 tracer; grpc_core::RefCountedPtr 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( - channel_tracer_max_nodes); - channel->tracer->AddTraceEvent( - grpc_core::ChannelTrace::Severity::Info, - grpc_slice_from_static_string("Channel created")); + channel_tracer_max_nodes = 10; channel->channelz_channel = - grpc_core::MakeRefCounted(channel); + grpc_core::MakeRefCounted( + 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); -- cgit v1.2.3 From 9a1bb051812a53462b2deb7e472f20e3e1dd785f Mon Sep 17 00:00:00 2001 From: ncteisen Date: Wed, 30 May 2018 16:17:06 -0700 Subject: Build out the channelz unit test --- include/grpc/grpc.h | 3 - src/core/lib/channel/channel_trace.cc | 16 +-- src/core/lib/channel/channel_trace.h | 13 +- src/core/lib/channel/channelz.cc | 76 +++++++----- src/core/lib/channel/channelz.h | 6 +- src/core/lib/surface/channel.cc | 5 +- test/core/channel/BUILD | 24 ++-- test/core/channel/channel_trace_test.cc | 182 +++++++++++++++------------- test/core/channel/channelz_test.cc | 150 ++++++++++++++++++++++- test/cpp/util/channel_trace_proto_helper.cc | 11 +- 10 files changed, 340 insertions(+), 146 deletions(-) (limited to 'src/core/lib/channel/channel_trace.cc') diff --git a/include/grpc/grpc.h b/include/grpc/grpc.h index de7c39b183..dd8a5d7d5f 100644 --- a/include/grpc/grpc.h +++ b/include/grpc/grpc.h @@ -294,9 +294,6 @@ GRPCAPI char* grpc_channel_get_trace(grpc_channel* channel); later time. */ GRPCAPI intptr_t grpc_channel_get_uuid(grpc_channel* channel); -/** channelz support */ -GRPCAPI char* grpc_channelz_get_channel(intptr_t channel_id); - /** Error handling for grpc_call Most grpc_call functions return a grpc_error. If the error is not GRPC_OK then the operation failed due to some unsatisfied precondition. diff --git a/src/core/lib/channel/channel_trace.cc b/src/core/lib/channel/channel_trace.cc index efa034a188..c82e42d545 100644 --- a/src/core/lib/channel/channel_trace.cc +++ b/src/core/lib/channel/channel_trace.cc @@ -28,7 +28,6 @@ #include #include -#include "src/core/lib/channel/channelz_registry.h" #include "src/core/lib/channel/status_util.h" #include "src/core/lib/gpr/string.h" #include "src/core/lib/gpr/useful.h" @@ -63,15 +62,13 @@ ChannelTrace::TraceEvent::TraceEvent(Severity severity, grpc_slice data) ChannelTrace::TraceEvent::~TraceEvent() { grpc_slice_unref_internal(data_); } ChannelTrace::ChannelTrace(size_t max_events) - : channel_uuid_(-1), - num_events_logged_(0), + : num_events_logged_(0), list_size_(0), max_list_size_(max_events), head_trace_(nullptr), tail_trace_(nullptr) { if (max_list_size_ == 0) return; // tracing is disabled if max_events == 0 gpr_mu_init(&tracer_mu_); - channel_uuid_ = ChannelzRegistry::Register(this); time_created_ = grpc_millis_to_timespec(grpc_core::ExecCtx::Get()->Now(), GPR_CLOCK_REALTIME); } @@ -84,12 +81,9 @@ ChannelTrace::~ChannelTrace() { it = it->next(); Delete(to_free); } - ChannelzRegistry::Unregister(channel_uuid_); gpr_mu_destroy(&tracer_mu_); } -intptr_t ChannelTrace::GetUuid() const { return channel_uuid_; } - void ChannelTrace::AddTraceEventHelper(TraceEvent* new_trace_event) { ++num_events_logged_; // first event case @@ -212,7 +206,7 @@ void ChannelTrace::TraceEvent::RenderTraceEvent(grpc_json* json) const { } } -char* ChannelTrace::RenderTrace() const { +grpc_json* ChannelTrace::RenderJSON() const { if (!max_list_size_) return nullptr; // tracing is disabled if max_events == 0 grpc_json* json = grpc_json_create(GRPC_JSON_OBJECT); @@ -235,6 +229,12 @@ char* ChannelTrace::RenderTrace() const { it->RenderTraceEvent(json_iterator); it = it->next(); } + return json; +} + +char* ChannelTrace::RenderTrace() const { + grpc_json* json = RenderJSON(); + if (json == nullptr) return nullptr; char* json_str = grpc_json_dump_to_string(json, 0); grpc_json_destroy(json); return json_str; diff --git a/src/core/lib/channel/channel_trace.h b/src/core/lib/channel/channel_trace.h index 687c6bc063..b1224bb96c 100644 --- a/src/core/lib/channel/channel_trace.h +++ b/src/core/lib/channel/channel_trace.h @@ -40,9 +40,6 @@ class ChannelTrace { ChannelTrace(size_t max_events); ~ChannelTrace(); - // returns the tracer's uuid - intptr_t GetUuid() const; - enum Severity { Unset = 0, // never to be used Info, // we start at 1 to avoid using proto default values @@ -72,8 +69,13 @@ class ChannelTrace { Severity severity, grpc_slice data, RefCountedPtr referenced_tracer); - // Returns the tracing data rendered as a grpc json string. - // The string is owned by the caller and must be freed. + // Creates and returns the raw grpc_json object, so a parent channelz + // object may incorporate the json before rendering. + grpc_json* RenderJSON() const; + + // Returns the tracing data rendered as a grpc json string. The string + // is owned by the caller and must be freed. This is used for testing only + // so that we may unit test ChannelTrace in isolation. char* RenderTrace() const; private: @@ -117,7 +119,6 @@ class ChannelTrace { void AddTraceEventHelper(TraceEvent* new_trace_event); gpr_mu tracer_mu_; - intptr_t channel_uuid_; uint64_t num_events_logged_; size_t list_size_; size_t max_list_size_; diff --git a/src/core/lib/channel/channelz.cc b/src/core/lib/channel/channelz.cc index cf7545621e..5c3dc97e69 100644 --- a/src/core/lib/channel/channelz.cc +++ b/src/core/lib/channel/channelz.cc @@ -39,9 +39,6 @@ #include "src/core/lib/transport/connectivity_state.h" #include "src/core/lib/transport/error_utils.h" -// TODO(ncteisen): actually implement this -char* grpc_channelz_get_channel(intptr_t channel_id) { return nullptr; } - namespace grpc_core { namespace channelz { @@ -82,9 +79,9 @@ char* fmt_time(gpr_timespec tm) { // TODO(ncteisen); move this to json library grpc_json* add_num_str(grpc_json* parent, grpc_json* it, const char* name, - uint64_t num) { + int64_t num) { char* num_str; - gpr_asprintf(&num_str, "%" PRIu64, num); + gpr_asprintf(&num_str, "%" PRId64, num); return grpc_json_create_child(it, parent, name, num_str, GRPC_JSON_STRING, true); } @@ -96,10 +93,13 @@ Channel::Channel(grpc_channel* channel, size_t channel_tracer_max_nodes) trace_.Init(channel_tracer_max_nodes); target_ = grpc_channel_get_target(channel_); channel_uuid_ = ChannelzRegistry::Register(this); + last_call_started_timestamp_ = + grpc_millis_to_timespec(ExecCtx::Get()->Now(), GPR_CLOCK_REALTIME); } Channel::~Channel() { gpr_free(const_cast(target_)); + trace_.Destroy(); ChannelzRegistry::Unregister(channel_uuid_); } @@ -125,7 +125,7 @@ char* Channel::RenderJSON() { // create and fill the ref child json_iterator = grpc_json_create_child(json_iterator, json, "ref", nullptr, - GRPC_JSON_OBJECT, true); + GRPC_JSON_OBJECT, false); json = json_iterator; json_iterator = nullptr; json_iterator = add_num_str(json, json_iterator, "channelId", channel_uuid_); @@ -134,33 +134,55 @@ char* Channel::RenderJSON() { json = top_level_json; json_iterator = nullptr; - // create and fill the data child - json_iterator = grpc_json_create_child(json_iterator, json, "data", nullptr, - GRPC_JSON_OBJECT, true); - json = json_iterator; + // create and fill the data child. + grpc_json* data = grpc_json_create_child(json_iterator, json, "data", nullptr, + GRPC_JSON_OBJECT, false); + + json = data; json_iterator = nullptr; - 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_create_child( - json_iterator, json, "lastCallStartedTimestamp", - fmt_time(last_call_started_timestamp_), GRPC_JSON_STRING, true); + + // create and fill the connectivity state child. + grpc_connectivity_state connectivity_state = GetConnectivityState(); + json_iterator = grpc_json_create_child(json_iterator, json, "state", nullptr, + GRPC_JSON_OBJECT, false); + json = json_iterator; + grpc_json_create_child(nullptr, json, "state", + grpc_connectivity_state_name(connectivity_state), + GRPC_JSON_STRING, false); + + // reset the parent to be the data object. + json = data; json_iterator = grpc_json_create_child(json_iterator, json, "target", target_, GRPC_JSON_STRING, false); - grpc_connectivity_state connectivity_state = GetConnectivityState(); - json_iterator = - grpc_json_create_child(json_iterator, json, "state", - grpc_connectivity_state_name(connectivity_state), - GRPC_JSON_STRING, false); - char* trace = trace_->RenderTrace(); + + // fill in the channel trace if applicable + grpc_json* trace = trace_->RenderJSON(); if (trace != nullptr) { - json_iterator = grpc_json_create_child(json_iterator, json, "trace", trace, - GRPC_JSON_STRING, true); + // we manuall link up and fill the child since it was created for us in + // ChannelTrace::RenderJSON + json_iterator = grpc_json_link_child(json, trace, json_iterator); + trace->parent = json; + trace->value = nullptr; + trace->key = "trace"; + trace->owns_value = false; } + // reset the parent to be the data object. + json = data; + 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_ ? calls_started_ : -1); + json_iterator = add_num_str(json, json_iterator, "callsSucceeded", + calls_succeeded_ ? calls_succeeded_ : -1); + json_iterator = add_num_str(json, json_iterator, "callsFailed", + calls_failed_ ? calls_failed_ : -1); + json_iterator = grpc_json_create_child( + json_iterator, json, "lastCallStartedTimestamp", + fmt_time(last_call_started_timestamp_), GRPC_JSON_STRING, true); + // 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 7cea00392f..63b90e90a0 100644 --- a/src/core/lib/channel/channelz.h +++ b/src/core/lib/channel/channelz.h @@ -59,9 +59,9 @@ class Channel : public RefCounted { bool channel_destroyed_ = false; grpc_channel* channel_; const char* target_; - uint64_t calls_started_ = 0; - uint64_t calls_succeeded_ = 0; - uint64_t calls_failed_ = 0; + int64_t calls_started_ = 0; + int64_t calls_succeeded_ = 0; + int64_t calls_failed_ = 0; gpr_timespec last_call_started_timestamp_; intptr_t channel_uuid_; ManualConstructor trace_; diff --git a/src/core/lib/surface/channel.cc b/src/core/lib/surface/channel.cc index 22760f4ea9..be9869b08c 100644 --- a/src/core/lib/surface/channel.cc +++ b/src/core/lib/surface/channel.cc @@ -146,7 +146,6 @@ grpc_channel* grpc_channel_create_with_builder( } grpc_channel_args_destroy(args); - channel_tracer_max_nodes = 10; channel->channelz_channel = grpc_core::MakeRefCounted( channel, channel_tracer_max_nodes); @@ -201,7 +200,7 @@ grpc_core::channelz::Channel* grpc_channel_get_channelz_channel( } intptr_t grpc_channel_get_uuid(grpc_channel* channel) { - return channel->channelz_channel->Trace()->GetUuid(); + return channel->channelz_channel->channel_uuid(); } grpc_channel* grpc_channel_create(const char* target, @@ -430,6 +429,8 @@ void grpc_channel_destroy(grpc_channel* channel) { GRPC_ERROR_CREATE_FROM_STATIC_STRING("Channel Destroyed"); elem = grpc_channel_stack_element(CHANNEL_STACK_FROM_CHANNEL(channel), 0); elem->filter->start_transport_op(elem, op); + channel->channelz_channel->set_channel_destroyed(); + channel->channelz_channel.reset(); GRPC_CHANNEL_INTERNAL_UNREF(channel, "channel"); } diff --git a/test/core/channel/BUILD b/test/core/channel/BUILD index c336688209..81c4353129 100644 --- a/test/core/channel/BUILD +++ b/test/core/channel/BUILD @@ -84,13 +84,8 @@ grpc_cc_test( ) grpc_cc_test( -<<<<<<< HEAD - name = "channelz_registry_test", - srcs = ["channelz_registry_test.cc"], -======= name = "channelz_test", srcs = ["channelz_test.cc"], ->>>>>>> Add channelz test language = "C++", deps = [ "//:gpr", @@ -98,10 +93,23 @@ grpc_cc_test( "//:grpc++", "//test/core/util:gpr_test_util", "//test/core/util:grpc_test_util", -<<<<<<< HEAD -======= "//test/cpp/util:channel_trace_proto_helper", ->>>>>>> Add channelz test + ], + external_deps = [ + "gtest", + ], +) + +grpc_cc_test( + name = "channelz_registry_test", + srcs = ["channelz_registry_test.cc"], + language = "C++", + deps = [ + "//:gpr", + "//:grpc", + "//:grpc++", + "//test/core/util:gpr_test_util", + "//test/core/util:grpc_test_util", ], external_deps = [ "gtest", diff --git a/test/core/channel/channel_trace_test.cc b/test/core/channel/channel_trace_test.cc index 9befdb4fa3..c8619be007 100644 --- a/test/core/channel/channel_trace_test.cc +++ b/test/core/channel/channel_trace_test.cc @@ -25,6 +25,7 @@ #include #include "src/core/lib/channel/channel_trace.h" +#include "src/core/lib/channel/channelz.h" #include "src/core/lib/channel/channelz_registry.h" #include "src/core/lib/gpr/useful.h" #include "src/core/lib/iomgr/exec_ctx.h" @@ -39,6 +40,7 @@ #include namespace grpc_core { +namespace channelz { namespace testing { namespace { @@ -77,13 +79,13 @@ void ValidateChannelTraceData(grpc_json* json, ValidateJsonArraySize(json, "events", actual_num_events_expected); } -void AddSimpleTrace(RefCountedPtr tracer) { +void AddSimpleTrace(ChannelTrace* tracer) { tracer->AddTraceEvent(ChannelTrace::Severity::Info, grpc_slice_from_static_string("simple trace")); } // checks for the existence of all the required members of the tracer. -void ValidateChannelTrace(RefCountedPtr tracer, +void ValidateChannelTrace(ChannelTrace* tracer, size_t expected_num_event_logged, size_t max_nodes) { if (!max_nodes) return; char* json_str = tracer->RenderTrace(); @@ -95,16 +97,26 @@ void ValidateChannelTrace(RefCountedPtr tracer, gpr_free(json_str); } -void ValidateTraceDataMatchedUuidLookup(RefCountedPtr tracer) { - intptr_t uuid = tracer->GetUuid(); - if (uuid == -1) return; // Doesn't make sense to lookup if tracing disabled - char* tracer_json_str = tracer->RenderTrace(); - ChannelTrace* uuid_lookup = ChannelzRegistry::Get(uuid); - char* uuid_lookup_json_str = uuid_lookup->RenderTrace(); - EXPECT_EQ(strcmp(tracer_json_str, uuid_lookup_json_str), 0); - gpr_free(tracer_json_str); - gpr_free(uuid_lookup_json_str); -} +class ChannelFixture { + public: + ChannelFixture(int max_trace_nodes) { + grpc_arg client_a; + client_a.type = GRPC_ARG_INTEGER; + client_a.key = + const_cast(GRPC_ARG_MAX_CHANNEL_TRACE_EVENTS_PER_NODE); + client_a.value.integer = max_trace_nodes; + grpc_channel_args client_args = {1, &client_a}; + channel_ = + grpc_insecure_channel_create("fake_target", &client_args, nullptr); + } + + ~ChannelFixture() { grpc_channel_destroy(channel_); } + + grpc_channel* channel() { return channel_; } + + private: + grpc_channel* channel_; +}; } // anonymous namespace @@ -114,25 +126,22 @@ class ChannelTracerTest : public ::testing::TestWithParam {}; // lookups by uuid. TEST_P(ChannelTracerTest, BasicTest) { grpc_core::ExecCtx exec_ctx; - RefCountedPtr tracer = MakeRefCounted(GetParam()); - AddSimpleTrace(tracer); - AddSimpleTrace(tracer); - ValidateTraceDataMatchedUuidLookup(tracer); - tracer->AddTraceEvent(ChannelTrace::Severity::Info, - grpc_slice_from_static_string("trace three")); - tracer->AddTraceEvent(ChannelTrace::Severity::Error, - grpc_slice_from_static_string("trace four error")); - ValidateChannelTrace(tracer, 4, GetParam()); - AddSimpleTrace(tracer); - AddSimpleTrace(tracer); - ValidateChannelTrace(tracer, 6, GetParam()); - AddSimpleTrace(tracer); - AddSimpleTrace(tracer); - AddSimpleTrace(tracer); - AddSimpleTrace(tracer); - ValidateChannelTrace(tracer, 10, GetParam()); - ValidateTraceDataMatchedUuidLookup(tracer); - tracer.reset(nullptr); + ChannelTrace tracer(GetParam()); + AddSimpleTrace(&tracer); + AddSimpleTrace(&tracer); + tracer.AddTraceEvent(ChannelTrace::Severity::Info, + grpc_slice_from_static_string("trace three")); + tracer.AddTraceEvent(ChannelTrace::Severity::Error, + grpc_slice_from_static_string("trace four error")); + ValidateChannelTrace(&tracer, 4, GetParam()); + AddSimpleTrace(&tracer); + AddSimpleTrace(&tracer); + ValidateChannelTrace(&tracer, 6, GetParam()); + AddSimpleTrace(&tracer); + AddSimpleTrace(&tracer); + AddSimpleTrace(&tracer); + AddSimpleTrace(&tracer); + ValidateChannelTrace(&tracer, 10, GetParam()); } // Tests more complex functionality, like a parent channel tracking @@ -140,42 +149,43 @@ TEST_P(ChannelTracerTest, BasicTest) { // and this function will both hold refs to the subchannel. TEST_P(ChannelTracerTest, ComplexTest) { grpc_core::ExecCtx exec_ctx; - RefCountedPtr tracer = MakeRefCounted(GetParam()); - AddSimpleTrace(tracer); - AddSimpleTrace(tracer); - RefCountedPtr sc1 = MakeRefCounted(GetParam()); - tracer->AddTraceEventReferencingSubchannel( + ChannelTrace tracer(GetParam()); + AddSimpleTrace(&tracer); + AddSimpleTrace(&tracer); + ChannelFixture channel1(GetParam()); + RefCountedPtr sc1 = + MakeRefCounted(channel1.channel(), GetParam()); + tracer.AddTraceEventReferencingSubchannel( ChannelTrace::Severity::Info, grpc_slice_from_static_string("subchannel one created"), sc1); - ValidateChannelTrace(tracer, 3, GetParam()); - AddSimpleTrace(sc1); - AddSimpleTrace(sc1); - AddSimpleTrace(sc1); - ValidateChannelTrace(sc1, 3, GetParam()); - AddSimpleTrace(sc1); - AddSimpleTrace(sc1); - AddSimpleTrace(sc1); - ValidateChannelTrace(sc1, 6, GetParam()); - AddSimpleTrace(tracer); - AddSimpleTrace(tracer); - ValidateChannelTrace(tracer, 5, GetParam()); - ValidateTraceDataMatchedUuidLookup(tracer); - RefCountedPtr sc2 = MakeRefCounted(GetParam()); - tracer->AddTraceEventReferencingChannel( + ValidateChannelTrace(&tracer, 3, GetParam()); + AddSimpleTrace(sc1->Trace()); + AddSimpleTrace(sc1->Trace()); + AddSimpleTrace(sc1->Trace()); + ValidateChannelTrace(sc1->Trace(), 3, GetParam()); + AddSimpleTrace(sc1->Trace()); + AddSimpleTrace(sc1->Trace()); + AddSimpleTrace(sc1->Trace()); + ValidateChannelTrace(sc1->Trace(), 6, GetParam()); + AddSimpleTrace(&tracer); + AddSimpleTrace(&tracer); + ValidateChannelTrace(&tracer, 5, GetParam()); + ChannelFixture channel2(GetParam()); + RefCountedPtr sc2 = + MakeRefCounted(channel2.channel(), GetParam()); + tracer.AddTraceEventReferencingChannel( ChannelTrace::Severity::Info, grpc_slice_from_static_string("LB channel two created"), sc2); - tracer->AddTraceEventReferencingSubchannel( + tracer.AddTraceEventReferencingSubchannel( ChannelTrace::Severity::Warning, grpc_slice_from_static_string("subchannel one inactive"), sc1); - ValidateChannelTrace(tracer, 7, GetParam()); - AddSimpleTrace(tracer); - AddSimpleTrace(tracer); - AddSimpleTrace(tracer); - AddSimpleTrace(tracer); - AddSimpleTrace(tracer); - AddSimpleTrace(tracer); - ValidateTraceDataMatchedUuidLookup(tracer); - tracer.reset(nullptr); + ValidateChannelTrace(&tracer, 7, GetParam()); + AddSimpleTrace(&tracer); + AddSimpleTrace(&tracer); + AddSimpleTrace(&tracer); + AddSimpleTrace(&tracer); + AddSimpleTrace(&tracer); + AddSimpleTrace(&tracer); sc1.reset(nullptr); sc2.reset(nullptr); } @@ -185,39 +195,44 @@ TEST_P(ChannelTracerTest, ComplexTest) { // gets deleted. TEST_P(ChannelTracerTest, TestNesting) { grpc_core::ExecCtx exec_ctx; - RefCountedPtr tracer = MakeRefCounted(GetParam()); - AddSimpleTrace(tracer); - AddSimpleTrace(tracer); - ValidateChannelTrace(tracer, 2, GetParam()); - RefCountedPtr sc1 = MakeRefCounted(GetParam()); - tracer->AddTraceEventReferencingChannel( + ChannelTrace tracer(GetParam()); + AddSimpleTrace(&tracer); + AddSimpleTrace(&tracer); + ValidateChannelTrace(&tracer, 2, GetParam()); + ChannelFixture channel1(GetParam()); + RefCountedPtr sc1 = + MakeRefCounted(channel1.channel(), GetParam()); + tracer.AddTraceEventReferencingChannel( ChannelTrace::Severity::Info, grpc_slice_from_static_string("subchannel one created"), sc1); - ValidateChannelTrace(tracer, 3, GetParam()); - AddSimpleTrace(sc1); - RefCountedPtr conn1 = MakeRefCounted(GetParam()); + ValidateChannelTrace(&tracer, 3, GetParam()); + AddSimpleTrace(sc1->Trace()); + ChannelFixture channel2(GetParam()); + RefCountedPtr conn1 = + MakeRefCounted(channel2.channel(), GetParam()); // nesting one level deeper. - sc1->AddTraceEventReferencingSubchannel( + sc1->Trace()->AddTraceEventReferencingSubchannel( ChannelTrace::Severity::Info, grpc_slice_from_static_string("connection one created"), conn1); - ValidateChannelTrace(tracer, 3, GetParam()); - AddSimpleTrace(conn1); - AddSimpleTrace(tracer); - AddSimpleTrace(tracer); - ValidateChannelTrace(tracer, 5, GetParam()); - ValidateChannelTrace(conn1, 1, GetParam()); - RefCountedPtr sc2 = MakeRefCounted(GetParam()); - tracer->AddTraceEventReferencingSubchannel( + ValidateChannelTrace(&tracer, 3, GetParam()); + AddSimpleTrace(conn1->Trace()); + AddSimpleTrace(&tracer); + AddSimpleTrace(&tracer); + ValidateChannelTrace(&tracer, 5, GetParam()); + ValidateChannelTrace(conn1->Trace(), 1, GetParam()); + ChannelFixture channel3(GetParam()); + RefCountedPtr sc2 = + MakeRefCounted(channel3.channel(), GetParam()); + tracer.AddTraceEventReferencingSubchannel( 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.AddTraceEventReferencingChannel( ChannelTrace::Severity::Warning, grpc_slice_from_static_string("subchannel one inactive"), sc1); - AddSimpleTrace(tracer); - ValidateChannelTrace(tracer, 8, GetParam()); - tracer.reset(nullptr); + AddSimpleTrace(&tracer); + ValidateChannelTrace(&tracer, 8, GetParam()); sc1.reset(nullptr); sc2.reset(nullptr); conn1.reset(nullptr); @@ -227,6 +242,7 @@ INSTANTIATE_TEST_CASE_P(ChannelTracerTestSweep, ChannelTracerTest, ::testing::Values(0, 1, 2, 6, 10, 15)); } // namespace testing +} // namespace channelz } // namespace grpc_core int main(int argc, char** argv) { diff --git a/test/core/channel/channelz_test.cc b/test/core/channel/channelz_test.cc index 503bb9065b..c460642609 100644 --- a/test/core/channel/channelz_test.cc +++ b/test/core/channel/channelz_test.cc @@ -25,26 +25,170 @@ #include #include "src/core/lib/channel/channel_trace.h" +#include "src/core/lib/channel/channelz.h" #include "src/core/lib/channel/channelz_registry.h" #include "src/core/lib/gpr/useful.h" #include "src/core/lib/iomgr/exec_ctx.h" #include "src/core/lib/json/json.h" +#include "src/core/lib/surface/channel.h" #include "test/core/util/test_config.h" #include "test/cpp/util/channel_trace_proto_helper.h" -// remove me #include #include #include namespace grpc_core { +namespace channelz { namespace testing { -namespace {} // anonymous namespace +namespace { -TEST(ChannelzTest, Channel) {} +grpc_json* GetJsonChild(grpc_json* parent, const char* key) { + EXPECT_NE(parent, nullptr); + for (grpc_json* child = parent->child; child != nullptr; + child = child->next) { + if (child->key != nullptr && strcmp(child->key, key) == 0) return child; + } + return nullptr; +} + +class ChannelFixture { + public: + ChannelFixture(int max_trace_nodes) { + grpc_arg client_a; + client_a.type = GRPC_ARG_INTEGER; + client_a.key = + const_cast(GRPC_ARG_MAX_CHANNEL_TRACE_EVENTS_PER_NODE); + client_a.value.integer = max_trace_nodes; + grpc_channel_args client_args = {1, &client_a}; + channel_ = + grpc_insecure_channel_create("fake_target", &client_args, nullptr); + } + + ~ChannelFixture() { grpc_channel_destroy(channel_); } + + grpc_channel* channel() { return channel_; } + + private: + grpc_channel* channel_; +}; + +struct validate_channel_data_args { + int64_t calls_started; + int64_t calls_failed; + int64_t calls_succeeded; +}; + +void ValidateChildInteger(grpc_json* json, int64_t expect, const char* key) { + grpc_json* gotten_json = GetJsonChild(json, key); + EXPECT_NE(gotten_json, nullptr); + int64_t gotten_number = (int64_t)strtol(gotten_json->value, nullptr, 0); + EXPECT_EQ(gotten_number, expect); +} + +void ValidateChannel(Channel* channel, validate_channel_data_args args) { + char* json_str = channel->RenderJSON(); + grpc::testing::ValidateChannelProtoJsonTranslation(json_str); + grpc_json* json = grpc_json_parse_string(json_str); + EXPECT_NE(json, nullptr); + grpc_json* data = GetJsonChild(json, "data"); + ValidateChildInteger(data, args.calls_started, "callsStarted"); + ValidateChildInteger(data, args.calls_failed, "callsFailed"); + ValidateChildInteger(data, args.calls_succeeded, "callsSucceeded"); + grpc_json_destroy(json); + gpr_free(json_str); +} + +char* GetLastCallStartedTimestamp(Channel* channel) { + char* json_str = channel->RenderJSON(); + grpc_json* json = grpc_json_parse_string(json_str); + grpc_json* data = GetJsonChild(json, "data"); + grpc_json* timestamp = GetJsonChild(data, "lastCallStartedTimestamp"); + char* ts_str = grpc_json_dump_to_string(timestamp, 0); + grpc_json_destroy(json); + gpr_free(json_str); + return ts_str; +} + +void ChannelzSleep(int64_t sleep_us) { + gpr_sleep_until(gpr_time_add(gpr_now(GPR_CLOCK_REALTIME), + gpr_time_from_micros(sleep_us, GPR_TIMESPAN))); + grpc_core::ExecCtx::Get()->InvalidateNow(); +} + +} // anonymous namespace + +class ChannelzChannelTest : public ::testing::TestWithParam {}; + +TEST_P(ChannelzChannelTest, BasicChannel) { + grpc_core::ExecCtx exec_ctx; + ChannelFixture channel(GetParam()); + intptr_t uuid = grpc_channel_get_uuid(channel.channel()); + Channel* channelz_channel = ChannelzRegistry::Get(uuid); + ValidateChannel(channelz_channel, {-1, -1, -1}); +} + +TEST_P(ChannelzChannelTest, BasicChannelAPIFunctionality) { + grpc_core::ExecCtx exec_ctx; + ChannelFixture channel(GetParam()); + intptr_t uuid = grpc_channel_get_uuid(channel.channel()); + Channel* channelz_channel = ChannelzRegistry::Get(uuid); + channelz_channel->CallStarted(); + channelz_channel->CallFailed(); + channelz_channel->CallSucceeded(); + ValidateChannel(channelz_channel, {1, 1, 1}); + channelz_channel->CallStarted(); + channelz_channel->CallFailed(); + channelz_channel->CallSucceeded(); + channelz_channel->CallStarted(); + channelz_channel->CallFailed(); + channelz_channel->CallSucceeded(); + ValidateChannel(channelz_channel, {3, 3, 3}); +} + +TEST_P(ChannelzChannelTest, LastCallStartedTimestamp) { + grpc_core::ExecCtx exec_ctx; + ChannelFixture channel(GetParam()); + intptr_t uuid = grpc_channel_get_uuid(channel.channel()); + Channel* channelz_channel = ChannelzRegistry::Get(uuid); + + // start a call to set the last call started timestamp + channelz_channel->CallStarted(); + char* ts1 = GetLastCallStartedTimestamp(channelz_channel); + + // time gone by should not affect the timestamp + ChannelzSleep(100); + char* ts2 = GetLastCallStartedTimestamp(channelz_channel); + EXPECT_STREQ(ts1, ts2); + + // calls succeeded or failed should not affect the timestamp + ChannelzSleep(100); + channelz_channel->CallFailed(); + channelz_channel->CallSucceeded(); + char* ts3 = GetLastCallStartedTimestamp(channelz_channel); + EXPECT_STREQ(ts1, ts3); + + // another call started should affect the timestamp + // sleep for extra long to avoid flakes (since we cache Now()) + ChannelzSleep(5000); + grpc_core::ExecCtx::Get()->InvalidateNow(); + channelz_channel->CallStarted(); + char* ts4 = GetLastCallStartedTimestamp(channelz_channel); + EXPECT_STRNE(ts1, ts4); + + // clean up + gpr_free(ts1); + gpr_free(ts2); + gpr_free(ts3); + gpr_free(ts4); +} + +INSTANTIATE_TEST_CASE_P(ChannelzChannelTestSweep, ChannelzChannelTest, + ::testing::Values(0, 1, 2, 6, 10, 15)); } // namespace testing +} // namespace channelz } // namespace grpc_core int main(int argc, char** argv) { diff --git a/test/cpp/util/channel_trace_proto_helper.cc b/test/cpp/util/channel_trace_proto_helper.cc index db9390163b..ee310784c2 100644 --- a/test/cpp/util/channel_trace_proto_helper.cc +++ b/test/cpp/util/channel_trace_proto_helper.cc @@ -45,16 +45,21 @@ void VaidateProtoJsonTranslation(char* json_c_str) { // then compare the output, and determine what fields are missing. // // parse_options.ignore_unknown_fields = true; - ASSERT_EQ(google::protobuf::util::JsonStringToMessage(json_str, &msg, + EXPECT_EQ(google::protobuf::util::JsonStringToMessage(json_str, &msg, parse_options), google::protobuf::util::Status::OK); std::string proto_json_str; - ASSERT_EQ(google::protobuf::util::MessageToJsonString(msg, &proto_json_str), + google::protobuf::util::JsonPrintOptions print_options; + // We usually do not want this to be true, however it can be helpful to + // uncomment and see the output produced then all fields are printed. + // print_options.always_print_primitive_fields = true; + EXPECT_EQ(google::protobuf::util::MessageToJsonString(msg, &proto_json_str, + print_options), google::protobuf::util::Status::OK); // uncomment these to compare the the json strings. // gpr_log(GPR_ERROR, "tracer json: %s", json_str.c_str()); // gpr_log(GPR_ERROR, "proto json: %s", proto_json_str.c_str()); - ASSERT_EQ(json_str, proto_json_str); + EXPECT_EQ(json_str, proto_json_str); } } // namespace -- cgit v1.2.3 From c845ba66f30c44120f80098774652ba644ed7652 Mon Sep 17 00:00:00 2001 From: ncteisen Date: Thu, 7 Jun 2018 10:14:55 -0700 Subject: Reviewer feedback --- src/core/lib/channel/channel_trace.cc | 12 +----- src/core/lib/channel/channel_trace.h | 13 +++---- src/core/lib/channel/channelz.cc | 26 ++----------- src/core/lib/channel/channelz.h | 27 ++++++------- src/core/lib/surface/call.cc | 6 +-- src/core/lib/surface/channel.cc | 7 +++- test/core/channel/channel_trace_test.cc | 35 +++++++++-------- test/core/channel/channelz_test.cc | 68 ++++++++++++++++----------------- 8 files changed, 83 insertions(+), 111 deletions(-) (limited to 'src/core/lib/channel/channel_trace.cc') diff --git a/src/core/lib/channel/channel_trace.cc b/src/core/lib/channel/channel_trace.cc index c82e42d545..4c29df42f4 100644 --- a/src/core/lib/channel/channel_trace.cc +++ b/src/core/lib/channel/channel_trace.cc @@ -121,11 +121,11 @@ void ChannelTrace::AddTraceEventReferencingChannel( void ChannelTrace::AddTraceEventReferencingSubchannel( Severity severity, grpc_slice data, - RefCountedPtr referenced_channel) { + RefCountedPtr referenced_subchannel) { if (max_list_size_ == 0) return; // tracing is disabled if max_events == 0 // create and fill up the new event AddTraceEventHelper(New(severity, data, - std::move(referenced_channel), + std::move(referenced_subchannel), ReferencedType::Subchannel)); } @@ -232,13 +232,5 @@ grpc_json* ChannelTrace::RenderJSON() const { return json; } -char* ChannelTrace::RenderTrace() const { - grpc_json* json = RenderJSON(); - if (json == nullptr) return nullptr; - char* json_str = grpc_json_dump_to_string(json, 0); - grpc_json_destroy(json); - return json_str; -} - } // namespace channelz } // namespace grpc_core diff --git a/src/core/lib/channel/channel_trace.h b/src/core/lib/channel/channel_trace.h index b1224bb96c..17e95a32ee 100644 --- a/src/core/lib/channel/channel_trace.h +++ b/src/core/lib/channel/channel_trace.h @@ -22,11 +22,13 @@ #include #include +// #include "src/core/lib/channel/channelz.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" #include "src/core/lib/json/json.h" + namespace grpc_core { namespace channelz { @@ -64,20 +66,15 @@ class ChannelTrace { // slice. void AddTraceEventReferencingChannel( Severity severity, grpc_slice data, - RefCountedPtr referenced_tracer); + RefCountedPtr referenced_channel); void AddTraceEventReferencingSubchannel( Severity severity, grpc_slice data, - RefCountedPtr referenced_tracer); + RefCountedPtr referenced_subchannel); // Creates and returns the raw grpc_json object, so a parent channelz // object may incorporate the json before rendering. grpc_json* RenderJSON() const; - // Returns the tracing data rendered as a grpc json string. The string - // is owned by the caller and must be freed. This is used for testing only - // so that we may unit test ChannelTrace in isolation. - char* RenderTrace() const; - private: // Types of objects that can be references by trace events. enum class ReferencedType { Channel, Subchannel }; @@ -87,7 +84,7 @@ class ChannelTrace { public: // Constructor for a TraceEvent that references a different channel. TraceEvent(Severity severity, grpc_slice data, - RefCountedPtr referenced_tracer, ReferencedType type); + RefCountedPtr referenced_channel, ReferencedType type); // Constructor for a TraceEvent that does not reverence a different // channel. diff --git a/src/core/lib/channel/channelz.cc b/src/core/lib/channel/channelz.cc index 9e153a3f4f..41b3c4527e 100644 --- a/src/core/lib/channel/channelz.cc +++ b/src/core/lib/channel/channelz.cc @@ -101,28 +101,9 @@ Channel::~Channel() { ChannelzRegistry::Unregister(channel_uuid_); } -Channel::AtomicTimespec::AtomicTimespec() { - gpr_atm_no_barrier_store(&tv_sec_, (gpr_atm)0); - gpr_atm_no_barrier_store(&tv_nsec_, (gpr_atm)0); -} - -void Channel::AtomicTimespec::Update(gpr_timespec ts) { - gpr_atm_no_barrier_store(&tv_sec_, (gpr_atm)ts.tv_sec); - gpr_atm_no_barrier_store(&tv_nsec_, (gpr_atm)ts.tv_nsec); -} - -gpr_timespec Channel::AtomicTimespec::Get() { - gpr_timespec out; - out.clock_type = GPR_CLOCK_REALTIME; - out.tv_sec = static_cast(gpr_atm_no_barrier_load(&tv_sec_)); - out.tv_nsec = static_cast(gpr_atm_no_barrier_load(&tv_nsec_)); - return out; -} - -void Channel::CallStarted() { +void Channel::RecordCallStarted() { gpr_atm_no_barrier_fetch_add(&calls_started_, (gpr_atm)1); - last_call_started_timestamp_.Update( - grpc_millis_to_timespec(ExecCtx::Get()->Now(), GPR_CLOCK_REALTIME)); + gpr_atm_no_barrier_store(&last_call_started_millis_, (gpr_atm)ExecCtx::Get()->Now()); } grpc_connectivity_state Channel::GetConnectivityState() { @@ -195,9 +176,10 @@ char* Channel::RenderJSON() { calls_succeeded_ ? calls_succeeded_ : -1); json_iterator = add_num_str(json, json_iterator, "callsFailed", calls_failed_ ? calls_failed_ : -1); + 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(last_call_started_timestamp_.Get()), GRPC_JSON_STRING, true); + fmt_time(ts), GRPC_JSON_STRING, true); // render and return the over json object char* json_str = grpc_json_dump_to_string(top_level_json, 0); diff --git a/src/core/lib/channel/channelz.h b/src/core/lib/channel/channelz.h index 684402e08f..2d4c1a058a 100644 --- a/src/core/lib/channel/channelz.h +++ b/src/core/lib/channel/channelz.h @@ -28,27 +28,32 @@ #include "src/core/lib/gprpp/ref_counted.h" #include "src/core/lib/gprpp/ref_counted_ptr.h" #include "src/core/lib/iomgr/error.h" +#include "src/core/lib/iomgr/exec_ctx.h" #include "src/core/lib/json/json.h" namespace grpc_core { namespace channelz { +namespace testing { + class ChannelPeer; +} + class Channel : public RefCounted { public: Channel(grpc_channel* channel, size_t channel_tracer_max_nodes); ~Channel(); - void CallStarted(); - void CallFailed() { + void RecordCallStarted(); + void RecordCallFailed() { gpr_atm_no_barrier_fetch_add(&calls_failed_, (gpr_atm(1))); } - void CallSucceeded() { + void RecordCallSucceeded() { gpr_atm_no_barrier_fetch_add(&calls_succeeded_, (gpr_atm(1))); } char* RenderJSON(); - ChannelTrace* Trace() { return trace_.get(); } + ChannelTrace* trace() { return trace_.get(); } void set_channel_destroyed() { GPR_ASSERT(!channel_destroyed_); @@ -58,16 +63,8 @@ class Channel : public RefCounted { intptr_t channel_uuid() { return channel_uuid_; } private: - class AtomicTimespec { - public: - AtomicTimespec(); - void Update(gpr_timespec ts); - gpr_timespec Get(); - - private: - gpr_atm tv_sec_; - gpr_atm tv_nsec_; - }; + // testing peer friend. + friend class testing::ChannelPeer; bool channel_destroyed_ = false; grpc_channel* channel_; @@ -75,7 +72,7 @@ class Channel : public RefCounted { gpr_atm calls_started_ = 0; gpr_atm calls_succeeded_ = 0; gpr_atm calls_failed_ = 0; - AtomicTimespec last_call_started_timestamp_; + gpr_atm last_call_started_millis_; intptr_t channel_uuid_; ManualConstructor trace_; diff --git a/src/core/lib/surface/call.cc b/src/core/lib/surface/call.cc index 924d633cb2..e2212baa8c 100644 --- a/src/core/lib/surface/call.cc +++ b/src/core/lib/surface/call.cc @@ -1084,11 +1084,11 @@ static void recv_trailing_filter(void* args, grpc_metadata_batch* b) { : nullptr; if (status_code == GRPC_STATUS_OK) { if (channelz_channel != nullptr) { - channelz_channel->CallSucceeded(); + channelz_channel->RecordCallSucceeded(); } } else { if (channelz_channel != nullptr) { - channelz_channel->CallFailed(); + channelz_channel->RecordCallFailed(); } error = grpc_error_set_int( GRPC_ERROR_CREATE_FROM_STATIC_STRING("Error received from peer"), @@ -1677,7 +1677,7 @@ static grpc_call_error call_start_batch(grpc_call* call, const grpc_op* ops, } grpc_core::channelz::Channel* channelz_channel = grpc_channel_get_channelz_channel(call->channel); - channelz_channel->CallStarted(); + channelz_channel->RecordCallStarted(); break; } case GRPC_OP_SEND_MESSAGE: { diff --git a/src/core/lib/surface/channel.cc b/src/core/lib/surface/channel.cc index be9869b08c..5189c54b86 100644 --- a/src/core/lib/surface/channel.cc +++ b/src/core/lib/surface/channel.cc @@ -149,7 +149,7 @@ grpc_channel* grpc_channel_create_with_builder( channel->channelz_channel = grpc_core::MakeRefCounted( channel, channel_tracer_max_nodes); - channel->channelz_channel->Trace()->AddTraceEvent( + channel->channelz_channel->trace()->AddTraceEvent( grpc_core::channelz::ChannelTrace::Severity::Info, grpc_slice_from_static_string("Channel created")); return channel; @@ -187,7 +187,10 @@ static grpc_channel_args* build_channel_args( } char* grpc_channel_get_trace(grpc_channel* channel) { - return channel->channelz_channel->Trace()->RenderTrace(); + grpc_json* json = channel->channelz_channel->trace()->RenderJSON(); + char* json_str = grpc_json_dump_to_string(json, 0); + grpc_json_destroy(json); + return json_str; } char* grpc_channel_render_channelz(grpc_channel* channel) { diff --git a/test/core/channel/channel_trace_test.cc b/test/core/channel/channel_trace_test.cc index c8619be007..ef69e63141 100644 --- a/test/core/channel/channel_trace_test.cc +++ b/test/core/channel/channel_trace_test.cc @@ -88,12 +88,15 @@ void AddSimpleTrace(ChannelTrace* tracer) { void ValidateChannelTrace(ChannelTrace* tracer, size_t expected_num_event_logged, size_t max_nodes) { if (!max_nodes) return; - char* json_str = tracer->RenderTrace(); + grpc_json* json = tracer->RenderJSON(); + EXPECT_NE(json, nullptr); + char* json_str = grpc_json_dump_to_string(json, 0); + grpc_json_destroy(json); grpc::testing::ValidateChannelTraceProtoJsonTranslation(json_str); - grpc_json* json = grpc_json_parse_string(json_str); - ValidateChannelTraceData(json, expected_num_event_logged, + grpc_json* parsed_json = grpc_json_parse_string(json_str); + ValidateChannelTraceData(parsed_json, expected_num_event_logged, GPR_MIN(expected_num_event_logged, max_nodes)); - grpc_json_destroy(json); + grpc_json_destroy(parsed_json); gpr_free(json_str); } @@ -159,14 +162,14 @@ TEST_P(ChannelTracerTest, ComplexTest) { ChannelTrace::Severity::Info, grpc_slice_from_static_string("subchannel one created"), sc1); ValidateChannelTrace(&tracer, 3, GetParam()); - AddSimpleTrace(sc1->Trace()); - AddSimpleTrace(sc1->Trace()); - AddSimpleTrace(sc1->Trace()); - ValidateChannelTrace(sc1->Trace(), 3, GetParam()); - AddSimpleTrace(sc1->Trace()); - AddSimpleTrace(sc1->Trace()); - AddSimpleTrace(sc1->Trace()); - ValidateChannelTrace(sc1->Trace(), 6, GetParam()); + AddSimpleTrace(sc1->trace()); + AddSimpleTrace(sc1->trace()); + AddSimpleTrace(sc1->trace()); + ValidateChannelTrace(sc1->trace(), 3, GetParam()); + AddSimpleTrace(sc1->trace()); + AddSimpleTrace(sc1->trace()); + AddSimpleTrace(sc1->trace()); + ValidateChannelTrace(sc1->trace(), 6, GetParam()); AddSimpleTrace(&tracer); AddSimpleTrace(&tracer); ValidateChannelTrace(&tracer, 5, GetParam()); @@ -206,20 +209,20 @@ TEST_P(ChannelTracerTest, TestNesting) { ChannelTrace::Severity::Info, grpc_slice_from_static_string("subchannel one created"), sc1); ValidateChannelTrace(&tracer, 3, GetParam()); - AddSimpleTrace(sc1->Trace()); + AddSimpleTrace(sc1->trace()); ChannelFixture channel2(GetParam()); RefCountedPtr conn1 = MakeRefCounted(channel2.channel(), GetParam()); // nesting one level deeper. - sc1->Trace()->AddTraceEventReferencingSubchannel( + sc1->trace()->AddTraceEventReferencingSubchannel( ChannelTrace::Severity::Info, grpc_slice_from_static_string("connection one created"), conn1); ValidateChannelTrace(&tracer, 3, GetParam()); - AddSimpleTrace(conn1->Trace()); + AddSimpleTrace(conn1->trace()); AddSimpleTrace(&tracer); AddSimpleTrace(&tracer); ValidateChannelTrace(&tracer, 5, GetParam()); - ValidateChannelTrace(conn1->Trace(), 1, GetParam()); + ValidateChannelTrace(conn1->trace(), 1, GetParam()); ChannelFixture channel3(GetParam()); RefCountedPtr sc2 = MakeRefCounted(channel3.channel(), GetParam()); diff --git a/test/core/channel/channelz_test.cc b/test/core/channel/channelz_test.cc index c460642609..1c676e51cf 100644 --- a/test/core/channel/channelz_test.cc +++ b/test/core/channel/channelz_test.cc @@ -42,6 +42,16 @@ namespace grpc_core { namespace channelz { namespace testing { + +// testing peer to access channel internals +class ChannelPeer { + public: + ChannelPeer (Channel* channel) : channel_(channel) {} + grpc_millis last_call_started_millis() { return (grpc_millis)gpr_atm_no_barrier_load(&channel_->last_call_started_millis_); } + private: + Channel* channel_; +}; + namespace { grpc_json* GetJsonChild(grpc_json* parent, const char* key) { @@ -100,15 +110,9 @@ void ValidateChannel(Channel* channel, validate_channel_data_args args) { gpr_free(json_str); } -char* GetLastCallStartedTimestamp(Channel* channel) { - char* json_str = channel->RenderJSON(); - grpc_json* json = grpc_json_parse_string(json_str); - grpc_json* data = GetJsonChild(json, "data"); - grpc_json* timestamp = GetJsonChild(data, "lastCallStartedTimestamp"); - char* ts_str = grpc_json_dump_to_string(timestamp, 0); - grpc_json_destroy(json); - gpr_free(json_str); - return ts_str; +grpc_millis GetLastCallStartedMillis(Channel* channel) { + ChannelPeer peer(channel); + return peer.last_call_started_millis(); } void ChannelzSleep(int64_t sleep_us) { @@ -134,16 +138,16 @@ TEST_P(ChannelzChannelTest, BasicChannelAPIFunctionality) { ChannelFixture channel(GetParam()); intptr_t uuid = grpc_channel_get_uuid(channel.channel()); Channel* channelz_channel = ChannelzRegistry::Get(uuid); - channelz_channel->CallStarted(); - channelz_channel->CallFailed(); - channelz_channel->CallSucceeded(); + channelz_channel->RecordCallStarted(); + channelz_channel->RecordCallFailed(); + channelz_channel->RecordCallSucceeded(); ValidateChannel(channelz_channel, {1, 1, 1}); - channelz_channel->CallStarted(); - channelz_channel->CallFailed(); - channelz_channel->CallSucceeded(); - channelz_channel->CallStarted(); - channelz_channel->CallFailed(); - channelz_channel->CallSucceeded(); + channelz_channel->RecordCallStarted(); + channelz_channel->RecordCallFailed(); + channelz_channel->RecordCallSucceeded(); + channelz_channel->RecordCallStarted(); + channelz_channel->RecordCallFailed(); + channelz_channel->RecordCallSucceeded(); ValidateChannel(channelz_channel, {3, 3, 3}); } @@ -154,34 +158,28 @@ TEST_P(ChannelzChannelTest, LastCallStartedTimestamp) { Channel* channelz_channel = ChannelzRegistry::Get(uuid); // start a call to set the last call started timestamp - channelz_channel->CallStarted(); - char* ts1 = GetLastCallStartedTimestamp(channelz_channel); + channelz_channel->RecordCallStarted(); + grpc_millis millis1 = GetLastCallStartedMillis(channelz_channel); // time gone by should not affect the timestamp ChannelzSleep(100); - char* ts2 = GetLastCallStartedTimestamp(channelz_channel); - EXPECT_STREQ(ts1, ts2); + grpc_millis millis2 = GetLastCallStartedMillis(channelz_channel); + EXPECT_EQ(millis1, millis2); // calls succeeded or failed should not affect the timestamp ChannelzSleep(100); - channelz_channel->CallFailed(); - channelz_channel->CallSucceeded(); - char* ts3 = GetLastCallStartedTimestamp(channelz_channel); - EXPECT_STREQ(ts1, ts3); + channelz_channel->RecordCallFailed(); + channelz_channel->RecordCallSucceeded(); + grpc_millis millis3 = GetLastCallStartedMillis(channelz_channel); + EXPECT_EQ(millis1, millis3); // another call started should affect the timestamp // sleep for extra long to avoid flakes (since we cache Now()) ChannelzSleep(5000); grpc_core::ExecCtx::Get()->InvalidateNow(); - channelz_channel->CallStarted(); - char* ts4 = GetLastCallStartedTimestamp(channelz_channel); - EXPECT_STRNE(ts1, ts4); - - // clean up - gpr_free(ts1); - gpr_free(ts2); - gpr_free(ts3); - gpr_free(ts4); + channelz_channel->RecordCallStarted(); + grpc_millis millis4 = GetLastCallStartedMillis(channelz_channel); + EXPECT_NE(millis1, millis4); } INSTANTIATE_TEST_CASE_P(ChannelzChannelTestSweep, ChannelzChannelTest, -- cgit v1.2.3 From 6c987cbf0911b6d97eeec2e14bff2e8fc5bc40e5 Mon Sep 17 00:00:00 2001 From: ncteisen Date: Fri, 8 Jun 2018 09:30:12 -0700 Subject: Rename channelz Channel to ChannelNode --- src/core/lib/channel/channel_trace.cc | 10 +++++----- src/core/lib/channel/channel_trace.h | 11 ++++++----- src/core/lib/channel/channelz.cc | 10 +++++----- src/core/lib/channel/channelz.h | 10 +++++----- src/core/lib/surface/call.cc | 4 ++-- src/core/lib/surface/channel.cc | 6 +++--- src/core/lib/surface/channel.h | 2 +- test/core/channel/channelz_test.cc | 18 +++++++++--------- 8 files changed, 36 insertions(+), 35 deletions(-) (limited to 'src/core/lib/channel/channel_trace.cc') diff --git a/src/core/lib/channel/channel_trace.cc b/src/core/lib/channel/channel_trace.cc index 4c29df42f4..0f655d8716 100644 --- a/src/core/lib/channel/channel_trace.cc +++ b/src/core/lib/channel/channel_trace.cc @@ -41,9 +41,9 @@ namespace grpc_core { namespace channelz { -ChannelTrace::TraceEvent::TraceEvent(Severity severity, grpc_slice data, - RefCountedPtr referenced_channel, - ReferencedType type) +ChannelTrace::TraceEvent::TraceEvent( + Severity severity, grpc_slice data, + RefCountedPtr referenced_channel, ReferencedType type) : severity_(severity), data_(data), timestamp_(grpc_millis_to_timespec(grpc_core::ExecCtx::Get()->Now(), @@ -112,7 +112,7 @@ void ChannelTrace::AddTraceEvent(Severity severity, grpc_slice data) { void ChannelTrace::AddTraceEventReferencingChannel( Severity severity, grpc_slice data, - RefCountedPtr referenced_channel) { + RefCountedPtr referenced_channel) { if (max_list_size_ == 0) return; // tracing is disabled if max_events == 0 // create and fill up the new event AddTraceEventHelper(New( @@ -121,7 +121,7 @@ void ChannelTrace::AddTraceEventReferencingChannel( void ChannelTrace::AddTraceEventReferencingSubchannel( Severity severity, grpc_slice data, - RefCountedPtr referenced_subchannel) { + RefCountedPtr referenced_subchannel) { if (max_list_size_ == 0) return; // tracing is disabled if max_events == 0 // create and fill up the new event AddTraceEventHelper(New(severity, data, diff --git a/src/core/lib/channel/channel_trace.h b/src/core/lib/channel/channel_trace.h index f6021c3224..0dd162a777 100644 --- a/src/core/lib/channel/channel_trace.h +++ b/src/core/lib/channel/channel_trace.h @@ -30,7 +30,7 @@ namespace grpc_core { namespace channelz { -class Channel; +class ChannelNode; // Object used to hold live data for a channel. This data is exposed via the // channelz service: @@ -64,10 +64,10 @@ class ChannelTrace { // slice. void AddTraceEventReferencingChannel( Severity severity, grpc_slice data, - RefCountedPtr referenced_channel); + RefCountedPtr referenced_channel); void AddTraceEventReferencingSubchannel( Severity severity, grpc_slice data, - RefCountedPtr referenced_subchannel); + RefCountedPtr referenced_subchannel); // Creates and returns the raw grpc_json object, so a parent channelz // object may incorporate the json before rendering. @@ -82,7 +82,8 @@ class ChannelTrace { public: // Constructor for a TraceEvent that references a different channel. TraceEvent(Severity severity, grpc_slice data, - RefCountedPtr referenced_channel, ReferencedType type); + RefCountedPtr referenced_channel, + ReferencedType type); // Constructor for a TraceEvent that does not reverence a different // channel. @@ -104,7 +105,7 @@ class ChannelTrace { gpr_timespec timestamp_; TraceEvent* next_; // the tracer object for the (sub)channel that this trace event refers to. - RefCountedPtr referenced_channel_; + RefCountedPtr 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_; diff --git a/src/core/lib/channel/channelz.cc b/src/core/lib/channel/channelz.cc index 42b169dffd..799eb8bed1 100644 --- a/src/core/lib/channel/channelz.cc +++ b/src/core/lib/channel/channelz.cc @@ -89,7 +89,7 @@ grpc_json* add_num_str(grpc_json* parent, grpc_json* it, const char* name, } // namespace -Channel::Channel(grpc_channel* channel, size_t channel_tracer_max_nodes) +ChannelNode::ChannelNode(grpc_channel* channel, size_t channel_tracer_max_nodes) : channel_(channel), target_(UniquePtr(grpc_channel_get_target(channel_))), channel_uuid_(ChannelzRegistry::Register(this)) { @@ -98,18 +98,18 @@ Channel::Channel(grpc_channel* channel, size_t channel_tracer_max_nodes) (gpr_atm)ExecCtx::Get()->Now()); } -Channel::~Channel() { +ChannelNode::~ChannelNode() { trace_.Destroy(); ChannelzRegistry::Unregister(channel_uuid_); } -void Channel::RecordCallStarted() { +void ChannelNode::RecordCallStarted() { gpr_atm_no_barrier_fetch_add(&calls_started_, (gpr_atm)1); gpr_atm_no_barrier_store(&last_call_started_millis_, (gpr_atm)ExecCtx::Get()->Now()); } -grpc_connectivity_state Channel::GetConnectivityState() { +grpc_connectivity_state ChannelNode::GetConnectivityState() { if (channel_ == nullptr) { return GRPC_CHANNEL_SHUTDOWN; } else { @@ -117,7 +117,7 @@ grpc_connectivity_state Channel::GetConnectivityState() { } } -char* Channel::RenderJSON() { +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); grpc_json* json = top_level_json; diff --git a/src/core/lib/channel/channelz.h b/src/core/lib/channel/channelz.h index 68b7c8d26b..62dc817b6a 100644 --- a/src/core/lib/channel/channelz.h +++ b/src/core/lib/channel/channelz.h @@ -35,13 +35,13 @@ namespace grpc_core { namespace channelz { namespace testing { -class ChannelPeer; +class ChannelNodePeer; } -class Channel : public RefCounted { +class ChannelNode : public RefCounted { public: - Channel(grpc_channel* channel, size_t channel_tracer_max_nodes); - ~Channel(); + ChannelNode(grpc_channel* channel, size_t channel_tracer_max_nodes); + ~ChannelNode(); void RecordCallStarted(); void RecordCallFailed() { @@ -64,7 +64,7 @@ class Channel : public RefCounted { private: // testing peer friend. - friend class testing::ChannelPeer; + friend class testing::ChannelNodePeer; // helper for getting connectivity state. grpc_connectivity_state GetConnectivityState(); diff --git a/src/core/lib/surface/call.cc b/src/core/lib/surface/call.cc index 65071672f2..33b9908a57 100644 --- a/src/core/lib/surface/call.cc +++ b/src/core/lib/surface/call.cc @@ -478,7 +478,7 @@ grpc_error* grpc_call_create(const grpc_call_create_args* args, &call->pollent); } - grpc_core::channelz::Channel* channelz_channel = + grpc_core::channelz::ChannelNode* channelz_channel = grpc_channel_get_channelz_node(call->channel); channelz_channel->RecordCallStarted(); @@ -1260,7 +1260,7 @@ static void post_batch_completion(batch_control* bctl) { call->final_op.server.cancelled, nullptr, nullptr); } - grpc_core::channelz::Channel* channelz_channel = + grpc_core::channelz::ChannelNode* channelz_channel = grpc_channel_get_channelz_node(call->channel); if (*call->final_op.client.status != GRPC_STATUS_OK) { channelz_channel->RecordCallFailed(); diff --git a/src/core/lib/surface/channel.cc b/src/core/lib/surface/channel.cc index d2b52dad2f..7dcfbc97cc 100644 --- a/src/core/lib/surface/channel.cc +++ b/src/core/lib/surface/channel.cc @@ -67,7 +67,7 @@ struct grpc_channel { gpr_mu registered_call_mu; registered_call* registered_calls; - grpc_core::RefCountedPtr channelz_channel; + grpc_core::RefCountedPtr channelz_channel; char* target; }; @@ -147,7 +147,7 @@ grpc_channel* grpc_channel_create_with_builder( grpc_channel_args_destroy(args); channel->channelz_channel = - grpc_core::MakeRefCounted( + grpc_core::MakeRefCounted( channel, channel_tracer_max_nodes); channel->channelz_channel->trace()->AddTraceEvent( grpc_core::channelz::ChannelTrace::Severity::Info, @@ -190,7 +190,7 @@ char* grpc_channel_render_channelz(grpc_channel* channel) { return channel->channelz_channel->RenderJSON(); } -grpc_core::channelz::Channel* grpc_channel_get_channelz_node( +grpc_core::channelz::ChannelNode* grpc_channel_get_channelz_node( grpc_channel* channel) { return channel->channelz_channel.get(); } diff --git a/src/core/lib/surface/channel.h b/src/core/lib/surface/channel.h index 6d2e1931e6..e5ff2c3596 100644 --- a/src/core/lib/surface/channel.h +++ b/src/core/lib/surface/channel.h @@ -51,7 +51,7 @@ grpc_call* grpc_channel_create_pollset_set_call( /** Get a (borrowed) pointer to this channels underlying channel stack */ grpc_channel_stack* grpc_channel_get_channel_stack(grpc_channel* channel); -grpc_core::channelz::Channel* grpc_channel_get_channelz_node( +grpc_core::channelz::ChannelNode* grpc_channel_get_channelz_node( grpc_channel* channel); /** Get a grpc_mdelem of grpc-status: X where X is the numeric value of diff --git a/test/core/channel/channelz_test.cc b/test/core/channel/channelz_test.cc index 262ca38ee0..8ef91c3b09 100644 --- a/test/core/channel/channelz_test.cc +++ b/test/core/channel/channelz_test.cc @@ -44,16 +44,16 @@ namespace channelz { namespace testing { // testing peer to access channel internals -class ChannelPeer { +class ChannelNodePeer { public: - ChannelPeer(Channel* channel) : channel_(channel) {} + ChannelNodePeer(ChannelNode* channel) : channel_(channel) {} grpc_millis last_call_started_millis() { return (grpc_millis)gpr_atm_no_barrier_load( &channel_->last_call_started_millis_); } private: - Channel* channel_; + ChannelNode* channel_; }; namespace { @@ -111,15 +111,15 @@ void ValidateCounters(char* json_str, validate_channel_data_args args) { grpc_json_destroy(json); } -void ValidateChannel(Channel* channel, validate_channel_data_args args) { +void ValidateChannel(ChannelNode* channel, validate_channel_data_args args) { char* json_str = channel->RenderJSON(); grpc::testing::ValidateChannelProtoJsonTranslation(json_str); ValidateCounters(json_str, args); gpr_free(json_str); } -grpc_millis GetLastCallStartedMillis(Channel* channel) { - ChannelPeer peer(channel); +grpc_millis GetLastCallStartedMillis(ChannelNode* channel) { + ChannelNodePeer peer(channel); return peer.last_call_started_millis(); } @@ -137,7 +137,7 @@ TEST_P(ChannelzChannelTest, BasicChannel) { grpc_core::ExecCtx exec_ctx; ChannelFixture channel(GetParam()); intptr_t uuid = grpc_channel_get_uuid(channel.channel()); - Channel* channelz_channel = ChannelzRegistry::Get(uuid); + ChannelNode* channelz_channel = ChannelzRegistry::Get(uuid); char* json_str = channelz_channel->RenderJSON(); ValidateCounters(json_str, {0, 0, 0}); gpr_free(json_str); @@ -147,7 +147,7 @@ TEST_P(ChannelzChannelTest, BasicChannelAPIFunctionality) { grpc_core::ExecCtx exec_ctx; ChannelFixture channel(GetParam()); intptr_t uuid = grpc_channel_get_uuid(channel.channel()); - Channel* channelz_channel = ChannelzRegistry::Get(uuid); + ChannelNode* channelz_channel = ChannelzRegistry::Get(uuid); channelz_channel->RecordCallStarted(); channelz_channel->RecordCallFailed(); channelz_channel->RecordCallSucceeded(); @@ -165,7 +165,7 @@ TEST_P(ChannelzChannelTest, LastCallStartedMillis) { grpc_core::ExecCtx exec_ctx; ChannelFixture channel(GetParam()); intptr_t uuid = grpc_channel_get_uuid(channel.channel()); - Channel* channelz_channel = ChannelzRegistry::Get(uuid); + ChannelNode* channelz_channel = ChannelzRegistry::Get(uuid); // start a call to set the last call started timestamp channelz_channel->RecordCallStarted(); grpc_millis millis1 = GetLastCallStartedMillis(channelz_channel); -- cgit v1.2.3