I pushed updates to github taking into account much of the feedback I received. The updated code has the following: 1) Simplified 2.0 API for logger and adapter writers. The idea is to make it much easier to write adapters/loggers and reduce risk of bugs by removing application facing API specifics such as formatting and Logger method signatures from adapter/logger implementations. Future enhancements to Logger may be made with less concern about breaking binary compatibility. 2) Maintain binary compatibility with all pre 2.0 adapters while not restricting future additions to the Logger interface. New Logger methods will work with old adapters. The tradeoff is that old loggers will not have access to the raw messagePattern - LocationAwareLogger does not support this. Non LocationAwareLoggers will not have access to the message parameters. So, Logback filtering on the raw messagePattern (using a pre 2.0 compatible logback) will be slightly less accurate, but it will be no worse than in 1.6 when using a wrapper such as XLogger. New 2.0 style loggers and adapters should have a maven dependency for the 2.0 slf4j-api. This will in no way break existing apps that for whatever reason may also bring in "other" 1.6 components as long as the app is compiled and deployed on Java 5+. 3) Addition of log Entry objects similar to Message objects that have been discussed. This design does not attempt to unify audit and access log APIs, but otherwise provides all of the capabilities of Message objects with the additional benefit that Entry objects can control Level and Marker. For example, an advanced application writer may have a custom EventEntry class that properly translates domain specific details to Level & Marker, such as map EventType.SECURITY_VIOLATION to Level.WARN, etc. A StructuredData object may accept syslog severities, but automatically translate to the appropriate slf4j Level in order to support adapters that do not understand StructuredData. 4) The log Entry interface is minimal, with additional interfaces adding functionality. Adapters that support additional functionality can use "instanceof". Future version of SLF4J may introduce additional features exposed through new interfaces without breaking compatibility with older adapters. 5) Support for logger.withFormatter(Formatter x) while maintaining Logger immutability and support for pre 2.0 loggers. I am not proposing this be included in the 2.0 API. But I feel that it is valuable to explore how this type of feature (formatting or whatever else may come) could be added to loggers in the future while the 2.0 API is being worked out. 6) I removed supplementalData and the numerous arg1...arg6 methods. I agree with Joern that it is probably best to avoid Google Collections style methods given the relatively low cost of implicit Object[] creation. There may be some value to supplementalData, but under this 2.0 proposal, it could be added any time in the future, so no use worrying about it now. Disclaimers - Unit tests pass with both 1.6 and 2.0 style adapters, but I haven't fully tested them for things like FQCN. And, of course, claims of binary compatibility while introducing features are nothing more than theory until fully tested. - A new interface LoggingProvider specifies what must be implemented for 2.0 style adapters. But, adapters still also implement Logger by extending AbstractLogger. The class hierarchy and clarity may be better if 2.0 adapters simply implemented LoggingProvider with a LoggerImpl created by LoggerFactory. - Legacy adapters are wrapped each time they are created. It may be better to cache them in LoggerFactory with a WeakHashMap<[old]Logger, WeakReference<[wrapped]Logger>> to save some cycles and GC. - Class naming and package structure needs some work - LoggingProvider, Level, Entry and various Entry sub interfaces, etc. - deserialization needs work. The new code is available at https://github.com/jvasileff/slf4j/tree/topic-jdk5-varargs An additional test branch that reverts all adapters, XLogger, and EventLogger to 1.6.2 source for basic compatibility testing is available at https://github.com/jvasileff/slf4j/tree/topic-jdk5-legacy-adapter-test John
Joern, I can't help but think having Entry objects that include Level & Marker is a major sticking point with this. I put more thought into Entry objects, and have reached a couple conclusions. First, with the other proposed changes, this feature could be added later without breaking things using a LevelProvidingMessage and corresponding Logger method. Second, one of the major advantages is the ability for an Entry to determine its own Level and Marker. But this comes at a performance cost as the Entry would have to be constructed prior to checking isEnabled. So, with that, I reworked my branch to have Message objects that more closely resemble those in your branch. (The current Message objects are rudimentary and would need to be updated with the work from your branch.) To recap, this proposal includes: - Much simplified adapter implementation requirements while improving separation of concerns and freedom for future slf4j-api innovation. - Binary and source compatibility with 1.6.x adapters and application code. - Support for new Logger methods including Message objects when using legacy adapters. - No change in the package names for org.slf4j.Logger, LoggerFactory, etc. - Resolves the following: http://bugzilla.slf4j.org/show_bug.cgi?id=245 http://bugzilla.slf4j.org/show_bug.cgi?id=244 http://bugzilla.slf4j.org/show_bug.cgi?id=243 http://bugzilla.slf4j.org/show_bug.cgi?id=242 http://bugzilla.slf4j.org/show_bug.cgi?id=241 http://bugzilla.slf4j.org/show_bug.cgi?id=148 http://bugzilla.slf4j.org/show_bug.cgi?id=31 - Resolves confusion behind the following: http://bugzilla.slf4j.org/show_bug.cgi?id=213 http://bugzilla.slf4j.org/show_bug.cgi?id=240 - Allows easy addition of (if desired): http://bugzilla.slf4j.org/show_bug.cgi?id=133 John
I just checked out your code at https://github.com/jvasileff/slf4j/blob/topic-jdk5-varargs/slf4j-api/src/mai... (only that file so far) and I can tell you that this interface won't be compatible with jdk1.4. Yes, I know, this is the jdk5 proposal. But I agree with Ceki that there should be an interface staying jdk1.4-compatible. This is not only relevant for ordinary Java but also for stuff like JavaME. Also, the Message interface is only used in the log-method and not in case of the explicit methods like debug, info, etc. Third, Loggers in your proposal are mutable. Changing the formatter of a logger is a very bad idea™. I explained this earlier. Joern. On 17.09.2011, at 20:39, John Vasileff wrote:
Joern, I can't help but think having Entry objects that include Level & Marker is a major sticking point with this. I put more thought into Entry objects, and have reached a couple conclusions. First, with the other proposed changes, this feature could be added later without breaking things using a LevelProvidingMessage and corresponding Logger method. Second, one of the major advantages is the ability for an Entry to determine its own Level and Marker. But this comes at a performance cost as the Entry would have to be constructed prior to checking isEnabled.
So, with that, I reworked my branch to have Message objects that more closely resemble those in your branch. (The current Message objects are rudimentary and would need to be updated with the work from your branch.)
To recap, this proposal includes:
- Much simplified adapter implementation requirements while improving separation of concerns and freedom for future slf4j-api innovation. - Binary and source compatibility with 1.6.x adapters and application code. - Support for new Logger methods including Message objects when using legacy adapters. - No change in the package names for org.slf4j.Logger, LoggerFactory, etc.
- Resolves the following: http://bugzilla.slf4j.org/show_bug.cgi?id=245 http://bugzilla.slf4j.org/show_bug.cgi?id=244 http://bugzilla.slf4j.org/show_bug.cgi?id=243 http://bugzilla.slf4j.org/show_bug.cgi?id=242 http://bugzilla.slf4j.org/show_bug.cgi?id=241 http://bugzilla.slf4j.org/show_bug.cgi?id=148 http://bugzilla.slf4j.org/show_bug.cgi?id=31
- Resolves confusion behind the following: http://bugzilla.slf4j.org/show_bug.cgi?id=213 http://bugzilla.slf4j.org/show_bug.cgi?id=240
- Allows easy addition of (if desired): http://bugzilla.slf4j.org/show_bug.cgi?id=133
John _______________________________________________ slf4j-dev mailing list slf4j-dev@qos.ch http://qos.ch/mailman/listinfo/slf4j-dev
On Sep 17, 2011, at 3:05 PM, Joern Huxhorn wrote:
I just checked out your code at https://github.com/jvasileff/slf4j/blob/topic-jdk5-varargs/slf4j-api/src/mai... (only that file so far) and I can tell you that this interface won't be compatible with jdk1.4.
Yes, I know, this is the jdk5 proposal. But I agree with Ceki that there should be an interface staying jdk1.4-compatible. This is not only relevant for ordinary Java but also for stuff like JavaME.
My understanding is that Ceki would prefer not having a new package, and that forcing 2.0 users to deploy on Java 5+ is ok. My opinion is that having two interfaces would be confusing, especially since logging is something people don't want to put a lot of thought into. Should we really cater to apps deployed on Java 1.4, especially when those apps are unlikely to even attempt to upgrade to SLF4J v2? Any other dependencies that bring in SLF4J v2 as a transitive dependency are also likely to require Java 5. I'm sure it is no joy enhancing 1.4 apps, but a new library that requires Java 5 is nothing new to those that do. http://www.qos.ch/pipermail/slf4j-dev/2010-March/002857.html http://www.qos.ch/pipermail/slf4j-dev/2010-March/002866.html
Also, the Message interface is only used in the log-method and not in case of the explicit methods like debug, info, etc.
Easy enough to add info(Message), etc. if desired. The code works pretty well, but is certainly not final.
Third, Loggers in your proposal are mutable. Changing the formatter of a logger is a very bad idea™. I explained this earlier.
Actually, the way it is done the logger is not mutable. But, as I noted, I am not proposing 2.0 include withFormatter() - it is more of an experiment to see how something like that may be done in case it leads towards a more flexible/superior design.
Joern.
On 17.09.2011, at 20:39, John Vasileff wrote:
Joern, I can't help but think having Entry objects that include Level & Marker is a major sticking point with this. I put more thought into Entry objects, and have reached a couple conclusions. First, with the other proposed changes, this feature could be added later without breaking things using a LevelProvidingMessage and corresponding Logger method. Second, one of the major advantages is the ability for an Entry to determine its own Level and Marker. But this comes at a performance cost as the Entry would have to be constructed prior to checking isEnabled.
So, with that, I reworked my branch to have Message objects that more closely resemble those in your branch. (The current Message objects are rudimentary and would need to be updated with the work from your branch.)
To recap, this proposal includes:
- Much simplified adapter implementation requirements while improving separation of concerns and freedom for future slf4j-api innovation. - Binary and source compatibility with 1.6.x adapters and application code. - Support for new Logger methods including Message objects when using legacy adapters. - No change in the package names for org.slf4j.Logger, LoggerFactory, etc.
- Resolves the following: http://bugzilla.slf4j.org/show_bug.cgi?id=245 http://bugzilla.slf4j.org/show_bug.cgi?id=244 http://bugzilla.slf4j.org/show_bug.cgi?id=243 http://bugzilla.slf4j.org/show_bug.cgi?id=242 http://bugzilla.slf4j.org/show_bug.cgi?id=241 http://bugzilla.slf4j.org/show_bug.cgi?id=148 http://bugzilla.slf4j.org/show_bug.cgi?id=31
- Resolves confusion behind the following: http://bugzilla.slf4j.org/show_bug.cgi?id=213 http://bugzilla.slf4j.org/show_bug.cgi?id=240
- Allows easy addition of (if desired): http://bugzilla.slf4j.org/show_bug.cgi?id=133
John _______________________________________________ slf4j-dev mailing list slf4j-dev@qos.ch http://qos.ch/mailman/listinfo/slf4j-dev
_______________________________________________ slf4j-dev mailing list slf4j-dev@qos.ch http://qos.ch/mailman/listinfo/slf4j-dev
On 17.09.2011, at 21:35, John Vasileff wrote:
On Sep 17, 2011, at 3:05 PM, Joern Huxhorn wrote:
I just checked out your code at https://github.com/jvasileff/slf4j/blob/topic-jdk5-varargs/slf4j-api/src/mai... (only that file so far) and I can tell you that this interface won't be compatible with jdk1.4.
Yes, I know, this is the jdk5 proposal. But I agree with Ceki that there should be an interface staying jdk1.4-compatible. This is not only relevant for ordinary Java but also for stuff like JavaME.
My understanding is that Ceki would prefer not having a new package, and that forcing 2.0 users to deploy on Java 5+ is ok. My opinion is that having two interfaces would be confusing, especially since logging is something people don't want to put a lot of thought into.
Should we really cater to apps deployed on Java 1.4, especially when those apps are unlikely to even attempt to upgrade to SLF4J v2? Any other dependencies that bring in SLF4J v2 as a transitive dependency are also likely to require Java 5. I'm sure it is no joy enhancing 1.4 apps, but a new library that requires Java 5 is nothing new to those that do.
http://www.qos.ch/pipermail/slf4j-dev/2010-March/002857.html http://www.qos.ch/pipermail/slf4j-dev/2010-March/002866.html
Extending an interface can not be done in a binary-compatible way (at least not until jdk8 with extension methods, see http://www.javarants.com/2011/01/22/using-closures-method-handles-and-extens... ) simply because all outside implementations of the interface will go boom because they don't implement the new methods. That's one of the reasons why I suggested moving over to a fresh interface. This enables us to remove superfluous methods like the ones with one or two arguments while also extending it at other places, like Message support. This is also the way people are doing it in other projects, for example JUnit and commons-lang. It's not only people that are too lazy to update their Java installation. Some are simply forced to use old versions (I remember someone needed to work on AS/400). Those could still use the old API without being cut off from fixes. Having two interfaces isn't more confusing than the old and new junit packages. People that don't care can just stay on the old interface without any work on their side while people that do care can switch to the new interface if they like.
Also, the Message interface is only used in the log-method and not in case of the explicit methods like debug, info, etc.
Easy enough to add info(Message), etc. if desired. The code works pretty well, but is certainly not final.
I know. Been there, done that. ;)
Third, Loggers in your proposal are mutable. Changing the formatter of a logger is a very bad idea™. I explained this earlier.
Actually, the way it is done the logger is not mutable. But, as I noted, I am not proposing 2.0 include withFormatter() - it is more of an experiment to see how something like that may be done in case it leads towards a more flexible/superior design.
If they are not mutable than you have multiple instances of the same Logger that was originally meant to be a singleton. Joern.
Joern.
On 17.09.2011, at 20:39, John Vasileff wrote:
Joern, I can't help but think having Entry objects that include Level & Marker is a major sticking point with this. I put more thought into Entry objects, and have reached a couple conclusions. First, with the other proposed changes, this feature could be added later without breaking things using a LevelProvidingMessage and corresponding Logger method. Second, one of the major advantages is the ability for an Entry to determine its own Level and Marker. But this comes at a performance cost as the Entry would have to be constructed prior to checking isEnabled.
So, with that, I reworked my branch to have Message objects that more closely resemble those in your branch. (The current Message objects are rudimentary and would need to be updated with the work from your branch.)
To recap, this proposal includes:
- Much simplified adapter implementation requirements while improving separation of concerns and freedom for future slf4j-api innovation. - Binary and source compatibility with 1.6.x adapters and application code. - Support for new Logger methods including Message objects when using legacy adapters. - No change in the package names for org.slf4j.Logger, LoggerFactory, etc.
- Resolves the following: http://bugzilla.slf4j.org/show_bug.cgi?id=245 http://bugzilla.slf4j.org/show_bug.cgi?id=244 http://bugzilla.slf4j.org/show_bug.cgi?id=243 http://bugzilla.slf4j.org/show_bug.cgi?id=242 http://bugzilla.slf4j.org/show_bug.cgi?id=241 http://bugzilla.slf4j.org/show_bug.cgi?id=148 http://bugzilla.slf4j.org/show_bug.cgi?id=31
- Resolves confusion behind the following: http://bugzilla.slf4j.org/show_bug.cgi?id=213 http://bugzilla.slf4j.org/show_bug.cgi?id=240
- Allows easy addition of (if desired): http://bugzilla.slf4j.org/show_bug.cgi?id=133
John _______________________________________________ slf4j-dev mailing list slf4j-dev@qos.ch http://qos.ch/mailman/listinfo/slf4j-dev
_______________________________________________ slf4j-dev mailing list slf4j-dev@qos.ch http://qos.ch/mailman/listinfo/slf4j-dev
_______________________________________________ slf4j-dev mailing list slf4j-dev@qos.ch http://qos.ch/mailman/listinfo/slf4j-dev
On Sep 17, 2011, at 4:32 PM, Joern Huxhorn wrote: ...
Extending an interface can not be done in a binary-compatible way (at least not until jdk8 with extension methods, see http://www.javarants.com/2011/01/22/using-closures-method-handles-and-extens... ) simply because all outside implementations of the interface will go boom because they don't implement the new methods.
That's one of the reasons why I suggested moving over to a fresh interface. This enables us to remove superfluous methods like the ones with one or two arguments while also extending it at other places, like Message support. This is also the way people are doing it in other projects, for example JUnit and commons-lang.
But extending the interface while maintaining binary compatibility is exactly what I have done. Give it a try! In my testing, using new interface methods on 1.6.0 adapters from maven central works great. The technique I used isn't a general solution for things like say, the jdbc interfaces that keep evolving. But it works well for slf4j and promises to allow both long term compatibility while not preventing innovation. This both avoids and prevents issues like the 1.5 -> 1.6 update that broke compatibility due to a change in LocationAwareLogger.
It's not only people that are too lazy to update their Java installation. Some are simply forced to use old versions (I remember someone needed to work on AS/400). Those could still use the old API without being cut off from fixes.
Having two interfaces isn't more confusing than the old and new junit packages. People that don't care can just stay on the old interface without any work on their side while people that do care can switch to the new interface if they like.
...
Joern.
John
participants (2)
-
Joern Huxhorn -
John Vasileff