Merge pull request #15906 from ncteisen/channelz

Fix Channelz Linkage Error

9 files changed, 201 insertions, 32 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
new file mode 100644
index 0000000000..08ceb2dd05
--- /dev/null
+++ b/src/core/ext/filters/client_channel/client_channel_channelz.cc
@@ -0,0 +1,80 @@
+/*
+ *
+ * Copyright 2018 gRPC authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+#include <grpc/support/port_platform.h>
+
+#include "src/core/ext/filters/client_channel/client_channel.h"
+#include "src/core/ext/filters/client_channel/client_channel_channelz.h"
+#include "src/core/lib/gpr/useful.h"
+#include "src/core/lib/surface/channel.h"
+#include "src/core/lib/transport/connectivity_state.h"
+
+namespace grpc_core {
+namespace channelz {
+namespace {
+
+void* client_channel_channelz_copy(void* p) { return p; }
+
+void client_channel_channelz_destroy(void* p) {}
+
+int client_channel_channelz_cmp(void* a, void* b) { return GPR_ICMP(a, b); }
+
+}  // namespace
+
+static const grpc_arg_pointer_vtable client_channel_channelz_vtable = {
+    client_channel_channelz_copy, client_channel_channelz_destroy,
+    client_channel_channelz_cmp};
+
+ClientChannelNode::ClientChannelNode(grpc_channel* channel,
+                                     size_t channel_tracer_max_nodes)
+    : ChannelNode(channel, channel_tracer_max_nodes) {
+  client_channel_ =
+      grpc_channel_stack_last_element(grpc_channel_get_channel_stack(channel));
+  GPR_ASSERT(client_channel_->filter == &grpc_client_channel_filter);
+}
+
+void ClientChannelNode::PopulateConnectivityState(grpc_json* json) {
+  grpc_connectivity_state state;
+  if (ChannelIsDestroyed()) {
+    state = GRPC_CHANNEL_SHUTDOWN;
+  } else {
+    state =
+        grpc_client_channel_check_connectivity_state(client_channel_, false);
+  }
+  json = grpc_json_create_child(nullptr, json, "state", nullptr,
+                                GRPC_JSON_OBJECT, false);
+  grpc_json_create_child(nullptr, json, "state",
+                         grpc_connectivity_state_name(state), GRPC_JSON_STRING,
+                         false);
+}
+
+grpc_arg ClientChannelNode::CreateChannelArg() {
+  return grpc_channel_arg_pointer_create(
+      const_cast<char*>(GRPC_ARG_CHANNELZ_CHANNEL_NODE_CREATION_FUNC),
+      reinterpret_cast<void*>(MakeClientChannelNode),
+      &client_channel_channelz_vtable);
+}
+
+RefCountedPtr<ChannelNode> ClientChannelNode::MakeClientChannelNode(
+    grpc_channel* channel, size_t channel_tracer_max_nodes) {
+  return MakePolymorphicRefCounted<ChannelNode, ClientChannelNode>(
+      channel, channel_tracer_max_nodes);
+}
+
+}  // namespace channelz
+}  // namespace grpc_core
diff --git a/src/core/ext/filters/client_channel/client_channel_channelz.h b/src/core/ext/filters/client_channel/client_channel_channelz.h
new file mode 100644
index 0000000000..cf3ef7b6f2
--- /dev/null
+++ b/src/core/ext/filters/client_channel/client_channel_channelz.h
@@ -0,0 +1,58 @@
+/*
+ *
+ * Copyright 2018 gRPC authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+#ifndef GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_CLIENT_CHANNEL_CHANNELZ_H
+#define GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_CLIENT_CHANNEL_CHANNELZ_H
+
+#include <grpc/support/port_platform.h>
+
+#include "src/core/lib/channel/channel_args.h"
+#include "src/core/lib/channel/channelz.h"
+
+namespace grpc_core {
+namespace channelz {
+
+// Subtype of ChannelNode that overrides and provides client_channel specific
+// functionality like querying for connectivity_state and subchannel data.
+class ClientChannelNode : public ChannelNode {
+ public:
+  static RefCountedPtr<ChannelNode> MakeClientChannelNode(
+      grpc_channel* channel, size_t channel_tracer_max_nodes);
+
+  // Override this functionality since client_channels have a notion of
+  // channel connectivity.
+  void PopulateConnectivityState(grpc_json* json) override;
+
+  // Helper to create a channel arg to ensure this type of ChannelNode is
+  // created.
+  static grpc_arg CreateChannelArg();
+
+ 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);
+  virtual ~ClientChannelNode() {}
+
+ private:
+  grpc_channel_element* client_channel_;
+};
+
+}  // namespace channelz
+}  // namespace grpc_core
+
+#endif /* GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_CLIENT_CHANNEL_CHANNELZ_H */
diff --git a/src/core/ext/filters/client_channel/client_channel_plugin.cc b/src/core/ext/filters/client_channel/client_channel_plugin.cc
index 8385852d1b..e0784b7e5c 100644
--- a/src/core/ext/filters/client_channel/client_channel_plugin.cc
+++ b/src/core/ext/filters/client_channel/client_channel_plugin.cc
@@ -25,6 +25,7 @@
 #include <grpc/support/alloc.h>
 
 #include "src/core/ext/filters/client_channel/client_channel.h"
