Takeshi's submission is not a finished product so let's go easy on him. While I
have still not bought the idea of levels in enums or resource bundles, I think
the idea of using enums for resource bundle keys is an important step forward.
Having levels in the enum or resource bundles (I actually did not see that in
the code) is conceptually interesting and possibly quite correct but it
duplicates the data found in logging statements (=logger invocations in java
code). Moreover, we could dynamically change the level of a logging statement
based on rules placed in a configuration file. Here is an example of one such rule:
if event has I18 marker
if event.key == XYZ_10120 then event.level = ERROR;
if event.key == XYZ_11234 then event.level = WARN;
if event.key == XYZ_08173 then event.level = DEBUG;
endif
We would need to find a way to state the above in XML but that should not be a
problem. (We would also need to add a mechanism for dynamically modifying a
logging event after its creation.)
FYI, I have created a small project for experimenting with the enum as resource
bundles keys idea. You can see the result at http://git.qos.ch/ or on github
http://github.com/ceki/i18n/tree/master
Ralph Goers wrote:
I found a few minutes to review the code this evening. There are several
things I don't care for in this implementation:
1. While doable, plugging in new resolver implementations is kind of a
pain. You have to create an I18NLogManager, create the custom resolvers
and presumably new annotation resolvers, add them to the
CompositeResolvers then add those to the manager. Finally a new
I18nLoggerFactory has to be created taking the manager and setINSTANCE
called.
2. The resolvers provide no mechanism for reloading the bundles if they
are modified. Many environments want 24x7 up time and do not want to
restart servers just to change message text.
3. The implementation only supports a single Locale - the default.
4. I dislike immensely having the log level in one property file and the
message text in another. This is very error prone. If you want log
levels then get rid of the property files and switch to using XML where
the log level and message can be bound together. Of course, this runs
the risk that the level might be different from one language to another.
Another option - which makes far more sense to me - is to not even have
a LogLevelResolver, just require the log level be defined in the enum
and just use that.
5. Messages are always resolved in the writeLog method. This precludes
the option of the message keys being written to a database so the
message can be retrieved based on the user's locale or passing the
message to different machines where it can be logged using a Locale
appropriate for that environment.
6. If you really, really want to use ResourceBundle's then at least make
Java 6 the minimum version and provide support for
ResourceBundle.Control. However, since ResourceBundles don't provide
support for reloading I'd avoid using them at all.
7. It seems very strange to have the LogLevel enum have log methods and
that writeLog is actually calling them.
8. The callerData is going to be useless since the LogLevel enum is just
using the standard SLF4J apis. Instead, the Logger needs to extend
org.slf4j.ext.LoggerWrapper and actually log from that class.
9. The format of the LogMessages enum is:
public enum LogMessages {
@Error("error message {}") // The log level is bound to Error.
TEST0001,
@Message("waring message {}") // The log level is not specified.
TEST0002
}
a. why would you want to allow a log level to not be specified? (See
item 4 above). It is now possible to not specify the level in the
properties file and not specify it as an annotation. If that happens
then the level will be info since that is what is hardcoded in
I18NLoggerFactory. What is more annoying is that I can call logger.warn
and generate an info level message.
b. Other than showing that you know how to use annotations I don't see
what the benefit is over doing something like:
public enum LogMessages {
private final LogLevel level;
private final String message;
public LogMessages(LogLevel lvl, String msg) {
level = lvl;
message = msg;
}
public LogLevel getLogLevel() { return level };
public String getMessage() { return message };
TEST0001(LogLevel.ERROR, "error message {}");
TEST0002(LogLevel.WARN, "warning message {}"};
}
I only spent about an hour looking at this so this may not be the
complete set of issues I might find.
In short, this implementation is not very general purpose as it only
supports a single use case.
Ralph
On Aug 22, 2009, at 10:32 PM, Takeshi Kondo wrote:
<slf4j-i18n-1.5.9-SNAPSHOT.jar><slf4j-i18n-1.5.9-SNAPSHOT-sources.jar>
I've developed initial thought of SLF4j's i18n extension.
It was committed to my branch
(http://github.com/takeshi/slf4j/tree/master).
I've implemented 4 features as follows.
1. Logger interface using enum.
@see org.slf4j.i18n.I18NLogger
2. Extension point to bind log id's enum to log message and level.
@see org.slf4j.i18n.spi.LogLevelResolver
@see org.slf4j.i18n.spi.LogMessageFormatResolver
3. Resolving log message and log level from Annotation associated from
log id's enum.
@see org.slf4j.i18n.impl.AnnotationLogLevelResolver
@see org.slf4j.i18n.impl.AnnotationMessageFormatResolver
4. Resolving log message and log level from ResourceBundle associated
with log id's enum.
@see org.slf4j.i18n.impl.ResourceBundleLogLevelResolver
@see org.slf4j.i18n.impl.ResourceBundleMessageFormatResolver
For example:
----
Log Message Definition
----
public enum LogMessages {
@Error("error message {}") // log level is bound to Error.
TEST0001,
@Message("waring message {}") // log level is not specified.
TEST0002
}
----
Logging
----
public static void main(String[] args) {
I18NLogger logger = I18NLoggerFactory.getLogger(LogMain.class);
// write to error log , because LogMessages.TEST0001 is bound to
Error level.
logger.log(LogMessages.TEST0001, "xxxx");
logger.log(LogMessages.TEST0001, new NullPointerException());
// write log as error level.
logger.error(LogMessages.TEST0002, "xxxx");
// write log as warn level.
logger.warn(LogMessages.TEST0002, new NullPointerException());
}
Takeshi Kondo
_______________________________________________
dev mailing list
dev@slf4j.org
http://www.slf4j.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@slf4j.org
http://www.slf4j.org/mailman/listinfo/dev
--
Ceki Gülcü
Logback: The reliable, generic, fast and flexible logging framework for Java.
http://logback.qos.ch
_______________________________________________
dev mailing list
dev@slf4j.org
http://www.slf4j.org/mailman/listinfo/dev
