feat: Add RequestId filter#10274
Conversation
michalsn
left a comment
There was a problem hiding this comment.
Thanks for working on this. I agree that having a request ID can be useful. However, I am not sure we should add this directly to IncomingRequest. I would prefer to see this as documentation only, or provide an opt-in filter that's disabled by default. This is a path similar to those followed by Symfony and Laravel.
If we provide an example or filter, it should store the value in context()->set('request_id', $id) so $logGlobalContext can pick it up, set X-Request-ID on the response in after(), and validate incoming IDs: non-empty, bounded length, and safe characters only.
Let's see what others think.
|
Thanks for working on this. I agree that having a request ID is useful. I also use this in my projects, but with a filter-based approach, and it has worked well in practice. I think that shape fits better here than adding it directly to The filter can resolve or generate the ID early, validate any incoming This avoids keeping separate request-id state on the request object, keeps the feature opt-in, and makes the ID useful for logs, response headers, and support/debug workflows. |
memleakd
left a comment
There was a problem hiding this comment.
Thanks for updating this. I think the filter direction is much better. I added a few comments on things that still look worth checking.
memleakd
left a comment
There was a problem hiding this comment.
Thanks for the updates. This looks much better now. Just two small things left for me:
memleakd
left a comment
There was a problem hiding this comment.
I noticed one last thing with the filter ordering. I think requestid should stay after pagecache in after. PageCache::after() stores response headers, so if requestid runs first, we may accidentally cache X-Request-ID.
6c69a30 to
6a209d4
Compare
michalsn
left a comment
There was a problem hiding this comment.
Looks good, thanks! You can rebase to resolve the PHPStan error.
6a209d4 to
8364767
Compare
| debugging and logging purposes, as it allows you to trace a specific request through the application. | ||
|
|
||
| Framework-generated IDs are 32-character hexadecimal strings and are unique for practical purposes, however valid incoming IDs are reused. | ||
| It is added to the request's context and can be accessed via the ``request_id`` key. |
There was a problem hiding this comment.
Since Context is still a relatively new feature in CodeIgniter, I wonder if it would be helpful to include a small code example showing how to access the request ID from the context.
Something along the lines of:
$requestId = service('context')->get('request_id');This might make the feature easier to discover for users who are not yet familiar with the Context API.
Description
This PR adds
RequestIdfilter which will improve debugging, traceability, as well as better logging for each request.This filter is fully opt-in, that means there's no BC here. The filter line is commented so that users can easily add if needed.
Request id is generated during early lifecycle. Incoming header
X-Request-IDwill be prioritized over auto generation of request ids, so that CDN/web server/load balancer can generate request id for better traceability.If the header is not present, the framework will create request id via
bin2hex(random_bytes(16))function, that means the framework-generated ids will be of length 32.Why only
bin2hex?I initially thought of UUID v4, but due to lack of in-built support for UUID, I changed that to bin2hex. Adding a new external dependency for just this one thing would be less feasible, IMO. And also
random_bytes(16)should provide enough randomness so collision would be highly unlikely.If there is some other better way to generate id, then I would surely change to that.
Checklist: