diff --git a/pom.xml b/pom.xml index 93cf276..931b26e 100644 --- a/pom.xml +++ b/pom.xml @@ -96,6 +96,12 @@ 1.9.5 test + + com.github.stefanbirkner + system-rules + 1.16.0 + test + diff --git a/src/main/java/com/spotify/logging/LoggingConfigurator.java b/src/main/java/com/spotify/logging/LoggingConfigurator.java index f42ec89..256ef22 100644 --- a/src/main/java/com/spotify/logging/LoggingConfigurator.java +++ b/src/main/java/com/spotify/logging/LoggingConfigurator.java @@ -58,6 +58,8 @@ public class LoggingConfigurator { public static final String DEFAULT_IDENT = "java"; + public static final String SPOTIFY_SYSLOG_HOST = "SPOTIFY_SYSLOG_HOST"; + public static final String SPOTIFY_SYSLOG_PORT = "SPOTIFY_SYSLOG_PORT"; public enum Level { OFF(ch.qos.logback.classic.Level.OFF), @@ -169,8 +171,8 @@ public static void configureSyslogDefaults(final String ident) { * @param level logging level to use. */ public static void configureSyslogDefaults(final String ident, final Level level) { - final String syslogHost = getenv("SPOTIFY_SYSLOG_HOST"); - final String port = getenv("SPOTIFY_SYSLOG_PORT"); + final String syslogHost = getenv(SPOTIFY_SYSLOG_HOST); + final String port = getenv(SPOTIFY_SYSLOG_PORT); final int syslogPort = port == null ? -1 : Integer.valueOf(port); configureSyslogDefaults(ident, level, syslogHost, syslogPort); } @@ -448,11 +450,11 @@ private static String getMyPid() { } private static String getSyslogHost() { - return emptyToNull(getenv("SPOTIFY_SYSLOG_HOST")); + return emptyToNull(getenv(SPOTIFY_SYSLOG_HOST)); } private static int getSyslogPort() { - final String port = getenv("SPOTIFY_SYSLOG_PORT"); + final String port = getenv(SPOTIFY_SYSLOG_PORT); return isNullOrEmpty(port) ? -1 : Integer.valueOf(port); } diff --git a/src/main/java/com/spotify/logging/logback/SpotifyInternalAppender.java b/src/main/java/com/spotify/logging/logback/SpotifyInternalAppender.java new file mode 100644 index 0000000..321a766 --- /dev/null +++ b/src/main/java/com/spotify/logging/logback/SpotifyInternalAppender.java @@ -0,0 +1,99 @@ +package com.spotify.logging.logback; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Preconditions; + +import com.spotify.logging.LoggingConfigurator; + +import java.lang.management.ManagementFactory; + +import ch.qos.logback.classic.net.SyslogAppender; +import ch.qos.logback.core.CoreConstants; +import ch.qos.logback.core.net.SyslogAppenderBase; + +/** + * A {@link SyslogAppender} that uses millisecond precision, and that by default configures its + * values for {@link SyslogAppender#syslogHost} and {@link SyslogAppender#port} based on the + * environment variables specified in {@link LoggingConfigurator#SPOTIFY_SYSLOG_HOST} and + * {@link LoggingConfigurator#SPOTIFY_SYSLOG_PORT}. If the configuration explicitly sets the + * {@link SyslogAppenderBase#syslogHost} or {@link SyslogAppenderBase#port} values, the environment + * variables will not be used. Note that logback's configuration support allows you to use + * environment variables in your logback.xml file as well (see + * http://logback.qos.ch/manual/configuration.html#scopes). + */ +@SuppressWarnings("WeakerAccess") +public class SpotifyInternalAppender extends MillisecondPrecisionSyslogAppender { + + private String serviceName; + + private boolean portConfigured = false; + + @Override + public void start() { + Preconditions.checkState(serviceName != null, "serviceName must be configured"); + + // set up some defaults + setFacility("LOCAL0"); + + // 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()); + setSuffixPattern(serviceAndPid + ": %msg"); + setStackTracePattern(serviceAndPid + ": " + CoreConstants.TAB); + + if (getSyslogHost() == null) { + setSyslogHost(System.getenv(LoggingConfigurator.SPOTIFY_SYSLOG_HOST)); + } + checkSetPort(System.getenv(LoggingConfigurator.SPOTIFY_SYSLOG_PORT)); + + super.start(); + } + + private void checkSetPort(String environmentValue) { + if (environmentValue == null || portConfigured) { + return; + } + + try { + setPort(Integer.parseInt(environmentValue)); + } catch (NumberFormatException e) { + throw new IllegalArgumentException( + "unable to parse value for \"" + LoggingConfigurator.SPOTIFY_SYSLOG_PORT + "\" (" + + environmentValue + ") as an int", e); + } + } + + @Override + public void setPort(int port) { + portConfigured = true; + super.setPort(port); + } + + /** + * The service name you want to include in the logs - this is a mandatory setting, and determines + * where syslog-ng will send the log output (/spotify/log/${serviceName}). + * + * @param serviceName the service name + */ + public void setServiceName(String serviceName) { + this.serviceName = serviceName; + } + + // copied from LoggingConfigurator to avoid making public and exposing externally. + // TODO (bjorn): We probably want to move this to the utilities project. + // Also, the portability of this function is not guaranteed. + @VisibleForTesting + static String getMyPid() { + String pid = "0"; + try { + final String nameStr = ManagementFactory.getRuntimeMXBean().getName(); + + // XXX (bjorn): Really stupid parsing assuming that nameStr will be of the form + // "pid@hostname", which is probably not guaranteed. + pid = nameStr.split("@")[0]; + } catch (RuntimeException e) { + // Fall through. + } + return pid; + } +} diff --git a/src/test/java/com/spotify/logging/logback/SpotifyInternalAppenderTest.java b/src/test/java/com/spotify/logging/logback/SpotifyInternalAppenderTest.java new file mode 100644 index 0000000..224b97a --- /dev/null +++ b/src/test/java/com/spotify/logging/logback/SpotifyInternalAppenderTest.java @@ -0,0 +1,134 @@ +package com.spotify.logging.logback; + +import com.spotify.logging.LoggingConfigurator; + +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.contrib.java.lang.system.EnvironmentVariables; +import org.junit.rules.ExpectedException; +import org.slf4j.LoggerFactory; + +import ch.qos.logback.classic.Logger; + +import static com.spotify.logging.logback.SpotifyInternalAppender.getMyPid; +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.is; +import static org.junit.Assert.assertThat; + +public class SpotifyInternalAppenderTest { + + private SpotifyInternalAppender appender; + + @Rule + public final EnvironmentVariables environmentVariables = new EnvironmentVariables(); + @Rule + public final ExpectedException thrown = ExpectedException.none(); + + @Before + public void setUp() throws Exception { + appender = new SpotifyInternalAppender(); + appender.setContext(((Logger) LoggerFactory.getLogger(Logger.ROOT_LOGGER_NAME)).getLoggerContext()); + appender.setServiceName("myservice"); + } + + @Test + public void shouldUseEnvironmentVariableForHostByDefault() throws Exception { + setSyslogHostEnvVar(); + + appender.start(); + + assertThat(appender.getSyslogHost(), is("www.spotify.com")); + } + + @Test + public void shouldUseEnvironmentVariableForPortByDefault() throws Exception { + environmentVariables.set(LoggingConfigurator.SPOTIFY_SYSLOG_PORT, "7642"); + + appender.start(); + + assertThat(appender.getPort(), is(7642)); + } + + @Test + public void shouldFailForNonIntPort() throws Exception { + environmentVariables.set(LoggingConfigurator.SPOTIFY_SYSLOG_PORT, "76424356436234623462345"); + + thrown.expect(IllegalArgumentException.class); + appender.start(); + } + + @Test + public void shouldSupportOverridingHost() throws Exception { + setSyslogHostEnvVar(); + + appender.setSyslogHost("www.dn.se"); + appender.start(); + + assertThat(appender.getSyslogHost(), is("www.dn.se")); + } + + @Test + public void shouldSupportOverridingPort() throws Exception { + environmentVariables.set(LoggingConfigurator.SPOTIFY_SYSLOG_PORT, "7642"); + appender.setPort(9878); + + appender.start(); + + assertThat(appender.getPort(), is(9878)); + } + + @Test + public void shouldSupportOverridingPortTo514() throws Exception { + environmentVariables.set(LoggingConfigurator.SPOTIFY_SYSLOG_PORT, "7642"); + appender.setPort(514); + + appender.start(); + + assertThat(appender.getPort(), is(514)); + } + + @Test + public void shouldFailIfServiceNameMissing() throws Exception { + appender = new SpotifyInternalAppender(); + appender.setContext(((Logger) LoggerFactory.getLogger(Logger.ROOT_LOGGER_NAME)).getLoggerContext()); + + thrown.expect(IllegalStateException.class); + thrown.expectMessage("serviceName must be configured"); + + appender.start(); + } + + @Test + public void shouldAddServiceNameToSuffixPattern() throws Exception { + appender.start(); + + assertThat(appender.getSuffixPattern(), containsString("myservice")); + } + + @Test + public void shouldAddPidToSuffixPattern() throws Exception { + appender.start(); + + assertThat(appender.getSuffixPattern(), containsString(getMyPid())); + } + + @Test + public void shouldAddServiceNameToStackTracePattern() throws Exception { + appender.start(); + + assertThat(appender.getStackTracePattern(), containsString("myservice")); + } + + @Test + public void shouldAddPidToStackTracePattern() throws Exception { + appender.start(); + + assertThat(appender.getStackTracePattern(), containsString(getMyPid())); + } + + private void setSyslogHostEnvVar() { + // this must be a valid host name that can be looked up anywhere + environmentVariables.set(LoggingConfigurator.SPOTIFY_SYSLOG_HOST, "www.spotify.com"); + } +} \ No newline at end of file