reviewer feedback

reviewable/pr16346/r7
ncteisen 6 years ago
parent 6076b1d798
commit fe1f7f5813
  1. 3
      src/core/lib/channel/channelz.cc
  2. 2
      src/core/lib/channel/channelz.h
  3. 11
      src/core/lib/surface/call.cc
  4. 4
      src/core/lib/surface/channel.cc
  5. 17
      test/core/channel/channelz_test.cc
  6. 4
      test/core/end2end/tests/channelz.cc

@ -181,9 +181,6 @@ grpc_json* ServerNode::RenderJson() {
// ask CallCountingHelper to populate trace and call count data.
call_counter_.PopulateCallCounts(json);
json = top_level_json;
// template method. Child classes may override this to add their specific
// functionality.
PopulateSockets(json);
return top_level_json;
}

@ -174,8 +174,6 @@ class ServerNode : public BaseNode {
grpc_json* RenderJson() override;
void PopulateSockets(grpc_json* json) {}
// proxy methods to composed classes.
void AddTraceEvent(ChannelTrace::Severity severity, grpc_slice data) {
trace_.AddTraceEvent(severity, data);

@ -167,8 +167,6 @@ struct grpc_call {
grpc_completion_queue* cq;
grpc_polling_entity pollent;
grpc_channel* channel;
// backpointer to owning server if this is a server side call.
grpc_server* server;
gpr_timespec start_time;
/* parent_call* */ gpr_atm parent_call_atm;
child_call* child;
@ -250,6 +248,8 @@ struct grpc_call {
} client;
struct {
int* cancelled;
// backpointer to owning server if this is a server side call.
grpc_server* server;
} server;
} final_op;
@ -369,7 +369,6 @@ grpc_error* grpc_call_create(const grpc_call_create_args* args,
grpc_slice path = grpc_empty_slice();
if (call->is_client) {
GRPC_STATS_INC_CLIENT_CALLS_CREATED();
call->server = nullptr;
GPR_ASSERT(args->add_initial_metadata_count <
MAX_SEND_EXTRA_METADATA_COUNT);
for (i = 0; i < args->add_initial_metadata_count; i++) {
@ -384,7 +383,7 @@ grpc_error* grpc_call_create(const grpc_call_create_args* args,
static_cast<int>(args->add_initial_metadata_count);
} else {
GRPC_STATS_INC_SERVER_CALLS_CREATED();
call->server = args->server;
call->final_op.server.server = args->server;
GPR_ASSERT(args->add_initial_metadata_count == 0);
call->send_extra_metadata_count = 0;
}
@ -496,7 +495,7 @@ grpc_error* grpc_call_create(const grpc_call_create_args* args,
}
} else {
grpc_core::channelz::ServerNode* channelz_server =
grpc_server_get_channelz_node(call->server);
grpc_server_get_channelz_node(call->final_op.server.server);
if (channelz_server != nullptr) {
channelz_server->RecordCallStarted();
}
@ -1286,7 +1285,7 @@ static void post_batch_completion(batch_control* bctl) {
get_final_status(call, set_cancelled_value,
call->final_op.server.cancelled, nullptr, nullptr);
grpc_core::channelz::ServerNode* channelz_server =
grpc_server_get_channelz_node(call->server);
grpc_server_get_channelz_node(call->final_op.server.server);
if (channelz_server != nullptr) {
if (*call->final_op.server.cancelled) {
channelz_server->RecordCallFailed();

@ -166,8 +166,8 @@ grpc_channel* grpc_channel_create_with_builder(
}
grpc_channel_args_destroy(args);
// only track client channels since server channels are held in the
// grpc_server channel.
// we only need to do the channelz bookkeeping for clients here. The channelz
// bookkeeping for server channels occurs in src/core/lib/surface/server.cc
if (channelz_enabled && channel->is_client) {
channel->channelz_channel = channel_node_create_func(
channel, channel_tracer_max_nodes, !internal_channel);

@ -146,20 +146,21 @@ class ChannelFixture {
class ServerFixture {
public:
ServerFixture(int max_trace_nodes = 0) {
grpc_arg server_a[2];
server_a[0] = grpc_channel_arg_integer_create(
const_cast<char*>(GRPC_ARG_MAX_CHANNEL_TRACE_EVENTS_PER_NODE),
max_trace_nodes);
server_a[1] = grpc_channel_arg_integer_create(
const_cast<char*>(GRPC_ARG_ENABLE_CHANNELZ), true);
explicit ServerFixture(int max_trace_nodes = 0) {
grpc_arg server_a[] = {
grpc_channel_arg_integer_create(
const_cast<char*>(GRPC_ARG_MAX_CHANNEL_TRACE_EVENTS_PER_NODE),
max_trace_nodes),
grpc_channel_arg_integer_create(
const_cast<char*>(GRPC_ARG_ENABLE_CHANNELZ), true),
};
grpc_channel_args server_args = {GPR_ARRAY_SIZE(server_a), server_a};
server_ = grpc_server_create(&server_args, nullptr);
}
~ServerFixture() { grpc_server_destroy(server_); }
grpc_server* server() { return server_; }
grpc_server* server() const { return server_; }
private:
grpc_server* server_;

@ -240,7 +240,7 @@ static void test_channelz(grpc_end2end_test_config config) {
GPR_ASSERT(nullptr != strstr(json, "\"callsStarted\":\"2\""));
GPR_ASSERT(nullptr != strstr(json, "\"callsFailed\":\"1\""));
GPR_ASSERT(nullptr != strstr(json, "\"callsSucceeded\":\"1\""));
// channel tracing is not enables, so these should not be preset.
// channel tracing is not enabled, so these should not be preset.
GPR_ASSERT(nullptr == strstr(json, "\"trace\""));
GPR_ASSERT(nullptr == strstr(json, "\"description\":\"Channel created\""));
GPR_ASSERT(nullptr == strstr(json, "\"severity\":\"CT_INFO\""));
@ -252,7 +252,7 @@ static void test_channelz(grpc_end2end_test_config config) {
GPR_ASSERT(nullptr != strstr(json, "\"callsStarted\":\"2\""));
GPR_ASSERT(nullptr != strstr(json, "\"callsFailed\":\"1\""));
GPR_ASSERT(nullptr != strstr(json, "\"callsSucceeded\":\"1\""));
// channel tracing is not enables, so these should not be preset.
// channel tracing is not enabled, so these should not be preset.
GPR_ASSERT(nullptr == strstr(json, "\"trace\""));
GPR_ASSERT(nullptr == strstr(json, "\"description\":\"Channel created\""));
GPR_ASSERT(nullptr == strstr(json, "\"severity\":\"CT_INFO\""));

Loading…
Cancel
Save