-
Notifications
You must be signed in to change notification settings - Fork 603
expose generate_id function and make it configurable in RequestId #1294
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
base: main
Are you sure you want to change the base?
expose generate_id function and make it configurable in RequestId #1294
Conversation
lib/plug/request_id.ex
Outdated
| plug Plug.RequestId, logger_metadata_key: :my_request_id | ||
| * `:generate_request_id` - The function used to generate the request ID, defaults to Plug.RequestId.generate_request_id/0. |
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.
| * `:generate_request_id` - The function used to generate the request ID, defaults to Plug.RequestId.generate_request_id/0. | |
| * `:generate_request_id` - The function used to generate the request ID, defaults to `generate_request_id/0`. |
lib/plug/request_id.ex
Outdated
| * `:generate_request_id` - The function used to generate the request ID, defaults to Plug.RequestId.generate_request_id/0. | ||
| plug Plug.RequestId, generate_request_id: fn -> "myapp-" <> Plug.RequestId.generate_request_id end |
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.
| plug Plug.RequestId, generate_request_id: fn -> "myapp-" <> Plug.RequestId.generate_request_id end | |
| plug Plug.RequestId, generate_request_id: fn -> "myapp-" <> Plug.RequestId.generate_request_id() end | |
lib/plug/request_id.ex
Outdated
| Keyword.get(opts, :assign_as), | ||
| Keyword.get(opts, :logger_metadata_key, :request_id) | ||
| Keyword.get(opts, :logger_metadata_key, :request_id), | ||
| Keyword.get(opts, :generate_request_id, &generate_request_id/0) |
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.
@josevalim since this is such a frequently-called thing, is it better to do
| Keyword.get(opts, :generate_request_id, &generate_request_id/0) | |
| Keyword.get(opts, :generate_request_id, &__MODULE__.generate_request_id/0) |
here?
| end | ||
|
|
||
| defp generate_request_id do | ||
| def generate_request_id do |
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.
If we expose this we'll have to document it and spec it.
35eabd5 to
c103756
Compare
|
@whatyouhide Thanks for the review, I've amended the requested changes! |
This PR supersedes #1293