reviewer feedback

pull/16023/head
ncteisen 6 years ago
parent b47214952b
commit 18a9d7d57c
  1. 4
      BUILD
  2. 5
      include/grpcpp/ext/channelz_service_plugin.h
  3. 42
      src/cpp/server/channelz/channelz_service_plugin.cc
  4. 2
      test/cpp/end2end/channelz_service_test.cc

@ -2061,9 +2061,11 @@ grpc_cc_library(
], ],
hdrs = [ hdrs = [
"src/cpp/server/channelz/channelz_service.h", "src/cpp/server/channelz/channelz_service.h",
"src/cpp/server/channelz/channelz_service_plugin.h",
], ],
language = "c++", language = "c++",
public_hdrs = [
"include/grpcpp/ext/channelz_service_plugin.h",
],
deps = [ deps = [
":grpc++", ":grpc++",
"//src/proto/grpc/channelz:channelz_proto", "//src/proto/grpc/channelz:channelz_proto",

@ -30,8 +30,9 @@ namespace channelz {
namespace experimental { namespace experimental {
/// Add channelz server plugin to \a ServerBuilder. This function should /// Add channelz server plugin to \a ServerBuilder. This function should
/// be called at static initialization time. /// be called at static initialization time. This service is experimental
void InitChannelzServiceBuilderPlugin(); /// for now. Track progress in https://github.com/grpc/grpc/issues/15988.
void InitChannelzService();
} // namespace experimental } // namespace experimental
} // namespace channelz } // namespace channelz

@ -29,49 +29,37 @@ namespace grpc {
namespace channelz { namespace channelz {
namespace experimental { namespace experimental {
// This plugin is experimental for now. Track progress in
// https://github.com/grpc/grpc/issues/15988.
class ChannelzServicePlugin : public ::grpc::ServerBuilderPlugin { class ChannelzServicePlugin : public ::grpc::ServerBuilderPlugin {
public: public:
ChannelzServicePlugin(); ChannelzServicePlugin() : channelz_service_(new grpc::ChannelzService()) {}
::grpc::string name() override;
void InitServer(::grpc::ServerInitializer* si) override;
void Finish(::grpc::ServerInitializer* si) override;
void ChangeArguments(const ::grpc::string& name, void* value) override;
bool has_async_methods() const override;
bool has_sync_methods() const override;
private: grpc::string name() override { return "channelz_service"; }
std::shared_ptr<grpc::ChannelzService> channelz_service_;
};
ChannelzServicePlugin::ChannelzServicePlugin()
: channelz_service_(new grpc::ChannelzService()) {}
grpc::string ChannelzServicePlugin::name() { return "channelz_service"; } void InitServer(grpc::ServerInitializer* si) override {
void ChannelzServicePlugin::InitServer(grpc::ServerInitializer* si) {
si->RegisterService(channelz_service_); si->RegisterService(channelz_service_);
} }
void ChannelzServicePlugin::Finish(grpc::ServerInitializer* si) {} void Finish(grpc::ServerInitializer* si) override {}
void ChannelzServicePlugin::ChangeArguments(const grpc::string& name, void ChangeArguments(const grpc::string& name, void* value) override {}
void* value) {}
bool ChannelzServicePlugin::has_sync_methods() const { bool has_sync_methods() const override {
if (channelz_service_) { if (channelz_service_) {
return channelz_service_->has_synchronous_methods(); return channelz_service_->has_synchronous_methods();
} }
return false; return false;
} }
bool ChannelzServicePlugin::has_async_methods() const { bool has_async_methods() const override {
if (channelz_service_) { if (channelz_service_) {
return channelz_service_->has_async_methods(); return channelz_service_->has_async_methods();
} }
return false; return false;
} }
private:
std::shared_ptr<grpc::ChannelzService> channelz_service_;
};
static std::unique_ptr< ::grpc::ServerBuilderPlugin> static std::unique_ptr< ::grpc::ServerBuilderPlugin>
CreateChannelzServicePlugin() { CreateChannelzServicePlugin() {
@ -79,7 +67,7 @@ CreateChannelzServicePlugin() {
new ChannelzServicePlugin()); new ChannelzServicePlugin());
} }
void InitChannelzServiceBuilderPlugin() { void InitChannelzService() {
static bool already_here = false; static bool already_here = false;
if (already_here) return; if (already_here) return;
already_here = true; already_here = true;

@ -77,7 +77,7 @@ class ChannelzServerTest : public ::testing::Test {
void SetUp() override { void SetUp() override {
// ensure channel server is brought up on all severs we build. // ensure channel server is brought up on all severs we build.
::grpc::channelz::experimental::InitChannelzServiceBuilderPlugin(); ::grpc::channelz::experimental::InitChannelzService();
// We set up a proxy server with channelz enabled. // We set up a proxy server with channelz enabled.
proxy_port_ = grpc_pick_unused_port_or_die(); proxy_port_ = grpc_pick_unused_port_or_die();

Loading…
Cancel
Save