Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use java.util.logging instead of slf4j #1106

Closed
Artur- opened this issue May 27, 2013 · 13 comments
Closed

Use java.util.logging instead of slf4j #1106

Artur- opened this issue May 27, 2013 · 13 comments

Comments

@Artur-
Copy link
Contributor

Artur- commented May 27, 2013

The slf4j dependency is causing a lot of headache to us. Some servers bundle their own version of the slf4j API and will fail if you include another version. Some servers do not bundle any version and will fail if you do not include an API jar. Some servers require a certain version and some are fine with anything.

For a normal user with one dedicated deployment platform the slf4j dependency probably does not cause issues. For a framework most dependencies are a problem.

Would you consider a pull request which drops the slf4j dependency? I read you have changed from jul to slf4j in the past although I could not find the reason for it or where Atmosphere itself grealy benefits from slf4j (users can always use slf4j if they like).

@dhoffer
Copy link

dhoffer commented May 27, 2013

Components like Atmosphere should not be tightly bound to any concrete
logger, so the use of slf4j is the preferred approach. You should be able
to plug in your own logger. If they didn't use slf4j there is no way to
add that after the fact as slf4j is not a concrete logger but a logging API
that lets you choose your logger.

On Mon, May 27, 2013 at 6:47 AM, Artur notifications@github.com wrote:

The slf4j dependency is causing a lot of headache to us. Some servers
bundle their own version of the slf4j API and will fail if you include
another version. Some servers do not bundle any version and will fail if
you do not include an API jar. Some servers require a certain version and
some are fine with anything.

For a normal user with one dedicated deployment platform the slf4j
dependency probably does not cause issues. For a framework most
dependencies are a problem.

Would you consider a pull request which drops the slf4j dependency? I read
you have changed from jul to slf4j in the past although I could not find
the reason for it or where Atmosphere itself grealy benefits from slf4j
(users can always use slf4j if they like).


Reply to this email directly or view it on GitHubhttps://github.com//issues/1106
.

@Artur-
Copy link
Contributor Author

Artur- commented May 27, 2013

Isn't that what org.slf4j.bridge.SLF4JBridgeHandler is for?

@dhoffer
Copy link

dhoffer commented May 27, 2013

SLF4J has several 'workaround' modules so that if one cannot, for some
reason, fully use SLF4J it can co-exist with other logging frameworks. A
common situation is one component uses slf4j and another uses
commons-logging...in that case slf4j has a module that can stand-in for
commons-logging and route to slf4j (and vice-versa).

However these are all workarounds. The simple and most correct use is to
use slf4j for all components and then at the application level you plugin
whatever logger you want, e.g. log4j, jdk logging, etc.

It's an anti-pattern to log to a concrete logger and then redirect back to
a logging API so that you can then redirect to a logger.

On Mon, May 27, 2013 at 7:11 AM, Artur notifications@github.com wrote:

Isn't that what org.slf4j.bridge.SLF4JBridgeHandler is for?


Reply to this email directly or view it on GitHubhttps://github.com//issues/1106#issuecomment-18497887
.

@jfarcand
Copy link
Member

@artur You will need to convince me more about moving back...for me it sound like going back to 2005 with juli :-) I have integrated Atmosphere in many env and never had issue with it. The issue is with the server, not Atmosphere IMO. Can you list the server you are having issue with Vaadin?

@Artur-
Copy link
Contributor Author

Artur- commented May 30, 2013

The problem we are having is basically that we want to be able to produce a war file that you can deploy on any of our supported servers. This works great except for the slf4j api problem. Another issue is that the rest of the framework uses jul for logging and the users would end up having to configure both slf4j and jul. If you are not interested in going back to jul then I think we will have to do it anyway for our version.

@jfarcand
Copy link
Member

@Artur- Understand, but that would break a lot of other framework, applications, etc. Instead of forking, I recommend the design a pluggable logging API where the default would be the same as right now. I can certainly do it under Async-IO.org professionnal services (I did get a ping from your boss, but no response since).

@dhoffer
Copy link

dhoffer commented May 30, 2013

Regarding running a war in multiple containers, SLF4J is not the problem as
that is done all the time. We do it on Jetty,Tomcat & JBoss with no
issues. If your having trouble with a particular container you might want
to ask the question on the SLF4J newsgroup for help with configuration.

