diff options
author | 2018-07-17 11:57:31 -0700 | |
---|---|---|
committer | 2018-07-17 21:48:47 -0700 | |
commit | 5d373c40bdd616d2d0e65b1c6936cc2d07ba4044 (patch) | |
tree | 9373a743971a00e23a78b7dfeea6963640adc462 /src/core | |
parent | 97066fd0e46a82ffa6b012117cfd428888d255f3 (diff) |
reviewer feedback
Diffstat (limited to 'src/core')
-rw-r--r-- | src/core/ext/filters/client_channel/client_channel_channelz.cc | 10 | ||||
-rw-r--r-- | src/core/ext/filters/client_channel/client_channel_channelz.h | 6 | ||||
-rw-r--r-- | src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc | 3 | ||||
-rw-r--r-- | src/core/lib/channel/channelz.cc | 15 | ||||
-rw-r--r-- | src/core/lib/channel/channelz.h | 11 | ||||
-rw-r--r-- | src/core/lib/channel/channelz_registry.cc | 5 | ||||
-rw-r--r-- | src/core/lib/channel/channelz_registry.h | 5 | ||||
-rw-r--r-- | src/core/lib/surface/channel.cc | 8 |
8 files changed, 34 insertions, 29 deletions
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..88a936f9b7 100644 --- a/src/core/ext/filters/client_channel/client_channel_channelz.cc +++ b/src/core/ext/filters/client_channel/client_channel_channelz.cc @@ -41,8 +41,9 @@ static const grpc_arg_pointer_vtable client_channel_channelz_vtable = { client_channel_channelz_cmp}; ClientChannelNode::ClientChannelNode(grpc_channel* channel, - size_t channel_tracer_max_nodes) - : ChannelNode(channel, channel_tracer_max_nodes) { + size_t channel_tracer_max_nodes, + bool is_top_level_channel) + : ChannelNode(channel, channel_tracer_max_nodes, is_top_level_channel) { client_channel_ = grpc_channel_stack_last_element(grpc_channel_get_channel_stack(channel)); GPR_ASSERT(client_channel_->filter == &grpc_client_channel_filter); @@ -71,9 +72,10 @@ grpc_arg ClientChannelNode::CreateChannelArg() { } RefCountedPtr<ChannelNode> ClientChannelNode::MakeClientChannelNode( - grpc_channel* channel, size_t channel_tracer_max_nodes) { + grpc_channel* channel, size_t channel_tracer_max_nodes, + bool is_top_level_channel) { return MakePolymorphicRefCounted<ChannelNode, ClientChannelNode>( - channel, channel_tracer_max_nodes); + channel, channel_tracer_max_nodes, is_top_level_channel); } } // namespace channelz 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..6fb475399f 100644 --- a/src/core/ext/filters/client_channel/client_channel_channelz.h +++ b/src/core/ext/filters/client_channel/client_channel_channelz.h @@ -32,7 +32,8 @@ namespace channelz { class ClientChannelNode : public ChannelNode { public: static RefCountedPtr<ChannelNode> MakeClientChannelNode( - grpc_channel* channel, size_t channel_tracer_max_nodes); + grpc_channel* channel, size_t channel_tracer_max_nodes, + bool is_top_level_channel); // Override this functionality since client_channels have a notion of // channel connectivity. @@ -45,7 +46,8 @@ class ClientChannelNode : public ChannelNode { protected: GPRC_ALLOW_CLASS_TO_USE_NON_PUBLIC_DELETE GPRC_ALLOW_CLASS_TO_USE_NON_PUBLIC_NEW - ClientChannelNode(grpc_channel* channel, size_t channel_tracer_max_nodes); + ClientChannelNode(grpc_channel* channel, size_t channel_tracer_max_nodes, + bool is_top_level_channel); virtual ~ClientChannelNode() {} private: diff --git a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc index 01d05dbb14..e7526cb7e2 100644 --- a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc +++ b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc @@ -1004,7 +1004,8 @@ grpc_channel_args* BuildBalancerChannelArgs( // A channel arg indicating the target is a grpclb load balancer. grpc_channel_arg_integer_create( const_cast<char*>(GRPC_ARG_ADDRESS_IS_GRPCLB_LOAD_BALANCER), 1), - // A channel arg indicating the target is a grpclb load balancer. + // A channel arg indicating this is an internal channels, aka it is + // owned by components in Core, not by the user application. grpc_channel_arg_integer_create( const_cast<char*>(GRPC_ARG_CHANNELZ_CHANNEL_IS_INTERNAL_CHANNEL), 1), }; diff --git a/src/core/lib/channel/channelz.cc b/src/core/lib/channel/channelz.cc index c8a1d179fa..c78ea99e63 100644 --- a/src/core/lib/channel/channelz.cc +++ b/src/core/lib/channel/channelz.cc @@ -88,8 +88,12 @@ grpc_json* add_num_str(grpc_json* parent, grpc_json* it, const char* name, } // namespace -ChannelNode::ChannelNode(grpc_channel* channel, size_t channel_tracer_max_nodes) - : channel_(channel), target_(nullptr), channel_uuid_(-1) { +ChannelNode::ChannelNode(grpc_channel* channel, size_t channel_tracer_max_nodes, + bool is_top_level_channel) + : channel_(channel), + target_(nullptr), + channel_uuid_(-1), + is_top_level_channel_(is_top_level_channel) { trace_.Init(channel_tracer_max_nodes); target_ = UniquePtr<char>(grpc_channel_get_target(channel_)); channel_uuid_ = ChannelzRegistry::RegisterChannelNode(this); @@ -136,7 +140,7 @@ grpc_json* ChannelNode::RenderJson() { // fill in the channel trace if applicable grpc_json* trace = trace_->RenderJson(); if (trace != nullptr) { - // we manuall link up and fill the child since it was created for us in + // we manually 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; @@ -175,9 +179,10 @@ char* ChannelNode::RenderJsonString() { } RefCountedPtr<ChannelNode> ChannelNode::MakeChannelNode( - grpc_channel* channel, size_t channel_tracer_max_nodes) { + grpc_channel* channel, size_t channel_tracer_max_nodes, + bool is_top_level_channel) { return MakeRefCounted<grpc_core::channelz::ChannelNode>( - channel, channel_tracer_max_nodes); + channel, channel_tracer_max_nodes, is_top_level_channel); } } // namespace channelz diff --git a/src/core/lib/channel/channelz.h b/src/core/lib/channel/channelz.h index abcf907c2b..4794a280d0 100644 --- a/src/core/lib/channel/channelz.h +++ b/src/core/lib/channel/channelz.h @@ -49,7 +49,8 @@ class ChannelNodePeer; class ChannelNode : public RefCounted<ChannelNode> { public: static RefCountedPtr<ChannelNode> MakeChannelNode( - grpc_channel* channel, size_t channel_tracer_max_nodes); + grpc_channel* channel, size_t channel_tracer_max_nodes, + bool is_top_level_channel); void RecordCallStarted(); void RecordCallFailed() { @@ -78,14 +79,12 @@ class ChannelNode : public RefCounted<ChannelNode> { intptr_t channel_uuid() { return channel_uuid_; } bool is_top_level_channel() { return is_top_level_channel_; } - void set_is_top_level_channel(bool is_top_level_channel) { - is_top_level_channel_ = is_top_level_channel; - } protected: GPRC_ALLOW_CLASS_TO_USE_NON_PUBLIC_DELETE GPRC_ALLOW_CLASS_TO_USE_NON_PUBLIC_NEW - ChannelNode(grpc_channel* channel, size_t channel_tracer_max_nodes); + ChannelNode(grpc_channel* channel, size_t channel_tracer_max_nodes, + bool is_top_level_channel); virtual ~ChannelNode(); private: @@ -106,7 +105,7 @@ class ChannelNode : public RefCounted<ChannelNode> { // Creation functions typedef RefCountedPtr<ChannelNode> (*ChannelNodeCreationFunc)(grpc_channel*, - size_t); + size_t, bool); } // namespace channelz } // namespace grpc_core diff --git a/src/core/lib/channel/channelz_registry.cc b/src/core/lib/channel/channelz_registry.cc index b4dc3d1103..7ca3fc5836 100644 --- a/src/core/lib/channel/channelz_registry.cc +++ b/src/core/lib/channel/channelz_registry.cc @@ -86,7 +86,7 @@ char* ChannelzRegistry::InternalGetTopChannels(intptr_t start_channel_id) { grpc_json* json_iterator = nullptr; InlinedVector<ChannelNode*, 10> top_level_channels; // uuids index into entities one-off (idx 0 is really uuid 1, since 0 is - // reserver). However, we want to support requests coming in which + // reserved). However, we want to support requests coming in this // start_channel_id=0, which signifies "give me everything." Hence this // funky looking line below. size_t start_idx = start_channel_id == 0 ? 0 : start_channel_id - 1; @@ -108,9 +108,6 @@ char* ChannelzRegistry::InternalGetTopChannels(intptr_t start_channel_id) { json_iterator = grpc_json_link_child(array_parent, channel_json, json_iterator); channel_json->parent = array_parent; - channel_json->value = nullptr; - channel_json->key = nullptr; - channel_json->owns_value = false; } } // For now we do not have any pagination rules. In the future we could diff --git a/src/core/lib/channel/channelz_registry.h b/src/core/lib/channel/channelz_registry.h index 2fefc9f629..c378467f34 100644 --- a/src/core/lib/channel/channelz_registry.h +++ b/src/core/lib/channel/channelz_registry.h @@ -36,7 +36,7 @@ class ChannelzRegistry { // To be called in grpc_init() static void Init(); - // To be callen in grpc_shutdown(); + // To be called in grpc_shutdown(); static void Shutdown(); static intptr_t RegisterChannelNode(ChannelNode* channel_node) { @@ -51,7 +51,8 @@ class ChannelzRegistry { return gotten == nullptr ? nullptr : static_cast<ChannelNode*>(gotten); } - // todo, protect me + // Returns the allocated JSON string that represents the proto + // GetTopChannelsResponse as per channelz.proto. static char* GetTopChannels(intptr_t start_channel_id) { return Default()->InternalGetTopChannels(start_channel_id); } diff --git a/src/core/lib/surface/channel.cc b/src/core/lib/surface/channel.cc index dd3880f18e..7cbd61adef 100644 --- a/src/core/lib/surface/channel.cc +++ b/src/core/lib/surface/channel.cc @@ -167,11 +167,9 @@ grpc_channel* grpc_channel_create_with_builder( grpc_channel_args_destroy(args); if (channelz_enabled) { - channel->channelz_channel = - channel_node_create_func(channel, channel_tracer_max_nodes); - if (internal_channel || !channel->is_client) { - channel->channelz_channel->set_is_top_level_channel(false); - } + bool is_top_level_channel = channel->is_client && !internal_channel; + channel->channelz_channel = channel_node_create_func( + channel, channel_tracer_max_nodes, is_top_level_channel); channel->channelz_channel->trace()->AddTraceEvent( grpc_core::channelz::ChannelTrace::Severity::Info, grpc_slice_from_static_string("Channel created")); |