You are viewing a plain text version of this content. The canonical link for it is here.
Posted to log4j-dev@logging.apache.org by rg...@apache.org on 2010/10/25 16:25:09 UTC
svn commit: r1027132 - in
/logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers:
log4j2-api/src/main/java/org/apache/logging/log4j/message/
log4j2-core/src/main/java/org/apache/logging/log4j/core/
log4j2-core/src/main/java/org/apache/logging/log4j/cor...
Author: rgoers
Date: Mon Oct 25 14:25:09 2010
New Revision: 1027132
URL: http://svn.apache.org/viewvc?rev=1027132&view=rev
Log:
Comment on or address more of the @doubt comments raised
Modified:
logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-api/src/main/java/org/apache/logging/log4j/message/Message.java
logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-core/src/main/java/org/apache/logging/log4j/core/ErrorHandler.java
logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-core/src/main/java/org/apache/logging/log4j/core/Layout.java
logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-core/src/main/java/org/apache/logging/log4j/core/LogEvent.java
logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-core/src/main/java/org/apache/logging/log4j/core/appender/AppenderRuntimeException.java
logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-core/src/main/java/org/apache/logging/log4j/core/appender/ConsoleAppender.java
logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-core/src/main/java/org/apache/logging/log4j/core/appender/OutputStreamAppender.java
logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-core/src/main/java/org/apache/logging/log4j/core/config/AppenderControl.java
logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-core/src/main/java/org/apache/logging/log4j/core/layout/PatternLayout.java
logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-core/src/test/java/org/apache/logging/log4j/core/SimplePerfTest.java
Modified: logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-api/src/main/java/org/apache/logging/log4j/message/Message.java
URL: http://svn.apache.org/viewvc/logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-api/src/main/java/org/apache/logging/log4j/message/Message.java?rev=1027132&r1=1027131&r2=1027132&view=diff
==============================================================================
--- logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-api/src/main/java/org/apache/logging/log4j/message/Message.java (original)
+++ logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-api/src/main/java/org/apache/logging/log4j/message/Message.java Mon Oct 25 14:25:09 2010
@@ -20,8 +20,14 @@ import java.io.Serializable;
import java.util.Map;
/**
- * An interface for various Message implementations that can be logged.
+ * An interface for various Message implementations that can be logged. Messages can act as wrappers
+ * around Objects so that user can have control over converting Objects to Strings when necessary without
+ * requiring complicated formatters and as a way to manipulate the message based on information available
+ * at runtime such as the locale of the system.
* @doubt Interfaces should rarely extend Serializable according to Effective Java 2nd Ed pg 291.
+ * (RG) That section also says "If a class or interface exists primarily to participate in a framework that
+ * requires all participants to implement Serializable, then it makes perfect sense for the class or
+ * interface to implement or extend Serializable". Such is the case here as the LogEvent must be Serializable.
*/
public interface Message extends Serializable {
/**
@@ -32,10 +38,14 @@ public interface Message extends Seriali
String getFormattedMessage();
/**
- * Returns the format portion of the Message
+ * Returns the format portion of the Message.
*
* @return The message format.
* @doubt Do all messages have a format? What syntax? Using a Formatter object could be cleaner.
+ * (RG) In SimpleMessage the format is identical to the formatted message. In ParameterizedMessage and
+ * StructuredDataMessage itis not. It is up to the Message implementer to determine what this
+ * method will return. A Formatter is inappropriate as this is very specific to the Message
+ * implementation so it isn't clear to me how having a Formatter separate from the Message would be cleaner.
*/
String getMessageFormat();
@@ -53,7 +63,10 @@ public interface Message extends Seriali
* provide values for. The Message must be able to return a formatted message even if
* no hints are provided.
* @return The Message hints.
- * @doubt would seem to go better into a formatter or format object.
+ * @doubt would seem to go better into a formatter or format object. (RG) A Formatter would have
+ * to understand every type of object that could be passed to it or you would have to
+ * configure an endless number of formatters on loggers and somehow pick the correct one. A Message
+ * implementation formats based only on what can be placed into the Message and what hints are provided.
*/
Map<MessageHint, String> getHints();
}
Modified: logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-core/src/main/java/org/apache/logging/log4j/core/ErrorHandler.java
URL: http://svn.apache.org/viewvc/logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-core/src/main/java/org/apache/logging/log4j/core/ErrorHandler.java?rev=1027132&r1=1027131&r2=1027132&view=diff
==============================================================================
--- logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-core/src/main/java/org/apache/logging/log4j/core/ErrorHandler.java (original)
+++ logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-core/src/main/java/org/apache/logging/log4j/core/ErrorHandler.java Mon Oct 25 14:25:09 2010
@@ -21,7 +21,8 @@ import org.apache.logging.log4j.core.Log
/**
* Appenders may delegate their error handling to <code>ErrorHandlers</code>.
* @doubt if the appender interface is simplified, then error handling could just be done by wrapping
- * a nested appender.
+ * a nested appender. (RG) Please look at DefaultErrorHandler. It's purpose is to make sure the console
+ * or error log isn't flooded with messages. I'm still considering the Appender refactoring.
*/
public interface ErrorHandler {
Modified: logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-core/src/main/java/org/apache/logging/log4j/core/Layout.java
URL: http://svn.apache.org/viewvc/logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-core/src/main/java/org/apache/logging/log4j/core/Layout.java?rev=1027132&r1=1027131&r2=1027132&view=diff
==============================================================================
--- logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-core/src/main/java/org/apache/logging/log4j/core/Layout.java (original)
+++ logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-core/src/main/java/org/apache/logging/log4j/core/Layout.java Mon Oct 25 14:25:09 2010
@@ -3,6 +3,8 @@ package org.apache.logging.log4j.core;
/**
* @doubt There is still a need for a character-based layout for character based event sinks (databases, etc).
* Would introduce an EventEncoder, EventRenderer or something similar for the logging event to byte encoding.
+ * (RG) A layout can be configured with a Charset and then Strings can be converted to byte arrays. OTOH, it isn't
+ * possible to write byte arrays as character streams.
*/
public interface Layout {
// Note that the line.separator property can be looked up even by
@@ -17,7 +19,9 @@ public interface Layout {
* Formats the event suitable for display.
* @param event The Logging Event.
* @return The formatted event.
- * @doubt Likely better to write to a OutputStream instead of return a byte[].
+ * @doubt Likely better to write to a OutputStream instead of return a byte[]. (RG) That limits how the
+ * Appender can use the Layout. For example, it might concatenate information in front or behind the
+ * data and then write it all to the OutputStream in one call.
*/
byte[] format(LogEvent event);
@@ -25,6 +29,7 @@ public interface Layout {
* Returns the header for the layout format.
* @return The header.
* @doubt the concept of header and footer is not universal, should not be on the base interface.
+ * (RG) I agree with this.
*/
byte[] getHeader();
@@ -32,6 +37,7 @@ public interface Layout {
* Returns the format for the layout format.
* @return The footer.
* @doubt the concept of header and footer is not universal, should not be on the base interface.
+ * (RG) I agree with this.
*/
byte[] getFooter();
Modified: logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-core/src/main/java/org/apache/logging/log4j/core/LogEvent.java
URL: http://svn.apache.org/viewvc/logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-core/src/main/java/org/apache/logging/log4j/core/LogEvent.java?rev=1027132&r1=1027131&r2=1027132&view=diff
==============================================================================
--- logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-core/src/main/java/org/apache/logging/log4j/core/LogEvent.java (original)
+++ logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-core/src/main/java/org/apache/logging/log4j/core/LogEvent.java Mon Oct 25 14:25:09 2010
@@ -40,6 +40,7 @@ public interface LogEvent {
* Get thread name.
* @return thread name, may be null.
* @doubt guess this could go into a thread context object too.
+ * (RG) Why?
*/
String getThreadName();
@@ -63,6 +64,7 @@ public interface LogEvent {
*
* @return A copy of the Mapped Diagnostic Context or null.
* @doubt as mentioned elsewhere, think MDC and NDC should be combined into a thread context object.
+ * (RG) Still to do.
*/
Map<String, Object> getContextMap();
@@ -71,6 +73,7 @@ public interface LogEvent {
*
* @return A copy of the Nested Diagnostic Context of null;
* @doubt as mentioned elsewhere, think MDC and NDC should be combined into a thread context object.
+ * (RG) Still to do.
*/
Stack<String> getContextStack();
Modified: logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-core/src/main/java/org/apache/logging/log4j/core/appender/AppenderRuntimeException.java
URL: http://svn.apache.org/viewvc/logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-core/src/main/java/org/apache/logging/log4j/core/appender/AppenderRuntimeException.java?rev=1027132&r1=1027131&r2=1027132&view=diff
==============================================================================
--- logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-core/src/main/java/org/apache/logging/log4j/core/appender/AppenderRuntimeException.java (original)
+++ logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-core/src/main/java/org/apache/logging/log4j/core/appender/AppenderRuntimeException.java Mon Oct 25 14:25:09 2010
@@ -17,7 +17,9 @@
package org.apache.logging.log4j.core.appender;
/**
- * @doubt unchecked exception again
+ * @doubt unchecked exception again (RG) Why is that a problem? A runtime exception
+ * is appropriate in the case where the Appender encounters a checked exception and
+ * needs to percolate the exception to the application.
*/
public class AppenderRuntimeException extends RuntimeException {
Modified: logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-core/src/main/java/org/apache/logging/log4j/core/appender/ConsoleAppender.java
URL: http://svn.apache.org/viewvc/logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-core/src/main/java/org/apache/logging/log4j/core/appender/ConsoleAppender.java?rev=1027132&r1=1027131&r2=1027132&view=diff
==============================================================================
--- logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-core/src/main/java/org/apache/logging/log4j/core/appender/ConsoleAppender.java (original)
+++ logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-core/src/main/java/org/apache/logging/log4j/core/appender/ConsoleAppender.java Mon Oct 25 14:25:09 2010
@@ -28,7 +28,10 @@ import org.apache.logging.log4j.core.con
* <code>System.err</code> using a layout specified by the user. The
* default target is <code>System.out</code>.
* @doubt accessing System.out or .err as a byte stream instead of a writer
- * bypasses the JVM's knowledge of the proper encoding.
+ * bypasses the JVM's knowledge of the proper encoding. (RG) Encoding
+ * is handled within the Layout. Typically, a Layout will generate a String
+ * and then call getBytes which may use a configured encoding or the system
+ * default. OTOH, a Writer cannot print byte streams.
*/
@Plugin(name="Console",type="Core",elementType="appender")
public class ConsoleAppender extends OutputStreamAppender {
Modified: logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-core/src/main/java/org/apache/logging/log4j/core/appender/OutputStreamAppender.java
URL: http://svn.apache.org/viewvc/logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-core/src/main/java/org/apache/logging/log4j/core/appender/OutputStreamAppender.java?rev=1027132&r1=1027131&r2=1027132&view=diff
==============================================================================
--- logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-core/src/main/java/org/apache/logging/log4j/core/appender/OutputStreamAppender.java (original)
+++ logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-core/src/main/java/org/apache/logging/log4j/core/appender/OutputStreamAppender.java Mon Oct 25 14:25:09 2010
@@ -25,13 +25,7 @@ import java.io.IOException;
import java.io.OutputStream;
/**
- *
- *
- * @doubt This seems to be a cross between a character and byte-oriented appender.
- * appenders would likely be either one or the other.
- * Would prefer to base on java.nio. Using an explicit
- * encoding might be expensive since it has to make an encoding
- * name to an encoder on every call.
+ * Writes the byte output stream. The stream will already have been encoded.
*/
public abstract class OutputStreamAppender extends AppenderBase {
@@ -50,14 +44,6 @@ public abstract class OutputStreamAppend
protected boolean immediateFlush = true;
/**
- * The encoding to use when writing. <p>The
- * <code>encoding</code> variable is set to <code>null</null> by
- * default which results in the utilization of the system's default
- * encoding.
- */
- protected String encoding;
-
- /**
* This is the OutputStream where we will write to.
*/
protected InternalOutputStream os;
@@ -278,19 +264,6 @@ public abstract class OutputStreamAppend
}
}
- public void write(String msg) {
- try {
- if (encoding != null) {
- os.write(msg.getBytes(encoding));
- } else {
- os.write(msg.getBytes());
- }
-
- } catch (IOException ioe) {
- getHandler().error("Error writing to appender " + getName(), ioe);
- }
- }
-
@Override
public void write(byte[] bytes, int i, int i1) {
try {
Modified: logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-core/src/main/java/org/apache/logging/log4j/core/config/AppenderControl.java
URL: http://svn.apache.org/viewvc/logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-core/src/main/java/org/apache/logging/log4j/core/config/AppenderControl.java?rev=1027132&r1=1027131&r2=1027132&view=diff
==============================================================================
--- logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-core/src/main/java/org/apache/logging/log4j/core/config/AppenderControl.java (original)
+++ logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-core/src/main/java/org/apache/logging/log4j/core/config/AppenderControl.java Mon Oct 25 14:25:09 2010
@@ -70,6 +70,11 @@ public class AppenderControl {
try {
appender.append(event);
+ } catch (RuntimeException ex) {
+ appender.getHandler().error("An exception occurred processing Appender " + appender.getName(), ex);
+ if (!appender.suppressException()) {
+ throw ex;
+ }
} catch (Exception ex) {
appender.getHandler().error("An exception occurred processing Appender " + appender.getName(), ex);
if (!appender.suppressException()) {
Modified: logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-core/src/main/java/org/apache/logging/log4j/core/layout/PatternLayout.java
URL: http://svn.apache.org/viewvc/logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-core/src/main/java/org/apache/logging/log4j/core/layout/PatternLayout.java?rev=1027132&r1=1027131&r2=1027132&view=diff
==============================================================================
--- logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-core/src/main/java/org/apache/logging/log4j/core/layout/PatternLayout.java (original)
+++ logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-core/src/main/java/org/apache/logging/log4j/core/layout/PatternLayout.java Mon Oct 25 14:25:09 2010
@@ -25,6 +25,7 @@ import org.apache.logging.log4j.core.hel
import org.apache.logging.log4j.core.layout.pattern.PatternConverter;
import org.apache.logging.log4j.core.layout.pattern.PatternParser;
+import java.nio.charset.Charset;
import java.util.List;
/**
@@ -419,12 +420,26 @@ public class PatternLayout extends Layou
private boolean handlesExceptions;
/**
+ * The charset of the formatted message.
+ */
+ private Charset charset;
+
+ /**
* Constructs a EnhancedPatternLayout using the DEFAULT_LAYOUT_PATTERN.
* <p/>
* The default pattern just produces the application supplied message.
*/
public PatternLayout() {
- this(DEFAULT_CONVERSION_PATTERN);
+ this(DEFAULT_CONVERSION_PATTERN, Charset.defaultCharset());
+ }
+
+ /**
+ * Constructs a EnhancedPatternLayout using the DEFAULT_LAYOUT_PATTERN.
+ * <p/>
+ * The default pattern just produces the application supplied message.
+ */
+ public PatternLayout(final String pattern) {
+ this(pattern, Charset.defaultCharset());
}
/**
@@ -432,9 +447,10 @@ public class PatternLayout extends Layou
*
* @param pattern conversion pattern.
*/
- public PatternLayout(final String pattern) {
+ public PatternLayout(final String pattern, final Charset charset) {
this.conversionPattern = pattern;
+ this.charset = charset;
PatternParser parser = createPatternParser();
converters = parser.parse((pattern == null) ? DEFAULT_CONVERSION_PATTERN : pattern);
handlesExceptions = parser.handlesExceptions();
@@ -468,7 +484,7 @@ public class PatternLayout extends Layou
for (PatternConverter c : converters) {
c.format(event, buf);
}
- return buf.toString().getBytes();
+ return buf.toString().getBytes(charset);
}
private PatternParser createPatternParser() {
@@ -477,9 +493,18 @@ public class PatternLayout extends Layou
}
@PluginFactory
- public static PatternLayout createLayout(@PluginAttr("pattern") String pattern) {
+ public static PatternLayout createLayout(@PluginAttr("pattern") String pattern,
+ @PluginAttr("charset") String charset) {
+ Charset c = Charset.defaultCharset();
+ if (charset != null) {
+ if (Charset.isSupported(charset)) {
+ c = Charset.forName(charset);
+ } else {
+ logger.error("Charset " + charset + " is not supported for layout, using default.");
+ }
+ }
if (pattern != null) {
- return new PatternLayout(pattern);
+ return new PatternLayout(pattern, c);
}
logger.error("No pattern specified for PatternLayout");
return null;
Modified: logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-core/src/test/java/org/apache/logging/log4j/core/SimplePerfTest.java
URL: http://svn.apache.org/viewvc/logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-core/src/test/java/org/apache/logging/log4j/core/SimplePerfTest.java?rev=1027132&r1=1027131&r2=1027132&view=diff
==============================================================================
--- logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-core/src/test/java/org/apache/logging/log4j/core/SimplePerfTest.java (original)
+++ logging/log4j/branches/BRANCH_2_0_EXPERIMENTAL/rgoers/log4j2-core/src/test/java/org/apache/logging/log4j/core/SimplePerfTest.java Mon Oct 25 14:25:09 2010
@@ -18,10 +18,14 @@ package org.apache.logging.log4j.core;
import org.apache.logging.log4j.Level;
import org.apache.logging.log4j.LogManager;
-import org.apache.logging.log4j.core.config.Configuration;
-import org.apache.logging.log4j.core.config.ConfigurationFactory;
-import org.apache.logging.log4j.internal.StatusLogger;
+import org.junit.BeforeClass;
import org.junit.Test;
+import org.junit.Assert;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
/**
*
@@ -31,6 +35,28 @@ public class SimplePerfTest {
private static org.apache.logging.log4j.Logger logger = LogManager.getLogger(SimplePerfTest.class.getName());
private volatile Level lvl = Level.DEBUG;
private static final int LOOP_CNT = 100000000;
+ private static final int WARMUP = 1000;
+ private static long maxTime;
+
+ @BeforeClass
+ public static void setupClass() {
+ for (int i=0; i < WARMUP; ++i) {
+ if (overhead(i, LOOP_CNT)) {
+ System.out.println("help!");
+ }
+ }
+
+ Timer timer = new Timer("Setup", LOOP_CNT);
+ timer.start();
+ for (int i=0; i < LOOP_CNT; ++i) {
+ if (overhead(i, LOOP_CNT)) {
+ System.out.println("help!");
+ }
+ }
+ timer.stop();
+ maxTime = timer.getElapsedNanoTime();
+ System.out.println(timer.toString());
+ }
@Test
public void debugDisabled() {
@@ -41,6 +67,7 @@ public class SimplePerfTest {
}
timer.stop();
System.out.println(timer.toString());
+ assertTrue("Timer exceeded max time of " + maxTime, maxTime > timer.getElapsedNanoTime());
}
@Test
@@ -52,6 +79,7 @@ public class SimplePerfTest {
}
timer.stop();
System.out.println(timer.toString());
+ assertTrue("Timer exceeded max time of " + maxTime, maxTime > timer.getElapsedNanoTime());
}
/*
@Test
@@ -64,4 +92,23 @@ public class SimplePerfTest {
timer.stop();
System.out.println(timer.toString());
} */
+
+ /*
+ * Try to generate some overhead that can't be optimized well. Not sure how accurate this is,
+ * but the point is simply to insure that changes made don't suddenly cause performance issues.
+ */
+ private static boolean overhead(int i, int j) {
+ for (int k=j; k < j+10; ++k) {
+ if (i > k) {
+ return true;
+ }
+ if (i == k) {
+ return true;
+ }
+ if (i < 0) {
+ return true;
+ }
+ }
+ return false;
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org