From d82dff4ea042752009f23aa2a87f56016050cc6c Mon Sep 17 00:00:00 2001 From: ncteisen Date: Tue, 30 Oct 2018 09:04:41 -0700 Subject: [PATCH] reviewer feedback, debuging helper --- .../chttp2/transport/chttp2_transport.cc | 18 +++++++++++++++++- src/core/lib/channel/channelz.cc | 19 ++++--------------- src/core/lib/channel/channelz.h | 2 +- src/core/lib/channel/channelz_registry.cc | 11 +++++++++++ src/core/lib/channel/channelz_registry.h | 6 ++++++ test/core/end2end/tests/channelz.cc | 7 +++---- 6 files changed, 42 insertions(+), 21 deletions(-) diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc index d7934ab7aae..d4665d5245d 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc @@ -31,6 +31,7 @@ #include #include +#include "src/core/ext/filters/client_channel/uri_parser.h" #include "src/core/ext/transport/chttp2/transport/frame_data.h" #include "src/core/ext/transport/chttp2/transport/internal.h" #include "src/core/ext/transport/chttp2/transport/varint.h" @@ -38,6 +39,7 @@ #include "src/core/lib/compression/stream_compression.h" #include "src/core/lib/debug/stats.h" #include "src/core/lib/gpr/env.h" +#include "src/core/lib/gpr/host_port.h" #include "src/core/lib/gpr/string.h" #include "src/core/lib/gprpp/memory.h" #include "src/core/lib/http/parser.h" @@ -395,9 +397,23 @@ static bool read_channel_args(grpc_chttp2_transport* t, } } if (channelz_enabled) { + // pick out just the host port (maybe trims off scheme prefix). + grpc_uri* uri = grpc_uri_parse(t->peer_string, false); + GPR_ASSERT(uri != nullptr); + const char* host_port = uri->path; + if (*host_port == '/') ++host_port; + char* host; + char* port; + GPR_ASSERT(gpr_split_host_port(host_port, &host, &port)); + int port_num = -1; + if (port != nullptr) { + port_num = atoi(port); + } t->channelz_socket = grpc_core::MakeRefCounted( - (t->peer_string)); + grpc_core::UniquePtr(host), port_num); + grpc_uri_destroy(uri); + gpr_free(port); } return enable_bdp; } diff --git a/src/core/lib/channel/channelz.cc b/src/core/lib/channel/channelz.cc index bf8acebcd7e..6da8a172b50 100644 --- a/src/core/lib/channel/channelz.cc +++ b/src/core/lib/channel/channelz.cc @@ -30,7 +30,6 @@ #include "src/core/lib/channel/channelz_registry.h" #include "src/core/lib/channel/status_util.h" -#include "src/core/lib/gpr/host_port.h" #include "src/core/lib/gpr/string.h" #include "src/core/lib/gpr/useful.h" #include "src/core/lib/gprpp/memory.h" @@ -278,20 +277,10 @@ grpc_json* ServerNode::RenderJson() { return top_level_json; } -SocketNode::SocketNode(const char* remote_peer_string) - : BaseNode(EntityType::kSocket) { - const char* ipv6_prefix = "ipv6:"; - const int ipv6_prefix_len = 5; - char* host; - char* port; - int offset = (strncmp(remote_peer_string, ipv6_prefix, ipv6_prefix_len) == 0) - ? ipv6_prefix_len - : 0; - GPR_ASSERT(gpr_split_host_port(remote_peer_string + offset, &host, &port)); - remote_host_ = UniquePtr(host); - remote_port_ = atoi(port); - gpr_free(port); -} +SocketNode::SocketNode(UniquePtr remote_host, int remote_port) + : BaseNode(EntityType::kSocket), + remote_host_(std::move(remote_host)), + remote_port_(remote_port) {} void SocketNode::RecordStreamStartedFromLocal() { gpr_atm_no_barrier_fetch_add(&streams_started_, static_cast(1)); diff --git a/src/core/lib/channel/channelz.h b/src/core/lib/channel/channelz.h index ba10b1a5bfb..404d8341ffa 100644 --- a/src/core/lib/channel/channelz.h +++ b/src/core/lib/channel/channelz.h @@ -232,7 +232,7 @@ class ServerNode : public BaseNode { // Handles channelz bookkeeping for sockets class SocketNode : public BaseNode { public: - SocketNode(const char* remote_peer_string); + SocketNode(UniquePtr remote_host, int remote_port); ~SocketNode() override {} grpc_json* RenderJson() override; diff --git a/src/core/lib/channel/channelz_registry.cc b/src/core/lib/channel/channelz_registry.cc index 8fc1b58adc4..bc23b90a661 100644 --- a/src/core/lib/channel/channelz_registry.cc +++ b/src/core/lib/channel/channelz_registry.cc @@ -210,6 +210,17 @@ char* ChannelzRegistry::InternalGetServers(intptr_t start_server_id) { return json_str; } +void ChannelzRegistry::InternalLogAllEntities() { + MutexLock lock(&mu_); + for (size_t i = 0; i < entities_.size(); ++i) { + if (entities_[i] != nullptr) { + char* json = entities_[i]->RenderJsonString(); + gpr_log(GPR_INFO, "%s", json); + gpr_free(json); + } + } +} + } // namespace channelz } // namespace grpc_core diff --git a/src/core/lib/channel/channelz_registry.h b/src/core/lib/channel/channelz_registry.h index 326f0201c72..73b330785d2 100644 --- a/src/core/lib/channel/channelz_registry.h +++ b/src/core/lib/channel/channelz_registry.h @@ -62,6 +62,10 @@ class ChannelzRegistry { return Default()->InternalGetServers(start_server_id); } + // Test only helper function to dump the JSON representation to std out. + // This can aid in debugging channelz code. + static void LogAllEntities() { Default()->InternalLogAllEntities(); } + private: GPRC_ALLOW_CLASS_TO_USE_NON_PUBLIC_NEW GPRC_ALLOW_CLASS_TO_USE_NON_PUBLIC_DELETE @@ -96,6 +100,8 @@ class ChannelzRegistry { // Else, will return idx of the first uuid higher than the target. int FindByUuidLocked(intptr_t uuid, bool direct_hit_needed); + void InternalLogAllEntities(); + // protects members gpr_mu mu_; InlinedVector entities_; diff --git a/test/core/end2end/tests/channelz.cc b/test/core/end2end/tests/channelz.cc index 7c61b7910be..f860bdfc17f 100644 --- a/test/core/end2end/tests/channelz.cc +++ b/test/core/end2end/tests/channelz.cc @@ -29,6 +29,7 @@ #include #include #include +#include "src/core/lib/channel/channelz_registry.h" #include "src/core/lib/gpr/string.h" #include "test/core/end2end/cq_verifier.h" @@ -238,7 +239,6 @@ static void test_channelz(grpc_end2end_test_config config) { json = channelz_channel->RenderJsonString(); GPR_ASSERT(json != nullptr); - gpr_log(GPR_INFO, "%s", json); GPR_ASSERT(nullptr != strstr(json, "\"callsStarted\":\"2\"")); GPR_ASSERT(nullptr != strstr(json, "\"callsFailed\":\"1\"")); GPR_ASSERT(nullptr != strstr(json, "\"callsSucceeded\":\"1\"")); @@ -250,7 +250,6 @@ static void test_channelz(grpc_end2end_test_config config) { json = channelz_server->RenderJsonString(); GPR_ASSERT(json != nullptr); - gpr_log(GPR_INFO, "%s", json); GPR_ASSERT(nullptr != strstr(json, "\"callsStarted\":\"2\"")); GPR_ASSERT(nullptr != strstr(json, "\"callsFailed\":\"1\"")); GPR_ASSERT(nullptr != strstr(json, "\"callsSucceeded\":\"1\"")); @@ -264,6 +263,8 @@ static void test_channelz(grpc_end2end_test_config config) { GPR_ASSERT(nullptr != strstr(json, "\"socketRef\":")); gpr_free(json); + grpc_core::channelz::ChannelzRegistry::LogAllEntities(); + end_test(&f); config.tear_down_data(&f); } @@ -292,7 +293,6 @@ static void test_channelz_with_channel_trace(grpc_end2end_test_config config) { char* json = channelz_channel->RenderJsonString(); GPR_ASSERT(json != nullptr); - gpr_log(GPR_INFO, "%s", json); GPR_ASSERT(nullptr != strstr(json, "\"trace\"")); GPR_ASSERT(nullptr != strstr(json, "\"description\":\"Channel created\"")); GPR_ASSERT(nullptr != strstr(json, "\"severity\":\"CT_INFO\"")); @@ -300,7 +300,6 @@ static void test_channelz_with_channel_trace(grpc_end2end_test_config config) { json = channelz_server->RenderJsonString(); GPR_ASSERT(json != nullptr); - gpr_log(GPR_INFO, "%s", json); GPR_ASSERT(nullptr != strstr(json, "\"trace\"")); GPR_ASSERT(nullptr != strstr(json, "\"description\":\"Server created\"")); GPR_ASSERT(nullptr != strstr(json, "\"severity\":\"CT_INFO\""));