Merge pull request #16774 from ncteisen/channelz-socket

Fix Channelz Defaults and Initialization

include/grpc/impl/codegen/grpc_types.h

@@ -286,7 +286,7 @@ typedef struct {
 /** The grpc_socket_factory instance to create and bind sockets. A pointer. */
 #define GRPC_ARG_SOCKET_FACTORY "grpc.socket_factory"
 /** The maximum number of trace events to keep in the tracer for each channel or
- * subchannel. The default is 10. If set to 0, channel tracing is disabled. */
+ * subchannel. The default is 0. If set to 0, channel tracing is disabled. */
 #define GRPC_ARG_MAX_CHANNEL_TRACE_EVENTS_PER_NODE \
   "grpc.max_channel_trace_events_per_node"
 /** If non-zero, gRPC library will track stats and information at at per channel

src/core/ext/filters/client_channel/subchannel.cc

@@ -388,10 +388,12 @@ grpc_subchannel* grpc_subchannel_create(grpc_connector* connector,
 
   const grpc_arg* arg =
       grpc_channel_args_find(c->args, GRPC_ARG_ENABLE_CHANNELZ);
-  bool channelz_enabled = grpc_channel_arg_get_bool(arg, false);
+  bool channelz_enabled =
+      grpc_channel_arg_get_bool(arg, GRPC_ENABLE_CHANNELZ_DEFAULT);
   arg = grpc_channel_args_find(c->args,
                                GRPC_ARG_MAX_CHANNEL_TRACE_EVENTS_PER_NODE);
-  const grpc_integer_options options = {0, 0, INT_MAX};
+  const grpc_integer_options options = {
+      GRPC_MAX_CHANNEL_TRACE_EVENTS_PER_NODE_DEFAULT, 0, INT_MAX};
   size_t channel_tracer_max_nodes =
       (size_t)grpc_channel_arg_get_integer(arg, options);
   if (channelz_enabled) {

src/core/ext/transport/chttp2/transport/chttp2_transport.cc

@@ -239,6 +239,7 @@ static bool read_channel_args(grpc_chttp2_transport* t,
                               const grpc_channel_args* channel_args,
                               bool is_client) {
   bool enable_bdp = true;
+  bool channelz_enabled = GRPC_ENABLE_CHANNELZ_DEFAULT;
   size_t i;
   int j;
 
@@ -341,8 +342,8 @@ static bool read_channel_args(grpc_chttp2_transport* t,
       }
     } else if (0 ==
                strcmp(channel_args->args[i].key, GRPC_ARG_ENABLE_CHANNELZ)) {
-      t->channelz_socket =
-          grpc_core::MakeRefCounted<grpc_core::channelz::SocketNode>();
+      channelz_enabled = grpc_channel_arg_get_bool(
+          &channel_args->args[i], GRPC_ENABLE_CHANNELZ_DEFAULT);
     } else {
       static const struct {
         const char* channel_arg_name;
@@ -393,6 +394,10 @@ static bool read_channel_args(grpc_chttp2_transport* t,
       }
     }
   }
+  if (channelz_enabled) {
+    t->channelz_socket =
+        grpc_core::MakeRefCounted<grpc_core::channelz::SocketNode>();
+  }
   return enable_bdp;
 }
 

src/core/lib/channel/channelz.h

@@ -39,6 +39,15 @@
 #define GRPC_ARG_CHANNELZ_CHANNEL_IS_INTERNAL_CHANNEL \
   "grpc.channelz_channel_is_internal_channel"
 
+/** This is the default value for whether or not to enable channelz. If
+ * GRPC_ARG_ENABLE_CHANNELZ is set, it will override this default value. */
+#define GRPC_ENABLE_CHANNELZ_DEFAULT false
+
+/** This is the default value for number of trace events per node. If
+ * GRPC_ARG_MAX_CHANNEL_TRACE_EVENTS_PER_NODE is set, it will override this
+ * default value. */
+#define GRPC_MAX_CHANNEL_TRACE_EVENTS_PER_NODE_DEFAULT 0
+
 namespace grpc_core {
 namespace channelz {
 

src/core/lib/surface/channel.cc

@@ -103,7 +103,7 @@ grpc_channel* grpc_channel_create_with_builder(
   channel->target = target;
   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;
+  bool channelz_enabled = GRPC_ENABLE_CHANNELZ_DEFAULT;
   bool internal_channel = false;
   // this creates the default ChannelNode. Different types of channels may
   // override this to ensure a correct ChannelNode is created.
@@ -144,13 +144,15 @@ grpc_channel* grpc_channel_create_with_builder(
                            GRPC_ARG_MAX_CHANNEL_TRACE_EVENTS_PER_NODE)) {
       GPR_ASSERT(channel_tracer_max_nodes == 0);
       // max_nodes defaults to 0 (which is off), clamped between 0 and INT_MAX
-      const grpc_integer_options options = {0, 0, INT_MAX};
+      const grpc_integer_options options = {
+          GRPC_MAX_CHANNEL_TRACE_EVENTS_PER_NODE_DEFAULT, 0, INT_MAX};
       channel_tracer_max_nodes =
           (size_t)grpc_channel_arg_get_integer(&args->args[i], options);
     } else if (0 == strcmp(args->args[i].key, GRPC_ARG_ENABLE_CHANNELZ)) {
       // channelz will not be enabled by default until all concerns in
       // https://github.com/grpc/grpc/issues/15986 are addressed.
-      channelz_enabled = grpc_channel_arg_get_bool(&args->args[i], false);
+      channelz_enabled = grpc_channel_arg_get_bool(
+          &args->args[i], GRPC_ENABLE_CHANNELZ_DEFAULT);
     } else if (0 == strcmp(args->args[i].key,
                            GRPC_ARG_CHANNELZ_CHANNEL_NODE_CREATION_FUNC)) {
       GPR_ASSERT(args->args[i].type == GRPC_ARG_POINTER);

src/core/lib/surface/server.cc

@@ -1008,11 +1008,11 @@ grpc_server* grpc_server_create(const grpc_channel_args* args, void* reserved) {
   server->channel_args = grpc_channel_args_copy(args);
 
   const grpc_arg* arg = grpc_channel_args_find(args, GRPC_ARG_ENABLE_CHANNELZ);
-  if (grpc_channel_arg_get_bool(arg, false)) {
+  if (grpc_channel_arg_get_bool(arg, GRPC_ENABLE_CHANNELZ_DEFAULT)) {
     arg = grpc_channel_args_find(args,
                                  GRPC_ARG_MAX_CHANNEL_TRACE_EVENTS_PER_NODE);
-    size_t trace_events_per_node =
-        grpc_channel_arg_get_integer(arg, {0, 0, INT_MAX});
+    size_t trace_events_per_node = grpc_channel_arg_get_integer(
+        arg, {GRPC_MAX_CHANNEL_TRACE_EVENTS_PER_NODE_DEFAULT, 0, INT_MAX});
     server->channelz_server =
         grpc_core::MakeRefCounted<grpc_core::channelz::ServerNode>(
             trace_events_per_node);