[xwiki-devs] [Proposal] Logging improvements
Hi, I wanted to see if we could move our LogEnabled lifecycle phase to a Logging component. I think it's not going to work since this means injecting a LoggingFactory/LoggingManager component and you need to call getLogger(this.getClass()) to get access to the Logger which is awkward. What I propose: 1) Use SLF4J (drop the slf4j-log4j jar in our WEB-INF/lib so that SLF4J uses log4j by default) 2) Drop the JCL/JUL/LOG4j SLF4J legacy jars in our WEB-INF/lib too and exclude the JCL/JUL/LOG4J jars from our poms so that all third party logs go to our logging system 3) Non component code should use a SLF4J's LoggerFactory directly 4a) Keep LogEnabled and AbstractLogEnabled for our components or 4b) Automatically inject a Logger and a ComponentManager when there are fields with these types in a component class. I like 4b) for its simplicity but I'm worried by the "magical" aspect of it. WDYT? Thanks -Vincent
Vincent Massol wrote:
Hi,
I wanted to see if we could move our LogEnabled lifecycle phase to a Logging component. I think it's not going to work since this means injecting a LoggingFactory/LoggingManager component and you need to call getLogger(this.getClass()) to get access to the Logger which is awkward.
What I propose:
1) Use SLF4J (drop the slf4j-log4j jar in our WEB-INF/lib so that SLF4J uses log4j by default) 2) Drop the JCL/JUL/LOG4j SLF4J legacy jars in our WEB-INF/lib too and exclude the JCL/JUL/LOG4J jars from our poms so that all third party logs go to our logging system 3) Non component code should use a SLF4J's LoggerFactory directly
4a) Keep LogEnabled and AbstractLogEnabled for our components or 4b) Automatically inject a Logger and a ComponentManager when there are fields with these types in a component class.
I like 4b) for its simplicity but I'm worried by the "magical" aspect of it.
But... Why do we need 4 at all? -- Sergiu Dumitriu http://purl.org/net/sergiu/
On May 13, 2009, at 5:30 PM, Sergiu Dumitriu wrote:
Vincent Massol wrote:
Hi,
I wanted to see if we could move our LogEnabled lifecycle phase to a Logging component. I think it's not going to work since this means injecting a LoggingFactory/LoggingManager component and you need to call getLogger(this.getClass()) to get access to the Logger which is awkward.
What I propose:
1) Use SLF4J (drop the slf4j-log4j jar in our WEB-INF/lib so that SLF4J uses log4j by default) 2) Drop the JCL/JUL/LOG4j SLF4J legacy jars in our WEB-INF/lib too and exclude the JCL/JUL/LOG4J jars from our poms so that all third party logs go to our logging system 3) Non component code should use a SLF4J's LoggerFactory directly
4a) Keep LogEnabled and AbstractLogEnabled for our components or 4b) Automatically inject a Logger and a ComponentManager when there are fields with these types in a component class.
I like 4b) for its simplicity but I'm worried by the "magical" aspect of it.
But... Why do we need 4 at all?
You mean use a static and don't do IOC? I don't like it it has all the problems of static. -Vincent
Vincent Massol wrote:
On May 13, 2009, at 5:30 PM, Sergiu Dumitriu wrote:
Vincent Massol wrote:
Hi,
I wanted to see if we could move our LogEnabled lifecycle phase to a Logging component. I think it's not going to work since this means injecting a LoggingFactory/LoggingManager component and you need to call getLogger(this.getClass()) to get access to the Logger which is awkward.
What I propose:
1) Use SLF4J (drop the slf4j-log4j jar in our WEB-INF/lib so that SLF4J uses log4j by default) 2) Drop the JCL/JUL/LOG4j SLF4J legacy jars in our WEB-INF/lib too and exclude the JCL/JUL/LOG4J jars from our poms so that all third party logs go to our logging system 3) Non component code should use a SLF4J's LoggerFactory directly
4a) Keep LogEnabled and AbstractLogEnabled for our components or 4b) Automatically inject a Logger and a ComponentManager when there are fields with these types in a component class.
I like 4b) for its simplicity but I'm worried by the "magical" aspect of it. But... Why do we need 4 at all?
You mean use a static and don't do IOC?
I don't like it it has all the problems of static.
Why not just have a plain field, like: final Logger logger = LoggerFactory.getLogger(Wombat.class); Is the logger a component? It's just plain logging, we don't need to go that deep. IOC is fine where it's useful, but here it's just overhead IMO. -- Sergiu Dumitriu http://purl.org/net/sergiu/
On May 13, 2009, at 5:43 PM, Sergiu Dumitriu wrote:
Vincent Massol wrote:
On May 13, 2009, at 5:30 PM, Sergiu Dumitriu wrote:
Vincent Massol wrote:
Hi,
I wanted to see if we could move our LogEnabled lifecycle phase to a Logging component. I think it's not going to work since this means injecting a LoggingFactory/LoggingManager component and you need to call getLogger(this.getClass()) to get access to the Logger which is awkward.
What I propose:
1) Use SLF4J (drop the slf4j-log4j jar in our WEB-INF/lib so that SLF4J uses log4j by default) 2) Drop the JCL/JUL/LOG4j SLF4J legacy jars in our WEB-INF/lib too and exclude the JCL/JUL/LOG4J jars from our poms so that all third party logs go to our logging system 3) Non component code should use a SLF4J's LoggerFactory directly
4a) Keep LogEnabled and AbstractLogEnabled for our components or 4b) Automatically inject a Logger and a ComponentManager when there are fields with these types in a component class.
I like 4b) for its simplicity but I'm worried by the "magical" aspect of it. But... Why do we need 4 at all?
You mean use a static and don't do IOC?
I don't like it it has all the problems of static.
Why not just have a plain field, like:
final Logger logger = LoggerFactory.getLogger(Wombat.class);
Is the logger a component? It's just plain logging, we don't need to go that deep. IOC is fine where it's useful, but here it's just overhead IMO.
Funny you say this when I find this in your code: // TODO It would be better to use a custom logger class, how to do that? StringOutputStream log = new StringOutputStream(); PrintStream out = System.out; System.setOut(new PrintStream(log)); engine.evaluate(context, writer, "mytemplate", new StringReader("#set($foo = $date.getYear())$foo $date.month")); System.setOut(out); :) Your example makes it hard to unit test. I'd personally see it as a regression to what we currently have. Thanks -Vincent
Vincent Massol wrote:
On May 13, 2009, at 5:43 PM, Sergiu Dumitriu wrote:
Vincent Massol wrote:
On May 13, 2009, at 5:30 PM, Sergiu Dumitriu wrote:
Vincent Massol wrote:
Hi,
I wanted to see if we could move our LogEnabled lifecycle phase to a Logging component. I think it's not going to work since this means injecting a LoggingFactory/LoggingManager component and you need to call getLogger(this.getClass()) to get access to the Logger which is awkward.
What I propose:
1) Use SLF4J (drop the slf4j-log4j jar in our WEB-INF/lib so that SLF4J uses log4j by default) 2) Drop the JCL/JUL/LOG4j SLF4J legacy jars in our WEB-INF/lib too and exclude the JCL/JUL/LOG4J jars from our poms so that all third party logs go to our logging system 3) Non component code should use a SLF4J's LoggerFactory directly
4a) Keep LogEnabled and AbstractLogEnabled for our components or 4b) Automatically inject a Logger and a ComponentManager when there are fields with these types in a component class.
I like 4b) for its simplicity but I'm worried by the "magical" aspect of it. But... Why do we need 4 at all? You mean use a static and don't do IOC?
I don't like it it has all the problems of static.
Why not just have a plain field, like:
final Logger logger = LoggerFactory.getLogger(Wombat.class);
Is the logger a component? It's just plain logging, we don't need to go that deep. IOC is fine where it's useful, but here it's just overhead IMO.
Funny you say this when I find this in your code:
// TODO It would be better to use a custom logger class, how to do that? StringOutputStream log = new StringOutputStream(); PrintStream out = System.out; System.setOut(new PrintStream(log)); engine.evaluate(context, writer, "mytemplate", new StringReader("#set($foo = $date.getYear())$foo $date.month")); System.setOut(out);
:)
Your example makes it hard to unit test. I'd personally see it as a regression to what we currently have.
In the meantime I found the answer for this question, and it's simply to use a different log4j.properties file for tests (src/test/resources/). -- Sergiu Dumitriu http://purl.org/net/sergiu/
On May 13, 2009, at 5:53 PM, Sergiu Dumitriu wrote:
Vincent Massol wrote:
On May 13, 2009, at 5:43 PM, Sergiu Dumitriu wrote:
Vincent Massol wrote:
On May 13, 2009, at 5:30 PM, Sergiu Dumitriu wrote:
Vincent Massol wrote:
Hi,
I wanted to see if we could move our LogEnabled lifecycle phase to a Logging component. I think it's not going to work since this means injecting a LoggingFactory/LoggingManager component and you need to call getLogger(this.getClass()) to get access to the Logger which is awkward.
What I propose:
1) Use SLF4J (drop the slf4j-log4j jar in our WEB-INF/lib so that SLF4J uses log4j by default) 2) Drop the JCL/JUL/LOG4j SLF4J legacy jars in our WEB-INF/lib too and exclude the JCL/JUL/LOG4J jars from our poms so that all third party logs go to our logging system 3) Non component code should use a SLF4J's LoggerFactory directly
4a) Keep LogEnabled and AbstractLogEnabled for our components or 4b) Automatically inject a Logger and a ComponentManager when there are fields with these types in a component class.
I like 4b) for its simplicity but I'm worried by the "magical" aspect of it. But... Why do we need 4 at all? You mean use a static and don't do IOC?
I don't like it it has all the problems of static.
Why not just have a plain field, like:
final Logger logger = LoggerFactory.getLogger(Wombat.class);
Is the logger a component? It's just plain logging, we don't need to go that deep. IOC is fine where it's useful, but here it's just overhead IMO.
Funny you say this when I find this in your code:
// TODO It would be better to use a custom logger class, how to do that? StringOutputStream log = new StringOutputStream(); PrintStream out = System.out; System.setOut(new PrintStream(log)); engine.evaluate(context, writer, "mytemplate", new StringReader("#set($foo = $date.getYear())$foo $date.month")); System.setOut(out);
:)
Your example makes it hard to unit test. I'd personally see it as a regression to what we currently have.
In the meantime I found the answer for this question, and it's simply to use a different log4j.properties file for tests (src/test/resources/).
That's not very nice and wrong IMO since it creates a strong dependency with our logging system and log4j when our system is independent of the logging system chosen. -Vincent
On May 13, 2009, at 6:01 PM, Vincent Massol wrote:
On May 13, 2009, at 5:53 PM, Sergiu Dumitriu wrote:
Vincent Massol wrote:
On May 13, 2009, at 5:43 PM, Sergiu Dumitriu wrote:
Vincent Massol wrote:
On May 13, 2009, at 5:30 PM, Sergiu Dumitriu wrote:
Vincent Massol wrote: > Hi, > > I wanted to see if we could move our LogEnabled lifecycle phase > to a > Logging component. I think it's not going to work since this > means > injecting a LoggingFactory/LoggingManager component and you > need to > call getLogger(this.getClass()) to get access to the Logger > which > is > awkward. > > What I propose: > > 1) Use SLF4J (drop the slf4j-log4j jar in our WEB-INF/lib so > that > SLF4J uses log4j by default) > 2) Drop the JCL/JUL/LOG4j SLF4J legacy jars in our WEB-INF/lib > too > and exclude the JCL/JUL/LOG4J jars from our poms so that all > third > party logs go to our logging system > 3) Non component code should use a SLF4J's LoggerFactory > directly > > 4a) Keep LogEnabled and AbstractLogEnabled for our components > or > 4b) Automatically inject a Logger and a ComponentManager when > there > are fields with these types in a component class. > > I like 4b) for its simplicity but I'm worried by the "magical" > aspect > of it. But... Why do we need 4 at all? You mean use a static and don't do IOC?
I don't like it it has all the problems of static.
Why not just have a plain field, like:
final Logger logger = LoggerFactory.getLogger(Wombat.class);
Is the logger a component? It's just plain logging, we don't need to go that deep. IOC is fine where it's useful, but here it's just overhead IMO.
Funny you say this when I find this in your code:
// TODO It would be better to use a custom logger class, how to do that? StringOutputStream log = new StringOutputStream(); PrintStream out = System.out; System.setOut(new PrintStream(log)); engine.evaluate(context, writer, "mytemplate", new StringReader("#set($foo = $date.getYear())$foo $date.month")); System.setOut(out);
:)
Your example makes it hard to unit test. I'd personally see it as a regression to what we currently have.
In the meantime I found the answer for this question, and it's simply to use a different log4j.properties file for tests (src/test/ resources/).
That's not very nice and wrong IMO since it creates a strong dependency with our logging system and log4j when our system is independent of the logging system chosen.
A mock Logger with an expectation would be much better. -Vincent
On May 13, 2009, at 5:39 PM, Vincent Massol wrote:
On May 13, 2009, at 5:30 PM, Sergiu Dumitriu wrote:
Vincent Massol wrote:
Hi,
I wanted to see if we could move our LogEnabled lifecycle phase to a Logging component. I think it's not going to work since this means injecting a LoggingFactory/LoggingManager component and you need to call getLogger(this.getClass()) to get access to the Logger which is awkward.
What I propose:
1) Use SLF4J (drop the slf4j-log4j jar in our WEB-INF/lib so that SLF4J uses log4j by default) 2) Drop the JCL/JUL/LOG4j SLF4J legacy jars in our WEB-INF/lib too and exclude the JCL/JUL/LOG4J jars from our poms so that all third party logs go to our logging system 3) Non component code should use a SLF4J's LoggerFactory directly
4a) Keep LogEnabled and AbstractLogEnabled for our components or 4b) Automatically inject a Logger and a ComponentManager when there are fields with these types in a component class.
I like 4b) for its simplicity but I'm worried by the "magical" aspect of it.
Note that if we wanted to remove the magical aspect of it we could simply add a @LogEnabled and @Composable annotations and the class fields would only be injected if those annotations are found. But maybe it's not even necessary. -Vincent
But... Why do we need 4 at all?
You mean use a static and don't do IOC?
I don't like it it has all the problems of static.
-Vincent
On May 13, 2009, at 5:19 PM, Vincent Massol wrote:
Hi,
I wanted to see if we could move our LogEnabled lifecycle phase to a Logging component. I think it's not going to work since this means injecting a LoggingFactory/LoggingManager component and you need to call getLogger(this.getClass()) to get access to the Logger which is awkward.
What I propose:
1) Use SLF4J (drop the slf4j-log4j jar in our WEB-INF/lib so that SLF4J uses log4j by default) 2) Drop the JCL/JUL/LOG4j SLF4J legacy jars in our WEB-INF/lib too and exclude the JCL/JUL/LOG4J jars from our poms so that all third party logs go to our logging system 3) Non component code should use a SLF4J's LoggerFactory directly
and have a setLogger(Logger) method so that they can be unit tested easily. -Vincent
4a) Keep LogEnabled and AbstractLogEnabled for our components or 4b) Automatically inject a Logger and a ComponentManager when there are fields with these types in a component class.
I like 4b) for its simplicity but I'm worried by the "magical" aspect of it.
WDYT?
Thanks -Vincent
participants (2)
-
Sergiu Dumitriu -
Vincent Massol