-
Notifications
You must be signed in to change notification settings - Fork 95
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
Add formatter for Elastic Common Schema (ECS) #116
Conversation
lib/logger_json/formatters/ecs.ex
Outdated
@@ -0,0 +1,236 @@ | |||
defmodule LoggerJSON.Formatters.ECS do |
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.
Let's name it in a way that anyone can understand. We also will need to add it to README.md and moduledoc logger_json.ex
:
defmodule LoggerJSON.Formatters.ECS do | |
defmodule LoggerJSON.Formatters.ElasticStack do |
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.
Yeah makes sense. I think just Elastic
as a name would be a bit cleaner though, are you happy with that too?
defmodule LoggerJSON.Formatters.ECS do | |
defmodule LoggerJSON.Formatters.Elastic do |
I'll then also rename this file to elastic.ex
and update the readme and moduledoc as you say
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_json/formatters/ecs.ex
Outdated
|
||
@spec format(any(), any()) :: none() | ||
def format(%{level: level, meta: meta, msg: msg}, opts) do | ||
metadata_keys_or_selector = Keyword.get(opts, :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.
We also need redactors support here, this is something that was recently added to master.
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.
Yup, I saw that was something you added there, I'll add it in here too!
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!
test/formatters/ecs_test.exs
Outdated
} = log_entry | ||
end | ||
|
||
test "logs exception http context" do |
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.
Please check what additional tests were added to other loggers, I recently got "100% coverage" and would like to keep that :).
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.
Yes will do! Good to know 100% coverage is the target.
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, with 100% coverage!
@bvobart Hello 👋, this contribution is very welcome! Will be happy to review it after rebase |
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.
This is perfect, thank you! ❤️
Fixes #89
Since I also require this functionality for an open-source project I'm working on professionally, I've implemented a formatter for use with ElasticSearch, which aims to output Elastic Common Schema (ECS) compliant log messages.
@AndrewDryga I see you recently took over this repository and added a bunch of changes and improvements that I haven't pulled into my fork yet. I'll rebase this PR and incorporate those changes later (though if you wanna do that before I do, feel free, you're more than welcome to ;) ).
In the mean time, it'd be good to get some early feedback. I'd like to hear what you think of my work :)