Skip to content

Customize the Logger metadata key name #1267

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

Merged
merged 2 commits into from
Apr 21, 2025

Conversation

crbelaus
Copy link
Contributor

@crbelaus crbelaus commented Apr 21, 2025

In certain scenarios we may want to use the Plug.RequestID plug multiple times with different headers or assigns.
While this is possible, the issue is that every call adds a :request_id key to the Logger metadata. When using the plug multiple times, the last call will override the metadata of the previous one.

defmodule MyAppWeb.Endpoint do
  use Phoenix.Endpoint, otp_app: :my_app

  plug Plug.RequestId
  # This works, but overwrites the previous `:request_id` value in the logger metadata
  plug Plug.RequestId, http_header: "x-correlation-id", assign_as: :correlation_id
end

This commit adds a new :logger_metadata_key option that allows users to customize how the request ID will be added to the logger metadata. If not provided, this option defaults to :request_id for backwards compatibility.

Previous code could be modified as:

defmodule MyAppWeb.Endpoint do
  use Phoenix.Endpoint, otp_app: :my_app

  plug Plug.RequestId
  plug Plug.RequestId, http_header: "x-correlation-id", assign_as: :correlation_id, logger_metadata_key: :correlation_id
end

In certain scenarios we may want to use the Plug.RequestID multiple
times with different configuration options (maybe different headers or
different assigns).

While this is possible, the issue is that every call adds a
`:request_id` key to the Logger metadata. When using the plug muliple
times, the last call will override the metadata of the previous one.

This commit adds a new `:log_as` option that allows users to customize
how the request ID will be added to the logger metadata. If not
provided, this option defaults to `:request_id` for backwards
compatibility.
@crbelaus
Copy link
Contributor Author

It is worth noting that I initially planned to use the same key as the :assign_as option and fall back to :request_id if none was provided.

At the end I decided against it because it could break existing applications that were using a custom :assign_as option but relying on the :request_id key in the logger metadata. Configuring it as a separate option does not cause this problem.

Copy link
Member

@whatyouhide whatyouhide left a comment

Choose a reason for hiding this comment

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

I think this is a nice addition. 👍 Left a couple comments first.

@@ -47,6 +47,12 @@ defmodule Plug.RequestId do

plug Plug.RequestId, assign_as: :plug_request_id

* `:log_as` - The name of the key that will be used to store the
Copy link
Member

Choose a reason for hiding this comment

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

I would name this :logger_metadata_key.

@@ -47,6 +47,12 @@ defmodule Plug.RequestId do

plug Plug.RequestId, assign_as: :plug_request_id

* `:log_as` - The name of the key that will be used to store the
discovered or generated request id in `Logger` metadata. If not provided,
the request id will be stored as `:request_id`.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
the request id will be stored as `:request_id`.
the request ID Logger metadata key defaults to `:request_id`.

discovered or generated request id in `Logger` metadata. If not provided,
the request id will be stored as `:request_id`.

plug Plug.RequestId, log_as: :my_request_id
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
plug Plug.RequestId, log_as: :my_request_id
plug Plug.RequestId, log_as: :my_request_id
*Available since v1.18.0*.

@crbelaus
Copy link
Contributor Author

Thanks for the feedback @whatyouhide! 🙇‍♂️

I've applied your suggestions and this is ready for another review.

@crbelaus crbelaus requested a review from whatyouhide April 21, 2025 18:39
@josevalim josevalim closed this Apr 21, 2025
@josevalim josevalim reopened this Apr 21, 2025
@josevalim josevalim merged commit 67faa57 into elixir-plug:main Apr 21, 2025
2 of 6 checks passed
@josevalim
Copy link
Member

💚 💙 💜 💛 ❤️

@gmile
Copy link

gmile commented Apr 24, 2025

Thanks for this addition 🎉 It just so happened that I had to roll my own plug to do this a few days ago, roughly at the time when this was being submitted :) Crazy timing!

FWIW, here's my use-case for this. Using this makes it possible to link all HTTP requests made by browser during a test in Playwright - with logs:

  1. in Playwright (see testInfo and setExtraHTTPHeaders):

    // in Playwright
    context = browser.newContext({ extraHTTPHeaders: {'x-test-id': testInfo.testId} })
  2. in Plug:

    plug Plug.RequestId, http_header: "x-test-id", assign_as: :test_id, logger_metadata_key: :test_id
  3. in logger config:

    config :logger, :default_formatter,
      format: "$time $metadata[$level] $message\n",
      metadata: [:request_id, :test_id]

In other words, after running a Playwright suite, it's now possible to filter out all entries logged during a specific test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants