add user-agent header to all http clients#8786
Conversation
pass user_agent as a parameter to http client constructors instead of depending on lighthouse_version from library crates. this avoids cargo cache invalidation cascading through the dependency tree on every git change since lighthouse_version uses git_version!().
f13b4f9 to
328c0e0
Compare
328c0e0 to
35c726b
Compare
|
If I understand correctly, after this change, when the VC calls the HTTP requests to the BN, it should include its version. I tested a running BN and VC and run: |
There was a problem hiding this comment.
I believe the reason it isn't working is because we never actually set the user agent when constructing the BeaconNodeHttpClient instance that is used by the VC on startup. We need to add a
.user_agent(&config.user_agent)here:
lighthouse/validator_client/src/lib.rs
Lines 287 to 291 in edba56b
Also, I noticed that while the PR mentions updating HttpJsonRpc and MonitoringHttpClient (and web3signer) but I don't see any diffs for those. Are those still required?
|
Clawdia seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
|
Clawdia seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
|
Hey @dapplion, is there an update on this one? |
|
Hi @dapplion, this pull request has been closed automatically due to 30 days of inactivity. If you’d like to continue working on it, feel free to reopen at any time. |
|
Just did some minor fix. I believe @dapplion has addressed the comments. I tested with BN + VC and ca see that the This PR is ready for another review. |
| pub initialized_validators: InitializedValidatorsConfig, | ||
| pub disable_attesting: bool, | ||
| /// User-Agent string for HTTP clients (set by the top-level binary). | ||
| #[serde(skip)] |
There was a problem hiding this comment.
For testing we might need to take off serde(skip).
| pub allow_insecure_genesis_sync: bool, | ||
| /// User-Agent string for HTTP clients (set by the top-level binary). | ||
| #[serde(skip)] | ||
| pub user_agent: String, |
There was a problem hiding this comment.
For testing:
- One BN test
- One VC test
Check that all "downstream" configs with user_agent are initialised when the top-level config is initialised.
|
Some required checks have failed. Could you please take a look @dapplion? 🙏 |
|
Hey guys, is there any chance we can get this merged and tagged before gloas? We'd love to keep get data of node operators which are on pre-gloas versions of their VCs and raise warnings to update. |
|
Hi @KaloyanTanev, most of the team is pretty flat out with Gloas at the moment but I'll try to review this by the end of the week and try to get this over the line |
Issue Addressed
Closes #7963
Proposed Changes
Adds a
User-AgentHTTP header to all outgoing HTTP requests. The header format isLighthouse/v<version>-<commit>(e.g.,Lighthouse/v8.0.0-67da032).This helps distributed validator middleware operators identify what their peers are running.
Clients updated:
BeaconNodeHttpClient(BN API client)ValidatorClientHttpClient(VC API client)HttpJsonRpc(execution layer engine API)BuilderHttpClient(already had user-agent)MonitoringHttpClient(metrics endpoint)ProductionValidatorClientbeacon node builderKey design decision: the
user_agentis passed as a&strconstructor parameter rather than having library crates depend onlighthouse_versiondirectly. This avoids cargo cache invalidation cascading through the dependency tree on every git change, sincelighthouse_versionusesgit_version!().Only top-level/binary crates reference
lighthouse_version::VERSION. Library crates (eth2,initialized_validators,beacon_node_fallback) receive it as a parameter.Related PRs
BeaconNodeHttpClientonlyTest plan
cargo checkpassescargo fmt --allcleanmake lint-fixpasses