aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
-rw-r--r--src/core/ext/filters/client_channel/client_channel_channelz.cc10
-rw-r--r--src/core/ext/filters/client_channel/client_channel_channelz.h6
-rw-r--r--src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc3
-rw-r--r--src/core/lib/channel/channelz.cc15
-rw-r--r--src/core/lib/channel/channelz.h11
-rw-r--r--src/core/lib/channel/channelz_registry.cc5
-rw-r--r--src/core/lib/channel/channelz_registry.h5
-rw-r--r--src/core/lib/surface/channel.cc8
-rw-r--r--test/core/channel/channel_trace_test.cc12
-rw-r--r--test/core/channel/channelz_registry_test.cc45
-rw-r--r--test/core/channel/channelz_test.cc27
11 files changed, 55 insertions, 92 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"));
diff --git a/test/core/channel/channel_trace_test.cc b/test/core/channel/channel_trace_test.cc
index 07b0e3de06..99d9a4847f 100644
--- a/test/core/channel/channel_trace_test.cc
+++ b/test/core/channel/channel_trace_test.cc
@@ -34,8 +34,6 @@
#include "test/core/util/test_config.h"
#include "test/cpp/util/channel_trace_proto_helper.h"
-// remove me
-#include <grpc/support/string_util.h>
#include <stdlib.h>
#include <string.h>
@@ -157,7 +155,7 @@ TEST_P(ChannelTracerTest, ComplexTest) {
AddSimpleTrace(&tracer);
ChannelFixture channel1(GetParam());
RefCountedPtr<ChannelNode> sc1 =
- MakeRefCounted<ChannelNode>(channel1.channel(), GetParam());
+ MakeRefCounted<ChannelNode>(channel1.channel(), GetParam(), true);
tracer.AddTraceEventReferencingSubchannel(
ChannelTrace::Severity::Info,
grpc_slice_from_static_string("subchannel one created"), sc1);
@@ -175,7 +173,7 @@ TEST_P(ChannelTracerTest, ComplexTest) {
ValidateChannelTrace(&tracer, 5, GetParam());
ChannelFixture channel2(GetParam());
RefCountedPtr<ChannelNode> sc2 =
- MakeRefCounted<ChannelNode>(channel2.channel(), GetParam());
+ MakeRefCounted<ChannelNode>(channel2.channel(), GetParam(), true);
tracer.AddTraceEventReferencingChannel(
ChannelTrace::Severity::Info,
grpc_slice_from_static_string("LB channel two created"), sc2);
@@ -204,7 +202,7 @@ TEST_P(ChannelTracerTest, TestNesting) {
ValidateChannelTrace(&tracer, 2, GetParam());
ChannelFixture channel1(GetParam());
RefCountedPtr<ChannelNode> sc1 =
- MakeRefCounted<ChannelNode>(channel1.channel(), GetParam());
+ MakeRefCounted<ChannelNode>(channel1.channel(), GetParam(), true);
tracer.AddTraceEventReferencingChannel(
ChannelTrace::Severity::Info,
grpc_slice_from_static_string("subchannel one created"), sc1);
@@ -212,7 +210,7 @@ TEST_P(ChannelTracerTest, TestNesting) {
AddSimpleTrace(sc1->trace());
ChannelFixture channel2(GetParam());
RefCountedPtr<ChannelNode> conn1 =
- MakeRefCounted<ChannelNode>(channel2.channel(), GetParam());
+ MakeRefCounted<ChannelNode>(channel2.channel(), GetParam(), true);
// nesting one level deeper.
sc1->trace()->AddTraceEventReferencingSubchannel(
ChannelTrace::Severity::Info,
@@ -225,7 +223,7 @@ TEST_P(ChannelTracerTest, TestNesting) {
ValidateChannelTrace(conn1->trace(), 1, GetParam());
ChannelFixture channel3(GetParam());
RefCountedPtr<ChannelNode> sc2 =
- MakeRefCounted<ChannelNode>(channel3.channel(), GetParam());
+ MakeRefCounted<ChannelNode>(channel3.channel(), GetParam(), true);
tracer.AddTraceEventReferencingSubchannel(
ChannelTrace::Severity::Info,
grpc_slice_from_static_string("subchannel two created"), sc2);
diff --git a/test/core/channel/channelz_registry_test.cc b/test/core/channel/channelz_registry_test.cc
index 06363d00e2..990cd3d205 100644
--- a/test/core/channel/channelz_registry_test.cc
+++ b/test/core/channel/channelz_registry_test.cc
@@ -42,36 +42,9 @@
namespace grpc_core {
namespace channelz {
namespace testing {
-namespace {
-class ChannelFixture {
- public:
- ChannelFixture() {
- grpc_arg client_a[1];
- client_a[0].type = GRPC_ARG_INTEGER;
- client_a[0].key = const_cast<char*>(GRPC_ARG_ENABLE_CHANNELZ);
- client_a[0].value.integer = true;
- grpc_channel_args client_args = {GPR_ARRAY_SIZE(client_a), 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_;
-};
-
-} // namespace
-
-// Tests basic ChannelTrace functionality like construction, adding trace, and
-// lookups by uuid.
TEST(ChannelzRegistryTest, UuidStartsAboveZeroTest) {
- ChannelFixture channel;
- ChannelNode* channelz_channel =
- grpc_channel_get_channelz_node(channel.channel());
+ ChannelNode* channelz_channel = nullptr;
intptr_t uuid = ChannelzRegistry::RegisterChannelNode(channelz_channel);
EXPECT_GT(uuid, 0) << "First uuid chose must be greater than zero. Zero if "
"reserved according to "
@@ -81,9 +54,7 @@ TEST(ChannelzRegistryTest, UuidStartsAboveZeroTest) {
}
TEST(ChannelzRegistryTest, UuidsAreIncreasing) {
- ChannelFixture channel;
- ChannelNode* channelz_channel =
- grpc_channel_get_channelz_node(channel.channel());
+ ChannelNode* channelz_channel = nullptr;
std::vector<intptr_t> uuids;
uuids.reserve(10);
for (int i = 0; i < 10; ++i) {
@@ -96,18 +67,14 @@ TEST(ChannelzRegistryTest, UuidsAreIncreasing) {
}
TEST(ChannelzRegistryTest, RegisterGetTest) {
- ChannelFixture channel;
- ChannelNode* channelz_channel =
- grpc_channel_get_channelz_node(channel.channel());
+ ChannelNode* channelz_channel = nullptr;
intptr_t uuid = ChannelzRegistry::RegisterChannelNode(channelz_channel);
ChannelNode* retrieved = ChannelzRegistry::GetChannelNode(uuid);
EXPECT_EQ(channelz_channel, retrieved);
}
TEST(ChannelzRegistryTest, RegisterManyItems) {
- ChannelFixture channel;
- ChannelNode* channelz_channel =
- grpc_channel_get_channelz_node(channel.channel());
+ ChannelNode* channelz_channel = nullptr;
for (int i = 0; i < 100; i++) {
intptr_t uuid = ChannelzRegistry::RegisterChannelNode(channelz_channel);
ChannelNode* retrieved = ChannelzRegistry::GetChannelNode(uuid);
@@ -116,9 +83,7 @@ TEST(ChannelzRegistryTest, RegisterManyItems) {
}
TEST(ChannelzRegistryTest, NullIfNotPresentTest) {
- ChannelFixture channel;
- ChannelNode* channelz_channel =
- grpc_channel_get_channelz_node(channel.channel());
+ ChannelNode* channelz_channel = nullptr;
intptr_t uuid = ChannelzRegistry::RegisterChannelNode(channelz_channel);
// try to pull out a uuid that does not exist.
ChannelNode* nonexistant = ChannelzRegistry::GetChannelNode(uuid + 1);
diff --git a/test/core/channel/channelz_test.cc b/test/core/channel/channelz_test.cc
index 1ed36acb61..d12f529726 100644
--- a/test/core/channel/channelz_test.cc
+++ b/test/core/channel/channelz_test.cc
@@ -80,7 +80,7 @@ void ValidateJsonArraySize(grpc_json* json, const char* key,
for (grpc_json* child = arr->child; child != nullptr; child = child->next) {
++count;
}
- ASSERT_EQ(count, expected_size);
+ EXPECT_EQ(count, expected_size);
}
void ValidateGetTopChannels(size_t expected_channels) {
@@ -91,7 +91,7 @@ void ValidateGetTopChannels(size_t expected_channels) {
// tracked: https://github.com/grpc/grpc/issues/16019.
ValidateJsonArraySize(parsed_json, "channel", expected_channels);
grpc_json* end = GetJsonChild(parsed_json, "end");
- EXPECT_NE(end, nullptr);
+ ASSERT_NE(end, nullptr);
EXPECT_EQ(end->type, GRPC_JSON_TRUE);
grpc_json_destroy(parsed_json);
gpr_free(json_str);
@@ -101,13 +101,11 @@ class ChannelFixture {
public:
ChannelFixture(int max_trace_nodes = 0) {
grpc_arg client_a[2];
- client_a[0].type = GRPC_ARG_INTEGER;
- client_a[0].key =
- const_cast<char*>(GRPC_ARG_MAX_CHANNEL_TRACE_EVENTS_PER_NODE);
- client_a[0].value.integer = max_trace_nodes;
- client_a[1].type = GRPC_ARG_INTEGER;
- client_a[1].key = const_cast<char*>(GRPC_ARG_ENABLE_CHANNELZ);
- client_a[1].value.integer = true;
+ client_a[0] = grpc_channel_arg_integer_create(
+ const_cast<char*>(GRPC_ARG_MAX_CHANNEL_TRACE_EVENTS_PER_NODE),
+ max_trace_nodes);
+ client_a[1] = grpc_channel_arg_integer_create(
+ const_cast<char*>(GRPC_ARG_ENABLE_CHANNELZ), true);
grpc_channel_args client_args = {GPR_ARRAY_SIZE(client_a), client_a};
channel_ =
grpc_insecure_channel_create("fake_target", &client_args, nullptr);
@@ -255,13 +253,10 @@ TEST(ChannelzGetTopChannelsTest, InternalChannelTest) {
(void)channels; // suppress unused variable error
// create an internal channel
grpc_arg client_a[2];
- client_a[0].type = GRPC_ARG_INTEGER;
- client_a[0].key =
- const_cast<char*>(GRPC_ARG_CHANNELZ_CHANNEL_IS_INTERNAL_CHANNEL);
- client_a[0].value.integer = 1;
- client_a[1].type = GRPC_ARG_INTEGER;
- client_a[1].key = const_cast<char*>(GRPC_ARG_ENABLE_CHANNELZ);
- client_a[1].value.integer = true;
+ client_a[0] = grpc_channel_arg_integer_create(
+ const_cast<char*>(GRPC_ARG_CHANNELZ_CHANNEL_IS_INTERNAL_CHANNEL), true);
+ client_a[1] = grpc_channel_arg_integer_create(
+ const_cast<char*>(GRPC_ARG_ENABLE_CHANNELZ), true);
grpc_channel_args client_args = {GPR_ARRAY_SIZE(client_a), client_a};
grpc_channel* internal_channel =
grpc_insecure_channel_create("fake_target", &client_args, nullptr);