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

Commit 774d0f5

Browse files
author
Daniel Norberg
authored
nonnull + spotbugs + cleanup (#36)
1 parent b6c290b commit 774d0f5

13 files changed

+153
-63
lines changed

‎pom.xml‎

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
1+
<project xmlns="http://maven.apache.org/POM/4.0.0"
2+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
3+
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
24
<modelVersion>4.0.0</modelVersion>
35

46
<artifactId>logging</artifactId>
@@ -93,10 +95,11 @@
9395
<version>4.11</version>
9496
</dependency>
9597

98+
<!-- Annotations-->
9699
<dependency>
97-
<groupId>com.intellij</groupId>
98-
<artifactId>annotations</artifactId>
99-
<version>9.0.4</version>
100+
<groupId>io.norberg</groupId>
101+
<artifactId>some</artifactId>
102+
<version>1.0.0</version>
100103
<scope>provided</scope>
101104
</dependency>
102105

@@ -168,6 +171,25 @@
168171
</execution>
169172
</executions>
170173
</plugin>
174+
<plugin>
175+
<groupId>com.github.spotbugs</groupId>
176+
<artifactId>spotbugs-maven-plugin</artifactId>
177+
<version>4.1.4</version>
178+
<configuration>
179+
<effort>Max</effort>
180+
<threshold>Low</threshold>
181+
<failOnError>true</failOnError>
182+
<xmlOutput>true</xmlOutput>
183+
</configuration>
184+
<executions>
185+
<execution>
186+
<phase>verify</phase>
187+
<goals>
188+
<goal>check</goal>
189+
</goals>
190+
</execution>
191+
</executions>
192+
</plugin>
171193
</plugins>
172194
</build>
173195
</project>

‎src/main/java/com/spotify/logging/LoggingConfigurator.java‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
import java.io.File;
5757
import java.lang.management.ManagementFactory;
5858
import java.nio.charset.StandardCharsets;
59+
import javax.annotation.Nullable;
5960
import net.logstash.logback.composite.loggingevent.ArgumentsJsonProvider;
6061
import org.slf4j.LoggerFactory;
6162

@@ -91,6 +92,7 @@ public static String getMsgPattern(final ReplaceNewLines replaceNewLines) {
9192
public static final String SPOTIFY_SYSLOG_PORT = "SPOTIFY_SYSLOG_PORT";
9293
private static final String USE_JSON_LOGGING = "USE_JSON_LOGGING";
9394

95+
@SuppressWarnings("unused")
9496
public enum Level {
9597
OFF(ch.qos.logback.classic.Level.OFF),
9698
ERROR(ch.qos.logback.classic.Level.ERROR),
@@ -189,7 +191,7 @@ public static void configureDefaults(
189191
// Call configureSyslogDefaults if the SPOTIFY_SYSLOG_HOST or SPOTIFY_SYSLOG_PORT env var is
190192
// set. If this causes a problem, we could introduce a configureConsoleDefaults method which
191193
// users could call instead to avoid this behavior.
192-
final String syslogHost = getSyslogHost();
194+
final @Nullable String syslogHost = getSyslogHost();
193195
final int syslogPort = getSyslogPort();
194196
if (syslogHost != null || syslogPort != -1) {
195197
configureSyslogDefaults(ident, level, syslogHost, syslogPort, replaceNewLines);
@@ -229,7 +231,7 @@ public static void configureLogstashEncoderDefaults(final Level level) {
229231
encoder.addProvider(new ArgumentsJsonProvider());
230232
encoder.start();
231233

232-
final ConsoleAppender<ILoggingEvent> appender = new ConsoleAppender<ILoggingEvent>();
234+
final ConsoleAppender<ILoggingEvent> appender = new ConsoleAppender<>();
233235
appender.setTarget("System.out");
234236
appender.setName("stdout");
235237
appender.setEncoder(encoder);
@@ -262,7 +264,7 @@ public static void configureSyslogDefaults(
262264
final String ident, final Level level, final ReplaceNewLines replaceNewLines) {
263265
final String syslogHost = getenv(SPOTIFY_SYSLOG_HOST);
264266
final String port = getenv(SPOTIFY_SYSLOG_PORT);
265-
final int syslogPort = port == null ? -1 : Integer.valueOf(port);
267+
final int syslogPort = port == null ? -1 : Integer.parseInt(port);
266268
configureSyslogDefaults(ident, level, syslogHost, syslogPort, replaceNewLines);
267269
}
268270

@@ -278,7 +280,7 @@ public static void configureSyslogDefaults(
278280
public static void configureSyslogDefaults(
279281
final String ident,
280282
final Level level,
281-
final String host,
283+
final @Nullable String host,
282284
final int port,
283285
final ReplaceNewLines replaceNewLines) {
284286
configureSyslogDefaults(ident, level, host, port, Logger.ROOT_LOGGER_NAME, replaceNewLines);
@@ -297,7 +299,7 @@ public static void configureSyslogDefaults(
297299
public static void configureSyslogDefaults(
298300
final String ident,
299301
final Level level,
300-
final String host,
302+
final @Nullable String host,
301303
final int port,
302304
final String loggerName,
303305
final ReplaceNewLines replaceNewLines) {
@@ -374,7 +376,7 @@ private static Appender<ILoggingEvent> getStdErrAppender(
374376
encoder.start();
375377

376378
// Setup stderr appender
377-
final ConsoleAppender<ILoggingEvent> appender = new ConsoleAppender<ILoggingEvent>();
379+
final ConsoleAppender<ILoggingEvent> appender = new ConsoleAppender<>();
378380
appender.setTarget("System.err");
379381
appender.setName("stderr");
380382
appender.setEncoder(encoder);
@@ -395,7 +397,7 @@ private static Appender<ILoggingEvent> getStdErrAppender(
395397
*/
396398
static Appender<ILoggingEvent> getSyslogAppender(
397399
final LoggerContext context,
398-
final String host,
400+
final @Nullable String host,
399401
final int port,
400402
final ReplaceNewLines replaceNewLines) {
401403
final String h = (host == null || host.isEmpty()) ? "localhost" : host;
@@ -430,6 +432,7 @@ static Appender<ILoggingEvent> getSyslogAppender(
430432
*
431433
* @deprecated Don't use, see docs.
432434
*/
435+
@Deprecated
433436
static void configure(final JewelCliLoggingOptions opts) {
434437
// Use logback config file to setup logging if specified, discarding any other logging options.
435438
if (!opts.logFileName().isEmpty()) {
@@ -448,7 +451,7 @@ static void configure(final JewelCliLoggingOptions opts) {
448451
// See if syslog host was specified via command line or environment variable.
449452
// The command line value takes precedence, which defaults to an empty string.
450453
String syslogHost = opts.syslogHost();
451-
if (syslogHost == null || syslogHost.isEmpty()) {
454+
if (syslogHost.isEmpty()) {
452455
syslogHost = getSyslogHost();
453456
}
454457

@@ -561,7 +564,7 @@ private static String getMyPid() {
561564
return pid;
562565
}
563566

564-
private static String getSyslogHost() {
567+
private static @Nullable String getSyslogHost() {
565568
final String host = System.getenv().getOrDefault(SPOTIFY_SYSLOG_HOST, "");
566569
return host.isEmpty() ? null : host;
567570
}
@@ -571,7 +574,7 @@ private static int getSyslogPort() {
571574
return port.isEmpty() ? -1 : Integer.parseInt(port);
572575
}
573576

574-
private static String getSpotifyHostname() {
577+
private static @Nullable String getSpotifyHostname() {
575578
final String hostname = System.getenv().getOrDefault(SPOTIFY_HOSTNAME, "");
576579
return hostname.isEmpty() ? null : hostname;
577580
}

‎src/main/java/com/spotify/logging/LoggingSupport.java‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
package com.spotify.logging;
3838

3939
import java.util.concurrent.atomic.AtomicLong;
40+
import javax.annotation.Nullable;
4041
import org.slf4j.Logger;
4142

4243
/** Utility for emitting log messages in the Spotify log format. */
@@ -109,7 +110,7 @@ public static void debug(
109110
final Logger logger,
110111
final String type,
111112
final int version,
112-
final Ident ident,
113+
final @Nullable Ident ident,
113114
final Object... args) {
114115

115116
logger.debug(buildLogLine(type, version, ident, args));
@@ -124,7 +125,7 @@ public static void info(
124125
final Logger logger,
125126
final String type,
126127
final int version,
127-
final Ident ident,
128+
final @Nullable Ident ident,
128129
final Object... args) {
129130

130131
logger.info(buildLogLine(type, version, ident, args));
@@ -139,7 +140,7 @@ public static void warn(
139140
final Logger logger,
140141
final String type,
141142
final int version,
142-
final Ident ident,
143+
final @Nullable Ident ident,
143144
final Object... args) {
144145

145146
logger.warn(buildLogLine(type, version, ident, args));
@@ -154,14 +155,14 @@ public static void error(
154155
final Logger logger,
155156
final String type,
156157
final int version,
157-
final Ident ident,
158+
final @Nullable Ident ident,
158159
final Object... args) {
159160

160161
logger.error(buildLogLine(type, version, ident, args));
161162
}
162163

163164
protected static String buildLogLine(
164-
final String type, final int version, final Ident ident, final Object... args) {
165+
final String type, final int version, @Nullable final Ident ident, final Object... args) {
165166
final StringBuilder line = new StringBuilder();
166167
line.append(LoggingSupport.rid.getAndIncrement()).append(' ');
167168

@@ -182,7 +183,7 @@ protected static String buildLogLine(
182183
return line.toString();
183184
}
184185

185-
protected static void appendEscaped(final Object o, final StringBuilder out) {
186+
protected static void appendEscaped(final @Nullable Object o, final StringBuilder out) {
186187
if (o == null) {
187188
return;
188189
}

‎src/main/java/com/spotify/logging/UncaughtExceptionLogger.java‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,11 +45,6 @@ public class UncaughtExceptionLogger {
4545

4646
public static void setDefaultUncaughtExceptionHandler() {
4747
Thread.setDefaultUncaughtExceptionHandler(
48-
new Thread.UncaughtExceptionHandler() {
49-
@Override
50-
public void uncaughtException(final Thread th, final Throwable t) {
51-
logger.error("Uncaught exception in thread " + th, t);
52-
}
53-
});
48+
(th, t) -> logger.error("Uncaught exception in thread " + th, t));
5449
}
5550
}

‎src/main/java/com/spotify/logging/logback/LevelRangeFilter.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import ch.qos.logback.classic.spi.ILoggingEvent;
4141
import ch.qos.logback.core.filter.Filter;
4242
import ch.qos.logback.core.spi.FilterReply;
43+
import javax.annotation.Nullable;
4344

4445
/**
4546
* Filters events within the levels levelMin and levelMax, inclusive.
@@ -50,8 +51,8 @@
5051
*/
5152
public class LevelRangeFilter extends Filter<ILoggingEvent> {
5253

53-
private Level levelMax;
54-
private Level levelMin;
54+
private @Nullable Level levelMax;
55+
private @Nullable Level levelMin;
5556

5657
@Override
5758
public FilterReply decide(final ILoggingEvent event) {

‎src/main/java/com/spotify/logging/logback/LoggerThresholdFilter.java‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import ch.qos.logback.classic.spi.ILoggingEvent;
4141
import ch.qos.logback.core.filter.Filter;
4242
import ch.qos.logback.core.spi.FilterReply;
43+
import javax.annotation.Nullable;
4344

4445
/**
4546
* Comparable to ch.qos.logback.classic.filter.ThresholdFilter, but for a specific logger. Filters
@@ -48,20 +49,27 @@
4849
*/
4950
public class LoggerThresholdFilter extends Filter<ILoggingEvent> {
5051

51-
private String logger;
52-
private Level level;
53-
private String exceptLogger;
52+
private @Nullable String logger;
53+
private @Nullable Level level;
54+
private @Nullable String exceptLogger;
5455

5556
@Override
5657
public FilterReply decide(ILoggingEvent event) {
57-
if (!isStarted()) return FilterReply.NEUTRAL;
58+
if (!isStarted()) {
59+
return FilterReply.NEUTRAL;
60+
}
5861

59-
if (logger != null && !event.getLoggerName().startsWith(logger)) return FilterReply.NEUTRAL;
62+
if (logger != null && !event.getLoggerName().startsWith(logger)) {
63+
return FilterReply.NEUTRAL;
64+
}
6065

61-
if (exceptLogger != null && event.getLoggerName().startsWith(exceptLogger))
66+
if (exceptLogger != null && event.getLoggerName().startsWith(exceptLogger)) {
6267
return FilterReply.NEUTRAL;
68+
}
6369

64-
if (level != null && !event.getLevel().isGreaterOrEqual(level)) return FilterReply.DENY;
70+
if (level != null && !event.getLevel().isGreaterOrEqual(level)) {
71+
return FilterReply.DENY;
72+
}
6573

6674
return FilterReply.NEUTRAL;
6775
}

‎src/main/java/com/spotify/logging/logback/MillisecondPrecisionSyslogAppender.java‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -49,12 +49,13 @@
4949
import java.lang.reflect.Field;
5050
import java.nio.charset.Charset;
5151
import java.nio.charset.StandardCharsets;
52+
import javax.annotation.Nullable;
5253

5354
/** A {@link SyslogAppender} with millisecond timestamp precision. */
5455
public class MillisecondPrecisionSyslogAppender extends SyslogAppender {
5556
private Charset charset = StandardCharsets.UTF_8;
56-
PatternLayout stackTraceLayout = new PatternLayout();
57-
private OutputStream sos;
57+
private final PatternLayout stackTraceLayout = new PatternLayout();
58+
private @Nullable OutputStream sos;
5859

5960
@Override
6061
public void start() {
@@ -82,6 +83,7 @@ protected void append(ILoggingEvent eventObject) {
8283
if (msg.length() > getMaxMessageSize()) {
8384
msg = msg.substring(0, getMaxMessageSize());
8485
}
86+
assert sos != null;
8587
sos.write(msg.getBytes(charset));
8688
sos.flush();
8789
postProcess(eventObject, sos);
@@ -112,7 +114,7 @@ private void recursiveWrite(
112114
final String stackTracePrefix,
113115
final IThrowableProxy tp,
114116
final int indent,
115-
final String firstLinePrefix) {
117+
final @Nullable String firstLinePrefix) {
116118
final StackTraceElementProxy[] stepArray = tp.getStackTraceElementProxyArray();
117119
try {
118120
handleThrowableFirstLine(sw, tp, stackTracePrefix, indent, firstLinePrefix);
@@ -121,7 +123,7 @@ private void recursiveWrite(
121123
sb.append(stackTracePrefix);
122124
addIndent(sb, indent);
123125
sb.append(step);
124-
sw.write(sb.toString().getBytes());
126+
sw.write(sb.toString().getBytes(StandardCharsets.UTF_8));
125127
sw.flush();
126128
}
127129
} catch (IOException e) {
@@ -153,15 +155,15 @@ private void handleThrowableFirstLine(
153155
final IThrowableProxy tp,
154156
final String stackTracePrefix,
155157
final int indent,
156-
final String prefix)
158+
final @Nullable String prefix)
157159
throws IOException {
158160
StringBuilder sb = new StringBuilder().append(stackTracePrefix);
159161
addIndent(sb, indent);
160162
if (prefix != null) {
161163
sb.append(prefix);
162164
}
163165
sb.append(tp.getClassName()).append(": ").append(tp.getMessage());
164-
sw.write(sb.toString().getBytes());
166+
sw.write(sb.toString().getBytes(StandardCharsets.UTF_8));
165167
sw.flush();
166168
}
167169

0 commit comments

Comments
 (0)