SLF4J usage

[mballoni]

Many libraries and projects today use SLF4J in order to provide a more flexible logging usage.
  It gives the developer the freedom to chose whatever log framework he or she wants.

  Jedis just aproved this same sort of migration:

  Https://github.com/xetorthio/jedis/issues/1340

  I think that this lib could follow the same example in order to follow this industry pattern and be more friendly with developers.

  I would also make a request with this feature that seems simple. May I?

[mballoni]

Created PR: #22

[ccheetham]

At a general level, replacing the pre-configured (java.util.logging) logging engine with a logging facade (SLF4J) is a welcome change. Ideally, such a change would not incur any changes to current behavior "out of the box."

PR #22 changes behavior in a couple ways; one somewhat benign, one somewhat not.

PR #22 results in some additional output when Tomcat first starts:

SLF4J: Failed to load class "org.slf4j.impl.StaticLoggerBinder".
SLF4J: Defaulting to no-operation (NOP) logger implementation
SLF4J: See http://www.slf4j.org/codes.html#StaticLoggerBinder for further details.

Perhaps white noise, but to the unsuspecting, it could be perplexing since the message would not have been previously seen until an update to redis-store was applied.

A bit more worrisome is that by default, PR #22 results in slf4j using the NOP binding in the absence of a user-provided one, so by default no logging will be output from redis-store. (At least this is my understanding from reading the SLF4J docs.)

So how to address those two items ...

One could add a binding to the redis-store distribution (ie java.util.logging). This approach is not a recommended practice (per SLF4J); and (worse) if a user provides their own binding, SLF4J's behavior is undefined as to which binding is used.

Do you know if the redis-store distribution, with SLF4J support, can continue to use java.util.logging by default and not introduce any new stdout messages when Tomcat starts? If it can, we can move forward with a tweaked version of PR #22.

If not, perhaps we can address this with documentation. While a viable alternative, for the 2 reasons listed above, I'd like to first to pursue further how we can maintain current behavior while adding SLF4J support.

Thoughts?

[mballoni]

Interesting, you are right about your understanding of the SLF4J docs.
An approach that we could make is to add, by default, the slf4j-jdk14 binding (is how they call the JUL binder). This way the warnings will disappear.
Also it would be preferable to add a line into the README to explicit alert users that they should configure an exclusion from the dependency declaration in case he or she desires a different binder.

As a collateral effect it is possible to have multiple bindings into the classpath in case the use don't configure the exclusion and also add another binder into the classpath. That is kind of a common problem with the slf4j usage with the unaware programmer. However that's something that already happens very often...

So, I think that we have two options

1. Add a jul default binder with an explanation into the docs;
2. Just leave a note into the doc's without a default binder.

Thank you

[ccheetham]

I'm encouraged by your option 1 suggestion. Can you create a PR? Please include a doc update that describes how to configure the dependency exclusion.

[mballoni]

Sure, I'll work on that today and update the PR.
Thank you!

[mballoni]

Hi @ccheetham , both #22 and #23 were updated as we discussed.
Thank you!