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

Set auction_id as request_id in autopilot #3243

Merged
merged 5 commits into from
Jan 20, 2025

Conversation

MartinquaXD
Copy link
Contributor

@MartinquaXD MartinquaXD commented Jan 20, 2025

Description

We have some machinery in place to trace logs of different pods that relate to the same request by generating request_ids and forwarding them using HTTP headers to the next pod. The next pod will then read this header and attach the id to all its logs.
If an incoming request does not have an id the pod will generate a new id based on a counter and use that for the request.
The component that generates request ids for incoming user requests actually lives outside the services pod. That means for incoming user requests we never have to worry about generating these ids ourselves. But the autopilot does respond to external requests so it only relies on its internal counter to generate these ids.
This is not great since these autogenerated ids don't relate to any auction id in the autopilot.

Changes

To make tracing of requests that originated from the autopilot easier we now use the current auction's id as the request id. This should hopefully allow us to identify the auction a requests we sent to the mevblocker proxy was associated with.
Also we now for the first time even set the necessary X-REQUEST-ID header in the autopilot -> driver requests.

I also changed the code to not use the underlying REQUEST_ID anymore since we have helper functions that are supposed to be used for that.

How to test

added a unit test to demonstrate the behavior of the helper functions
Since the CI only prints logs on failures I copied a log from a local run showing that the request_id is the same as the auction_id:

request{id="84"}:/solve{solver=mock_solver auction_id=84}: driver::infra::observe: postprocessing solutions solutions=1 remaining=Ok(1.486682s)

@MartinquaXD MartinquaXD requested a review from a team as a code owner January 20, 2025 11:26
@MartinquaXD MartinquaXD marked this pull request as draft January 20, 2025 11:43
Copy link
Contributor

@mstrug mstrug left a comment

Choose a reason for hiding this comment

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

Look ok.

crates/observe/src/request_id.rs Show resolved Hide resolved
@MartinquaXD MartinquaXD marked this pull request as ready for review January 20, 2025 11:55
Copy link
Contributor

@squadgazzz squadgazzz left a comment

Choose a reason for hiding this comment

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

LGTM, even though we will have duplicated data in the driver logs where both auction and request IDs are equal.

@MartinquaXD
Copy link
Contributor Author

even though we will have duplicated data in the driver logs where both auction and request IDs are equal.

True. But this will only happen for requests related to solving auctions. For requests originated by users (e.g. create order) there will be a different value.

@MartinquaXD MartinquaXD merged commit ab2f47c into main Jan 20, 2025
11 checks passed
@MartinquaXD MartinquaXD deleted the set-auction-id-as-request-id branch January 20, 2025 14:44
@github-actions github-actions bot locked and limited conversation to collaborators Jan 20, 2025
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.

3 participants