Skip to content
This repository was archived by the owner on Jan 21, 2025. It is now read-only.

Add support for wrapping the ConsoleAppender in an AsyncAppender. #62

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 18 additions & 21 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
</scm>

<properties>
<maven.compiler.release>17</maven.compiler.release>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<logback.version>1.4.12</logback.version>
<slf4j.version>2.0.7</slf4j.version>
Expand Down Expand Up @@ -98,6 +99,11 @@
<artifactId>sentry-logback</artifactId>
<version>6.11.0</version>
</dependency>
<dependency>
<groupId>jakarta.annotation</groupId>
<artifactId>jakarta.annotation-api</artifactId>
<version>1.3.5</version>
</dependency>

<!--test scope-->
<dependency>
Expand All @@ -108,8 +114,8 @@
</dependency>
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-all</artifactId>
<version>1.9.5</version>
<artifactId>mockito-core</artifactId>
<version>5.14.2</version>
<scope>test</scope>
</dependency>
<dependency>
Expand All @@ -127,6 +133,16 @@

<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<version>3.1.2</version>
<configuration>
<argLine>@{argLine} -javaagent:${org.mockito:mockito-core:jar}</argLine>
<argLine>--add-opens java.base/java.lang=ALL-UNNAMED</argLine>
<argLine>--add-opens java.base/java.util=ALL-UNNAMED</argLine>
</configuration>
</plugin>
<plugin>
<artifactId>maven-checkstyle-plugin</artifactId>
</plugin>
Expand All @@ -140,25 +156,6 @@
<groupId>com.coveo</groupId>
<artifactId>fmt-maven-plugin</artifactId>
</plugin>
<plugin>
<groupId>com.github.spotbugs</groupId>
<artifactId>spotbugs-maven-plugin</artifactId>
<version>4.1.4</version>
<configuration>
<effort>Max</effort>
<threshold>Low</threshold>
<failOnError>true</failOnError>
<xmlOutput>true</xmlOutput>
</configuration>
<executions>
<execution>
<phase>verify</phase>
<goals>
<goal>check</goal>
</goals>
</execution>
</executions>
</plugin>
<plugin>
<artifactId>maven-javadoc-plugin</artifactId>
<executions>
Expand Down
36 changes: 29 additions & 7 deletions src/main/java/com/spotify/logging/LoggingConfigurator.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import static ch.qos.logback.classic.Level.OFF;
import static java.lang.System.getenv;

