-
Notifications
You must be signed in to change notification settings - Fork 50
Conversation
710c507
to
ef003d8
Compare
ef003d8
to
5a5280f
Compare
API Developer Docs Preview: Ready https://wordpress.github.io/openverse-api/_preview/777 Please note that GitHub pages takes a little time to deploy newly pushed code, if the links above don't work or you see old versions, wait 5 minutes and try again. You can check the GitHub pages deployment action list to see the current status of the deployments. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! How would the request id be used?
I added a blurb in the PR description to describe how we would use this. Essentially it is a way to be able to get all the logs from a particular request to be able to "trace" the request through the logs. Sometimes this is also called a "trace id" for the same reason. If we ever had multiple services, we can forwards this request ID to those services so that their logs also include the same request ID for the same request "session" and we would be able to tie everything together. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice addition, local logs are appearing correctly.
5a5280f
to
32374ec
Compare
This is so cool, great idea! |
Fixes
Part of WordPress/openverse#689 by @sarayourfriend
Description
Adds request IDs to all logs so that we can trace them easily throughout a request without having to rebuild it ourselves (we can simply query for the request ID and then follow a request's logs in their entirety).
I want to split out this part from #776 because it applies cleanly without interrupting the revert of #741 that @dhruvkb is going to work on. Getting request IDs so that we can trace logs through requests is useful at any point that we deploy next, even if we don't get in new diagnostic logging.
Initially I was going to write the filter and middleware myself but this library appears well enough maintained and is simple enough that I am not worried about adding it. It also has apparently figured out some problems and edge cases that we would inevitably run into if we hand-spun this functionality.
Testing Instructions
CI should pass. Checkout the branch locally and run the app and make a request (like to
/v1/images?q=waves
) and confirm you see something like this in your logs (noting the request id):Here,
7d3d1f0b82334242a352eb0e4361283e
is the request id.Checklist
Update index.md
).main
) or a parent feature branch.Developer Certificate of Origin
Developer Certificate of Origin