Now you mentioned that the rest of the Atmosphere framework uses JUL, that
indeed is a problem if that's the case. Atmosphere should use one and only
one logging framework and it should be a logging API (like SLF4J) so others
can easily configure their desired logger, JUL, Log4j, etc.

On Thu, May 30, 2013 at 4:21 AM, Artur notifications@github.com wrote:

The problem we are having is basically that we want to be able to produce
a war file that you can deploy on any of our supported servers. This works
great except for the slf4j api problem. Another issue is that the rest of
the framework uses jul for logging and the users would end up having to
configure both slf4j and jul. If you are not interested in going back to
jul then I think we will have to do it anyway for our version.


Reply to this email directly or view it on GitHubhttps://github.com//issues/1106#issuecomment-18672199
.

@jfarcand
Copy link
Member

@dhoffer Atmosphere only use one, e.g slf4 . The issue they are seeing is more related to several version of that library, which seems to clash.

@stephenc
Copy link

Application containers should not be polluting the contained application's classpath by exposing slf4j to the application's classloader, so IMHO this is an issue of the application container and not an issue with slf4j or atmosphere

@dhoffer
Copy link

dhoffer commented May 30, 2013

Okay good news, that's what I expected. Then to solve the version problem,
seems one could solve this a few ways. Either by adding a maven exclusion
in the their application and then adding the desired version (as I doubt
atmosphere cares what version is used)...or just marking as provided if the
container already has that. However perhaps a better solution would be to
prevent the application container from interfering with the application's
classpath. One question I have is the context.xml, I think Atmosphere
might be recommending to set Loader delegate="true" which I think is
backwards as it will cause it to look to the parent first. Is there any
reason atmosphere needs it this way?

On Thu, May 30, 2013 at 6:59 AM, Jeanfrancois Arcand <
notifications@github.com> wrote:

@dhoffer https://github.com/dhoffer Atmosphere only use one, e.g slf4 .
The issue they are seeing is more related to several version of that
library, which seems to clash.


Reply to this email directly or view it on GitHubhttps://github.com//issues/1106#issuecomment-18678341
.

@dhoffer
Copy link

dhoffer commented May 30, 2013

I just checked the docs, looks like for 1.1.x there is no need for
context.xml anymore? If so, that won't be changing the default classloader
anymore.

On Thu, May 30, 2013 at 7:43 AM, David Hoffer dhoffer6@gmail.com wrote:

Okay good news, that's what I expected. Then to solve the version
problem, seems one could solve this a few ways. Either by adding a maven
exclusion in the their application and then adding the desired version (as
I doubt atmosphere cares what version is used)...or just marking as
provided if the container already has that. However perhaps a better
solution would be to prevent the application container from interfering
with the application's classpath. One question I have is the context.xml,
I think Atmosphere might be recommending to set Loader delegate="true"
which I think is backwards as it will cause it to look to the parent first.
Is there any reason atmosphere needs it this way?

On Thu, May 30, 2013 at 6:59 AM, Jeanfrancois Arcand <
notifications@github.com> wrote:

@dhoffer https://github.com/dhoffer Atmosphere only use one, e.g slf4
. The issue they are seeing is more related to several version of that
library, which seems to clash.


Reply to this email directly or view it on GitHubhttps://github.com//issues/1106#issuecomment-18678341
.

@jfarcand
Copy link
Member

Loader delegate="true" is ONLY used if you are using the Native Comet implementation in Tomcat 6/7. Since Native is no longer the default in 1.1, that's why context.xml is no longer needed.

@dhoffer
Copy link

dhoffer commented May 30, 2013

Got it, that makes sense. That should eliminate the app from forcing to
use the parent classloader, that's a good move, imho.

On Thu, May 30, 2013 at 7:59 AM, Jeanfrancois Arcand <
notifications@github.com> wrote:

Loader delegate="true" is ONLY used if you are using the Native Comet
implementation in Tomcat 6/7. Since Native is no longer the default in 1.1,
that's why context.xml is no longer needed.


Reply to this email directly or view it on GitHubhttps://github.com//issues/1106#issuecomment-18681717
.

@jfarcand jfarcand closed this as completed Jun 5, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants