jschniper / gelf_logger

An Elixir Logger backend for GELF
MIT License
29 stars 27 forks source link

Support arbitrary lists in metadata (for Elixir 1.10 support) #24

Closed dhaspden closed 4 years ago

dhaspden commented 4 years ago

Elixir adds metadata to Logger automatically that breaks this library out-of-the-box on Elixir 1.10. It attempts to call to_string/1 on lists which is no longer supported. I have updated log_event/5 to now call inspect/1 on a list instead of to_string/1.

For reference, this is the error that pops up when you try to use this library with Elixir 1.10.

:gen_event handler {Logger.Backends.Gelf, :gelf_logger} installed in Logger terminating
** (ArgumentError) cannot convert the given list to a string.

To be converted to a string, a list must either be empty or only
contain the following elements:

  * strings
  * integers representing Unicode code points
  * a list containing one of these three elements

Please check the given list or call inspect/1 to get the list representation, got:

[:elixir]
chazsconi commented 4 years ago

Hi @dhaspden I assume the reason for this is because Elixir 1.10 is adding an extra metadata field, domain which is a list of atoms? I'm not sure if to_string([:foo, :bar]) was ever supported. I've tried on Elixir 1.6 and it fails there also.

Also is there a reason for the try/rescue solution?

Would a cleaner solution be this?

        if is_list(v) or String.Chars.impl_for(v) == nil do
          {"_#{k}", inspect(v)}
        else
          {"_#{k}", to_string(v)}
        end
dhaspden commented 4 years ago

Hi @dhaspden I assume the reason for this is because Elixir 1.10 is adding an extra metadata field, domain which is a list of atoms? I'm not sure if to_string([:foo, :bar]) was ever supported. I've tried on Elixir 1.6 and it fails there also.

Also is there a reason for the try/rescue solution?

Would a cleaner solution be this?

        if is_list(v) or String.Chars.impl_for(v) == nil do
          {"_#{k}", inspect(v)}
        else
          {"_#{k}", to_string(v)}
        end

That would be a possible solution as well. I will take some time and test that in our dev environment and see if that works as well. I'll get back to you on that.

jschniper commented 4 years ago

Out in version 0.10.0

Sorry for the delay!