XdsClient: inject version string at construction time (#31812)

* Decouple XdsClient from grpc_version_string

* Updated tests too

* clang format fixes

src/core/ext/xds/xds_api.cc

@@ -45,7 +45,6 @@
 #include "upb/upb.h"
 #include "upb/upb.hpp"
 
-#include <grpc/grpc.h>
 #include <grpc/impl/codegen/gpr_types.h>
 #include <grpc/status.h>
 #include <grpc/support/log.h>
@@ -58,36 +57,15 @@
 
 namespace grpc_core {
 
-// If gRPC is built with -DGRPC_XDS_USER_AGENT_NAME_SUFFIX="...", that string
-// will be appended to the user agent name reported to the xDS server.
-#ifdef GRPC_XDS_USER_AGENT_NAME_SUFFIX
-#define GRPC_XDS_USER_AGENT_NAME_SUFFIX_STRING \
-  " " GRPC_XDS_USER_AGENT_NAME_SUFFIX
-#else
-#define GRPC_XDS_USER_AGENT_NAME_SUFFIX_STRING ""
-#endif
-
-// If gRPC is built with -DGRPC_XDS_USER_AGENT_VERSION_SUFFIX="...", that string
-// will be appended to the user agent version reported to the xDS server.
-#ifdef GRPC_XDS_USER_AGENT_VERSION_SUFFIX
-#define GRPC_XDS_USER_AGENT_VERSION_SUFFIX_STRING \
-  " " GRPC_XDS_USER_AGENT_VERSION_SUFFIX
-#else
-#define GRPC_XDS_USER_AGENT_VERSION_SUFFIX_STRING ""
-#endif
-
 XdsApi::XdsApi(XdsClient* client, TraceFlag* tracer,
-               const XdsBootstrap::Node* node, upb::SymbolTable* symtab)
+               const XdsBootstrap::Node* node, upb::SymbolTable* symtab,
+               std::string user_agent_name, std::string user_agent_version)
     : client_(client),
       tracer_(tracer),
       node_(node),
       symtab_(symtab),
-      user_agent_name_(absl::StrCat("gRPC C-core ", GPR_PLATFORM_STRING,
-                                    GRPC_XDS_USER_AGENT_NAME_SUFFIX_STRING)),
-      user_agent_version_(
-          absl::StrCat("C-core ", grpc_version_string(),
-                       GRPC_XDS_USER_AGENT_NAME_SUFFIX_STRING,
-                       GRPC_XDS_USER_AGENT_VERSION_SUFFIX_STRING)) {}
+      user_agent_name_(std::move(user_agent_name)),
+      user_agent_version_(std::move(user_agent_version)) {}
 
 namespace {
 

src/core/ext/xds/xds_api.h

@@ -147,7 +147,8 @@ class XdsApi {
                 "");
 
   XdsApi(XdsClient* client, TraceFlag* tracer, const XdsBootstrap::Node* node,
-         upb::SymbolTable* symtab);
+         upb::SymbolTable* symtab, std::string user_agent_name,
+         std::string user_agent_version);
 
   // Creates an ADS request.
   std::string CreateAdsRequest(absl::string_view type_url,

src/core/ext/xds/xds_client.cc

@@ -1463,6 +1463,7 @@ XdsClient::XdsClient(
     std::unique_ptr<XdsBootstrap> bootstrap,
     OrphanablePtr<XdsTransportFactory> transport_factory,
     std::shared_ptr<grpc_event_engine::experimental::EventEngine> engine,
+    std::string user_agent_name, std::string user_agent_version,
     Duration resource_request_timeout)
     : DualRefCounted<XdsClient>(
           GRPC_TRACE_FLAG_ENABLED(grpc_xds_client_refcount_trace) ? "XdsClient"
@@ -1471,7 +1472,8 @@ XdsClient::XdsClient(
       transport_factory_(std::move(transport_factory)),
       request_timeout_(resource_request_timeout),
       xds_federation_enabled_(XdsFederationEnabled()),
-      api_(this, &grpc_xds_client_trace, bootstrap_->node(), &symtab_),
+      api_(this, &grpc_xds_client_trace, bootstrap_->node(), &symtab_,
+           std::move(user_agent_name), std::move(user_agent_version)),
       engine_(std::move(engine)) {
   if (GRPC_TRACE_FLAG_ENABLED(grpc_xds_client_trace)) {
     gpr_log(GPR_INFO, "[xds_client %p] creating xds client", this);

src/core/ext/xds/xds_client.h

@@ -75,6 +75,7 @@ class XdsClient : public DualRefCounted<XdsClient> {
       std::unique_ptr<XdsBootstrap> bootstrap,
       OrphanablePtr<XdsTransportFactory> transport_factory,
       std::shared_ptr<grpc_event_engine::experimental::EventEngine> engine,
+      std::string user_agent_name, std::string user_agent_version,
       Duration resource_request_timeout = Duration::Seconds(15));
   ~XdsClient() override;
 

src/core/ext/xds/xds_client_grpc.cc

@@ -57,6 +57,24 @@
 
 namespace grpc_core {
 
+// If gRPC is built with -DGRPC_XDS_USER_AGENT_NAME_SUFFIX="...", that string
+// will be appended to the user agent name reported to the xDS server.
+#ifdef GRPC_XDS_USER_AGENT_NAME_SUFFIX
+#define GRPC_XDS_USER_AGENT_NAME_SUFFIX_STRING \
+  " " GRPC_XDS_USER_AGENT_NAME_SUFFIX
+#else
+#define GRPC_XDS_USER_AGENT_NAME_SUFFIX_STRING ""
+#endif
+
+// If gRPC is built with -DGRPC_XDS_USER_AGENT_VERSION_SUFFIX="...", that string
+// will be appended to the user agent version reported to the xDS server.
+#ifdef GRPC_XDS_USER_AGENT_VERSION_SUFFIX
+#define GRPC_XDS_USER_AGENT_VERSION_SUFFIX_STRING \
+  " " GRPC_XDS_USER_AGENT_VERSION_SUFFIX
+#else
+#define GRPC_XDS_USER_AGENT_VERSION_SUFFIX_STRING ""
+#endif
+
 //
 // GrpcXdsClient
 //
@@ -157,6 +175,11 @@ GrpcXdsClient::GrpcXdsClient(std::unique_ptr<GrpcXdsBootstrap> bootstrap,
     : XdsClient(
           std::move(bootstrap), MakeOrphanable<GrpcXdsTransportFactory>(args),
           grpc_event_engine::experimental::GetDefaultEventEngine(),
+          absl::StrCat("gRPC C-core ", GPR_PLATFORM_STRING,
+                       GRPC_XDS_USER_AGENT_NAME_SUFFIX_STRING),
+          absl::StrCat("C-core ", grpc_version_string(),
+                       GRPC_XDS_USER_AGENT_NAME_SUFFIX_STRING,
+                       GRPC_XDS_USER_AGENT_VERSION_SUFFIX_STRING),
           std::max(Duration::Zero(),
                    args.GetDurationFromIntMillis(
                            GRPC_ARG_XDS_RESOURCE_DOES_NOT_EXIST_TIMEOUT_MS)

test/core/xds/xds_client_fuzzer.cc

@@ -62,7 +62,8 @@ class Fuzzer {
     transport_factory_ = transport_factory.get();
     xds_client_ = MakeRefCounted<XdsClient>(
         std::move(*bootstrap), std::move(transport_factory),
-        grpc_event_engine::experimental::GetDefaultEventEngine());
+        grpc_event_engine::experimental::GetDefaultEventEngine(), "foo agent",
+        "foo version");
   }
 
   void Act(const xds_client_fuzzer::Action& action) {

test/core/xds/xds_client_test.cc

@@ -39,7 +39,6 @@
 #include "gtest/gtest.h"
 #include "upb/def.h"
 
-#include <grpc/grpc.h>
 #include <grpc/support/log.h>
 #include <grpcpp/impl/codegen/config_protobuf.h>
 
@@ -569,8 +568,8 @@ class XdsClientTest : public ::testing::Test {
     transport_factory_ = transport_factory->Ref();
     xds_client_ = MakeRefCounted<XdsClient>(
         bootstrap_builder.Build(), std::move(transport_factory),
-        grpc_event_engine::experimental::GetDefaultEventEngine(),
-        resource_request_timeout * grpc_test_slowdown_factor());
+        grpc_event_engine::experimental::GetDefaultEventEngine(), "foo agent",
+        "foo version", resource_request_timeout * grpc_test_slowdown_factor());
   }
 
   // Starts and cancels a watch for a Foo resource.
@@ -721,12 +720,9 @@ class XdsClientTest : public ::testing::Test {
           << Json{xds_client_->bootstrap().node()->metadata()}.Dump()
           << "\nactual: " << metadata_json->Dump();
     }
-    // These are hard-coded by XdsClient.
-    EXPECT_EQ(request.node().user_agent_name(),
-              absl::StrCat("gRPC C-core ", GPR_PLATFORM_STRING))
+    EXPECT_EQ(request.node().user_agent_name(), "foo agent")
         << location.file() << ":" << location.line();
-    EXPECT_EQ(request.node().user_agent_version(),
-              absl::StrCat("C-core ", grpc_version_string()))
+    EXPECT_EQ(request.node().user_agent_version(), "foo version")
         << location.file() << ":" << location.line();
   }
 

test/core/xds/xds_cluster_resource_type_test.cc

@@ -111,7 +111,8 @@ class XdsClusterTest : public ::testing::Test {
     }
     return MakeRefCounted<XdsClient>(std::move(*bootstrap),
                                      /*transport_factory=*/nullptr,
-                                     /*event_engine=*/nullptr);
+                                     /*event_engine=*/nullptr, "foo agent",
+                                     "foo version");
   }
 
   RefCountedPtr<XdsClient> xds_client_;

test/core/xds/xds_common_types_test.cc

@@ -103,7 +103,8 @@ class XdsCommonTypesTest : public ::testing::Test {
     }
     return MakeRefCounted<XdsClient>(std::move(*bootstrap),
                                      /*transport_factory=*/nullptr,
-                                     /*event_engine=*/nullptr);
+                                     /*event_engine=*/nullptr, "foo agent",
+                                     "foo version");
   }
 
   RefCountedPtr<XdsClient> xds_client_;

test/core/xds/xds_endpoint_resource_type_test.cc

@@ -89,7 +89,8 @@ class XdsEndpointTest : public ::testing::Test {
     }
     return MakeRefCounted<XdsClient>(std::move(*bootstrap),
                                      /*transport_factory=*/nullptr,
-                                     /*event_engine=*/nullptr);
+                                     /*event_engine=*/nullptr, "foo agent",
+                                     "foo version");
   }
 
   RefCountedPtr<XdsClient> xds_client_;

test/core/xds/xds_listener_resource_type_test.cc

@@ -116,7 +116,8 @@ class XdsListenerTest : public ::testing::Test {
     }
     return MakeRefCounted<XdsClient>(std::move(*bootstrap),
                                      /*transport_factory=*/nullptr,
-                                     /*event_engine=*/nullptr);
+                                     /*event_engine=*/nullptr, "foo agent",
+                                     "foo version");
   }
 
   RefCountedPtr<XdsClient> xds_client_;

test/core/xds/xds_route_config_resource_type_test.cc

@@ -103,7 +103,8 @@ class XdsRouteConfigTest : public ::testing::Test {
     }
     return MakeRefCounted<XdsClient>(std::move(*bootstrap),
                                      /*transport_factory=*/nullptr,
-                                     /*event_engine=*/nullptr);
+                                     /*event_engine=*/nullptr, "foo agent",
+                                     "foo version");
   }
 
   RefCountedPtr<XdsClient> xds_client_;