Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

feat/requestclient: propagate original User-Agent as X-Forwarded-For-User-Agent#64113

Merged
bobheadxi merged 2 commits intomainfrom
requestclient-forwardedforuseragent
Jul 29, 2024
Merged

feat/requestclient: propagate original User-Agent as X-Forwarded-For-User-Agent#64113
bobheadxi merged 2 commits intomainfrom
requestclient-forwardedforuseragent

Conversation

@bobheadxi
Copy link
Member

@bobheadxi bobheadxi commented Jul 26, 2024

Propagates a for-reference-only record of the first User-Agent seen when a request gets into Sourcegraph across services and contexts. This allows telemetry to try and indicate where a request originates from (https://github.com/sourcegraph/sourcegraph/pull/64112), rather than only having the most recent user-agent.

A new header and requestclient.Client property X-Forwarded-For-User-Agent and ForwardedForUserAgent is used to explicitly forward this. Strictly speaking I think we're supposed to just forward User-Agent but it looks like in multiple places we add/clobber the User-Agent ourselves.

The gRPC propagator currently sets user-agent on outgoing requests, this change also makes that consistent with the HTTP transport, such that both only explicitly propagate X-Forwarded-For-User-Agent

Test plan

Unit tests

@bobheadxi bobheadxi requested review from a team, dadlerj and ggilmore July 26, 2024 20:28
@cla-bot cla-bot bot added the cla-signed label Jul 26, 2024
@bobheadxi bobheadxi force-pushed the requestclient-forwardedforuseragent branch from 68922c1 to 73db77a Compare July 26, 2024 20:30
Copy link
Member

@eseliger eseliger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We talked through this, seems fine

@bobheadxi bobheadxi merged commit 38d4e83 into main Jul 29, 2024
@bobheadxi bobheadxi deleted the requestclient-forwardedforuseragent branch July 29, 2024 21:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants