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

Simplify log configuration in servlet environment #61

Open
mjiderhamn opened this issue Oct 12, 2016 · 2 comments
Open

Simplify log configuration in servlet environment #61

mjiderhamn opened this issue Oct 12, 2016 · 2 comments

Comments

@mjiderhamn
Copy link
Owner

As reported by @hdeadman in #60:

In the 1.x version of this listener I could extend the listener and override the logging methods. I am currently using SLF4J/Logback and I was going to provide an implementation of Logger that used that but I didn't see an easy way to extend the listener and pass in my instance of the Logger so it could be set on ClassLoaderLeakPreventorFactory. Should "void contextInitialized(final ServletContext servletContext)" be overloaded with a version that takes a Logger?

It's not a big deal for me because I am using the org.slf4/jul-to-slf4j bridge so the default JULLogger happens to work for me but it isn't clear to me how one is supposed to specify their own logger (or use StdLogger).

Another option would be to have more implementations of Logger and have a context parameter for choosing between them. If the context parameter specified the Logger impl class name then you wouldn't have to bundle all the impl classes.

@mjiderhamn
Copy link
Owner Author

@hdeadman would you care to confirm that the changes in #86 also simplifies setting a logger on the ClassLoaderLeakPreventorFactory?

@hdeadman
Copy link

That looks like it would allow someone to provide their own logger. Feel free to close this issue.

I haven't been using this library recently b/c at the place I work now we run all our apps in docker (typically using spring-boot). We can't really do zero downtime deployments via deploying a new version of an app to a permanently running app server so we don't run into classloader memory leaks anymore. Our deployments are mostly zero downtime b/c most state has moved to the browser and we have redundant services behind a load balancer. I say mostly b/c there is plenty of room for improvement around orchestrating container restarts and moving the authentication state of the otherwise stateless services to an api gateway.

I will continue to recommend this library to anyone I run into that is having classloader memory leaks. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants