Fix the muddled linkeage of channelz

7 files changed, 180 insertions, 29 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..df78dd5932
--- /dev/null
+++ b/src/core/ext/filters/client_channel/client_channel_channelz.cc
@@ -0,0 +1,62 @@
+/*
+ *
+ * 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_channelz.h"
+#include "src/core/lib/gpr/useful.h"
+
+namespace grpc_core {
+namespace channelz {
+
+static void* client_channel_channelz_copy(void* p) { return p; }
+
+static void client_channel_channelz_destroy(void* p) {}
+
+static int client_channel_channelz_cmp(void* a, void* b) {
+  return GPR_ICMP(a, b);
+}
+
+static const grpc_arg_pointer_vtable client_channel_channelz_vtable = {
+    client_channel_channelz_copy, client_channel_channelz_destroy,
+    client_channel_channelz_cmp};
+
+bool ClientChannelNode::GetConnectivityState(grpc_connectivity_state* state) {
+  if (channel()) {
+    *state = grpc_channel_check_connectivity_state(channel(), false);
+  } else {
+    *state = GRPC_CHANNEL_SHUTDOWN;
+  }
+  return true;
+}
+
+grpc_arg ClientChannelNode::CreateArg() {
+  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> MakeClientChannelNode(
+    grpc_channel* channel, size_t channel_tracer_max_nodes) {
+  return RefCountedPtr<ChannelNode>(
+      New<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..7d05cda7da
--- /dev/null
+++ b/src/core/ext/filters/client_channel/client_channel_channelz.h
@@ -0,0 +1,55 @@
+/*
+ *
+ * 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:
+  ClientChannelNode(grpc_channel* channel, size_t channel_tracer_max_nodes)
+      : ChannelNode(channel, channel_tracer_max_nodes) {}
+  virtual ~ClientChannelNode() {}
+
+  // Override this functionality since client_channels have a notion of
+  // channel connectivity.
+  bool GetConnectivityState(grpc_connectivity_state* state) override;
+
+  // Helper to create a channel arg to ensure this type of ChannelNode is
+  // created.
+  static grpc_arg CreateArg();
+};
+
+RefCountedPtr<ChannelNode> MakeClientChannelNode(
+    grpc_channel* channel, size_t channel_tracer_max_nodes);
+
+grpc_arg BlahBlah();
+
+}  // namespace channelz
+}  // namespace grpc_core
+
+#endif /* GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_CLIENT_CHANNEL_CHANNELZ_H */
diff --git a/src/core/ext/transport/chttp2/client/insecure/channel_create.cc b/src/core/ext/transport/chttp2/client/insecure/channel_create.cc
index e6c8c38260..420629f6b7 100644
--- a/src/core/ext/transport/chttp2/client/insecure/channel_create.cc
+++ b/src/core/ext/transport/chttp2/client/insecure/channel_create.cc
@@ -26,6 +26,7 @@
 #include <grpc/support/string_util.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/resolver_registry.h"
 #include "src/core/ext/transport/chttp2/client/authority.h"
 #include "src/core/ext/transport/chttp2/client/chttp2_connector.h"
@@ -92,10 +93,11 @@ grpc_channel* grpc_insecure_channel_create(const char* target,
       "grpc_insecure_channel_create(target=%s, args=%p, reserved=%p)", 3,
       (target, args, reserved));
   GPR_ASSERT(reserved == nullptr);
-  // Add channel arg containing the client channel factory.
-  grpc_arg arg =
-      grpc_client_channel_factory_create_channel_arg(&client_channel_factory);
-  grpc_channel_args* new_args = grpc_channel_args_copy_and_add(args, &arg, 1);
+  grpc_arg args_to_add[] = {
+      grpc_client_channel_factory_create_channel_arg(&client_channel_factory),
+      grpc_core::channelz::ClientChannelNode::CreateArg()};
+  grpc_channel_args* new_args = grpc_channel_args_copy_and_add(
+      args, args_to_add, GPR_ARRAY_SIZE(args_to_add));
   // Create channel.
   grpc_channel* channel = client_channel_factory_create_channel(
       &client_channel_factory, target, GRPC_CLIENT_CHANNEL_TYPE_REGULAR,
diff --git a/src/core/ext/transport/chttp2/client/secure/secure_channel_create.cc b/src/core/ext/transport/chttp2/client/secure/secure_channel_create.cc
index 5ce73a95d7..82691d4e25 100644
--- a/src/core/ext/transport/chttp2/client/secure/secure_channel_create.cc
+++ b/src/core/ext/transport/chttp2/client/secure/secure_channel_create.cc
@@ -26,6 +26,7 @@
 #include <grpc/support/string_util.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/resolver_registry.h"
 #include "src/core/ext/filters/client_channel/uri_parser.h"
 #include "src/core/ext/transport/chttp2/client/chttp2_connector.h"
@@ -213,7 +214,8 @@ grpc_channel* grpc_secure_channel_create(grpc_channel_credentials* creds,
     // credentials.
     grpc_arg args_to_add[] = {
         grpc_client_channel_factory_create_channel_arg(&client_channel_factory),
-        grpc_channel_credentials_to_arg(creds)};
+        grpc_channel_credentials_to_arg(creds),
+        grpc_core::channelz::ClientChannelNode::CreateArg()};
     grpc_channel_args* new_args = grpc_channel_args_copy_and_add(
         args, args_to_add, GPR_ARRAY_SIZE(args_to_add));
     // Create channel.
diff --git a/src/core/lib/channel/channelz.cc b/src/core/lib/channel/channelz.cc
index a49271c3a1..62bc4d4330 100644
--- a/src/core/lib/channel/channelz.cc
+++ b/src/core/lib/channel/channelz.cc
@@ -109,15 +109,8 @@ 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;
-  }
+bool ChannelNode::GetConnectivityState(grpc_connectivity_state* state) {
+  return false;
 }
 
 char* ChannelNode::RenderJSON() {
@@ -140,15 +133,17 @@ char* ChannelNode::RenderJSON() {
   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;
+  grpc_connectivity_state connectivity_state;
+  if (GetConnectivityState(&connectivity_state)) {
+    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;
+  }
   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 +179,11 @@ char* ChannelNode::RenderJSON() {
   return json_str;
 }
 
+RefCountedPtr<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..5c91658dd1 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 {
 
@@ -41,7 +45,7 @@ class ChannelNodePeer;
 class ChannelNode : public RefCounted<ChannelNode> {
  public:
   ChannelNode(grpc_channel* channel, size_t channel_tracer_max_nodes);
-  ~ChannelNode();
+  virtual ~ChannelNode();
 
   void RecordCallStarted();
   void RecordCallFailed() {
@@ -53,6 +57,13 @@ class ChannelNode : public RefCounted<ChannelNode> {
 
   char* RenderJSON();
 
+  // helper for getting connectivity state. It is virtual because it allows
+  // the client_channel code to live in ext/ instead of lib/
+  //
+  // returns true if the channel has a notion of a connectivity state. In that
+  // case it also sets state to the correct connectivity state.
+  virtual bool GetConnectivityState(grpc_connectivity_state* state);
+
   ChannelTrace* trace() { return trace_.get(); }
 
   void set_channel_destroyed() {
@@ -62,13 +73,13 @@ class ChannelNode : public RefCounted<ChannelNode> {
 
   intptr_t channel_uuid() { return channel_uuid_; }
 
+ protected:
+  grpc_channel* channel() { return channel_; }
+
  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 +90,14 @@ class ChannelNode : public RefCounted<ChannelNode> {
   ManualConstructor<ChannelTrace> trace_;
 };
 
+// Creation functions
+
+typedef RefCountedPtr<ChannelNode> (*ChannelNodeCreationFunc)(grpc_channel*,
+                                                              size_t);
+
+RefCountedPtr<ChannelNode> MakeChannelNode(grpc_channel* channel,
+                                           size_t channel_tracer_max_nodes);
+
 }  // namespace channelz
 }  // namespace grpc_core
 
diff --git a/src/core/lib/surface/channel.cc b/src/core/lib/surface/channel.cc
index d5d75fcb2a..a17fde015f 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::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"));