Skip to content
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

Metadata Duplication #17

Closed
novaugust opened this issue Apr 16, 2019 · 5 comments · Fixed by #18
Closed

Metadata Duplication #17

novaugust opened this issue Apr 16, 2019 · 5 comments · Fixed by #18

Comments

@novaugust
Copy link
Contributor

novaugust commented Apr 16, 2019

Logster copies the Logger.metadata into its log message.
However, the standard for logging metadata through a backend is to configure that backend's metadata option, specifying which keys should be printed.
This means that with a correctly configured backend, like

config :logger, :console,
  format: "[$level] $levelpad $message $metadata",
  metadata: [:request_id, :trace_id, :span_id]

logs will be produced that have request_id, trace_id, and span_id logged twice: once in the message itself via Logster, and then again via the backend.

Example

[info]   state=set duration=... status=200 
params={...} path=... method=GET format=json-api controller=... action=show
span_id=1691184670686183571 trace_id=2439927720751528365 request_id=FZX--3x64qa8bTUAAFfx
request_id=FZX--3x64qa8bTUAAFfx trace_id=2439927720751528365 span_id=1691184670686183571 

I think the easiest fix for this would be to revert the work in #6 and update the readme or changelog with a small guide on how to configure a backend to include metadata (removing the responsibility from Logster)

@novaugust
Copy link
Contributor Author

I should add, I'd be happy to do that work if it sounds good.

@pmenhart
Copy link

I am using a workaround with a custom formatter:
Since Elixir 1.7, the format/1 function can alternatively return a tuple {message, list_of_keywords}. Keywords are added to logger metadata. I format the message to my liking (e.g. Processed <method> <path> -> <status> (duration ms) <response_body_if error>).

format/1 can return all params as list_of_keywords (or exclude unwanted ones). Keywords are merged with metadata, i.e. any duplicates are eliminated.

However, I support your suggestion.
I am worried to break compatibility for existing users (if that's desirable). Instead of completely reverting #6, I propose to change Logster formatters to have 2 parameters: params and metadata.
Perhaps a new configuration option would tell whether to merge them together, or ignore metadata (defauting to "merge" for compatibility).

@novaugust
Copy link
Contributor Author

I am worried to break compatibility for existing users

Luckily, there's semver! This is currently 0.10 after all =) The version they're using will still exist.

Anyone looking to upgrade will get the added benefit of correctly configuring their backends to include metadata in all their logs.

@navinpeiris
Copy link
Owner

That sounds excellent to me @novaugust, if you could send me a pull request with some info in the README like you suggested, I'll get the merged in and cut a release.

@navinpeiris
Copy link
Owner

Thanks @novaugust, this has been merged in and released with version 1.0.0

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 a pull request may close this issue.

3 participants