Skip to content

Comments

Add new LogFormat: StructuredJSON#1951

Merged
akshaymankar merged 1 commit intodevelopfrom
fs-62/structured-json-logs
Dec 1, 2021
Merged

Add new LogFormat: StructuredJSON#1951
akshaymankar merged 1 commit intodevelopfrom
fs-62/structured-json-logs

Conversation

@akshaymankar
Copy link
Member

This format is intended as an imporvement over the JSON format. It tries to
improve in these ways:

  • Explicitly encode log level as a key in the main JSON object logged

  • Encode field values as key-value pairs in the main JSON object

Encoding logs like this makes it easy for external tools like fluent-bit and
elasticsearch to understand the log level and the fields and index them as such
without requiring bonanza to be present while processing logs.

https://wearezeta.atlassian.net/browse/FS-62

Checklist

  • The PR Title explains the impact of the change.
  • The PR description provides context as to why the change should occur and what the code contributes to that effect. This could also be a link to a JIRA ticket or a Github issue, if there is one.
  • changelog.d contains the following bits of information (details):
    • A file with the changelog entry in one or more suitable sub-sections. The sub-sections are marked by directories inside changelog.d.

This format is intended as an imporvement over the JSON format. It tries to
improve in these ways:

- Explicitly encode log level as a key in the main JSON object logged

- Encode field values as key-value pairs in the main JSON object

Encoding logs like this makes it easy for external tools like fluent-bit and
elasticsearch to understand the log level and the fields and index them as such
without requiring bonanza to be present while processing logs.
Copy link
Member

@jschaul jschaul left a comment

Choose a reason for hiding this comment

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

This just adds the new log format, right? But doesn't turn it on anywhere yet - how/where is it turned on to log that way?

The first part, the log level - could that not also be a separate structural element rather than the first item in the list of msgs? Or do you configure fluent-bit to parse the log level as first element?

@akshaymankar
Copy link
Member Author

This just adds the new log format, right? But doesn't turn it on anywhere yet - how/where is it turned on to log that way?

Yes, it only adds a new log format. I will turn it on in our federation environments as a test. If all looks fine after a month or so, I will propose that we make it the default.

The first part, the log level - could that not also be a separate structural element rather than the first item in the list of msgs? Or do you configure fluent-bit to parse the log level as first element?

The way tinylog works, it sends all the logs as [Element] to the renderer, this does contain the log level encoded as 1 character string (I, D, etc.). But it also sends the log level separately. Instead of relying on this contract of parsing the first 1 character string, I decided to just ignore it and put the log level separately sent by tiny log into the final JSON as level. I will comment inline to show where. This ensures that no configuration on fleunt-bit is required.

let structuredJSON = toStructuredJSONOutput logElems
in fromEncoding . toEncoding $
object
( [ "level" Aeson..= lvl,
Copy link
Member Author

Choose a reason for hiding this comment

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

@jschaul This line adds the level to the final JSON.

Copy link
Member

Choose a reason for hiding this comment

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

Cool, thanks!

@akshaymankar akshaymankar merged commit f992c7d into develop Dec 1, 2021
@akshaymankar akshaymankar deleted the fs-62/structured-json-logs branch December 1, 2021 11:14
@akshaymankar
Copy link
Member Author

Turns out this doc is a lie. The actual LogLevel passed here is just the configured setting, so all logs are currently being marked whatever log level is configured for the service 🤦
The only way to get the log level of the event is to parse it from the list of messages. I will create a follow up PR.

@akshaymankar
Copy link
Member Author

I also created an issue upstream: https://gitlab.com/twittner/tinylog/-/issues/5

@smatting smatting mentioned this pull request Dec 2, 2021
jschaul added a commit that referenced this pull request Jul 13, 2022
jschaul added a commit that referenced this pull request Jul 14, 2022
* charts/*: default logFormat=StructuredJSON format

This format was introduced in #1951 and #1959.

* Hi CI

* spar: Ensure integration test for testing logging works with StructuredJSON logs

Co-authored-by: Akshay Mankar <akshay@wire.com>
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