Community
Participate
Working Groups
By default, if there is no slf4j implementation present jetty will log to their built-in StdErrLog which posts everything to standard out. We need to replace this with our own which disables all log messages by default. In jetty 6 we seemed to have our own fake org.slf4j package included in our own bundle and then set the context class loader to our own bundle before invoking some jetty Log APIs and hoped that jetty would attempt to load our fake Logger. Well in jetty 8 they no logger ask the context class loader for an slf4j implementation. That is fine by me because this approach seemed rather clunky anyway. For jetty 8 I think we should simply implement org.eclipse.jetty.util.log.Logger directly and use a StdErrLog impl under the covers to delegate to if our logging properties are set.
I released a fix in commit: http://git.eclipse.org/c/equinox/rt.equinox.bundles.git/commit/?id=b28c8833ba841e3b1df5345ab0a8ea370e3756e0 I would appreciate a review of the approach by Hugues, and the two Simons if possible.
(In reply to comment #1) > I released a fix in commit: > > http://git.eclipse.org/c/equinox/rt.equinox.bundles.git/commit/?id=b28c8833ba841e3b1df5345ab0a8ea370e3756e0 > > I would appreciate a review of the approach by Hugues, and the two Simons if > possible. Hi Tom, sorry for the delay. Joakim and I reviewed this change. Joakim's initial feedback was to use the system property "org.eclipse.jetty.util.log.class" to plugin a different Logger's implementation. This is back by this class: http://git.eclipse.org/c/jetty/org.eclipse.jetty.project.git/tree/jetty-util/src/main/java/org/eclipse/jetty/util/log/Log.java However, this type of plugin mechanism won't work nicely in OSGi. We concluded that probably you wanted to leave the logging configuration of Jetty unchanged and override the default stderr logger. Could you double confirm that is what we are doing here? We are certainly not comfortable with the clunky approach that forces adopters of jetty in OSGi to put in place such clunky glue code. Should we look into changing jetty-util's log to behave as nicely as possible in OSGi?
(In reply to comment #2) > Hi Tom, sorry for the delay. > > Joakim and I reviewed this change. Thanks! > > Joakim's initial feedback was to use the system property > "org.eclipse.jetty.util.log.class" > to plugin a different Logger's implementation. > This is back by this class: > http://git.eclipse.org/c/jetty/org.eclipse.jetty.project.git/tree/jetty-util/src/main/java/org/eclipse/jetty/util/log/Log.java > > However, this type of plugin mechanism won't work nicely in OSGi. Right, looking at the code, this seems similar to how we were overriding the log in jetty 6 although through a little different mechanism. In Jetty 6 we would set the TCCL to our own bundle and fake jetty 6 out into thinking we provided an slf4j impl that return jetty loggers. I really did not care for that and the same code path did not seem to exist in jetty 8. So I did take a look at the org.eclipse.jetty.util.log.class property, but I do not like this because it also would require us to set the TCCL to our own bundle's class loader so that jetty 8 could find our log implementation class while we called Log.initialized(). I also do not like configuring this through system properties and was not sure if we could configure it differently. > > We concluded that probably you wanted to leave the logging configuration of > Jetty unchanged and override the default stderr logger. Yes, we basically want to be able to configure what level of errors get written to stderr without setting system properties since jetty my be embedded in a java process for which we don't want to set the system wide properties on. > > Could you double confirm that is what we are doing here? > We are certainly not comfortable with the clunky approach that forces adopters > of jetty in OSGi to put in place such clunky glue code. > > Should we look into changing jetty-util's log to behave as nicely as possible > in OSGi?
Looks good -- the extra check for -- !(rootLogger instanceof EquinoxStdErrLog) is no longer needed and can be removed.
