-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Expose :logger's global metadata #12413
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
Conversation
lib/logger/lib/logger.ex
Outdated
* `:metadata` - global primary metadata to be included in all log messages. | ||
Defaults to `[]`. See [the configuration | ||
section](https://www.erlang.org/doc/man/kernel_app.html#configuration) of | ||
the documentation for the `kernel` Erlang application. |
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.
* `:metadata` - global primary metadata to be included in all log messages. | |
Defaults to `[]`. See [the configuration | |
section](https://www.erlang.org/doc/man/kernel_app.html#configuration) of | |
the documentation for the `kernel` Erlang application. | |
* `:metadata` - global primary metadata to be included in all log messages. | |
Defaults to `[]`. This can be overridden at the process level with `metadata/1` | |
or each on log call as desired. |
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.
We should also add a note to Logger.metadata/0
that it only returns the process metadata and not the global one.
Let's also add a test in logger_test
. You will already find some tests that call App.stop
and Application.start(:logger)
to test the level is read correctly. We can do similar for metadata.
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.
Done and done 👍
lib/logger/lib/logger/app.ex
Outdated
@@ -34,6 +34,12 @@ defmodule Logger.App do | |||
primary_config = :logger.get_primary_config() | |||
:ok = :logger.set_primary_config(:level, default_level()) | |||
|
|||
:ok = | |||
Application.get_env(:logger, :metadata, []) |
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.
Application.get_env(:logger, :metadata, []) | |
Application.fetch_env!(:logger, :metadata) |
And set a default on mix.exs
.
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.
Done!
lib/logger/lib/logger/app.ex
Outdated
:ok = | ||
Application.get_env(:logger, :metadata, []) | ||
|> Map.new() | ||
|> then(&Map.merge(primary_config.metadata, &1)) | ||
|> then(&:logger.set_primary_config(:metadata, &1)) |
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.
:ok = | |
Application.get_env(:logger, :metadata, []) | |
|> Map.new() | |
|> then(&Map.merge(primary_config.metadata, &1)) | |
|> then(&:logger.set_primary_config(:metadata, &1)) | |
with [_ | _] = metadata <- Application.fetch_env!(:logger, :metadata) do | |
:ok = :logger.set_primary_config(:metadata, Enum.into(metadata, primary_config.metadata)) | |
end |
So we don't bother setting up a metadata if nothing changed?
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.
Really nice. Done.
Part of #9465.