[channel] canonify target and set channel arg in only one place (#36134)

Channel target will now be reported in canonified form (e.g., in channelz).

Closes #36134

COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/36134 from markdroth:channel_target 22e49e73f8
PiperOrigin-RevId: 616911536

BUILD

@@ -1790,6 +1790,7 @@ grpc_cc_library(
         "channel",
         "channel_arg_names",
         "channelz",
+        "config",
         "gpr",
         "grpc_base",
         "grpc_public_hdrs",

src/core/ext/transport/binder/client/channel_create.cc

@@ -121,33 +121,21 @@ std::shared_ptr<grpc::Channel> CreateCustomBinderChannel(
   grpc_binder::TryEstablishConnectionWithUri(static_cast<JNIEnv*>(jni_env_void),
                                              context, uri, connection_id);
 
-  grpc_channel_args channel_args;
-  args.SetChannelArgs(&channel_args);
+  grpc_binder::GetSecurityPolicySetting()->Set(connection_id, security_policy);
 
   // Set server URI to a URI that contains connection id. The URI will be used
   // by subchannel connector to obtain correct endpoint binder from
   // `EndpointBinderPool`.
-  grpc_channel_args* new_args;
-  {
-    string server_uri = "binder:" + connection_id;
-    grpc_arg server_uri_arg =
-        grpc_channel_arg_string_create(const_cast<char*>(GRPC_ARG_SERVER_URI),
-                                       const_cast<char*>(server_uri.c_str()));
-    const char* to_remove[] = {GRPC_ARG_SERVER_URI};
-    new_args = grpc_channel_args_copy_and_add_and_remove(
-        &channel_args, to_remove, 1, &server_uri_arg, 1);
-  }
+  string server_uri = "binder:" + connection_id;
 
-  grpc_binder::GetSecurityPolicySetting()->Set(connection_id, security_policy);
+  grpc_channel_args channel_args;
+  args.SetChannelArgs(&channel_args);
 
-  auto channel = CreateChannelInternal(
-      "", grpc::internal::CreateClientBinderChannelImpl(new_args),
+  return CreateChannelInternal(
+      "",
+      grpc::internal::CreateClientBinderChannelImpl(server_uri, &channel_args),
       std::vector<
           std::unique_ptr<experimental::ClientInterceptorFactoryInterface>>());
-
-  grpc_channel_args_destroy(new_args);
-
-  return channel;
 }
 
 bool InitializeBinderChannelJavaClass(void* jni_env_void) {

src/core/ext/transport/binder/client/channel_create_impl.cc

@@ -65,7 +65,8 @@ grpc_channel* CreateDirectBinderChannelImplForTesting(
   return channel->release()->c_ptr();
 }
 
-grpc_channel* CreateClientBinderChannelImpl(const grpc_channel_args* args) {
+grpc_channel* CreateClientBinderChannelImpl(std::string target,
+                                            const grpc_channel_args* args) {
   grpc_core::ExecCtx exec_ctx;
 
   gpr_once_init(&g_factory_once, FactoryInit);
@@ -75,14 +76,12 @@ grpc_channel* CreateClientBinderChannelImpl(const grpc_channel_args* args) {
                           .PreconditionChannelArgs(args)
                           .SetObject(g_factory);
 
-  auto channel =
-      grpc_core::ChannelCreate("binder_channel_target_placeholder",
-                               channel_args, GRPC_CLIENT_CHANNEL, nullptr);
+  auto channel = grpc_core::ChannelCreate(target, channel_args,
+                                          GRPC_CLIENT_CHANNEL, nullptr);
 
   if (!channel.ok()) {
     return grpc_lame_client_channel_create(
-        "binder_channel_target_placeholder",
-        static_cast<grpc_status_code>(channel.status().code()),
+        target.c_str(), static_cast<grpc_status_code>(channel.status().code()),
         "Failed to create binder channel");
   }
 

src/core/ext/transport/binder/client/channel_create_impl.h

@@ -34,7 +34,8 @@ grpc_channel* CreateDirectBinderChannelImplForTesting(
         security_policy);
 
 // Creates a GRPC_CLIENT_CHANNEL channel
-grpc_channel* CreateClientBinderChannelImpl(const grpc_channel_args* args);
+grpc_channel* CreateClientBinderChannelImpl(std::string target,
+                                            const grpc_channel_args* args);
 
 }  // namespace internal
 }  // namespace grpc

src/core/ext/transport/chaotic_good/client/chaotic_good_connector.cc

@@ -370,15 +370,11 @@ grpc_channel* grpc_chaotic_good_channel_create(const char* target,
   grpc_channel* channel = nullptr;
   grpc_error_handle error;
   // Create channel.
-  std::string canonical_target = grpc_core::CoreConfiguration::Get()
-                                     .resolver_registry()
-                                     .AddDefaultPrefixIfNeeded(target);
   auto r = grpc_core::ChannelCreate(
       target,
       grpc_core::CoreConfiguration::Get()
           .channel_args_preconditioning()
           .PreconditionChannelArgs(args)
-          .Set(GRPC_ARG_SERVER_URI, canonical_target)
           .SetObject(
               grpc_core::NoDestructSingleton<
                   grpc_core::chaotic_good::ChaoticGoodChannelFactory>::Get()),

src/core/ext/transport/chttp2/client/chttp2_connector.cc

@@ -314,12 +314,7 @@ absl::StatusOr<OrphanablePtr<Channel>> CreateChannel(const char* target,
     gpr_log(GPR_ERROR, "cannot create channel with NULL target name");
     return absl::InvalidArgumentError("channel target is NULL");
   }
-  // Add channel arg containing the server URI.
-  std::string canonical_target =
-      CoreConfiguration::Get().resolver_registry().AddDefaultPrefixIfNeeded(
-          target);
-  return ChannelCreate(target, args.Set(GRPC_ARG_SERVER_URI, canonical_target),
-                       GRPC_CLIENT_CHANNEL, nullptr);
+  return ChannelCreate(target, args, GRPC_CLIENT_CHANNEL, nullptr);
 }
 
 }  // namespace

src/core/lib/surface/channel_create.cc

@@ -22,6 +22,7 @@
 
 #include "src/core/lib/channel/channel_args.h"
 #include "src/core/lib/channel/channelz.h"
+#include "src/core/lib/config/core_configuration.h"
 #include "src/core/lib/debug/stats.h"
 #include "src/core/lib/debug/stats_data.h"
 #include "src/core/lib/surface/channel.h"
@@ -34,6 +35,14 @@ absl::StatusOr<OrphanablePtr<Channel>> ChannelCreate(
     std::string target, ChannelArgs args,
     grpc_channel_stack_type channel_stack_type, Transport* optional_transport) {
   global_stats().IncrementClientChannelsCreated();
+  // For client channels, canonify target string and add channel arg.
+  // Note: We don't do this for direct channels or lame channels.
+  if (channel_stack_type == GRPC_CLIENT_CHANNEL) {
+    target =
+        CoreConfiguration::Get().resolver_registry().AddDefaultPrefixIfNeeded(
+            target);
+    args = args.Set(GRPC_ARG_SERVER_URI, target);
+  }
   // Set default authority if needed.
   if (!args.GetString(GRPC_ARG_DEFAULT_AUTHORITY).has_value()) {
     auto ssl_override = args.GetString(GRPC_SSL_TARGET_NAME_OVERRIDE_ARG);

src/php/tests/unit_tests/CallInvokerTest.php

@@ -194,7 +194,7 @@ class CallInvokerTest extends \PHPUnit\Framework\TestCase
         $stub = new \Grpc\BaseStub('localhost:50051',
           ['credentials' => \Grpc\ChannelCredentials::createInsecure(),
             'grpc_call_invoker' => $call_invoker]);
-        $this->assertEquals($call_invoker->getChannel()->getTarget(), 'localhost:50050');
+        $this->assertEquals($call_invoker->getChannel()->getTarget(), 'dns:///localhost:50050');
         $call_invoker->getChannel()->close();
     }
 

src/python/grpcio_tests/tests_aio/channelz/channelz_servicer_test.py

@@ -94,7 +94,7 @@ class _ChannelServerPair:
             channelz_pb2.GetTopChannelsRequest(start_channel_id=0)
         )
         for channel in resp.channel:
-            if channel.data.target == self.address:
+            if channel.data.target == "dns:///" + self.address:
                 self.channel_ref_id = channel.ref.channel_id
 
         resp = await channelz_stub.GetServers(