Skip to content

Commit 906c195

Browse files
committed
Improve log formatters
Fixes the following issues: - SimpleDateFormat is not thread safe, so the shared instance should be protected by synchronization. - Logged exceptions are hidden on low log levels, even if the handler is configured to show messages on that level. - IndexDatabase re-implements printStackTrace() in order to work around exceptions not being logged on low log levels.
1 parent d7440ea commit 906c195

File tree

3 files changed

+56
-65
lines changed

3 files changed

+56
-65
lines changed

src/org/opensolaris/opengrok/SimpleConsoleFormatter.java

Lines changed: 24 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,15 @@
1818
*/
1919

2020
/*
21-
* Copyright (c) 2010, 2011, Oracle and/or its affiliates. All rights reserved.
21+
* Copyright (c) 2010, 2013, Oracle and/or its affiliates. All rights reserved.
2222
*/
2323

2424
package org.opensolaris.opengrok;
2525

26+
import java.io.PrintWriter;
27+
import java.io.StringWriter;
2628
import java.util.Date;
2729
import java.util.logging.Formatter;
28-
import java.util.logging.Level;
2930
import java.util.logging.LogRecord;
3031

3132
/**
@@ -38,29 +39,30 @@ final public class SimpleConsoleFormatter extends Formatter {
3839

3940
private final java.text.SimpleDateFormat formatter =
4041
new java.text.SimpleDateFormat("HH:mm:ss");
41-
private static final String lineSeparator = System.
42-
getProperty("line.separator");
4342

44-
private String ts(Date date) {
43+
// Format the time stamp. Must be synchronized since SimpleDateFormatter
44+
// isn't thread safe.
45+
private synchronized String ts(Date date) {
4546
return formatter.format(date);
4647
}
4748

4849
@Override
49-
public String format(LogRecord record) {
50-
StringBuilder sb = new StringBuilder();
51-
sb.append(ts(new Date(record.getMillis())));
52-
sb.append(" ");
53-
sb.append(record.getLevel().getName());
54-
sb.append(": ");
55-
sb.append(formatMessage(record));
56-
Throwable thrown = record.getThrown();
57-
if (null != thrown && record.getLevel().intValue() > Level.CONFIG.intValue()) {
58-
sb.append(lineSeparator);
59-
java.io.ByteArrayOutputStream ba=new java.io.ByteArrayOutputStream();
60-
thrown.printStackTrace(new java.io.PrintStream(ba, true));
61-
sb.append(ba.toString());
62-
}
63-
sb.append(lineSeparator);
64-
return sb.toString();
65-
}
50+
public String format(LogRecord record) {
51+
StringWriter sw = new StringWriter();
52+
53+
try (PrintWriter pw = new PrintWriter(sw)) {
54+
pw.print(ts(new Date(record.getMillis())));
55+
pw.print(' ');
56+
pw.print(record.getLevel().getName());
57+
pw.print(": ");
58+
pw.print(formatMessage(record));
59+
pw.println();
60+
Throwable thrown = record.getThrown();
61+
if (thrown != null) {
62+
thrown.printStackTrace(pw);
63+
}
64+
}
65+
66+
return sw.toString();
67+
}
6668
}

src/org/opensolaris/opengrok/SimpleFileLogFormatter.java

Lines changed: 30 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,15 @@
1818
*/
1919

2020
/*
21-
* Copyright (c) 2010, 2011, Oracle and/or its affiliates. All rights reserved.
21+
* Copyright (c) 2010, 2013, Oracle and/or its affiliates. All rights reserved.
2222
*/
2323

2424
package org.opensolaris.opengrok;
2525

26+
import java.io.PrintWriter;
27+
import java.io.StringWriter;
2628
import java.util.Date;
2729
import java.util.logging.Formatter;
28-
import java.util.logging.Level;
2930
import java.util.logging.LogRecord;
3031

