support more metadata types #28
Conversation
| "level" => "debug", | ||
| "line" => line, | ||
| "mfa" => "ExJsonLoggerTest.\"test format/4 structured\"/1", | ||
| "msg" => "[msg: \"this is a message\", where: \"somewhere\", ok: :ok]", |
There was a problem hiding this comment.
Ideally this would be JSON, but it'd probably require a custom :logger formatter.
There was a problem hiding this comment.
Here's a demo of what it could be like: https://github.com/ruslandoga/logger_json_formatter
There was a problem hiding this comment.
Should format/2 be added to this project to support the new formatters or should it be left for another project?
| @@ -1,5 +1,5 @@ | |||
| defmodule ExJsonLogger.Plug.LoggerTest do | |||
| use ExUnit.Case, async: true | |||
There was a problem hiding this comment.
I was getting interleaved messages in capture_log. Plus most tests mutate global state (logger config) which doesn't work well with async tests.
ruslandoga
force-pushed
the
more-metadata-types
branch
2 times, most recently
from
0927541
to
2b264e1
Compare
ruslandoga
force-pushed
the
more-metadata-types
branch
from
2b264e1
to
6ebac0d
Compare
| defp format_metadata({key, atom}) when is_atom(atom) do | ||
| value = | ||
| case Atom.to_string(atom) do | ||
| "Elixir." <> rest -> rest |
There was a problem hiding this comment.
I'd be in favor of leaving these untouched, even if they do contain an Elixir. prefix. Thoughts?
There was a problem hiding this comment.
I was trying to keep the behavior the same as in the default formatter :)
| {"time", format_timestamp(timestamp)}, | ||
| {"msg", IO.iodata_to_binary(msg)} | ||
| | metadata(metadata) | ||
| ]) |
There was a problem hiding this comment.
Was this change from map to list done for performance reasons?
There was a problem hiding this comment.
It was mostly done for simplicity. This way, metadata/1 does both processing and filtering without the need for extra Map.merge/2 and Map.delete/2 operations.
This PR adds support for more metadata types.
Closes #25