Enable logging configuration via config file

pom.xml

@@ -96,6 +96,12 @@
       <version>1.9.5</version>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>com.github.stefanbirkner</groupId>
+      <artifactId>system-rules</artifactId>
+      <version>1.16.0</version>
+      <scope>test</scope>
+    </dependency>
   </dependencies>
 
   <build>

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);
   }
 

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;
+  }
+}

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");
+  }
+}