+#include "src/core/ext/filters/client_channel/client_channel_channelz.h"
 #include "src/core/ext/filters/client_channel/http_connect_handshaker.h"
 #include "src/core/ext/filters/client_channel/http_proxy.h"
 #include "src/core/ext/filters/client_channel/lb_policy_registry.h"
@@ -35,6 +36,14 @@
 #include "src/core/lib/surface/channel_init.h"
 
 static bool append_filter(grpc_channel_stack_builder* builder, void* arg) {
+  const grpc_channel_args* args =
+      grpc_channel_stack_builder_get_channel_arguments(builder);
+  grpc_arg args_to_add[] = {
+      grpc_core::channelz::ClientChannelNode::CreateChannelArg()};
+  grpc_channel_args* new_args = grpc_channel_args_copy_and_add(
+      args, args_to_add, GPR_ARRAY_SIZE(args_to_add));
+  grpc_channel_stack_builder_set_channel_arguments(builder, new_args);
+  grpc_channel_args_destroy(new_args);
   return grpc_channel_stack_builder_append_filter(
       builder, static_cast<const grpc_channel_filter*>(arg), nullptr, nullptr);
 }
diff --git a/src/core/lib/channel/channelz.cc b/src/core/lib/channel/channelz.cc
index a49271c3a1..2074cb0cc5 100644
--- a/src/core/lib/channel/channelz.cc
+++ b/src/core/lib/channel/channelz.cc
@@ -36,7 +36,6 @@
 #include "src/core/lib/iomgr/error.h"
 #include "src/core/lib/slice/slice_internal.h"
 #include "src/core/lib/surface/channel.h"
-#include "src/core/lib/transport/connectivity_state.h"
 #include "src/core/lib/transport/error_utils.h"
 
 namespace grpc_core {
@@ -109,16 +108,7 @@ void ChannelNode::RecordCallStarted() {
                            (gpr_atm)ExecCtx::Get()->Now());
 }
 
-grpc_connectivity_state ChannelNode::GetConnectivityState() {
-  if (channel_ == nullptr) {
-    return GRPC_CHANNEL_SHUTDOWN;
-  } else {
-    // TODO(ncteisen): re-enable this once we have cleaned up all of the
-    // internal dependency issues.
-    // return grpc_channel_check_connectivity_state(channel_, false);
-    return GRPC_CHANNEL_IDLE;
-  }
-}
+void ChannelNode::PopulateConnectivityState(grpc_json* json) {}
 
 char* ChannelNode::RenderJSON() {
   // We need to track these three json objects to build our object
@@ -139,16 +129,7 @@ char* ChannelNode::RenderJSON() {
                                            GRPC_JSON_OBJECT, false);
   json = data;
   json_iterator = nullptr;
-  // create and fill the connectivity state child.
-  grpc_connectivity_state connectivity_state = GetConnectivityState();
-  json_iterator = grpc_json_create_child(json_iterator, json, "state", nullptr,
-                                         GRPC_JSON_OBJECT, false);
-  json = json_iterator;
-  grpc_json_create_child(nullptr, json, "state",
-                         grpc_connectivity_state_name(connectivity_state),
-                         GRPC_JSON_STRING, false);
-  // reset the parent to be the data object.
-  json = data;
+  PopulateConnectivityState(json);
   json_iterator = grpc_json_create_child(
       json_iterator, json, "target", target_.get(), GRPC_JSON_STRING, false);
   // fill in the channel trace if applicable
@@ -184,5 +165,11 @@ char* ChannelNode::RenderJSON() {
   return json_str;
 }
 
+RefCountedPtr<ChannelNode> ChannelNode::MakeChannelNode(
+    grpc_channel* channel, size_t channel_tracer_max_nodes) {
+  return MakeRefCounted<grpc_core::channelz::ChannelNode>(
+      channel, channel_tracer_max_nodes);
+}
+
 }  // namespace channelz
 }  // namespace grpc_core
