google-cloud-rust icon indicating copy to clipboard operation
google-cloud-rust copied to clipboard

Support user agent over gRPC

Open dbolduc opened this issue 8 months ago • 4 comments

tonic (as of 0.13.0) does not support user agents. They override any value for the header set in a request. So we cannot add our own.

We need to:

  • [x] Add support to tonic
    • https://github.com/hyperium/tonic/pull/2250
  • [ ] Add support to tonic (for real)
    • https://github.com/hyperium/tonic/pull/2290
  • [ ] Wait for a tonic release, and adopt latest version
  • [ ] Add support for user agents in the gRPC client

Note that while we can write the code that sets the user-agent in our gRPC client, it will be difficult to verify the observable behavior until we use a newer tonic. It is best to wait.

dbolduc avatar Apr 22 '25 15:04 dbolduc

I think this is in the 0.13.1 release, so it is no longer blocked.

coryan avatar May 29 '25 22:05 coryan

Crap:

https://github.com/hyperium/tonic/blob/b303caa52ba8bbe8172310be7165a80b7c2a53f8/tonic/src/metadata/map.rs#L219

https://github.com/hyperium/tonic/blob/b303caa52ba8bbe8172310be7165a80b7c2a53f8/tonic/src/metadata/map.rs#L266-L271

dbolduc avatar May 30 '25 04:05 dbolduc

Ok, I am actually testing against a local version of tonic now, instead of making assumptions.

This is where they drop the user-agent header:

stack backtrace:
   0: rust_begin_unwind
             at /rustc/05f9846f893b09a1be1fc8560e33fc3c815cfecb/library/std/src/panicking.rs:695:5
   1: core::panicking::panic_fmt
             at /rustc/05f9846f893b09a1be1fc8560e33fc3c815cfecb/library/core/src/panicking.rs:75:14
   2: tonic::metadata::map::MetadataMap::into_sanitized_headers
             at /usr/local/google/home/dbolduc/remote-build/dbolduc-glaptop.roam.corp.google.com/tonic/tonic/src/metadata/map.rs:268:13
   3: tonic::request::Request<T>::into_http
             at /usr/local/google/home/dbolduc/remote-build/dbolduc-glaptop.roam.corp.google.com/tonic/tonic/src/request.rs:187:37
   4: tonic::client::grpc::GrpcConfig::prepare_request
             at /usr/local/google/home/dbolduc/remote-build/dbolduc-glaptop.roam.corp.google.com/tonic/tonic/src/client/grpc.rs:390:27
   5: tonic::client::grpc::Grpc<T>::streaming::{{closure}}
             at /usr/local/google/home/dbolduc/remote-build/dbolduc-glaptop.roam.corp.google.com/tonic/tonic/src/client/grpc.rs:308:23
   6: tonic::client::grpc::Grpc<T>::client_streaming::{{closure}}
             at /usr/local/google/home/dbolduc/remote-build/dbolduc-glaptop.roam.corp.google.com/tonic/tonic/src/client/grpc.rs:242:50
   7: tonic::client::grpc::Grpc<T>::unary::{{closure}}
             at /usr/local/google/home/dbolduc/remote-build/dbolduc-glaptop.roam.corp.google.com/tonic/tonic/src/client/grpc.rs:222:53
   8: google_cloud_gax_internal::grpc::Client::request_attempt::{{closure}}
             at ./src/grpc.rs:174:14

My new unit test passes with this edit. Is it safe? uhhhhh maybe?

diff --git a/tonic/src/metadata/map.rs b/tonic/src/metadata/map.rs
index dc6d4d3..8e08421 100644
--- a/tonic/src/metadata/map.rs
+++ b/tonic/src/metadata/map.rs
@@ -214,9 +214,8 @@ pub(crate) const GRPC_TIMEOUT_HEADER: &str = "grpc-timeout";
 
 impl MetadataMap {
     // Headers reserved by the gRPC protocol.
-    pub(crate) const GRPC_RESERVED_HEADERS: [HeaderName; 6] = [
+    pub(crate) const GRPC_RESERVED_HEADERS: [HeaderName; 5] = [
         HeaderName::from_static("te"),
-        HeaderName::from_static("user-agent"),
         HeaderName::from_static("content-type"),
         HeaderName::from_static("grpc-message"),
         HeaderName::from_static("grpc-message-type"),

dbolduc avatar May 30 '25 04:05 dbolduc

For when I pick this up again:

https://github.com/dbolduc/google-cloud-rust/tree/user-agent-grpc

dbolduc avatar May 30 '25 15:05 dbolduc

For when I pick this up again:

https://github.com/dbolduc/google-cloud-rust/tree/user-agent-grpc

Thanks @dbolduc, this was super helpful!

dbolduc avatar Aug 08 '25 16:08 dbolduc