uohzoaix / spymemcached

Automatically exported from code.google.com/p/spymemcached
0 stars 0 forks source link

logging change over to slf4j #51

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
a couple of people have suggested this over on the logging wiki page, but
doesn't look like any one has yet opened a ticket.

it would be great if spymemcached could support the logging facade:
http://www.slf4j.org/

that would make the logging setup more standardized, and might reduce (
eliminate? ) the dependency on the extra spy.jar.

Original issue reported on code.google.com by shalmaneser@gmail.com on 31 Jan 2009 at 3:10

GoogleCodeExporter commented 9 years ago
the spy.jar dependency is removed in master.

slf4j is kind of interesting, but I've already built it with printf syntax and 
ensure
all of my objects have loggers currently.  I don't think I'd put effort into 
swapping
out a new logging abstraction (though I may want to make it smarter about 
figuring
out what kind of logging to do by default).

Original comment by dsalli...@gmail.com on 31 Jan 2009 at 6:40

GoogleCodeExporter commented 9 years ago
It would be great to update to a standard logging system.  Would you accept a 
patch
to do so?

Original comment by ddlat...@gmail.com on 24 Feb 2010 at 9:41

GoogleCodeExporter commented 9 years ago
It supports both sun's logger and log4j.  Which standard are you referring to?

Original comment by dsalli...@gmail.com on 24 Feb 2010 at 9:43

GoogleCodeExporter commented 9 years ago
slf4j would be great.

It would be great to remove the step of adding a new library, like this 
excellent
one, noticing different log output from everything else, tracking down the 
logging
documentation, and needing to update all the launch scripts to include a new 
system
property specific to this library.

Original comment by ddlat...@gmail.com on 24 Feb 2010 at 9:47

GoogleCodeExporter commented 9 years ago
slf4j is less of a standard logging mechanism than the built-in java logger or 
log4j -- 
both of which I support.  I could also add support for commons logger and 
slf4j, but I 
don't see why this would add value.

Is there something you want to accomplish that you can't with log4j or 
java.util.logging?

Original comment by dsalli...@gmail.com on 25 Feb 2010 at 8:14

GoogleCodeExporter commented 9 years ago
Nope, there's nothing I need to accomplish that the current setup is failing 
for me.
 I've got it set up now, and it's working fine.

The goal would be to make it easier to integrate this library in projects that 
use
many others, so that each library would not use it's own logging facade and 
require
it's own property/config to setup.  It looks like 
net.spy.memcached.compat.log.Logger
is a decent facade layer in front of logging choices, and I can see why it was 
put in
before something like slf4j came along.  

Original comment by ddlat...@gmail.com on 25 Feb 2010 at 4:36

GoogleCodeExporter commented 9 years ago
I don't understand what does this means

Using log4j

Set the logger impl to net.spy.log.Log4JLogger. For example:

  -Dnet.spy.log.LoggerImpl=net.spy.memcached.compat.log.Log4JLogger 

I am using log4j.xml and i tried creating a logger for the package 
net.spy.memcached but that doesn't help.

Original comment by agraw...@gmail.com on 23 Jun 2010 at 10:22

