Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add request id header for rpc requests #2625

Merged
merged 2 commits into from
Apr 19, 2024

Conversation

rferrandop
Copy link
Contributor

Description

Add a request id header for the RPC requests.

Changes

This PR adds the following request HTTP headers on RPC requests:

  • X-RPC-Request-Id - The unique ID of the RPC request.
  • X-Request-Id - The top level request id passed as a request header for the RPC request generated.

How to test

I couldn't find a suitable way to test this. Maybe, it would be useful to create tests for the RPC request execution and assert that those headers are been sending.

Related Issues

Fixes #2519

@rferrandop rferrandop requested a review from a team as a code owner April 16, 2024 16:39
Copy link

github-actions bot commented Apr 16, 2024

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@rferrandop
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

@rferrandop rferrandop force-pushed the rpc-request-id-header branch from 6bb8ded to 8741c6d Compare April 16, 2024 16:41
Copy link
Contributor

@MartinquaXD MartinquaXD left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution.
Looks very good!

@rferrandop rferrandop force-pushed the rpc-request-id-header branch from 8741c6d to cfbec7a Compare April 18, 2024 11:36
@rferrandop
Copy link
Contributor Author

Hi, I fixed the code formatting. Now the lint workflow should be working OK.
The test-forked-node failed, but I think that is a runtime issue: https://github.com/cowprotocol/services/actions/runs/8709367754/job/23972858160

@MartinquaXD
Copy link
Contributor

The test-forked-node failed, but I think that is a runtime issue

Yeah, unfortunately these tests always fail for external contributions.
Will rebase the PR and merge it.
Regarding the reward: Can you post your reward gnosis chain address here or send me a DM on discord (in case you prefer that for privacy reasons)?

@MartinquaXD MartinquaXD enabled auto-merge (squash) April 19, 2024 12:13
@MartinquaXD MartinquaXD merged commit 8b62faa into cowprotocol:main Apr 19, 2024
8 of 9 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Apr 19, 2024
@rferrandop rferrandop deleted the rpc-request-id-header branch April 20, 2024 11:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: Add Request ID header to RPC requests for easier debugging
4 participants