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

Log-shipping fix: adopt crosswordv2 log settings #31

Merged
merged 2 commits into from
Oct 1, 2024

Conversation

bryophyta
Copy link
Contributor

What does this change?

Copies the logback config that we're using in crosswordv2

It includes a number of changes to build.sbt as well. It might be that not all of these changes are necessary but I don't understand the jackson eviction stuff well enough to know what's needed and what's not.

Initially created because we thought that the broken log shipping was due to our config, though have since realised that there was an upstream issue with logging. Nevertheless, could be a QoL improvement to have nicer logs?

How to test

How can we measure success?

Have we considered potential risks?

Images

Accessibility

@bryophyta bryophyta requested a review from a team as a code owner September 26, 2024 13:44
@twrichards
Copy link
Contributor

do we know if this ensures log output is JSON? (so it gets parsed in a structured way by ELK)

similarly does it support stack traces (currently they come out as separate lines, which makes debugging v. painful)

@bryophyta
Copy link
Contributor Author

do we know if this ensures log output is JSON? (so it gets parsed in a structured way by ELK)

similarly does it support stack traces (currently they come out as separate lines, which makes debugging v. painful)

If I'm honest I don't understand all the wizardry which @andrew-nowak put into this for crosswords, but in my experience it's worked really nicely there. An example record from crosswordv2 with separate message and stackTrace fields can be seen here: https://logs.gutools.co.uk/s/editorial-tools/app/discover#/doc/67604290-baa6-11e9-bea2-633437abb232/logstash-ed-tools-2024.09.30?id=T0ajQpIBvaA8WQZa0ffv

<pattern>%d{yyyy-MM-dd HH:mm:ss} %highlight(%-5level) %cyan(%logger{36}) %magenta(%X{pekkoSource}) %msg%n</pattern>
</encoder>
<appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
<if condition='property("STAGE").contains("DEV")'>
Copy link
Member

Choose a reason for hiding this comment

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

add the .sbtopts file to make this work https://github.com/guardian/crosswordv2/blob/main/.sbtopts (this condition turns off json encoding for running locally, which is much more human readable)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ty! 93a41c8

possibly can be replaced with a JsonEncoder when we can upgrade to
logback 1.3 https://logback.qos.ch/manual/encoders.html#JsonEncoder
-->
<encoder class="net.logstash.logback.encoder.LogstashEncoder" />
Copy link
Member

Choose a reason for hiding this comment

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

this encoder outputs JSON, and does handle newline fields

@bryophyta bryophyta merged commit 9384b0e into main Oct 1, 2024
3 checks passed
@bryophyta bryophyta deleted the pf/logback-config branch October 1, 2024 12:43
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.

3 participants