GoogleCodeExporter commented 9 years ago
FWIW, I'm another very happy user who just spent a couple of hours tracking 
down the source of the odd STDERR error messages I'm seeing... Adding the 
'-Dnet.spy.log.LoggerImpl=net.spy.memcached.compat.log.Log4JLogger ' to all of 
my JBoss launch instances is a bit of a PITA.  I wish that this was 
configurable in the usage code somehow, if Log4J isn't going to be the default 
(as it appears that it won't be).

Original comment by rjstanf...@gmail.com on 1 Sep 2010 at 10:42

GoogleCodeExporter commented 9 years ago
Is there any way to configure skymemcached logging for web application in 
Tomcat without setting JVM-wide system properties?

Original comment by max.valj...@gmail.com on 27 Nov 2010 at 8:40

GoogleCodeExporter commented 9 years ago
i would vote for code/config file based configuration too.
it is pain to set system wide setting on all of prod machines. easier to deploy 
a config file than setting in env.

Original comment by mukesh.a...@gmail.com on 3 Dec 2010 at 9:48

GoogleCodeExporter commented 9 years ago
+1 too.  Also note that SLF4J is written by the author of Log4j and is intended 
to be the one logging facade to use, preferably with logback as the Log4j 
replacement.

SLF4J is the de-facto standard logging facade, and having built-in support for 
it in spymemcached would be really nice.

Original comment by tsuna...@gmail.com on 8 Nov 2011 at 12:42

GoogleCodeExporter commented 9 years ago
So, I have a project using slf4j + logback, and the whole project uses the same 
standard, using a single logging configuration, etc. But spymemcached logs 
require extra tweaking. Would be nice if you supported slf4j, as it's gotten 
more popular and standardized since this ticket was opened.

Original comment by ikromc...@gmail.com on 14 Dec 2011 at 10:52

GoogleCodeExporter commented 9 years ago
Maybe solved with log4j-over-slf4j

Original comment by andrey.b...@gmail.com on 14 Feb 2012 at 10:02

GoogleCodeExporter commented 9 years ago
+1

dsallings, slf4j does not compete with log4j or java logger - it would replace 
the logging abstraction you have here.  So rather than talk of adding support 
for other logging frameworks, you could let slf4j replace this whole directory 
of code.

That would save having to hunt down the seperate, unique logging setup for this 
package.  And give you all off the logging impls supported by slf4j, for free.  
For instance, I'm using logback, which this lib doesn't support as an impl, but 
slf4j does.

You basically have an in-house version of slf4j in the compat dir.  It'd be 
nice to consolidate.

Original comment by l...@janrain.com on 6 Mar 2012 at 11:30

GoogleCodeExporter commented 9 years ago
we're definitely open to a patch here, but since it's a change it'll 
necessitate a new minor release.  Marking it low priority and accepted.

Original comment by ingen...@gmail.com on 6 Mar 2012 at 11:38

GoogleCodeExporter commented 9 years ago
Oh man, this is disappointing to find.  I just switched over all of my projects 
from log4j to logback.  logback is the next generation of log4j and many of the 
open source frameworks are using it with the slf4j wrapper. While painful, it 
was pretty straightforward migration.  Now my very last project depends upon 
netspy and was quite surprised to discover this problem dating back to 2009.  
Sigh...

Original comment by dcp4...@gmail.com on 20 Jul 2012 at 7:31

GoogleCodeExporter commented 9 years ago
We've had more requests around slf4j I think.  What do you think of it?

Also, would be glad to work with you on integration of patches.  Can you 
contribute?

Original comment by ingen...@gmail.com on 20 Jul 2012 at 8:06

GoogleCodeExporter commented 9 years ago
I could contribute a patch when I get time, unless someone else gets to it 
first.

Original comment by l...@janrain.com on 20 Jul 2012 at 8:17

GoogleCodeExporter commented 9 years ago
It looks like someone contributed a patch a while ago:

https://github.com/hgschmie/java-memcached-client/commit/bff35690f54c2a882f9a682
b277a3a23410b8272

Is this acceptable?

Original comment by dsu...@gmail.com on 15 Aug 2012 at 5:37

GoogleCodeExporter commented 9 years ago
I glanced over that patch, and it looks good to me.  Though the slf4j version 
it specifies could stand to be updated - 1.6.6 is out now.

Original comment by l...@janrain.com on 15 Aug 2012 at 5:44

GoogleCodeExporter commented 9 years ago
@dsugar,

Can you upload your patch to our code review system at 
(http://review.couchbase.org/)? This way we can make sure and give you credit 
for the change. If not I can upload it for you and note that you submitted the 
code in the commit message.

Original comment by mikewie...@gmail.com on 15 Aug 2012 at 5:51

GoogleCodeExporter commented 9 years ago
By the way, more info on how to set up for contributing is here:
http://www.couchbase.com/wiki/display/couchbase/Contributing+Changes

Some of it isn't relevant to this project (the CLA, repo) but it will probably 
help with understanding what's happening.

Original comment by ingen...@gmail.com on 15 Aug 2012 at 5:55

GoogleCodeExporter commented 9 years ago
Replying to comment 21:

To give credit where it is due, Henning Schmiedehausen (hgschmie) actually 
contributed that code.  I was about to start working on a patch and noticed 
that he had already done the work.

Original comment by dsu...@gmail.com on 15 Aug 2012 at 6:27

GoogleCodeExporter commented 9 years ago
Okay. To make things easier on you I will upload the patch and give Henning 
credit for writing it and dsugar credit for submitting it. If you have any 
objections to this please let me know.

Original comment by mikewie...@gmail.com on 15 Aug 2012 at 6:38

GoogleCodeExporter commented 9 years ago
I have added this pull request to our code review system here: 
http://review.couchbase.org/#change,19755. When the commit goes through I will 
decline the pull request from dsugar on github. In the commit message I gave 
credit to both Henning (Written By) and Dennis (Submitted By).

Original comment by mikewie...@gmail.com on 17 Aug 2012 at 5:34

GoogleCodeExporter commented 9 years ago

Original comment by mikewie...@gmail.com on 17 Aug 2012 at 5:48

GoogleCodeExporter commented 9 years ago
I've updated this on the review.  We can't add this in a micro update, because 
it's incompatible.  I think it could be done in a compatible way, but it'd be a 
bigger change.  See the review referenced above.

Do we want to try to do this in the 2.8 series, or handle it in a 2.9.0?

Just to be clear, I really want to have this change, but wouldn't like breaking 
the use of log compat in a micro release which is what this change does 
currently.

Original comment by ingen...@gmail.com on 19 Aug 2012 at 8:14

GoogleCodeExporter commented 9 years ago
As far as compatibility - the current logging supports JRE, log4j, and stderr.  
Assuming logging defaults to going through slf4j, the first two could be 
supported by the addition of a couple jars (slf4j-api.jar  + slf4j-jdk14.jar or 
slf4j-log4j12.jar).  As far as stderr logging, there is slf4j-simple.jar which 
writes to there, but I don't know if the format would be the same.  Perhaps one 
of the other loggers could be configured to mimic the behaviour of 
spymemcached's stderr logger.  For serious (logging) use, you'd think people 
would already be using log4j or JRE logging, anyways - the change there should 
be transparent - the 
-Dnet.spy.log.LoggerImpl=net.spy.memcached.compat.log.Log4JLogger setting would 
be ignored, replaced by the addition of a couple jars to the classpath.  Again, 
not sure how much requiring a couple extra jars to get the same behavior would 
be a minor change - there could be wider implications in their system.
Another option would be to simply have slf4j as another option in the current 
logging switched on the net.spy.log.LoggerImpl property.  That would be more 
work, and I think people using slf4j want to avoid having to specify the logger 
used by each lib in system properties, and just have them default to slf4j.  
Changing the default from stderr to slf4j in such a system might be an option 
(maybe in the next (compat-breaking) release).  Again, I don't think people who 
let the logging default to stderr are serious about their logging, anyways.

Original comment by l...@janrain.com on 19 Aug 2012 at 9:11

GoogleCodeExporter commented 9 years ago
I agree with what you're saying, but the current documentation (and for years) 
has documented a define to switch to different log compat, either log4j or JDK 
logging.  The problem with the patch as it currently stands is that neither of 
those would work any longer.

I see two options:
1) add this as another log compat (easy)
2) add slf4j as a dependency and make the old defines work using slf4j (harder)

