Re: [Patch review please] Further work for Receivers backport to 1.2
On 19/04/2007, at 2:57 PM, Curt Arnold wrote:
On Apr 18, 2007, at 10:31 PM, Paul Smith wrote:
Maybe you could fake out the getLogger() calls like
private static final class LogLogger {
public void debug(final String msg) {
LogLog.debug(msg);
}
public void warn(final String msg) {
LogLog.warn(msg);
}
public void error(final String msg, final Throwable ex) {
LogLog.error(msg, ex);
}
}
private static final LogLogger getLogger() {
return new LogLogger();
}
Done, nice suggestion. Bit clunky, but works for now.
Did my second suggestion, replacing that who chunk with
private static final LogLog getLogger() {
return new LogLog();
}
not work? Actually, since they are static methods, you could
probably get away with returning null, but I thought that might be
pushing it too much.
I'm a dumbo. I realise now that your suggestion wasn't for both.
I'll try the second technique exclusively.
I'm still working through some maven-eclipse issues. I'm not
_immediately_ loving Maven, but lets say I dislike it quite a bit
less than I used to. I can see some good things that have been done
since the bad old 1.x days.
My reading the diffs had both the declaration and assignment as new
code, so don't see how the split minimizes code change.
I'll take a peek.
I've taken your LoggeRepositoryExImpl class and currently trying it
out. I think I've got that bit working, and now have a problem with
some of the receivers not having the fqnLoggerClass set. This is
something that Scott pointed out with the DBAppender, and I think I
need to go back and check the others to make sure they work.
Getting this pluginRegistry working again has shown to me the 'age'
of the Chainsaw code base. The LogUI class could do with some
serious gutting/cleaning, because the startup logic is... brittle.
I'm sure I'm responsible for a good chunk of that evilness.A
Chainsaw 3 could probably be built on top of quite a bit of code
ripped out from the 2.0 codebase.
Paul
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
Re: [Patch review please] Further work for Receivers backport to 1.2
On 18/04/2007, at 2:46 PM, Scott Deboy wrote: In DBAppender: insertStatement.setLong(0, 0); should be insertStatement.setLong(1, 0); done, well spotted. Also, in CustomSQLDBReceiver: Should LoggingEvent event = new LoggingEvent(null, eventLogger, timeStamp, levelImpl, message, threadName, throwableInfo, ndc, locationInfo, properties); be LoggingEvent event = new LoggingEvent(eventLogger.getName(), eventLogger, timeStamp, levelImpl, message, threadName, throwableInfo, ndc, locationInfo, properties); ? Done, that makes sense, not sure what I was smoking. In dbreceiverjob: Should we pass a logger name instead of null in loggingevent constructor (same as above)? done. Question: are we going to support event properties in the 1.2 stream? Being able to populate properties from MDC, throwableinfo and NDC are all pretty important...would be nice if all of the receivers could use this functionality (multicastappender/receiver, etc). That's probably a next step. Paul - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: [Patch review please] Further work for Receivers backport to 1.2
Maybe you could fake out the getLogger() calls like
private static final class LogLogger {
public void debug(final String msg) {
LogLog.debug(msg);
}
public void warn(final String msg) {
LogLog.warn(msg);
}
public void error(final String msg, final Throwable ex) {
LogLog.error(msg, ex);
}
}
private static final LogLogger getLogger() {
return new LogLogger();
}
Done, nice suggestion. Bit clunky, but works for now.
and access the sequence number through another private method:
private static long getSequenceNumber(final LoggingEvent event) {
return 0;
}
I left this out. The discussion about what to do with sequence
numbers and the 1.2 tree is probably best left for a future thread.
In DBReceiverJob.java, I'm unclear why you are doing:
Object msg = null;
...
msg = rs.getString(3);
instead of:
Object msg = rs.getString(3);
I tried to leave the diffs as simple as possible. You are correct; I
just tried to limit the amount of text change.
p.s. Only my Aunt Louise gets to call me Curty
:-/ No idea what my fingers were doing there. I use the Dvorak
layout, so the 'y' is actually the 't' key on a qwerty. I can only
blame it on a high sugar condition after a can of coke.
Paul
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
Re: [Patch review please] Further work for Receivers backport to 1.2
On Apr 18, 2007, at 10:31 PM, Paul Smith wrote:
Maybe you could fake out the getLogger() calls like
private static final class LogLogger {
public void debug(final String msg) {
LogLog.debug(msg);
}
public void warn(final String msg) {
LogLog.warn(msg);
}
public void error(final String msg, final Throwable ex) {
LogLog.error(msg, ex);
}
}
private static final LogLogger getLogger() {
return new LogLogger();
}
Done, nice suggestion. Bit clunky, but works for now.
Did my second suggestion, replacing that who chunk with
private static final LogLog getLogger() {
return new LogLog();
}
not work? Actually, since they are static methods, you could
probably get away with returning null, but I thought that might be
pushing it too much.
and access the sequence number through another private method:
private static long getSequenceNumber(final LoggingEvent event) {
return 0;
}
I left this out. The discussion about what to do with sequence
numbers and the 1.2 tree is probably best left for a future thread.
In DBReceiverJob.java, I'm unclear why you are doing:
Object msg = null;
...
msg = rs.getString(3);
instead of:
Object msg = rs.getString(3);
I tried to leave the diffs as simple as possible. You are correct;
I just tried to limit the amount of text change.
My reading the diffs had both the declaration and assignment as new
code, so don't see how the split minimizes code change.
From my code reading PluginRegistry is always accessed by code like:
LoggerRepository repo = LogManager.getLoggerRepository();
if (repo instanceof LoggerRepositoryEx) {
PluginRegistry pluginRegistry = ((LoggerRepositoryEx)
repo).getPluginRegistry();
...
Some variation of that code appears in about 8 or 9 places. If you
could replace that code with a call to a function (maybe in LogUI):
private PluginRegistry plugRegistry;
public PluginRegistry getPluginRegistry() {
LoggerRepository repo = LogManager.getLoggerRepository();
if (repo instanceof LoggerRepositoryEx) {
return ((LoggerRepositoryEx) repo).getPluginRegistry();
}
synchronized(this) {
if(plugRegistry == null) {
plugRegistry = new PluginRegistry(repo);
}
return plugRegistry;
}
}
(with adding PluginRegistry(LoggerRepository) constructor in the
components project). That would likely get you over the initial hump
until can look whether we can add the notification hooks that would
be necessary to catch the repo shutdown and change notifications that
plug in registry would like.
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
Re: [Patch review please] Further work for Receivers backport to 1.2
On Apr 17, 2007, at 10:42 PM, Paul Smith wrote:
Hi all,
I've completed porting the remaining Receiver classes to the
sandbox area. The below patch is what I have locally, and taking
the generated jar and adding it as a dependency to Chainsaw makes
everything compile nicely. I am not able to do any testing on this
as yet, and figure a patch review is probably worthwhile at this
stage.
These are all baseline copies from the log4j trunk. The DB* stuff
needed more fiddling than the others. I had to comment out all
internal logging statements in the DBAppender because
AppenderSkeleton in 1.2 does not provide that. There is no concept
of a sequence # either, so I munged that in the insert statement so
that all the inserts have '0' as a sequence value. Given
LoggingEvent in 1.2 doesn't have that anyway, I figure that's ok..
Curty, Scott and others, any chance you could review this please
before I check it in?
(Note: I'll fix checkstyle issues later, I know there's tabs
everywhere sorry)
Maybe you could fake out the getLogger() calls like
private static final class LogLogger {
public void debug(final String msg) {
LogLog.debug(msg);
}
public void warn(final String msg) {
LogLog.warn(msg);
}
public void error(final String msg, final Throwable ex) {
LogLog.error(msg, ex);
}
}
private static final LogLogger getLogger() {
return new LogLogger();
}
and access the sequence number through another private method:
private static long getSequenceNumber(final LoggingEvent event) {
return 0;
}
That should allow the body of the functional methods to be unchanged
between 1.3 and 1.2.x. Only the kludge methods would need to change
to switch between 1.3 and 1.2.
In DBReceiverJob.java, I'm unclear why you are doing:
Object msg = null;
...
msg = rs.getString(3);
instead of:
Object msg = rs.getString(3);
p.s. Only my Aunt Louise gets to call me Curty
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
Re: [Patch review please] Further work for Receivers backport to 1.2
On Apr 17, 2007, at 11:27 PM, Curt Arnold wrote:
On Apr 17, 2007, at 10:42 PM, Paul Smith wrote:
Hi all,
I've completed porting the remaining Receiver classes to the
sandbox area. The below patch is what I have locally, and taking
the generated jar and adding it as a dependency to Chainsaw makes
everything compile nicely. I am not able to do any testing on
this as yet, and figure a patch review is probably worthwhile at
this stage.
These are all baseline copies from the log4j trunk. The DB* stuff
needed more fiddling than the others. I had to comment out all
internal logging statements in the DBAppender because
AppenderSkeleton in 1.2 does not provide that. There is no
concept of a sequence # either, so I munged that in the insert
statement so that all the inserts have '0' as a sequence value.
Given LoggingEvent in 1.2 doesn't have that anyway, I figure
that's ok..
Curty, Scott and others, any chance you could review this please
before I check it in?
(Note: I'll fix checkstyle issues later, I know there's tabs
everywhere sorry)
Maybe you could fake out the getLogger() calls like
private static final class LogLogger {
public void debug(final String msg) {
LogLog.debug(msg);
}
public void warn(final String msg) {
LogLog.warn(msg);
}
public void error(final String msg, final Throwable ex) {
LogLog.error(msg, ex);
}
}
private static final LogLogger getLogger() {
return new LogLogger();
}
or even better
private static final LogLog getLogger() {
return new LogLog();
}
You could likely get style warnings that you shouldn't access static
members using instance member syntax, but it should still work.
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
RE: [Patch review please] Further work for Receivers backport to 1.2
In DBAppender:
insertStatement.setLong(0, 0);
should be
insertStatement.setLong(1, 0);
Also, in CustomSQLDBReceiver:
Should
LoggingEvent event = new LoggingEvent(null,
eventLogger, timeStamp, levelImpl, message,
threadName,
throwableInfo,
ndc,
locationInfo,
properties);
be
LoggingEvent event = new LoggingEvent(eventLogger.getName(),
eventLogger, timeStamp, levelImpl, message,
threadName,
throwableInfo,
ndc,
locationInfo,
properties);
?
In dbreceiverjob:
Should we pass a logger name instead of null in loggingevent constructor (same
as above)?
Question: are we going to support event properties in the 1.2 stream? Being
able to populate properties from MDC, throwableinfo and NDC are all pretty
important...would be nice if all of the receivers could use this functionality
(multicastappender/receiver, etc).
Scott Deboy
COMOTIV SYSTEMS
111 SW Columbia Street Ste. 950
Portland, OR 97201
Telephone: 503.224.7496
Cell: 503.997.1367
Fax:503.222.0185
[EMAIL PROTECTED]
www.comotivsystems.com
-Original Message-
From: Curt Arnold [mailto:[EMAIL PROTECTED]
Sent: Tue 4/17/2007 9:40 PM
To: Log4J Developers List
Subject: Re: [Patch review please] Further work for Receivers backport to 1.2
On Apr 17, 2007, at 11:27 PM, Curt Arnold wrote:
On Apr 17, 2007, at 10:42 PM, Paul Smith wrote:
Hi all,
I've completed porting the remaining Receiver classes to the
sandbox area. The below patch is what I have locally, and taking
the generated jar and adding it as a dependency to Chainsaw makes
everything compile nicely. I am not able to do any testing on
this as yet, and figure a patch review is probably worthwhile at
this stage.
These are all baseline copies from the log4j trunk. The DB* stuff
needed more fiddling than the others. I had to comment out all
internal logging statements in the DBAppender because
AppenderSkeleton in 1.2 does not provide that. There is no
concept of a sequence # either, so I munged that in the insert
statement so that all the inserts have '0' as a sequence value.
Given LoggingEvent in 1.2 doesn't have that anyway, I figure
that's ok..
Curty, Scott and others, any chance you could review this please
before I check it in?
(Note: I'll fix checkstyle issues later, I know there's tabs
everywhere sorry)
Maybe you could fake out the getLogger() calls like
private static final class LogLogger {
public void debug(final String msg) {
LogLog.debug(msg);
}
public void warn(final String msg) {
LogLog.warn(msg);
}
public void error(final String msg, final Throwable ex) {
LogLog.error(msg, ex);
}
}
private static final LogLogger getLogger() {
return new LogLogger();
}
or even better
private static final LogLog getLogger() {
return new LogLog();
}
You could likely get style warnings that you shouldn't access static
members using instance member syntax, but it should still work.
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
winmail.dat-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
