From 18a9d7d57c44e071a722b399c27758786542cf56 Mon Sep 17 00:00:00 2001
From: ncteisen <ncteisen@gmail.com>
Date: Fri, 20 Jul 2018 09:29:05 -0700
Subject: [PATCH] reviewer feedback

---
 BUILD                                         |  4 +-
 include/grpcpp/ext/channelz_service_plugin.h  |  5 +-
 .../channelz/channelz_service_plugin.cc       | 56 ++++++++-----------
 test/cpp/end2end/channelz_service_test.cc     |  2 +-
 4 files changed, 29 insertions(+), 38 deletions(-)

diff --git a/BUILD b/BUILD
index 8bb98862e33..1c80e83ccae 100644
--- a/BUILD
+++ b/BUILD
@@ -2061,9 +2061,11 @@ grpc_cc_library(
     ],
     hdrs = [
         "src/cpp/server/channelz/channelz_service.h",
-        "src/cpp/server/channelz/channelz_service_plugin.h",
     ],
     language = "c++",
+    public_hdrs = [
+        "include/grpcpp/ext/channelz_service_plugin.h",
+    ],
     deps = [
         ":grpc++",
         "//src/proto/grpc/channelz:channelz_proto",
diff --git a/include/grpcpp/ext/channelz_service_plugin.h b/include/grpcpp/ext/channelz_service_plugin.h
index 4a748a98800..af3192d4513 100644
--- a/include/grpcpp/ext/channelz_service_plugin.h
+++ b/include/grpcpp/ext/channelz_service_plugin.h
@@ -30,8 +30,9 @@ namespace channelz {
 namespace experimental {
 
 /// Add channelz server plugin to \a ServerBuilder. This function should
-/// be called at static initialization time.
-void InitChannelzServiceBuilderPlugin();
+/// be called at static initialization time. This service is experimental
+/// for now. Track progress in https://github.com/grpc/grpc/issues/15988.
+void InitChannelzService();
 
 }  // namespace experimental
 }  // namespace channelz
diff --git a/src/cpp/server/channelz/channelz_service_plugin.cc b/src/cpp/server/channelz/channelz_service_plugin.cc
index c1c41bb57c8..b93e5b551e1 100644
--- a/src/cpp/server/channelz/channelz_service_plugin.cc
+++ b/src/cpp/server/channelz/channelz_service_plugin.cc
@@ -29,49 +29,37 @@ namespace grpc {
 namespace channelz {
 namespace experimental {
 
-// This plugin is experimental for now. Track progress in
-// https://github.com/grpc/grpc/issues/15988.
 class ChannelzServicePlugin : public ::grpc::ServerBuilderPlugin {
  public:
-  ChannelzServicePlugin();
-  ::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;
+  ChannelzServicePlugin() : channelz_service_(new grpc::ChannelzService()) {}
 
- private:
-  std::shared_ptr<grpc::ChannelzService> channelz_service_;
-};
-
-ChannelzServicePlugin::ChannelzServicePlugin()
-    : channelz_service_(new grpc::ChannelzService()) {}
+  grpc::string name() override { return "channelz_service"; }
 
-grpc::string ChannelzServicePlugin::name() { return "channelz_service"; }
-
-void ChannelzServicePlugin::InitServer(grpc::ServerInitializer* si) {
-  si->RegisterService(channelz_service_);
-}
+  void InitServer(grpc::ServerInitializer* si) override {
+    si->RegisterService(channelz_service_);
+  }
 
-void ChannelzServicePlugin::Finish(grpc::ServerInitializer* si) {}
+  void Finish(grpc::ServerInitializer* si) override {}
 
-void ChannelzServicePlugin::ChangeArguments(const grpc::string& name,
-                                            void* value) {}
+  void ChangeArguments(const grpc::string& name, void* value) override {}
 
-bool ChannelzServicePlugin::has_sync_methods() const {
-  if (channelz_service_) {
-    return channelz_service_->has_synchronous_methods();
+  bool has_sync_methods() const override {
+    if (channelz_service_) {
+      return channelz_service_->has_synchronous_methods();
+    }
+    return false;
   }
-  return false;
-}
 
-bool ChannelzServicePlugin::has_async_methods() const {
-  if (channelz_service_) {
-    return channelz_service_->has_async_methods();
+  bool has_async_methods() const override {
+    if (channelz_service_) {
+      return channelz_service_->has_async_methods();
+    }
+    return false;
   }
-  return false;
-}
+
+ private:
+  std::shared_ptr<grpc::ChannelzService> channelz_service_;
+};
 
 static std::unique_ptr< ::grpc::ServerBuilderPlugin>
 CreateChannelzServicePlugin() {
@@ -79,7 +67,7 @@ CreateChannelzServicePlugin() {
       new ChannelzServicePlugin());
 }
 
-void InitChannelzServiceBuilderPlugin() {
+void InitChannelzService() {
   static bool already_here = false;
   if (already_here) return;
   already_here = true;
diff --git a/test/cpp/end2end/channelz_service_test.cc b/test/cpp/end2end/channelz_service_test.cc
index d52324d7b36..933e4a1ff67 100644
--- a/test/cpp/end2end/channelz_service_test.cc
+++ b/test/cpp/end2end/channelz_service_test.cc
@@ -77,7 +77,7 @@ class ChannelzServerTest : public ::testing::Test {
 
   void SetUp() override {
     // 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.
     proxy_port_ = grpc_pick_unused_port_or_die();