3132
/**
@@ -38,11 +39,10 @@ final public class SimpleFileLogFormatter extends Formatter {
3839

3940
private final java.text.SimpleDateFormat formatter =
4041
new java.text.SimpleDateFormat("yyyy-MM-dd' 'HH:mm:ss.SSSZ");
41-
private static final String lineSeparator = System.
42-
getProperty("line.separator");
43-
Date dat = new Date();
4442

45-
private String ts(Date date) {
43+
// Format the time stamp. Must be synchronized since SimpleDateFormatter
44+
// isn't thread safe.
45+
private synchronized String ts(Date date) {
4646
return formatter.format(date);
4747
}
4848

@@ -52,30 +52,28 @@ private String classNameOnly(String name) {
5252
}
5353

5454
@Override
55-
public String format(LogRecord record) {
56-
StringBuilder sb = new StringBuilder();
57-
dat.setTime(record.getMillis());
58-
sb.append(ts(dat));
59-
sb.append(" ");
60-
String loglevel = record.getLevel().getName();
61-
sb.append(loglevel);
62-
sb.append(" ");
63-
sb.append("t");
64-
sb.append(record.getThreadID());
65-
sb.append(" ");
66-
sb.append(classNameOnly(record.getSourceClassName()));
67-
sb.append('.');
68-
sb.append(record.getSourceMethodName());
69-
sb.append(": ");
70-
sb.append(formatMessage(record));
71-
Throwable thrown = record.getThrown();
72-
if (null != thrown && record.getLevel().intValue() > Level.CONFIG.intValue()) {
73-
sb.append(lineSeparator);
74-
java.io.ByteArrayOutputStream ba=new java.io.ByteArrayOutputStream();
75-
thrown.printStackTrace(new java.io.PrintStream(ba, true));
76-
sb.append(ba.toString());
77-
}
78-
sb.append(lineSeparator);
79-
return sb.toString();
80-
}
55+
public String format(LogRecord record) {
56+
StringWriter sw = new StringWriter();
57+
58+
try (PrintWriter pw = new PrintWriter(sw)) {
59+
pw.print(ts(new Date(record.getMillis())));
60+
pw.print(' ');
61+
pw.print(record.getLevel().getName());
62+
pw.print(" t");
63+
pw.print(record.getThreadID());
64+
pw.print(' ');
65+
pw.print(classNameOnly(record.getSourceClassName()));
66+
pw.print('.');
67+
pw.print(record.getSourceMethodName());
68+
pw.print(": ");
69+
pw.print(formatMessage(record));
70+
pw.println();
71+
Throwable thrown = record.getThrown();
72+
if (thrown != null) {
73+
thrown.printStackTrace(pw);
74+
}
75+
}
76+
77+
return sw.toString();
78+
}
8179
}

src/org/opensolaris/opengrok/index/IndexDatabase.java

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -640,17 +640,8 @@ private void addFile(File file, String path) throws IOException {
640640
"Skipped file ''{0}'' because the analyzer didn''t "
641641
+ "understand it.",
642642
path);
643-
StringBuilder stack = new StringBuilder();
644-
for (StackTraceElement ste : e.getStackTrace()) {
645-
stack.append(ste.toString()).append(System.lineSeparator());
646-
}
647-
StringBuilder sstack = new StringBuilder();
648-
for (Throwable t : e.getSuppressed()) {
649-
for (StackTraceElement ste : t.getStackTrace()) {
650-
sstack.append(ste.toString()).append(System.lineSeparator());
651-
}
652-
}
653-
log.log(Level.FINE, "Exception from analyzer {0}: {1} {2}{3}{4}{5}{6}", new String[]{fa.getClass().getName(), e.toString(), System.lineSeparator(), stack.toString(), System.lineSeparator(), sstack.toString()});
643+
log.log(Level.FINE,
644+
"Exception from analyzer " + fa.getClass().getName(), e);
654645
cleanupResources(doc);
655646
return;
656647
}

0 commit comments

Comments
 (0)