Bug 39691 - DBAppender doesn't log long events
Summary: DBAppender doesn't log long events
Status: ASSIGNED
Alias: None
Product: Log4j - Now in Jira
Classification: Unclassified
Component: Appender (show other bugs)
Version: 1.3alpha
Hardware: Other Windows XP
: P2 normal
Target Milestone: ---
Assignee: log4j-dev
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-05-31 10:39 UTC by Jan Novotn
Modified: 2008-08-05 20:21 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Novotn 2006-05-31 10:39:29 UTC
DBAppender contains scripts for creating tables with columns with maximal length
 - for example renderedmessage is VARCHAR2(4000). However while appending
records length is not controlled, so if I try to append long log database throws
exception and nothing is logged.

The same problem as with rendered message is with writing stack trace. So I
extended your classes with mine, although I think this functionality should be
in base classes.

############## Repaired Log4JTrimmedLoggingEvent ################

package cz.corpus.f1.commons.log;

import org.apache.log4j.Level;
import org.apache.log4j.Logger;
import org.apache.log4j.spi.LocationInfo;
import org.apache.log4j.spi.LoggingEvent;
import org.apache.log4j.spi.ThrowableInformation;

import java.util.Hashtable;
import java.util.Map;
import java.util.Set;

/**
 * @author Jan Novotný
 */
public class Log4JTrimmedLoggingEvent extends LoggingEvent {
    LoggingEvent original;

    public Log4JTrimmedLoggingEvent(LoggingEvent original) {
        this.original = original;
    }

    public boolean equals(Object rObject) {
        return original.equals(rObject);
    }

    public String getFQNOfLoggerClass() {
        return original.getFQNOfLoggerClass();
    }

    public Level getLevel() {
        return original.getLevel();
    }

    public LocationInfo getLocationInformation() {
        return original.getLocationInformation();
    }

    public Logger getLogger() {
        return original.getLogger();
    }

    public String getLoggerName() {
        return original.getLoggerName();
    }

    public Object getMDC(final String key) {
        return original.getMDC(key);
    }

    public void getMDCCopy() {
        original.getMDCCopy();
    }

    public Object getMessage() {
        return original.getMessage();
    }

    public String getNDC() {
        return original.getNDC();
    }

    public Map getProperties() {
        return original.getProperties();
    }

    public String getProperty(String key) {
        return original.getProperty(key);
    }

    public Set getPropertyKeySet() {
        return original.getPropertyKeySet();
    }

    public String getRenderedMessage() {
        String origMsg = original.getRenderedMessage();
        if(origMsg.length() > 3800){
        	return origMsg.substring(0, 3800) + " ... msg too long - trimed " +
(origMsg.length() - 3800) + " characters";
        }	
        return origMsg;
    }

    public long getSequenceNumber() {
        return original.getSequenceNumber();
    }

    public String getThreadName() {
        return original.getThreadName();
    }

    public ThrowableInformation getThrowableInformation() {
        return new
Log4JTrimmedThrowableInformation(original.getThrowableInformation());
    }

    public String[] getThrowableStrRep() {
        return new
Log4JTrimmedThrowableInformation(original.getThrowableInformation()).getThrowableStrRep();
    }

    public long getTimeStamp() {
        return original.getTimeStamp();
    }

    public int hashCode() {
        return original.hashCode();
    }

    public void initializeProperties() {
        original.initializeProperties();
    }

    public boolean locationInformationExists() {
        return original.locationInformationExists();
    }

    public void prepareForDeferredProcessing() {
        original.prepareForDeferredProcessing();
    }

    public void setFQNOfLoggerClass(String fqnOfLoggerClass) {
        original.setFQNOfLoggerClass(fqnOfLoggerClass);
    }

    public void setLevel(Level level) {
        original.setLevel(level);
    }

    public void setLocationInformation(LocationInfo li) {
        original.setLocationInformation(li);
    }

    public void setLogger(Logger logger) {
        original.setLogger(logger);
    }

