Merge remote-tracking branch 'upstream/master' into cert_provider_remove_pollset_set

.github/workflows/pr-auto-fix.yaml

@@ -48,7 +48,7 @@ jobs:
         with:
           script: |
             // If you'd like not to run this code on your commits, add your github user id here:
-            NO_AUTOFIX_USERS = []
+            NO_AUTOFIX_USERS = ["copybara-service"]
             const { owner, repo } = context.repo
             if (NO_AUTOFIX_USERS.includes(context.actor)) {
               console.log('Cancelling');

include/grpc/event_engine/event_engine.h

@@ -79,7 +79,7 @@ namespace experimental {
 ///
 ///
 /// Blocking EventEngine Callbacks
-/// -----------------------------
+/// ------------------------------
 ///
 /// Doing blocking work in EventEngine callbacks is generally not advisable.
 /// While gRPC's default EventEngine implementations have some capacity to scale
@@ -90,6 +90,15 @@ namespace experimental {
 /// *Best Practice* : Occasional blocking work may be fine, but we do not
 /// recommend running a mostly blocking workload in EventEngine threads.
 ///
+///
+/// Thread-safety guarantees
+/// ------------------------
+///
+/// All EventEngine methods are guaranteed to be thread-safe, no external
+/// synchronization is required to call any EventEngine method. Please note that
+/// this does not apply to application callbacks, which may be run concurrently;
+/// application state synchronization must be managed by the application.
+///
 ////////////////////////////////////////////////////////////////////////////////
 class EventEngine : public std::enable_shared_from_this<EventEngine> {
  public:
@@ -405,8 +414,8 @@ class EventEngine : public std::enable_shared_from_this<EventEngine> {
 
   /// Asynchronously executes a task as soon as possible.
   ///
-  /// \a Closures scheduled with \a Run cannot be cancelled. The \a closure will
-  /// not be deleted after it has been run, ownership remains with the caller.
+  /// \a Closures passed to \a Run cannot be cancelled. The \a closure will not
+  /// be deleted after it has been run, ownership remains with the caller.
   ///
   /// Implementations must not execute the closure in the calling thread before
   /// \a Run returns. For example, if the caller must release a lock before the
@@ -415,9 +424,9 @@ class EventEngine : public std::enable_shared_from_this<EventEngine> {
   virtual void Run(Closure* closure) = 0;
   /// Asynchronously executes a task as soon as possible.
   ///
-  /// \a Closures scheduled with \a Run cannot be cancelled. Unlike the
-  /// overloaded \a Closure alternative, the absl::AnyInvocable version's \a
-  /// closure will be deleted by the EventEngine after the closure has been run.
+  /// \a Closures passed to \a Run cannot be cancelled. Unlike the overloaded \a
+  /// Closure alternative, the absl::AnyInvocable version's \a closure will be
+  /// deleted by the EventEngine after the closure has been run.
   ///
   /// This version of \a Run may be less performant than the \a Closure version
   /// in some scenarios. This overload is useful in situations where performance
@@ -453,13 +462,12 @@ class EventEngine : public std::enable_shared_from_this<EventEngine> {
                               absl::AnyInvocable<void()> closure) = 0;
   /// Request cancellation of a task.
   ///
-  /// If the associated closure has already been scheduled to run, it will not
-  /// be cancelled, and this function will return false.
+  /// If the associated closure cannot be cancelled for any reason, this
+  /// function will return false.
   ///
-  /// If the associated closure has not been scheduled to run, it will be
-  /// cancelled, and this method will return true. The associated
-  /// absl::AnyInvocable or \a Closure* will not be called. If the closure type
-  /// was an absl::AnyInvocable, it will be destroyed before the method returns.
+  /// If the associated closure can be cancelled, the associated callback will
+  /// never be run, and this method will return true. If the callback type was
+  /// an absl::AnyInvocable, it will be destroyed before the method returns.
   virtual bool Cancel(TaskHandle handle) = 0;
 };
 

include/grpc/grpc_security.h

@@ -894,7 +894,10 @@ GRPCAPI void grpc_tls_credentials_options_set_identity_cert_name(
 GRPCAPI void grpc_tls_credentials_options_set_cert_request_type(
     grpc_tls_credentials_options* options,
     grpc_ssl_client_certificate_request_type type);
-/**
+
+/** Deprecated in favor of grpc_tls_credentials_options_set_crl_provider. The
+ * crl provider interface provides a significantly more flexible approach to
+ * using CRLs. See gRFC A69 for details.
  * EXPERIMENTAL API - Subject to change
  *
  * If set, gRPC will read all hashed x.509 CRL files in the directory and

include/grpcpp/security/tls_credentials_options.h

@@ -99,8 +99,9 @@ class TlsCredentialsOptions {
   // verifiers other than the host name verifier is used.
   void set_check_call_host(bool check_call_host);
 
-  // TODO(zhenlian): This is an experimental API is likely to change in the
-  // future. Before de-experiementalizing, verify the API is up to date.
+  // Deprecated in favor of set_crl_provider. The
+  // crl provider interface provides a significantly more flexible approach to
+  // using CRLs. See gRFC A69 for details.
   // If set, gRPC will read all hashed x.509 CRL files in the directory and
   // enforce the CRL files on all TLS handshakes. Only supported for OpenSSL
   // version > 1.1.

src/core/lib/event_engine/posix_engine/posix_engine_listener_utils.cc

@@ -25,6 +25,8 @@
 
 #include "absl/cleanup/cleanup.h"
 #include "absl/status/status.h"
+#include "absl/strings/str_cat.h"
+#include "absl/strings/str_replace.h"
 
 #include <grpc/event_engine/event_engine.h>
 #include <grpc/support/log.h>
@@ -44,8 +46,6 @@
 #include <netinet/in.h>  // IWYU pragma: keep
 #include <sys/socket.h>  // IWYU pragma: keep
 #include <unistd.h>      // IWYU pragma: keep
-
-#include "absl/strings/str_cat.h"
 #endif
 
 namespace grpc_event_engine {
@@ -176,8 +176,16 @@ absl::Status PrepareSocket(const PosixTcpOptions& options,
       GRPC_FD_SERVER_LISTENER_USAGE, options));
 
   if (bind(fd, socket.addr.address(), socket.addr.size()) < 0) {
+    auto sockaddr_str = ResolvedAddressToString(socket.addr);
+    if (!sockaddr_str.ok()) {
+      gpr_log(GPR_ERROR, "Could not convert sockaddr to string: %s",
+              sockaddr_str.status().ToString().c_str());
+      sockaddr_str = "<unparsable>";
+    }
+    sockaddr_str = absl::StrReplaceAll(*sockaddr_str, {{"\0", "@"}});
     return absl::FailedPreconditionError(
-        absl::StrCat("Error in bind: ", std::strerror(errno)));
+        absl::StrCat("Error in bind for address '", *sockaddr_str,
+                     "': ", std::strerror(errno)));
   }
 
   if (listen(fd, GetMaxAcceptQueueSize()) < 0) {

src/core/lib/event_engine/posix_engine/tcp_socket_utils.cc

@@ -75,6 +75,7 @@ int AdjustValue(int default_value, int min_value, int max_value,
   return *actual_value;
 }
 
+#ifdef GRPC_POSIX_SOCKET_UTILS_COMMON
 // The default values for TCP_USER_TIMEOUT are currently configured to be in
 // line with the default values of KEEPALIVE_TIMEOUT as proposed in
 // https://github.com/grpc/proposal/blob/master/A18-tcp-user-timeout.md */
@@ -83,8 +84,6 @@ int kDefaultServerUserTimeoutMs = 20000;
 bool kDefaultClientUserTimeoutEnabled = false;
 bool kDefaultServerUserTimeoutEnabled = true;
 
-#ifdef GRPC_POSIX_SOCKET_UTILS_COMMON
-
 absl::Status ErrorForFd(
     int fd, const experimental::EventEngine::ResolvedAddress& addr) {
   if (fd >= 0) return absl::OkStatus();

src/core/lib/security/credentials/external/aws_external_account_credentials.cc

@@ -525,4 +525,8 @@ void AwsExternalAccountCredentials::FinishRetrieveSubjectToken(
   }
 }
 
+absl::string_view AwsExternalAccountCredentials::CredentialSourceType() {
+  return "aws";
+}
+
 }  // namespace grpc_core

src/core/lib/security/credentials/external/aws_external_account_credentials.h

@@ -24,6 +24,8 @@
 #include <string>
 #include <vector>
 
+#include "absl/strings/string_view.h"
+
 #include "src/core/lib/gprpp/orphanable.h"
 #include "src/core/lib/gprpp/ref_counted_ptr.h"
 #include "src/core/lib/http/httpcli.h"
@@ -72,6 +74,8 @@ class AwsExternalAccountCredentials final : public ExternalAccountCredentials {
 
   void AddMetadataRequestHeaders(grpc_http_request* request);
 
+  absl::string_view CredentialSourceType() override;
+
   std::string audience_;
   OrphanablePtr<HttpRequest> http_request_;
 

src/core/lib/security/credentials/external/external_account_credentials.cc

@@ -26,6 +26,7 @@
 
 #include "absl/status/status.h"
 #include "absl/status/statusor.h"
+#include "absl/strings/escaping.h"
 #include "absl/strings/match.h"
 #include "absl/strings/numbers.h"
 #include "absl/strings/str_cat.h"
@@ -53,7 +54,6 @@
 #include "src/core/lib/security/credentials/external/file_external_account_credentials.h"
 #include "src/core/lib/security/credentials/external/url_external_account_credentials.h"
 #include "src/core/lib/security/util/json_util.h"
-#include "src/core/lib/slice/b64.h"
 #include "src/core/lib/uri/uri_parser.h"
 
 #define EXTERNAL_ACCOUNT_CREDENTIALS_GRANT_TYPE \
@@ -271,6 +271,20 @@ std::string ExternalAccountCredentials::debug_string() {
                          grpc_oauth2_token_fetcher_credentials::debug_string());
 }
 
+std::string ExternalAccountCredentials::MetricsHeaderValue() {
+  return absl::StrFormat(
+      "gl-cpp/unknown auth/%s google-byoid-sdk source/%s sa-impersonation/%v "
+      "config-lifetime/%v",
+      grpc_version_string(), CredentialSourceType(),
+      !options_.service_account_impersonation_url.empty(),
+      options_.service_account_impersonation.token_lifetime_seconds !=
+          IMPERSONATED_CRED_DEFAULT_LIFETIME_IN_SECONDS);
+}
+
+absl::string_view ExternalAccountCredentials::CredentialSourceType() {
+  return "unknown";
+}
+
 // The token fetching flow:
 // 1. Retrieve subject token - Subclass's RetrieveSubjectToken() gets called
 // and the subject token is received in OnRetrieveSubjectTokenInternal().
@@ -317,27 +331,21 @@ void ExternalAccountCredentials::ExchangeToken(
   }
   grpc_http_request request;
   memset(&request, 0, sizeof(grpc_http_request));
-  grpc_http_header* headers = nullptr;
-  if (!options_.client_id.empty() && !options_.client_secret.empty()) {
-    request.hdr_count = 2;
-    headers = static_cast<grpc_http_header*>(
-        gpr_malloc(sizeof(grpc_http_header) * request.hdr_count));
-    headers[0].key = gpr_strdup("Content-Type");
-    headers[0].value = gpr_strdup("application/x-www-form-urlencoded");
+  const bool add_authorization_header =
+      !options_.client_id.empty() && !options_.client_secret.empty();
+  request.hdr_count = add_authorization_header ? 3 : 2;
+  auto* headers = static_cast<grpc_http_header*>(
+      gpr_malloc(sizeof(grpc_http_header) * request.hdr_count));
+  headers[0].key = gpr_strdup("Content-Type");
+  headers[0].value = gpr_strdup("application/x-www-form-urlencoded");
+  headers[1].key = gpr_strdup("x-goog-api-client");
+  headers[1].value = gpr_strdup(MetricsHeaderValue().c_str());
+  if (add_authorization_header) {
     std::string raw_cred =
         absl::StrFormat("%s:%s", options_.client_id, options_.client_secret);
-    char* encoded_cred =
-        grpc_base64_encode(raw_cred.c_str(), raw_cred.length(), 0, 0);
-    std::string str = absl::StrFormat("Basic %s", std::string(encoded_cred));
-    headers[1].key = gpr_strdup("Authorization");
-    headers[1].value = gpr_strdup(str.c_str());
-    gpr_free(encoded_cred);
-  } else {
-    request.hdr_count = 1;
-    headers = static_cast<grpc_http_header*>(
-        gpr_malloc(sizeof(grpc_http_header) * request.hdr_count));
-    headers[0].key = gpr_strdup("Content-Type");
-    headers[0].value = gpr_strdup("application/x-www-form-urlencoded");
+    std::string str = absl::StrFormat("Basic %s", absl::Base64Escape(raw_cred));
+    headers[2].key = gpr_strdup("Authorization");
+    headers[2].value = gpr_strdup(str.c_str());
   }
   request.hdrs = headers;
   std::vector<std::string> body_parts;

src/core/lib/security/credentials/external/external_account_credentials.h

@@ -101,6 +101,10 @@ class ExternalAccountCredentials
       HTTPRequestContext* ctx, const Options& options,
       std::function<void(std::string, grpc_error_handle)> cb) = 0;
 
+  virtual absl::string_view CredentialSourceType();
+
+  std::string MetricsHeaderValue();
+
  private:
   // This method implements the common token fetch logic and it will be called
   // when grpc_oauth2_token_fetcher_credentials request a new access token.

src/core/lib/security/credentials/external/file_external_account_credentials.cc

@@ -137,4 +137,8 @@ void FileExternalAccountCredentials::RetrieveSubjectToken(
   cb(std::string(content), absl::OkStatus());
 }
 
+absl::string_view FileExternalAccountCredentials::CredentialSourceType() {
+  return "file";
+}
+
 }  // namespace grpc_core

src/core/lib/security/credentials/external/file_external_account_credentials.h

@@ -23,6 +23,8 @@
 #include <string>
 #include <vector>
 
+#include "absl/strings/string_view.h"
+
 #include "src/core/lib/gprpp/ref_counted_ptr.h"
 #include "src/core/lib/iomgr/error.h"
 #include "src/core/lib/security/credentials/external/external_account_credentials.h"
@@ -44,6 +46,8 @@ class FileExternalAccountCredentials final : public ExternalAccountCredentials {
       HTTPRequestContext* ctx, const Options& options,
       std::function<void(std::string, grpc_error_handle)> cb) override;
 
+  absl::string_view CredentialSourceType() override;
+
   // Fields of credential source
   std::string file_;
   std::string format_type_;

src/core/lib/security/credentials/external/url_external_account_credentials.cc

@@ -240,4 +240,8 @@ void UrlExternalAccountCredentials::FinishRetrieveSubjectToken(
   }
 }
 
+absl::string_view UrlExternalAccountCredentials::CredentialSourceType() {
+  return "url";
+}
+
 }  // namespace grpc_core

src/core/lib/security/credentials/external/url_external_account_credentials.h

@@ -24,6 +24,8 @@
 #include <string>
 #include <vector>
 
+#include "absl/strings/string_view.h"
+
 #include "src/core/lib/gprpp/orphanable.h"
 #include "src/core/lib/gprpp/ref_counted_ptr.h"
 #include "src/core/lib/http/httpcli.h"
@@ -48,6 +50,8 @@ class UrlExternalAccountCredentials final : public ExternalAccountCredentials {
       HTTPRequestContext* ctx, const Options& options,
       std::function<void(std::string, grpc_error_handle)> cb) override;
 
+  absl::string_view CredentialSourceType() override;
+
   static void OnRetrieveSubjectToken(void* arg, grpc_error_handle error);
   void OnRetrieveSubjectTokenInternal(grpc_error_handle error);
 

src/python/grpcio/grpc/_cython/_cygrpc/aio/server.pyx.pxi

@@ -470,6 +470,7 @@ async def _finish_handler_with_stream_responses(RPCState rpc_state,
     """
     cdef object async_response_generator
     cdef object response_message
+    install_context_from_request_call_event_aio(rpc_state)
 
     if inspect.iscoroutinefunction(stream_handler):
         # Case 1: Coroutine async handler - using reader-writer API
@@ -523,6 +524,7 @@ async def _finish_handler_with_stream_responses(RPCState rpc_state,
     rpc_state.metadata_sent = True
     rpc_state.status_sent = True
     await execute_batch(rpc_state, finish_ops, loop)
+    uninstall_context()
 
 
 async def _handle_unary_unary_rpc(object method_handler,

test/core/security/credentials_test.cc

@@ -2127,12 +2127,12 @@ void validate_external_account_creds_token_exchage_request(
   // Check the rest of the request.
   GPR_ASSERT(strcmp(host, "foo.com:5555") == 0);
   GPR_ASSERT(strcmp(path, "/token") == 0);
-  GPR_ASSERT(request->hdr_count == 2);
+  GPR_ASSERT(request->hdr_count == 3);
   GPR_ASSERT(strcmp(request->hdrs[0].key, "Content-Type") == 0);
   GPR_ASSERT(
       strcmp(request->hdrs[0].value, "application/x-www-form-urlencoded") == 0);
-  GPR_ASSERT(strcmp(request->hdrs[1].key, "Authorization") == 0);
-  GPR_ASSERT(strcmp(request->hdrs[1].value,
+  GPR_ASSERT(strcmp(request->hdrs[2].key, "Authorization") == 0);
+  GPR_ASSERT(strcmp(request->hdrs[2].value,
                     "Basic Y2xpZW50X2lkOmNsaWVudF9zZWNyZXQ=") == 0);
 }
 
@@ -2155,12 +2155,12 @@ void validate_external_account_creds_token_exchage_request_with_url_encode(
   // Check the rest of the request.
   GPR_ASSERT(strcmp(host, "foo.com:5555") == 0);
   GPR_ASSERT(strcmp(path, "/token_url_encode") == 0);
-  GPR_ASSERT(request->hdr_count == 2);
+  GPR_ASSERT(request->hdr_count == 3);
   GPR_ASSERT(strcmp(request->hdrs[0].key, "Content-Type") == 0);
   GPR_ASSERT(
       strcmp(request->hdrs[0].value, "application/x-www-form-urlencoded") == 0);
-  GPR_ASSERT(strcmp(request->hdrs[1].key, "Authorization") == 0);
-  GPR_ASSERT(strcmp(request->hdrs[1].value,
+  GPR_ASSERT(strcmp(request->hdrs[2].key, "Authorization") == 0);
+  GPR_ASSERT(strcmp(request->hdrs[2].value,
                     "Basic Y2xpZW50X2lkOmNsaWVudF9zZWNyZXQ=") == 0);
 }
 
@@ -2311,12 +2311,18 @@ void validate_aws_external_account_creds_token_exchage_request(
   // Check the rest of the request.
   GPR_ASSERT(strcmp(host, "foo.com:5555") == 0);
   GPR_ASSERT(strcmp(path, "/token") == 0);
-  GPR_ASSERT(request->hdr_count == 2);
+  GPR_ASSERT(request->hdr_count == 3);
   GPR_ASSERT(strcmp(request->hdrs[0].key, "Content-Type") == 0);
   GPR_ASSERT(
       strcmp(request->hdrs[0].value, "application/x-www-form-urlencoded") == 0);
-  GPR_ASSERT(strcmp(request->hdrs[1].key, "Authorization") == 0);
-  GPR_ASSERT(strcmp(request->hdrs[1].value,
+  GPR_ASSERT(strcmp(request->hdrs[1].key, "x-goog-api-client") == 0);
+  EXPECT_EQ(
+      request->hdrs[1].value,
+      absl::StrFormat("gl-cpp/unknown auth/%s google-byoid-sdk source/aws "
+                      "sa-impersonation/false config-lifetime/false",
+                      grpc_version_string()));
+  GPR_ASSERT(strcmp(request->hdrs[2].key, "Authorization") == 0);
+  GPR_ASSERT(strcmp(request->hdrs[2].value,
                     "Basic Y2xpZW50X2lkOmNsaWVudF9zZWNyZXQ=") == 0);
 }
 
@@ -2385,6 +2391,8 @@ class TestExternalAccountCredentials final : public ExternalAccountCredentials {
                                  std::vector<std::string> scopes)
       : ExternalAccountCredentials(std::move(options), std::move(scopes)) {}
 
+  std::string GetMetricsValue() { return MetricsHeaderValue(); }
+
  protected:
   void RetrieveSubjectToken(
       HTTPRequestContext* /*ctx*/, const Options& /*options*/,
@@ -2393,6 +2401,91 @@ class TestExternalAccountCredentials final : public ExternalAccountCredentials {
   }
 };
 
+TEST(CredentialsTest, TestExternalAccountCredsMetricsHeader) {
+  Json credential_source = Json::FromString("");
+  TestExternalAccountCredentials::ServiceAccountImpersonation
+      service_account_impersonation;
+  service_account_impersonation.token_lifetime_seconds = 3600;
+  TestExternalAccountCredentials::Options options = {
+      "external_account",                 // type;
+      "audience",                         // audience;
+      "subject_token_type",               // subject_token_type;
+      "",                                 // service_account_impersonation_url;
+      service_account_impersonation,      // service_account_impersonation;
+      "https://foo.com:5555/token",       // token_url;
+      "https://foo.com:5555/token_info",  // token_info_url;
+      credential_source,                  // credential_source;
+      "quota_project_id",                 // quota_project_id;
+      "client_id",                        // client_id;
+      "client_secret",                    // client_secret;
+      "",                                 // workforce_pool_user_project;
+  };
+  TestExternalAccountCredentials creds(options, {});
+
+  EXPECT_EQ(
+      creds.GetMetricsValue(),
+      absl::StrFormat("gl-cpp/unknown auth/%s google-byoid-sdk source/unknown "
+                      "sa-impersonation/false config-lifetime/false",
+                      grpc_version_string()));
+}
+
+TEST(CredentialsTest,
+     TestExternalAccountCredsMetricsHeaderWithServiceAccountImpersonation) {
+  Json credential_source = Json::FromString("");
+  TestExternalAccountCredentials::ServiceAccountImpersonation
+      service_account_impersonation;
+  service_account_impersonation.token_lifetime_seconds = 3600;
+  TestExternalAccountCredentials::Options options = {
+      "external_account",    // type;
+      "audience",            // audience;
+      "subject_token_type",  // subject_token_type;
+      "https://foo.com:5555/service_account_impersonation",  // service_account_impersonation_url;
+      service_account_impersonation,      // service_account_impersonation;
+      "https://foo.com:5555/token",       // token_url;
+      "https://foo.com:5555/token_info",  // token_info_url;
+      credential_source,                  // credential_source;
+      "quota_project_id",                 // quota_project_id;
+      "client_id",                        // client_id;
+      "client_secret",                    // client_secret;
+      "",                                 // workforce_pool_user_project;
+  };
+  TestExternalAccountCredentials creds(options, {});
+
+  EXPECT_EQ(
+      creds.GetMetricsValue(),
+      absl::StrFormat("gl-cpp/unknown auth/%s google-byoid-sdk source/unknown "
+                      "sa-impersonation/true config-lifetime/false",
+                      grpc_version_string()));
+}
+
+TEST(CredentialsTest, TestExternalAccountCredsMetricsHeaderWithConfigLifetime) {
+  Json credential_source = Json::FromString("");
+  TestExternalAccountCredentials::ServiceAccountImpersonation
+      service_account_impersonation;
+  service_account_impersonation.token_lifetime_seconds = 5000;
+  TestExternalAccountCredentials::Options options = {
+      "external_account",    // type;
+      "audience",            // audience;
+      "subject_token_type",  // subject_token_type;
+      "https://foo.com:5555/service_account_impersonation",  // service_account_impersonation_url;
+      service_account_impersonation,      // service_account_impersonation;
+      "https://foo.com:5555/token",       // token_url;
+      "https://foo.com:5555/token_info",  // token_info_url;
+      credential_source,                  // credential_source;
+      "quota_project_id",                 // quota_project_id;
+      "client_id",                        // client_id;
+      "client_secret",                    // client_secret;
+      "",                                 // workforce_pool_user_project;
+  };
+  TestExternalAccountCredentials creds(options, {});
+
+  EXPECT_EQ(
+      creds.GetMetricsValue(),
+      absl::StrFormat("gl-cpp/unknown auth/%s google-byoid-sdk source/unknown "
+                      "sa-impersonation/true config-lifetime/true",
+                      grpc_version_string()));
+}
+
 TEST(CredentialsTest, TestExternalAccountCredsSuccess) {
   ExecCtx exec_ctx;
   Json credential_source = Json::FromString("");

test/cpp/interop/xds_stats_watcher.cc

@@ -51,6 +51,19 @@ std::unordered_set<std::string> ToLowerCase(
   return result;
 }
 
+bool HasNonEmptyMetadata(
+    const std::map<std::string, LoadBalancerStatsResponse::MetadataByPeer>&
+        metadata_by_peer) {
+  for (const auto& entry : metadata_by_peer) {
+    for (const auto& rpc_metadata : entry.second.rpc_metadata()) {
+      if (rpc_metadata.metadata_size() > 0) {
+        return true;
+      }
+    }
+  }
+  return false;
+}
+
 }  // namespace
 
 XdsStatsWatcher::XdsStatsWatcher(int start_id, int end_id,
@@ -113,8 +126,12 @@ LoadBalancerStatsResponse XdsStatsWatcher::WaitForRpcStatsResponse(
                [this] { return rpcs_needed_ == 0; });
   response.mutable_rpcs_by_peer()->insert(rpcs_by_peer_.begin(),
                                           rpcs_by_peer_.end());
-  response.mutable_metadatas_by_peer()->insert(metadata_by_peer_.begin(),
-                                               metadata_by_peer_.end());
+  // Return metadata if at least one RPC had relevant metadata. Note that empty
+  // entries would be returned for RCPs with no relevant metadata in this case.
+  if (HasNonEmptyMetadata(metadata_by_peer_)) {
+    response.mutable_metadatas_by_peer()->insert(metadata_by_peer_.begin(),
+                                                 metadata_by_peer_.end());
+  }
   auto& response_rpcs_by_method = *response.mutable_rpcs_by_method();
   for (const auto& rpc_by_type : rpcs_by_type_) {
     std::string method_name;

test/cpp/interop/xds_stats_watcher_test.cc

@@ -152,7 +152,7 @@ TEST(XdsStatsWatcherTest, WaitForRpcStatsResponseReturnsAll) {
             watcher.WaitForRpcStatsResponse(0).DebugString());
 }
 
-TEST(XdsStatsWatcherTest, WaitForRpcStatsResponseIgnoresMetadata) {
+TEST(XdsStatsWatcherTest, WaitForRpcStatsResponseExcludesMetadata) {
   XdsStatsWatcher watcher(0, 3, {});
   // RPC had metadata - but watcher should ignore it
   watcher.RpcCompleted(BuildCallResult(0), "peer1",
@@ -163,11 +163,6 @@ TEST(XdsStatsWatcherTest, WaitForRpcStatsResponseIgnoresMetadata) {
                        {{"k1", "v5"}, {"k2", "v6"}, {"k3", "v7"}});
   LoadBalancerStatsResponse expected;
   expected.mutable_rpcs_by_peer()->insert({{"peer1", 2}, {"peer2", 1}});
-  // There will still be an empty metadata collection for each RPC
-  expected.mutable_metadatas_by_peer()->insert({
-      {"peer1", BuildMetadatas({{}, {}})},
-      {"peer2", BuildMetadatas({{}})},
-  });
   (*expected.mutable_rpcs_by_method())["UnaryCall"]
       .mutable_rpcs_by_peer()
       ->insert({{"peer1", 2}, {"peer2", 1}});