salvo icon indicating copy to clipboard operation
salvo copied to clipboard

add support for x-request-id

Open prabirshrestha opened this issue 2 years ago • 5 comments

Is your feature request related to a problem? Please describe. In order to correlate logs we need to have some sort of ids.

Describe the solution you'd like Add support for request id similar to the following.

  • https://docs.acquia.com/cloud-platform/develop/drupal/requestid/
  • https://devcenter.heroku.com/articles/http-request-id.
  • https://docs.microsoft.com/en-us/rest/api/storageservices/troubleshooting-api-operations
  • https://cloud.google.com/appengine/docs/standard/go/reference/request-response-headers

Describe alternatives you've considered Roll my own handler. This is common pattern that may be worth having as extras.

Additional context While there are multiple types of id that could be used for logging I usually look for 2 ids one that is generated by the client and the other that is generated by the server.

For client request id.

// default
.hoop(RequestIdHandler::new())

// customized
.hoop(RequestIdHandler::new()
    .with_request_header("x-request-id")
    .with_response_header("x-request-id")
    .with_id_generator(some_fn)) // if the id is absent probably want to auto generate.

For server request id. Since client request can't be trusted as they can replay the requests using curl/fiddler tools. We can also have an option to always force generate the id.

// default
.hoop(ResponseIdHandler::new())

// customized
.hoop(ResponseIdHandler::new()
    .with_response_header("x-server-request-id")
    .with_id_generator(some_fn))

Should also have ext methods so the id for RequestIdHandler and ResponseIdHandler can be retrieved.

prabirshrestha avatar Sep 04 '22 06:09 prabirshrestha

Why we need to write request id to response header?

chrislearn avatar Sep 05 '22 01:09 chrislearn

There are two ids I'm interested in the client request id (x-client-request-id) and the server request id (x-server-request-id).

If a client doesn't send the x-client-request-id or is invalid such as too long or too short or doesn't match certain patterns such as guid, the server may decide to generate the generate a new client request. In this case server might want to generate the id the x-client-request-id.

https://devcenter.heroku.com/articles/http-request-id#how-it-works

curl https://api.heroku.com/schema -H "Accept: application/vnd.heroku+json; version=3" -v 1> /dev/null

image

Because the x-client-request-id is provided by the client and it can't be trusted, the server may wish to always generate another id.

prabirshrestha avatar Sep 05 '22 06:09 prabirshrestha

  1. When we get request, we try to parse requestid.
  2. validate requestid, if it is not exists or invalid, generate a new one.

I just puzzled why we write it to response header, do we need to generate a new request id and send it back to client?

.hoop(ResponseIdHandler::new()
    .with_response_header("x-server-request-id")
    .with_id_generator(some_fn))

chrislearn avatar Sep 05 '22 10:09 chrislearn

There are two different ids - x-client-request-id used by RequestIdHandler and x-server-request-id use by the ResponseIdHandler.

x-client-request-id is sent by the client and cannot be trusted. Clients may not pass this at all or reuse the same id. Most of the time this is enough. Server should always generate another id independent of x-client-request id so it never conflicts.

Here are some examples:

request a: curl -vv http://localhost:8080
response a: 
x-client-request-id: c1 // auto generate by server since it wasn't in request
x-server-request-id: s1 // always auto generated
request b: curl -vv http://localhost:8080 -H 'x-client-request-id: c1'
response b: 
x-client-request-id: c1 // reuse the request header since this was passed by client
x-server-request-id: s1 // always auto generate

// assume after a second the same request was replayed
request c: curl -vv http://localhost:8080 -H 'x-client-request-id: c1'
response c: 
x-client-request-id: c1 // reuse the request header since this was passed by client (this id c1 is the same as the previous request b)
x-server-request-id: s2 // always auto generate (even though the request was the same as previous we autogenerate a new id)

It is for the 2nd case when debugging request b and c that the server needs to generate and send the id.

Confusion might be due to the fact that most servers just use one x-request-id and treat it as x-client-request-id while in my case I want both x-client-request-id and x-server-request-id.

x-server-request-id could also contain other interesting data such as location of datacenter/nodes which could look like westus:node1:78ecd68e4b4040eea00292189337cd4c or could be broken down in to different headers such as x-location: westus and so on.

prabirshrestha avatar Sep 05 '22 17:09 prabirshrestha

I'm afraid I will not implement it in salvo repository, It looks the core feature of it just read and write HeaderMap. Then common id generator may just a uuid generator, but you need to define you custom generator in most situation. Some guys may just care about client request id, and some guys may care both client and server id. I haven't been able to find an elegant and general implementation of it.

You can create your customed request id crate if you like.

chrislearn avatar Sep 07 '22 08:09 chrislearn