You make a good point about those who just let things go to stderr.

Original comment by ingen...@gmail.com on 19 Aug 2012 at 9:29

GoogleCodeExporter commented 9 years ago
So was a decision ever made whether to to add slf4j as another log compat or 
replace compat completely with slf4j?

If so, will this go into version 8.x or 9?

Original comment by thriceth...@gmail.com on 11 Dec 2012 at 12:43

GoogleCodeExporter commented 9 years ago
yes, and it was partially done, but was breaking some existing behavior in 
2.8.x so we set it aside for now.  We want to get it in to 2.8.x without 
breaking anything.

Original comment by ingen...@gmail.com on 11 Dec 2012 at 1:08

GoogleCodeExporter commented 9 years ago
By the way, the work in progress is right here:
http://review.couchbase.org/#/c/9640/

There is not a lot of work, in case you might be able to help get it to the 
right place.

Original comment by ingen...@gmail.com on 11 Dec 2012 at 1:22

GoogleCodeExporter commented 9 years ago
Looks like the change has now been abandoned. When will this work now be done?

Original comment by helen.wi...@mindcandy.com on 31 Jan 2013 at 11:29

GoogleCodeExporter commented 9 years ago
We're planning to get this done for the next minor release, but there is no 
roadmap with a fixed date yet.

Original comment by michael....@gmail.com on 31 Jan 2013 at 11:34

GoogleCodeExporter commented 9 years ago
Note that we dont completely switched over to slf4j, but we provide an SLF4J 
adapter as we do with log4j for example.. its a larger change and is expected 
to be done for a 3.0 release.

See this blog post for more info on how to do it: 
http://nitschinger.at/Logging-with-the-Couchbase-Java-Client

Original comment by michael....@gmail.com on 6 Jun 2013 at 1:09