Hi guys,
I think there is something wrong with the current implementation of the
LogFilter.
The idea is to be able to log something _if_ a specific event is set to
a certain level. For instance, one may log as debug when a
MessageReceived event is received, but only log errors when a
MessageSent event is received.
So when you initialize a logLevel, you inject two arguments :
- the eventType
- the desired log level
public void setLogLevel(IoEventType eventType, LogLevel logLevel) {
The problem with this approach is that the way it's implemented, there
is no way to get some logs for two different event with two different
log level, simply because there is only one Logger which is used
globally, and set to a level globally, too.
So we ned to fix that, and I'm wondering which is the best approach.
- we can simply consider that this is totally useless to tie an event to
a log level, and just log all events to the same level (pretty much what
we currently have right now).
- or we consider that the original idea is ok, but then we have to
create a logger per event.
In both case, I think that the utter complexity of the current
implementation need to be simplified a lot. We don't need a map to store
the relation between an event and a logLevel, as the log level will be
implicit if we have one logger per event (or implicitly set to the
global logger if we chose option #1), we don't need a LogLevel enum
either, as the implementation is straightforward in both case :
Option #1 :
public void messageSent(NextFilter nextFilter, IoSession session,
WriteRequest writeRequest) throws Exception {
if (logger.isDebugEnabled()) {
logger.debug("SENT: {}", writeRequest.getMessage());
} else if (logger.isInfoEnabled()) {
logger.info("SENT: {}", writeRequest.getMessage());
} else if (logger.isTraceEnabled()) {
logger.trace("SENT: {}", writeRequest.getMessage());
} else if (logger.isWarnEnabled()) {
logger.warn("SENT: {}", writeRequest.getMessage());
} else if (logger.isErrorEnabled()) {
logger.error("SENT: {}", writeRequest.getMessage());
}
nextFilter.messageSent(session, writeRequest);
}
Option #2 :
public void messageSent(NextFilter nextFilter, IoSession session,
WriteRequest writeRequest) throws Exception {
if (logger.isDebugEnabled()) {
loggerMessageSent.debug("SENT: {}", writeRequest.getMessage());
} else if (logger.isInfoEnabled()) {
loggerMessageSent.info("SENT: {}", writeRequest.getMessage());
} else if (logger.isTraceEnabled()) {
loggerMessageSent.trace("SENT: {}", writeRequest.getMessage());
} else if (logger.isWarnEnabled()) {
loggerMessageSent.warn("SENT: {}", writeRequest.getMessage());
} else if (logger.isErrorEnabled()) {
loggerMessageSent.error("SENT: {}", writeRequest.getMessage());
}
nextFilter.messageSent(session, writeRequest);
}
FYI, the current implemention is :
private final Map<IoEventType, LogLevel> logSettings = new
CopyOnWriteMap<IoEventType, LogLevel>();
private final Logger logger;
public LoggingFilter(String name) {
if (name == null) {
throw new NullPointerException("name should not be null");
}
this.name = name;
logger = LoggerFactory.getLogger(name);
...
public void messageSent(NextFilter nextFilter, IoSession session,
Object message) throws Exception {
log(IoEventType.MESSAGE_RECEIVED, "RECEIVED: {}", message);
nextFilter.messageReceived(session, message);
}
protected void log(IoEventType eventType, String format, Object arg) {
getLogLevel(eventType).log(logger, format, arg);
}
public LogLevel getLogLevel(IoEventType eventType) {
if (eventType == null) {
throw new NullPointerException("eventType");
}
return logSettings.get(eventType);
}
// For Debug mode :
/**
* [EMAIL PROTECTED] LogLevel} which logs messages on the DEBUG level.
*/
DEBUG(new LogLevelLogger() {
public void log(Logger logger, String message, Object arg) {
logger.debug(message, arg);
}
public void log(Logger logger, String message, Object[] args) {
logger.debug(message, args);
}
public void log(Logger logger, String message, Throwable cause) {
logger.debug(message, cause);
}
}),
(same thing for the other levels)
pretty complicated, IMHO :)
Thoughts ?
--
--
cordialement, regards,
Emmanuel Lécharny
www.iktek.com
directory.apache.org