    public void setLoggerName(String loggerName)
            throws IllegalStateException {
        original.setLoggerName(loggerName);
    }

    public void setMessage(Object message) {
        original.setMessage(message);
    }

    public void setNDC(String ndcString) {
        original.setNDC(ndcString);
    }

    public void setProperties(Hashtable properties) {
        original.setProperties(properties);
    }

    public void setProperty(String key, String value) {
        original.setProperty(key, value);
    }

    public void setRenderedMessage(String renderedMessage)
            throws IllegalStateException {
        original.setRenderedMessage(renderedMessage);
    }

    public void setSequenceNumber(long sequenceNumber) {
        original.setSequenceNumber(sequenceNumber);
    }

    public void setThreadName(String threadName)
            throws IllegalStateException {
        original.setThreadName(threadName);
    }

    public void setThrowableInformation(ThrowableInformation ti) {
        original.setThrowableInformation(ti);
    }

    public void setTimeStamp(long timeStamp) {
        original.setTimeStamp(timeStamp);
    }
}


############## Repaired Log4JTrimmedThrowableInformation ###############

package cz.corpus.f1.commons.log;

import org.apache.log4j.spi.ThrowableInformation;

/**
 * @author Jan Novotný
 */
public class Log4JTrimmedThrowableInformation extends ThrowableInformation {
    ThrowableInformation original;

    public Log4JTrimmedThrowableInformation(ThrowableInformation ti) {
        super(ti.getThrowableStrRep());
        original = ti;
    }

    public boolean equals(Object o) {
        return original.equals(o);
    }

    public Throwable getThrowable() {
        return original.getThrowable();
    }

    public String[] getThrowableStrRep() {
        String[] throwableStrRep = original.getThrowableStrRep();
        String[] result = new String[throwableStrRep.length];
        for (int i = 0; i < throwableStrRep.length; i++) {
            String msg = throwableStrRep[i];
            int lng = msg.length();
            if (msg != null && lng > 254) {
                msg = "..." + msg.substring(lng - 230, lng);
            }
            result[i] = msg;
        }
        return throwableStrRep;
    }
}
Comment 1 Elias Ross 2007-01-29 12:54:34 UTC
Truncation should be the resposibility of the appender, since it has the domain
knowledge on the length to trim at.

It also appears that the stack trace is now in separate table rows.

Although trimming can be done in other places, I'm thinking of adding a 

setMaxMessageLength
or
setMaxRenderedMessageLength

to DBAppender and truncating using this value.  By default, no truncation would
be done.
Comment 2 Thorbjørn Ravn Andersen 2008-07-03 12:50:23 UTC
(In reply to comment #1)
> Truncation should be the resposibility of the appender, since it has the domain
> knowledge on the length to trim at.

It should be expected that the logger logs as much as it can of the data it gets, so I disagree with this.

The suggested code contains magical constants - 3800, 254 and 230 - which should be better documented.  Best would be if the creation scripts use the same values to create the needed tables, otherwise they may get out of synchronization later.
Comment 3 Thorbjørn Ravn Andersen 2008-07-03 16:40:30 UTC
(In reply to comment #2)
> (In reply to comment #1)
> > Truncation should be the resposibility of the appender, since it has the domain
> > knowledge on the length to trim at.
> 
> It should be expected that the logger logs as much as it can of the data it
> gets, so I disagree with this.

I was apparently thinking of the logger and not the appender when writing the above.  Please disregard.
Comment 4 Thorbjørn Ravn Andersen 2008-08-02 10:06:01 UTC
Please provide testcase demonstrating the issue reported to verify that provided fix makes it go away :)

It would be best if it uses the Derby database as it can be easily used in Java unit tests.
Comment 5 Curt Arnold 2008-08-05 20:21:03 UTC
NEEDINFO isn't appropriate for this bug since there is enough information for someone to deal with it.  It would be great if the submitter would provide a test case, but it isn't essential for problem resolution.  Marking a bug as NEEDINFO basically puts it at the bottom of the work queue since it should only be used when the issue is stalled due to lack of info.