transport: re-think user agents - change from replace to prepend?

[broady]

Currently, we allow users to set the User-Agent header via the WithUserAgent client option.

There's also the "UserAgent" field in the JSON/REST generated clients (this repo) which is appended to the default user agent (googleapi.UserAgent).

This option sets the User-Agent in its entirety, which means the client doesn't send its default User-Agent. It also makes it a bit more difficult for intermediary libraries to allow their users to set the User-Agent (think about a terraform library, for instance, you could end up with three user agents: end user, terraform, api client).

We might need some way to prepend user agent strings.

Questions:

• do we actually to make a change? is our default User-Agent important?
• prepend or append?

see PR #490

https://github.com/googleapis/google-api-go-client/blob/8ecc69ffe2df19a1b579676c8b1e0e3b5690d5e7/transport/http/dial.go#L144-L147

[kenegozi]

Worth mentioning that with the current setup, there is an inherent difference between calling WithUserAgent(...) (replace the default, would not work with a custom httpClient) and setting Service.UserAgent (would append to the default, and would work with a custom httpClient)

[codyoss]

Also related: #320.

[Capstan]

As the author of #320, I am in favor of prepending and not allowing replacement.

[wyuan1704]

I am wondering why we don't use the User-Agent in the caller's request. Below is an example from iamcredentials/v1/iamcredentials-gen.go.

func (c *ProjectsServiceAccountsGenerateAccessTokenCall) doRequest(alt string) (*http.Response, error) {
	reqHeaders := make(http.Header)
	reqHeaders.Set("x-goog-api-client", "gl-go/"+gensupport.GoVersion()+" gdcl/20200605")
	for k, v := range c.header_ {
		reqHeaders[k] = v
	}
	reqHeaders.Set("User-Agent", c.s.userAgent())

The caller can specify c.header_.UserAgent. But it's abandoned. Can we make it something like

      if reqHeaders["User-Agent"] == "" {
          reqHeaders["User-Agent"] = c.s.userAgent()
      } else {
         reqHeaders.Set("User-Agent", reqHeaders["User-Agent"] + " " + c.s.userAgent())
         // Or do nothing. Just use Caller's User-Agent?
      }

[broady]

@wyuan1704 are you referring to if you had used

c := s.Method()
c.Header().Set("User-Agent", "...")
resp, err := c.Do()

?

[wyuan1704]

Hi Chris, Yes, you are right. I am planning to do it, but I find that I can't do it now.

We have a user case in GKE. In a GKE cluster, customers can run multiple pods. Let's say POD1 has UserAgent POD1-UA. POD1 makes requests to a system daemon-set DS1 to get something. DS1 has UserAgent DS1-UA. DS1 uses google-api-go-client to makes requests to a google internal service SRV1. At SRV1 side, they want to know POD1-UA, which is the real end clients, so that they do some analysis.

Why not collect data at DS1? The logs belong to individual cluster and are not centralized. If using metrics, its cardinality is unbounded.

[wyuan1704]

@broady What do you think about my last comment? Thanks.

[enocom]

Side note: if a custom HTTP client is provided the user agent isn't used.

https://github.com/googleapis/google-api-go-client/blob/9d43e7dc08e2598d4dcb661324c4877dfd63c4f9/transport/http/dial.go#L40

[codyoss]

@enocom This was actually just recently codified in #1747