elixir-lang / elixir

Elixir is a dynamic, functional language for building scalable and maintainable applications
https://elixir-lang.org/
Apache License 2.0
24.51k stars 3.38k forks source link

Extra space in metadata part of log messages #8339

Closed albertored closed 6 years ago

albertored commented 6 years ago

Environment

Erlang/OTP 21 [erts-10.0] [source] [64-bit] [smp:4:4] [ds:4:4:10] [async-threads:1] [hipe]
Elixir 1.7.3 (compiled with Erlang/OTP 21)

Current behavior

At the moment the Logger.Formatter add a space at the end of metadata chardata:

iex> pattern = Logger.Formatter.compile("[$level] $metadata$message\n")
iex> formatted = Logger.Formatter.format(pattern, :info, "hello", nil, [foo: 1, bar: "baz"])
iex> IO.chardata_to_string(formatted)
"[info] foo=1 bar=baz hello\n"

this makes log formats like the follwowing a little bit ugly

iex> pattern = Logger.Formatter.compile("[$level] ($metadata) $message\n")
iex> formatted = Logger.Formatter.format(pattern, :info, "hello", nil, [foo: 1, bar: "baz"])
iex> IO.chardata_to_string(formatted)
"[info] (foo=1 bar=baz ) hello\n"

Expected behavior

The last space at the end of metadata part should be removed.

I took a look at the code and implementing this fix should be straightforward so I'm wondering if there is a reason that I'm not aware of for having that space.

One con I can think of is that this change can be viewed as a breaking one since all format strings like "$metadata$message" will then result in log messages with a missing space and maybe you are not interested in changing it for this reason. If not I can submit a PR.

josevalim commented 6 years ago

Hi @albertored! Yes, this would be a breaking change. The best option in your case is maybe to use a custom formatter. See Logger.Formatter for examples.

albertored commented 6 years ago

The better solution I got using a custom formatter and without duplicating a lot of the Logger.Formatter code is the following

defmodule MetadataFixedFormatter do

  def format(level, message, timestamp, []) do
    [:date, " ", :time, " [", :level, "] ", :metadata, :message]
    |> Logger.Formatter.format(level, message, timestamp, [])
  end

  def format(level, message, timestamp, metadata) do
    [:date, " ", :time, " [", :level, "] (", :metadata, ") ", :message]
    |> Logger.Formatter.format(level, message, timestamp, metadata)
    |> Enum.map(fn
      [[_k, ?=, _v, ?\s] | _tail] = list -> List.update_at(list, -1, fn elem -> Enum.drop(elem, -1) end)
      other -> other
    end)
  end
end

There is still one thing that bothers me and it is the need of putting the format compiled patter inside the custom logger module. I was wondering if we can add another way of setting up custom logger by passing an extra_arguments parameter like that

config :logger, :console,
  format: {MetadataFixedFormatter, :format, extra_args}

that should result in a MetadataFixedFormatter.format(level, message, timestamp, metadata, extra_args). In my specific situation I would use it for setting format pattern from config file but it can be used in general for setting options on custom loggers.

what do you think about?