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

Make pretty JSON formatting optional in basic formatter #114

Merged
merged 1 commit into from
Apr 29, 2024

Conversation

hkrutzer
Copy link
Contributor

Some log processors can't process JSON logs if they are spread across multiple lines.

I saw the other formatters don't have pretty at all so I'm not sure if it's intentional.

@AndrewDryga
Copy link
Member

@hkrutzer Good catch! Please instead of making it an option just remove it. This is just a leftover after a total rewrite.

@@ -44,7 +45,7 @@ defmodule LoggerJSON.Formatters.Basic do
|> maybe_put(:request, format_http_request(meta))
|> maybe_put(:span, format_span(meta))
|> maybe_put(:trace, format_trace(meta))
|> Jason.encode_to_iodata!(pretty: true)
|> Jason.encode_to_iodata!(pretty: pretty_json)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
|> Jason.encode_to_iodata!(pretty: pretty_json)
|> Jason.encode_to_iodata!()

@hkrutzer hkrutzer force-pushed the optional_pretty_json branch from 24cc0df to db6c491 Compare April 26, 2024 11:45
@hkrutzer hkrutzer force-pushed the optional_pretty_json branch from db6c491 to d77c75b Compare April 26, 2024 11:45
@hkrutzer
Copy link
Contributor Author

Thanks, removed it!

@AndrewDryga AndrewDryga merged commit cf2670e into Nebo15:master Apr 29, 2024
0 of 2 checks passed
@AndrewDryga
Copy link
Member

Thank you! @hkrutzer

@hkrutzer
Copy link
Contributor Author

hkrutzer commented May 2, 2024

Thanks @AndrewDryga. Are you planning on publishing 6.x to hex.pm anytime soon?

@AndrewDryga
Copy link
Member

@hkrutzer yes, I'm just trying to test it in our production to make sure it won't fail in yours. I think I will bump the version early next week.

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 this pull request may close these issues.

2 participants