From 3abc5ec3fe459659b47e5f48a4b2907c125a2f25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petter=20M=C3=A5hl=C3=A9n?= Date: Wed, 3 Aug 2016 11:57:34 +0200 Subject: [PATCH 1/4] create initial version of EnvironmentVariableSyslogAppender This is intended to support the same logging functionality as LoggingConfigurator, but to be configurable via the standard logback configuration methods. --- pom.xml | 6 + .../spotify/logging/LoggingConfigurator.java | 10 +- .../EnvironmentVariableSyslogAppender.java | 67 ++++++++++ ...EnvironmentVariableSyslogAppenderTest.java | 115 ++++++++++++++++++ 4 files changed, 194 insertions(+), 4 deletions(-) create mode 100644 src/main/java/com/spotify/logging/logback/EnvironmentVariableSyslogAppender.java create mode 100644 src/test/java/com/spotify/logging/logback/EnvironmentVariableSyslogAppenderTest.java 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/EnvironmentVariableSyslogAppender.java b/src/main/java/com/spotify/logging/logback/EnvironmentVariableSyslogAppender.java new file mode 100644 index 0000000..470320c --- /dev/null +++ b/src/main/java/com/spotify/logging/logback/EnvironmentVariableSyslogAppender.java @@ -0,0 +1,67 @@ +package com.spotify.logging.logback; + +import com.spotify.logging.LoggingConfigurator; + +import ch.qos.logback.classic.net.SyslogAppender; + +/** + * 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 #syslogHostEnvVar} and {@link #syslogPortEnvVar}. + */ +public class EnvironmentVariableSyslogAppender extends MillisecondPrecisionSyslogAppender { + + private String syslogHostEnvVar = LoggingConfigurator.SPOTIFY_SYSLOG_HOST; + private String syslogPortEnvVar = LoggingConfigurator.SPOTIFY_SYSLOG_PORT; + + private boolean portConfigured = false; + + @Override + public void start() { + // set up some defaults + setFacility("LOCAL0"); + + if (getSyslogHost() == null) { + setSyslogHost(System.getenv(syslogHostEnvVar)); + } + checkSetPort(System.getenv(syslogPortEnvVar)); + + 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 \"" + syslogPortEnvVar + "\" (" + + environmentValue + ") as an int", e); + } + } + + public String getSyslogHostEnvVar() { + return syslogHostEnvVar; + } + + public void setSyslogHostEnvVar(String syslogHostEnvVar) { + this.syslogHostEnvVar = syslogHostEnvVar; + } + + public String getSyslogPortEnvVar() { + return syslogPortEnvVar; + } + + public void setSyslogPortEnvVar(String syslogPortEnvVar) { + this.syslogPortEnvVar = syslogPortEnvVar; + } + + @Override + public void setPort(int port) { + portConfigured = true; + super.setPort(port); + } +} diff --git a/src/test/java/com/spotify/logging/logback/EnvironmentVariableSyslogAppenderTest.java b/src/test/java/com/spotify/logging/logback/EnvironmentVariableSyslogAppenderTest.java new file mode 100644 index 0000000..78793db --- /dev/null +++ b/src/test/java/com/spotify/logging/logback/EnvironmentVariableSyslogAppenderTest.java @@ -0,0 +1,115 @@ +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 org.hamcrest.CoreMatchers.is; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.fail; + +public class EnvironmentVariableSyslogAppenderTest { + + private EnvironmentVariableSyslogAppender appender; + + @Rule + public final EnvironmentVariables environmentVariables = new EnvironmentVariables(); + @Rule + public final ExpectedException thrown = ExpectedException.none(); + + @Before + public void setUp() throws Exception { + appender = new EnvironmentVariableSyslogAppender(); + appender.setContext(((Logger) LoggerFactory.getLogger(Logger.ROOT_LOGGER_NAME)).getLoggerContext()); + } + + @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 shouldSupportCustomisingEnvironmentVariableForHost() throws Exception { + environmentVariables.set("MY_HOST_ENVVAR", "www.spotify.com"); + + appender.setSyslogHostEnvVar("MY_HOST_ENVVAR"); + + appender.start(); + + assertThat(appender.getSyslogHost(), is("www.spotify.com")); + } + + @Test + public void shouldSupportCustomisingEnvironmentVariableForPort() throws Exception { + environmentVariables.set("MY_PORT_ENVVAR", "5242"); + + appender.setSyslogPortEnvVar("MY_PORT_ENVVAR"); + + appender.start(); + + assertThat(appender.getPort(), is(5242)); + } + + 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 From 7e20eb88be06c861627dbc8800a7064096c193d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petter=20M=C3=A5hl=C3=A9n?= Date: Wed, 3 Aug 2016 13:35:18 +0200 Subject: [PATCH 2/4] add suffix and stack trace pattern config to EnvironmentVariableSyslogAppender --- .../spotify/logging/LoggingConfigurator.java | 2 +- .../EnvironmentVariableSyslogAppender.java | 20 +++++++++ ...EnvironmentVariableSyslogAppenderTest.java | 42 ++++++++++++++++++- 3 files changed, 62 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/spotify/logging/LoggingConfigurator.java b/src/main/java/com/spotify/logging/LoggingConfigurator.java index 256ef22..6f29f8b 100644 --- a/src/main/java/com/spotify/logging/LoggingConfigurator.java +++ b/src/main/java/com/spotify/logging/LoggingConfigurator.java @@ -435,7 +435,7 @@ public static void configure(final File file, final String defaultIdent) { // TODO (bjorn): We probably want to move this to the utilities project. // Also, the portability of this function is not guaranteed. - private static String getMyPid() { + public static String getMyPid() { String pid = "0"; try { final String nameStr = ManagementFactory.getRuntimeMXBean().getName(); diff --git a/src/main/java/com/spotify/logging/logback/EnvironmentVariableSyslogAppender.java b/src/main/java/com/spotify/logging/logback/EnvironmentVariableSyslogAppender.java index 470320c..9a01415 100644 --- a/src/main/java/com/spotify/logging/logback/EnvironmentVariableSyslogAppender.java +++ b/src/main/java/com/spotify/logging/logback/EnvironmentVariableSyslogAppender.java @@ -1,8 +1,11 @@ package com.spotify.logging.logback; +import com.google.common.base.Preconditions; + import com.spotify.logging.LoggingConfigurator; import ch.qos.logback.classic.net.SyslogAppender; +import ch.qos.logback.core.CoreConstants; /** * A {@link SyslogAppender} that uses millisecond precision, and that by default configures its @@ -13,14 +16,23 @@ public class EnvironmentVariableSyslogAppender extends MillisecondPrecisionSyslo private String syslogHostEnvVar = LoggingConfigurator.SPOTIFY_SYSLOG_HOST; private String syslogPortEnvVar = LoggingConfigurator.SPOTIFY_SYSLOG_PORT; + 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, LoggingConfigurator.getMyPid()); + setSuffixPattern(serviceAndPid + ": %msg"); + setStackTracePattern(serviceAndPid + ": " + CoreConstants.TAB); + if (getSyslogHost() == null) { setSyslogHost(System.getenv(syslogHostEnvVar)); } @@ -64,4 +76,12 @@ public void setPort(int port) { portConfigured = true; super.setPort(port); } + + public String getServiceName() { + return serviceName; + } + + public void setServiceName(String serviceName) { + this.serviceName = serviceName; + } } diff --git a/src/test/java/com/spotify/logging/logback/EnvironmentVariableSyslogAppenderTest.java b/src/test/java/com/spotify/logging/logback/EnvironmentVariableSyslogAppenderTest.java index 78793db..d0f5390 100644 --- a/src/test/java/com/spotify/logging/logback/EnvironmentVariableSyslogAppenderTest.java +++ b/src/test/java/com/spotify/logging/logback/EnvironmentVariableSyslogAppenderTest.java @@ -11,9 +11,9 @@ import ch.qos.logback.classic.Logger; +import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.is; import static org.junit.Assert.assertThat; -import static org.junit.Assert.fail; public class EnvironmentVariableSyslogAppenderTest { @@ -28,6 +28,7 @@ public class EnvironmentVariableSyslogAppenderTest { public void setUp() throws Exception { appender = new EnvironmentVariableSyslogAppender(); appender.setContext(((Logger) LoggerFactory.getLogger(Logger.ROOT_LOGGER_NAME)).getLoggerContext()); + appender.setServiceName("myservice"); } @Test @@ -108,6 +109,45 @@ public void shouldSupportCustomisingEnvironmentVariableForPort() throws Exceptio assertThat(appender.getPort(), is(5242)); } + @Test + public void shouldFailIfServiceNameMissing() throws Exception { + appender = new EnvironmentVariableSyslogAppender(); + 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 shouldAddIdentToSuffixPattern() throws Exception { + appender.start(); + + assertThat(appender.getSuffixPattern(), containsString("myservice")); + } + + @Test + public void shouldAddPidToSuffixPattern() throws Exception { + appender.start(); + + assertThat(appender.getSuffixPattern(), containsString(LoggingConfigurator.getMyPid())); + } + + @Test + public void shouldAddIdentToStackTracePattern() throws Exception { + appender.start(); + + assertThat(appender.getStackTracePattern(), containsString("myservice")); + } + + @Test + public void shouldAddPidToStackTracePattern() throws Exception { + appender.start(); + + assertThat(appender.getStackTracePattern(), containsString(LoggingConfigurator.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"); From 099c4f8c2751fa51f07f93974c1ac526731fb0ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petter=20M=C3=A5hl=C3=A9n?= Date: Wed, 3 Aug 2016 15:35:14 +0200 Subject: [PATCH 3/4] address code review comments - rename EnvironmentVariableSyslogAppender to SpotifyInternalAppender - improve javadocs - remove ability to configure environment variables, since logback provides support for that out of the box. --- ...nder.java => SpotifyInternalAppender.java} | 45 ++++++++----------- ....java => SpotifyInternalAppenderTest.java} | 30 ++----------- 2 files changed, 22 insertions(+), 53 deletions(-) rename src/main/java/com/spotify/logging/logback/{EnvironmentVariableSyslogAppender.java => SpotifyInternalAppender.java} (58%) rename src/test/java/com/spotify/logging/logback/{EnvironmentVariableSyslogAppenderTest.java => SpotifyInternalAppenderTest.java} (81%) diff --git a/src/main/java/com/spotify/logging/logback/EnvironmentVariableSyslogAppender.java b/src/main/java/com/spotify/logging/logback/SpotifyInternalAppender.java similarity index 58% rename from src/main/java/com/spotify/logging/logback/EnvironmentVariableSyslogAppender.java rename to src/main/java/com/spotify/logging/logback/SpotifyInternalAppender.java index 9a01415..5261ddc 100644 --- a/src/main/java/com/spotify/logging/logback/EnvironmentVariableSyslogAppender.java +++ b/src/main/java/com/spotify/logging/logback/SpotifyInternalAppender.java @@ -6,16 +6,21 @@ 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 #syslogHostEnvVar} and {@link #syslogPortEnvVar}. + * 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). */ -public class EnvironmentVariableSyslogAppender extends MillisecondPrecisionSyslogAppender { +@SuppressWarnings("WeakerAccess") +public class SpotifyInternalAppender extends MillisecondPrecisionSyslogAppender { - private String syslogHostEnvVar = LoggingConfigurator.SPOTIFY_SYSLOG_HOST; - private String syslogPortEnvVar = LoggingConfigurator.SPOTIFY_SYSLOG_PORT; private String serviceName; private boolean portConfigured = false; @@ -34,9 +39,9 @@ public void start() { setStackTracePattern(serviceAndPid + ": " + CoreConstants.TAB); if (getSyslogHost() == null) { - setSyslogHost(System.getenv(syslogHostEnvVar)); + setSyslogHost(System.getenv(LoggingConfigurator.SPOTIFY_SYSLOG_HOST)); } - checkSetPort(System.getenv(syslogPortEnvVar)); + checkSetPort(System.getenv(LoggingConfigurator.SPOTIFY_SYSLOG_PORT)); super.start(); } @@ -50,37 +55,23 @@ private void checkSetPort(String environmentValue) { setPort(Integer.parseInt(environmentValue)); } catch (NumberFormatException e) { throw new IllegalArgumentException( - "unable to parse value for \"" + syslogPortEnvVar + "\" (" + + "unable to parse value for \"" + LoggingConfigurator.SPOTIFY_SYSLOG_PORT + "\" (" + environmentValue + ") as an int", e); } } - public String getSyslogHostEnvVar() { - return syslogHostEnvVar; - } - - public void setSyslogHostEnvVar(String syslogHostEnvVar) { - this.syslogHostEnvVar = syslogHostEnvVar; - } - - public String getSyslogPortEnvVar() { - return syslogPortEnvVar; - } - - public void setSyslogPortEnvVar(String syslogPortEnvVar) { - this.syslogPortEnvVar = syslogPortEnvVar; - } - @Override public void setPort(int port) { portConfigured = true; super.setPort(port); } - public String getServiceName() { - return serviceName; - } - + /** + * 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; } diff --git a/src/test/java/com/spotify/logging/logback/EnvironmentVariableSyslogAppenderTest.java b/src/test/java/com/spotify/logging/logback/SpotifyInternalAppenderTest.java similarity index 81% rename from src/test/java/com/spotify/logging/logback/EnvironmentVariableSyslogAppenderTest.java rename to src/test/java/com/spotify/logging/logback/SpotifyInternalAppenderTest.java index d0f5390..c0cf405 100644 --- a/src/test/java/com/spotify/logging/logback/EnvironmentVariableSyslogAppenderTest.java +++ b/src/test/java/com/spotify/logging/logback/SpotifyInternalAppenderTest.java @@ -15,9 +15,9 @@ import static org.hamcrest.CoreMatchers.is; import static org.junit.Assert.assertThat; -public class EnvironmentVariableSyslogAppenderTest { +public class SpotifyInternalAppenderTest { - private EnvironmentVariableSyslogAppender appender; + private SpotifyInternalAppender appender; @Rule public final EnvironmentVariables environmentVariables = new EnvironmentVariables(); @@ -26,7 +26,7 @@ public class EnvironmentVariableSyslogAppenderTest { @Before public void setUp() throws Exception { - appender = new EnvironmentVariableSyslogAppender(); + appender = new SpotifyInternalAppender(); appender.setContext(((Logger) LoggerFactory.getLogger(Logger.ROOT_LOGGER_NAME)).getLoggerContext()); appender.setServiceName("myservice"); } @@ -87,31 +87,9 @@ public void shouldSupportOverridingPortTo514() throws Exception { assertThat(appender.getPort(), is(514)); } - @Test - public void shouldSupportCustomisingEnvironmentVariableForHost() throws Exception { - environmentVariables.set("MY_HOST_ENVVAR", "www.spotify.com"); - - appender.setSyslogHostEnvVar("MY_HOST_ENVVAR"); - - appender.start(); - - assertThat(appender.getSyslogHost(), is("www.spotify.com")); - } - - @Test - public void shouldSupportCustomisingEnvironmentVariableForPort() throws Exception { - environmentVariables.set("MY_PORT_ENVVAR", "5242"); - - appender.setSyslogPortEnvVar("MY_PORT_ENVVAR"); - - appender.start(); - - assertThat(appender.getPort(), is(5242)); - } - @Test public void shouldFailIfServiceNameMissing() throws Exception { - appender = new EnvironmentVariableSyslogAppender(); + appender = new SpotifyInternalAppender(); appender.setContext(((Logger) LoggerFactory.getLogger(Logger.ROOT_LOGGER_NAME)).getLoggerContext()); thrown.expect(IllegalStateException.class); From ebda0ab61fb54e7c55e3ca2f552c4c03af0c3229 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petter=20M=C3=A5hl=C3=A9n?= Date: Wed, 3 Aug 2016 16:55:00 +0200 Subject: [PATCH 4/4] hide 'getMyPid' implementations and minor test method name changes --- .../spotify/logging/LoggingConfigurator.java | 2 +- .../logback/SpotifyInternalAppender.java | 23 ++++++++++++++++++- .../logback/SpotifyInternalAppenderTest.java | 9 ++++---- 3 files changed, 28 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/spotify/logging/LoggingConfigurator.java b/src/main/java/com/spotify/logging/LoggingConfigurator.java index 6f29f8b..256ef22 100644 --- a/src/main/java/com/spotify/logging/LoggingConfigurator.java +++ b/src/main/java/com/spotify/logging/LoggingConfigurator.java @@ -435,7 +435,7 @@ public static void configure(final File file, final String defaultIdent) { // TODO (bjorn): We probably want to move this to the utilities project. // Also, the portability of this function is not guaranteed. - public static String getMyPid() { + private static String getMyPid() { String pid = "0"; try { final String nameStr = ManagementFactory.getRuntimeMXBean().getName(); diff --git a/src/main/java/com/spotify/logging/logback/SpotifyInternalAppender.java b/src/main/java/com/spotify/logging/logback/SpotifyInternalAppender.java index 5261ddc..321a766 100644 --- a/src/main/java/com/spotify/logging/logback/SpotifyInternalAppender.java +++ b/src/main/java/com/spotify/logging/logback/SpotifyInternalAppender.java @@ -1,9 +1,12 @@ 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; @@ -34,7 +37,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, LoggingConfigurator.getMyPid()); + String serviceAndPid = String.format("%s[%s]", serviceName, getMyPid()); setSuffixPattern(serviceAndPid + ": %msg"); setStackTracePattern(serviceAndPid + ": " + CoreConstants.TAB); @@ -75,4 +78,22 @@ public void setPort(int port) { 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 index c0cf405..224b97a 100644 --- a/src/test/java/com/spotify/logging/logback/SpotifyInternalAppenderTest.java +++ b/src/test/java/com/spotify/logging/logback/SpotifyInternalAppenderTest.java @@ -11,6 +11,7 @@ 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; @@ -99,7 +100,7 @@ public void shouldFailIfServiceNameMissing() throws Exception { } @Test - public void shouldAddIdentToSuffixPattern() throws Exception { + public void shouldAddServiceNameToSuffixPattern() throws Exception { appender.start(); assertThat(appender.getSuffixPattern(), containsString("myservice")); @@ -109,11 +110,11 @@ public void shouldAddIdentToSuffixPattern() throws Exception { public void shouldAddPidToSuffixPattern() throws Exception { appender.start(); - assertThat(appender.getSuffixPattern(), containsString(LoggingConfigurator.getMyPid())); + assertThat(appender.getSuffixPattern(), containsString(getMyPid())); } @Test - public void shouldAddIdentToStackTracePattern() throws Exception { + public void shouldAddServiceNameToStackTracePattern() throws Exception { appender.start(); assertThat(appender.getStackTracePattern(), containsString("myservice")); @@ -123,7 +124,7 @@ public void shouldAddIdentToStackTracePattern() throws Exception { public void shouldAddPidToStackTracePattern() throws Exception { appender.start(); - assertThat(appender.getStackTracePattern(), containsString(LoggingConfigurator.getMyPid())); + assertThat(appender.getStackTracePattern(), containsString(getMyPid())); } private void setSyslogHostEnvVar() {