import ch.qos.logback.classic.AsyncAppender;
import ch.qos.logback.classic.Logger;
import ch.qos.logback.classic.LoggerContext;
import ch.qos.logback.classic.encoder.PatternLayoutEncoder;
Expand Down Expand Up @@ -222,6 +223,19 @@ public static void configureDefaults(
* a Docker container will capture these logs for further processing.
*/
public static void configureLogstashEncoderDefaults(final Level level) {
configureLogstashEncoderDefaults(level, null);
}

/**
* Configure logging with the LogstashEncoder library.
* (https://github.com/logstash/logstash-logback-encoder)
*
* <p>The supplied {@link AsyncAppender} will wrap an appender that is configured to send the log
* messages to stdout. It is the responsibility of the caller to stop the {@link AsyncAppender} to
* properly flush the logging events from its queue.
*/
public static void configureLogstashEncoderDefaults(
final Level level, @Nullable final AsyncAppender asyncAppender) {
final Logger rootLogger = (Logger) LoggerFactory.getLogger(Logger.ROOT_LOGGER_NAME);
final LoggerContext context = rootLogger.getLoggerContext();

Expand All @@ -232,14 +246,22 @@ public static void configureLogstashEncoderDefaults(final Level level) {
encoder.addProvider(new ArgumentsJsonProvider());
encoder.start();

final ConsoleAppender<ILoggingEvent> appender = new ConsoleAppender<>();
appender.setTarget("System.out");
appender.setName("stdout");
appender.setEncoder(encoder);
appender.setContext(context);
appender.start();
final ConsoleAppender<ILoggingEvent> consoleAppender = new ConsoleAppender<>();
consoleAppender.setTarget("System.out");
consoleAppender.setName("stdout");
consoleAppender.setEncoder(encoder);
consoleAppender.setContext(context);
consoleAppender.start();

if (asyncAppender != null) {
asyncAppender.setContext(context);
asyncAppender.addAppender(consoleAppender);
asyncAppender.start();
rootLogger.addAppender(asyncAppender);
} else {
rootLogger.addAppender(consoleAppender);
}

rootLogger.addAppender(appender);
rootLogger.setLevel(level.logbackLevel);

UncaughtExceptionLogger.setDefaultUncaughtExceptionHandler();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public void start() {

// our internal syslog-ng configuration splits logs up based on service name, and expects the
// format below.
String serviceAndPid = String.format("%s[%s]", serviceName, getMyPid());
String serviceAndPid = "%s[%s]".formatted(serviceName, getMyPid());
setSuffixPattern(
serviceAndPid
+ ": "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.runners.MockitoJUnitRunner;
import org.mockito.junit.MockitoJUnitRunner;
import org.slf4j.LoggerFactory;

@RunWith(MockitoJUnitRunner.class)
Expand All @@ -68,7 +68,6 @@ public void syslog() {

@Test
public void threadlog() throws InterruptedException {
when(mockOptions.syslog()).thenReturn(false);
when(mockOptions.logFileName()).thenReturn("logback_test.xml");
JewelCliLoggingConfigurator.configure(mockOptions);

Expand Down
46 changes: 31 additions & 15 deletions src/test/java/com/spotify/logging/LoggingConfiguratorTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,15 @@
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;

import ch.qos.logback.classic.AsyncAppender;
import ch.qos.logback.classic.Level;
import ch.qos.logback.classic.Logger;
import ch.qos.logback.classic.LoggerContext;
import ch.qos.logback.classic.encoder.PatternLayoutEncoder;
import ch.qos.logback.classic.net.SyslogAppender;
import ch.qos.logback.core.ConsoleAppender;
import ch.qos.logback.core.status.Status;
import com.spotify.logging.LoggingConfigurator.ReplaceNewLines;
import com.spotify.logging.logback.CustomLogstashEncoder;
import io.sentry.logback.SentryAppender;
import net.logstash.logback.composite.loggingevent.ArgumentsJsonProvider;
Expand All @@ -66,26 +68,19 @@ public void testGetSyslogAppender() {
final LoggerContext context = new LoggerContext();

SyslogAppender appender =
(SyslogAppender)
getSyslogAppender(context, "", -1, LoggingConfigurator.ReplaceNewLines.OFF);
(SyslogAppender) getSyslogAppender(context, "", -1, ReplaceNewLines.OFF);
assertEquals("wrong host", "localhost", appender.getSyslogHost());
assertEquals("wrong port", 514, appender.getPort());

appender =
(SyslogAppender)
getSyslogAppender(context, null, -1, LoggingConfigurator.ReplaceNewLines.OFF);
appender = (SyslogAppender) getSyslogAppender(context, null, -1, ReplaceNewLines.OFF);
assertEquals("wrong host", "localhost", appender.getSyslogHost());
assertEquals("wrong port", 514, appender.getPort());

appender =
(SyslogAppender)
getSyslogAppender(context, "host", -1, LoggingConfigurator.ReplaceNewLines.OFF);
appender = (SyslogAppender) getSyslogAppender(context, "host", -1, ReplaceNewLines.OFF);
assertEquals("wrong host", "host", appender.getSyslogHost());
assertEquals("wrong port", 514, appender.getPort());

appender =
(SyslogAppender)
getSyslogAppender(context, null, 999, LoggingConfigurator.ReplaceNewLines.OFF);
appender = (SyslogAppender) getSyslogAppender(context, null, 999, ReplaceNewLines.OFF);
assertEquals("wrong host", "localhost", appender.getSyslogHost());
assertEquals("wrong port", 999, appender.getPort());
}
Expand All @@ -95,15 +90,13 @@ public void testGetSyslogAppenderRespectsNewLineReplacement() {
final LoggerContext context = new LoggerContext();

SyslogAppender appender =
(SyslogAppender)
getSyslogAppender(context, "", -1, LoggingConfigurator.ReplaceNewLines.OFF);
(SyslogAppender) getSyslogAppender(context, "", -1, ReplaceNewLines.OFF);
assertEquals("%property{ident}[%property{pid}]: %msg", appender.getSuffixPattern());

appender = (SyslogAppender) getSyslogAppender(context, "", -1, null);
assertEquals("%property{ident}[%property{pid}]: %msg", appender.getSuffixPattern());

appender =
(SyslogAppender) getSyslogAppender(context, "", -1, LoggingConfigurator.ReplaceNewLines.ON);
appender = (SyslogAppender) getSyslogAppender(context, "", -1, ReplaceNewLines.ON);
assertEquals(
"%property{ident}[%property{pid}]: %replace(%msg){'[\\r\\n]', ''}",
appender.getSuffixPattern());
Expand Down Expand Up @@ -172,6 +165,29 @@ public void shouldConfigureLogstashEncoderWithLevel() {
assertLogstashEncoder(Level.DEBUG);
}

@Test
public void shouldConfigureLogstashEncoderWithLevelAndAsyncAppender() {
// Given
final AsyncAppender asyncAppender = new AsyncAppender();

// When
LoggingConfigurator.configureLogstashEncoderDefaults(
LoggingConfigurator.Level.DEBUG, asyncAppender);

// Then
final Logger rootLogger = (Logger) LoggerFactory.getLogger(Logger.ROOT_LOGGER_NAME);
final ConsoleAppender<?> stdout = (ConsoleAppender<?>) asyncAppender.getAppender("stdout");
final CustomLogstashEncoder encoder = (CustomLogstashEncoder) stdout.getEncoder();
assertTrue(stdout.getEncoder() instanceof CustomLogstashEncoder);
assertEquals(Level.DEBUG, rootLogger.getLevel());
assertEquals(
1,
encoder.getProviders().getProviders().stream()
.filter(ArgumentsJsonProvider.class::isInstance)
.count());
assertTrue(asyncAppender.isStarted());
}

@Test
public void shouldConfigureDefaultWithIdentAndLevelWhenSyslogEnvVarIsNotSet() {
LoggingConfigurator.configureDefaults("some-ident", LoggingConfigurator.Level.DEBUG);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,8 +157,7 @@ public void filterSpotifyAtInfoOthersAtWarn() {
actual = andFilterReplies(actual, nextReply);
}

assertEquals(
String.format("Logger: %s, Level: %s", logger, level.toString()), expected, actual);
assertEquals("Logger: %s, Level: %s".formatted(logger, level.toString()), expected, actual);
}
}
}
Expand Down