diff --git a/src/core/lib/channel/channelz.h b/src/core/lib/channel/channelz.h
index 2aad1e82f4..9bd01ece50 100644
--- a/src/core/lib/channel/channelz.h
+++ b/src/core/lib/channel/channelz.h
@@ -31,6 +31,10 @@
 #include "src/core/lib/iomgr/exec_ctx.h"
 #include "src/core/lib/json/json.h"
 
+// Channel arg key for client channel factory.
+#define GRPC_ARG_CHANNELZ_CHANNEL_NODE_CREATION_FUNC \
+  "grpc.channelz_channel_node_creation_func"
+
 namespace grpc_core {
 namespace channelz {
 
@@ -40,8 +44,8 @@ class ChannelNodePeer;
 
 class ChannelNode : public RefCounted<ChannelNode> {
  public:
-  ChannelNode(grpc_channel* channel, size_t channel_tracer_max_nodes);
-  ~ChannelNode();
+  static RefCountedPtr<ChannelNode> MakeChannelNode(
+      grpc_channel* channel, size_t channel_tracer_max_nodes);
 
   void RecordCallStarted();
   void RecordCallFailed() {
@@ -53,22 +57,32 @@ class ChannelNode : public RefCounted<ChannelNode> {
 
   char* RenderJSON();
 
+  // helper for getting and populating connectivity state. It is virtual
+  // because it allows the client_channel specific code to live in ext/
+  // instead of lib/
+  virtual void PopulateConnectivityState(grpc_json* json);
+
   ChannelTrace* trace() { return trace_.get(); }
 
-  void set_channel_destroyed() {
+  void MarkChannelDestroyed() {
     GPR_ASSERT(channel_ != nullptr);
     channel_ = nullptr;
   }
 
+  bool ChannelIsDestroyed() { return channel_ == nullptr; }
+
   intptr_t channel_uuid() { return channel_uuid_; }
 
+ 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);
+  virtual ~ChannelNode();
+
  private:
   // testing peer friend.
   friend class testing::ChannelNodePeer;
 
-  // helper for getting connectivity state.
-  grpc_connectivity_state GetConnectivityState();
-
   grpc_channel* channel_ = nullptr;
   UniquePtr<char> target_;
   gpr_atm calls_started_ = 0;
@@ -79,6 +93,11 @@ class ChannelNode : public RefCounted<ChannelNode> {
   ManualConstructor<ChannelTrace> trace_;
 };
 
+// Creation functions
+
+typedef RefCountedPtr<ChannelNode> (*ChannelNodeCreationFunc)(grpc_channel*,
+                                                              size_t);
+
 }  // namespace channelz
 }  // namespace grpc_core
 
diff --git a/src/core/lib/gprpp/memory.h b/src/core/lib/gprpp/memory.h
index 28fcdf1779..e90bedcd9b 100644
--- a/src/core/lib/gprpp/memory.h
+++ b/src/core/lib/gprpp/memory.h
@@ -31,12 +31,12 @@
 // protected destructor.
 #define GPRC_ALLOW_CLASS_TO_USE_NON_PUBLIC_DELETE \
   template <typename T>                           \
-  friend void Delete(T*);
+  friend void grpc_core::Delete(T*);
 // Add this to a class that want to use New(), but has a private or
 // protected constructor.
 #define GPRC_ALLOW_CLASS_TO_USE_NON_PUBLIC_NEW \
   template <typename T, typename... Args>      \
-  friend T* New(Args&&...);
+  friend T* grpc_core::New(Args&&...);
 
 namespace grpc_core {
 
diff --git a/src/core/lib/gprpp/ref_counted_ptr.h b/src/core/lib/gprpp/ref_counted_ptr.h
index 388e2ec410..534d3d03cb 100644
--- a/src/core/lib/gprpp/ref_counted_ptr.h
+++ b/src/core/lib/gprpp/ref_counted_ptr.h
@@ -107,6 +107,11 @@ inline RefCountedPtr<T> MakeRefCounted(Args&&... args) {
   return RefCountedPtr<T>(New<T>(std::forward<Args>(args)...));
 }
 
+template <typename Parent, typename Child, typename... Args>
+inline RefCountedPtr<Parent> MakePolymorphicRefCounted(Args&&... args) {
+  return RefCountedPtr<Parent>(New<Child>(std::forward<Args>(args)...));
+}
+
 }  // namespace grpc_core
 
 #endif /* GRPC_CORE_LIB_GPRPP_REF_COUNTED_PTR_H */
diff --git a/src/core/lib/surface/channel.cc b/src/core/lib/surface/channel.cc
index d5d75fcb2a..3e4e434a63 100644
--- a/src/core/lib/surface/channel.cc
+++ b/src/core/lib/surface/channel.cc
@@ -105,6 +105,10 @@ grpc_channel* grpc_channel_create_with_builder(
   channel->is_client = grpc_channel_stack_type_is_client(channel_stack_type);
   size_t channel_tracer_max_nodes = 0;  // default to off
   bool channelz_enabled = false;
+  // this creates the default ChannelNode. Different types of channels may
+  // override this to ensure a correct ChannelNode is created.
+  grpc_core::channelz::ChannelNodeCreationFunc channel_node_create_func =
+      grpc_core::channelz::ChannelNode::MakeChannelNode;
   gpr_mu_init(&channel->registered_call_mu);
   channel->registered_calls = nullptr;
 
@@ -145,14 +149,20 @@ grpc_channel* grpc_channel_create_with_builder(
           (size_t)grpc_channel_arg_get_integer(&args->args[i], options);
     } else if (0 == strcmp(args->args[i].key, GRPC_ARG_ENABLE_CHANNELZ)) {
       channelz_enabled = grpc_channel_arg_get_bool(&args->args[i], false);
+    } else if (0 == strcmp(args->args[i].key,
+                           GRPC_ARG_CHANNELZ_CHANNEL_NODE_CREATION_FUNC)) {
+      GPR_ASSERT(args->args[i].type == GRPC_ARG_POINTER);
+      GPR_ASSERT(args->args[i].value.pointer.p != nullptr);
+      channel_node_create_func =
+          reinterpret_cast<grpc_core::channelz::ChannelNodeCreationFunc>(
+              args->args[i].value.pointer.p);
     }
   }
 
   grpc_channel_args_destroy(args);
   if (channelz_enabled) {
     channel->channelz_channel =
-        grpc_core::MakeRefCounted<grpc_core::channelz::ChannelNode>(
-            channel, channel_tracer_max_nodes);
+        channel_node_create_func(channel, channel_tracer_max_nodes);
     channel->channelz_channel->trace()->AddTraceEvent(
         grpc_core::channelz::ChannelTrace::Severity::Info,
         grpc_slice_from_static_string("Channel created"));
@@ -400,7 +410,7 @@ void grpc_channel_internal_unref(grpc_channel* c REF_ARG) {
 static void destroy_channel(void* arg, grpc_error* error) {
   grpc_channel* channel = static_cast<grpc_channel*>(arg);
   if (channel->channelz_channel != nullptr) {
-    channel->channelz_channel->set_channel_destroyed();
+    channel->channelz_channel->MarkChannelDestroyed();
     channel->channelz_channel.reset();
   }
   grpc_channel_stack_destroy(CHANNEL_STACK_FROM_CHANNEL(channel));
diff --git a/src/python/grpcio/grpc_core_dependencies.py b/src/python/grpcio/grpc_core_dependencies.py
index 1746020b72..49185cc648 100644
--- a/src/python/grpcio/grpc_core_dependencies.py
+++ b/src/python/grpcio/grpc_core_dependencies.py
@@ -306,6 +306,7 @@ CORE_SOURCE_FILES = [
     'src/core/ext/filters/client_channel/backup_poller.cc',
     'src/core/ext/filters/client_channel/channel_connectivity.cc',
     'src/core/ext/filters/client_channel/client_channel.cc',
+    'src/core/ext/filters/client_channel/client_channel_channelz.cc',
     'src/core/ext/filters/client_channel/client_channel_factory.cc',
     'src/core/ext/filters/client_channel/client_channel_plugin.cc',
     'src/core/ext/filters/client_channel/connector.cc',