[jira] [Comment Edited] (LOG4J2-547) Update LoggerStream API
[ https://issues.apache.org/jira/browse/LOG4J2-547?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13972262#comment-13972262 ] Bruce Brouwer edited comment on LOG4J2-547 at 4/17/14 12:13 PM: I like the builder idea too. I was just following the pattern that all other Java related streams take. So I decided to take a stab at it with log4j2-547-builders.patch. Also in that patch are some things that I missed originally, so here's what it includes: * LoggerStreams.Builder (and related classes) * Updated log4j-bom to include streams * Moved helper classes into a .helpers package I haven't written the tests because I wanted to hear some feedback on the approach. was (Author: bruce.brouwer): I like the builder idea too. I was just following the pattern that all other Java related streams take. So I decided to take a stab at it with log4j2-547-builders-and-more.patch. Also in that patch are some things that I missed originally, so here's what it includes: * Builders * Updated log4j-bom to include streams * Moved helper classes into a .helpers package I haven't written the tests because I wanted to hear some feedback on the approach. > Update LoggerStream API > --- > > Key: LOG4J2-547 > URL: https://issues.apache.org/jira/browse/LOG4J2-547 > Project: Log4j 2 > Issue Type: Improvement > Components: API >Affects Versions: 2.0-rc1 >Reporter: Matt Sicker >Assignee: Ralph Goers > Fix For: 2.0-rc2 > > Attachments: 0001-PrintStream-API-update.patch, MyBenchmark.java, > PerfTestCalcLocation.java, log4j2-547-bbrouwer.patch, > log4j2-547-builders.patch, log4j2-547-new-module.patch, > log4j2-547-remove-streams.patch, log4j2-loggerStream.patch > > > I've got some ideas on how to improve the LoggerStream idea that I added a > little while ago. The main thing I'd like to do is extract an interface from > it, rename the default implementation to SimpleLoggerStream (part of the > SimpleLogger stuff), and allow log4j implementations to specify a different > implementation if desired. > In doing this, I'm not sure where specifically I'd prefer the getStream > methods to be. Right now, it's in Logger, but really, it could be in > LoggerContext instead. I don't think I should be required to get a Logger > just to get a LoggerStream. > Now if only the java.io package used interfaces instead of classes. This > would be so much easier to design! -- This message was sent by Atlassian JIRA (v6.2#6252) - To unsubscribe, e-mail: log4j-dev-unsubscr...@logging.apache.org For additional commands, e-mail: log4j-dev-h...@logging.apache.org
[jira] [Comment Edited] (LOG4J2-547) Update LoggerStream API
[ https://issues.apache.org/jira/browse/LOG4J2-547?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13917561#comment-13917561 ] Matt Sicker edited comment on LOG4J2-547 at 3/2/14 9:16 PM: I'm starting to see two ways this can go: as part of the API, or a separate module dependent on log4j-api (similar to fluent-hc from HttpComponents). I like the idea of going the full java.io route for spying and such (which can help log I/O as well as provide a path for logger-less code to transition to real loggers). As for the one root class, of course LogManager should have the API available to create the streams and reader/writers, but that could easily be delegated just like how most of the methods in LogManager already delegate to a LoggerContextFactory. Edit: if we made a fluent API, it would definitely make sense to put it in its own module such as fluent-logging or log4j-fluent (more consistent naming there at least). Fluent APIs are neat, but they're also limited in functionality (which is the point) and really belong in separate modules. Then again, I'm all about modules lately, so I might be a bit biased. ;) was (Author: jvz): I'm starting to see two ways this can go: as part of the API, or a separate module dependent on log4j-api (similar to fluent-hc from HttpComponents). I like the idea of going the full java.io route for spying and such (which can help log I/O as well as provide a path for logger-less code to transition to real loggers). As for the one root class, of course LogManager should have the API available to create the streams and reader/writers, but that could easily be delegated just like how most of the methods in LogManager already delegate to a LoggerContextFactory. > Update LoggerStream API > --- > > Key: LOG4J2-547 > URL: https://issues.apache.org/jira/browse/LOG4J2-547 > Project: Log4j 2 > Issue Type: Improvement > Components: API >Affects Versions: 2.0-rc1 >Reporter: Matt Sicker >Assignee: Ralph Goers > Fix For: 2.0 > > Attachments: 0001-PrintStream-API-update.patch, > Add_caller_info_tests.patch, log4j2-loggerStream.patch > > > I've got some ideas on how to improve the LoggerStream idea that I added a > little while ago. The main thing I'd like to do is extract an interface from > it, rename the default implementation to SimpleLoggerStream (part of the > SimpleLogger stuff), and allow log4j implementations to specify a different > implementation if desired. > In doing this, I'm not sure where specifically I'd prefer the getStream > methods to be. Right now, it's in Logger, but really, it could be in > LoggerContext instead. I don't think I should be required to get a Logger > just to get a LoggerStream. > Now if only the java.io package used interfaces instead of classes. This > would be so much easier to design! -- This message was sent by Atlassian JIRA (v6.2#6252) - To unsubscribe, e-mail: log4j-dev-unsubscr...@logging.apache.org For additional commands, e-mail: log4j-dev-h...@logging.apache.org
[jira] [Comment Edited] (LOG4J2-547) Update LoggerStream API
[
https://issues.apache.org/jira/browse/LOG4J2-547?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13917415#comment-13917415
]
Bruce Brouwer edited comment on LOG4J2-547 at 3/2/14 5:01 PM:
--
So, I've been thinking about the actual use case for this feature. I was
thinking it would be even more helpful if I grabbed a writer that actually
wrapped another writer. This way I could "spy" on the writer and send all its
content to a Logger and to the actual writer. I'm thinking of a method
signatures like this:
{code}
public Writer writer(Writer writer, Level level);
public Writer writer(Writer writer, Marker marker, Level level);
public OutputStream stream(OutputStream stream, Level level);
public OutputStream stream(OutputStream stream, Marker marker, Level level);
{code}
We could also provide ones basically like before:
{code}
public Writer writer(Level level);
public Writer writer(Marker marker, Level level);
public OutputStream stream(Level level);
public OutputStream stream(Marker marker, Level level);
{code}
Maybe we would keep the PrintWriter/PrintStream variants as well. But then why
stop there. We could spy on Readers as well:
{code}
public Reader reader(Reader reader, Level level);
public Reader reader(Reader reader, Marker marker, Level level);
public InputStream stream(InputStream stream, Level level);
public InputStream stream(InputStream stream, Marker marker, Level level);
{code}
Making this all work is not really any harder than what is already there.
But now we're starting to really pollute the Logger interface with stuff that
99% of the time I'll never want or use. What if we took it a different
direction and didn't put these reader/writer methods on Logger, but instead put
them on LogManager or even a new class LogStreaming. Now I could get one of
these things with code like this:
{code}
Writer writer = LogStreaming.writer(myWriter, Level.WARNING);
{code}
We could go even further and add debug/warn/info/error methods on here as well:
{code}
Writer writer = LogStreaming.debugWriter(MyExample.class, myWriter);
{code}
This goes along with another idea I had in LOG4J-242 which would make a fluent
interface for log4j. I eventually decided that all that stuff doesn't belong in
Logger either, so instead I would want a FluentLogManager instead of LogManager.
Now back to the whole FQCN thing. If I do the hack I mentioned coupled with
these new methods I described, if I were to pass in a PrintWriter to this
writer method, then using the FQCN of PrintWriter will determine the caller is
the caller of the PrintWriter I passed in, and not the one being created by the
writer method.
Furthermore, the reason I would want one of these Writers is to pass to some
function that expects a Writer, and therefore the call stack will not be from
my code anyway and then the caller location probably won't be as useful to me
as I would want.
So, what are your thoughts on all of these ideas? Am I making this more
complicated than it should be?
was (Author: bruce.brouwer):
So, I've been thinking about the actual use case for this feature. I was
thinking it would be even more helpful if I grabbed a writer that actually
wrapped another writer. This way I could "spy" on the writer and send all its
content to a Logger and to the actual writer. I'm thinking of a method
signatures like this:
{code}
public Writer writer(Writer writer, Level level);
public Writer writer(Writer writer, Marker marker, Level level);
public OutputStream stream(OutputStream stream, Level level);
public OutputStream stream(OutputStream stream, Marker marker, Level level);
{code}
We could also provide ones basically like before:
{code}
public Writer writer(Level level);
public Writer writer(Marker marker, Level level);
public OutputStream stream(Level level);
public OutputStream stream(Marker marker, Level level);
{code}
Maybe we would keep the PrintWriter/PrintStream variants as well. But then why
stop there. We could spy on Readers as well:
{code}
public Reader reader(Reader reader, Level level);
public Reader reader(Reader reader, Marker marker, Level level);
public InputStream stream(InputStream stream, Level level);
public InputStream stream(InputStream stream, Marker marker, Level level);
{code}
Making this all work is not really any harder than what is already there.
But now we're starting to really pollute the Logger interface with stuff that
99% of the time I'll never want or use. What if we took it a different
direction and didn't put these reader/writer methods on Logger, but instead put
them on LogManager or even a new class LogStreaming. Now I could get one of
these things with code like this:
{code}
Writer writer = LogStreaming.writer(myWriter, Level.WARNING);
{code}
We could go even further and add debug/warn/info/error methods on here as well:
{code}
Writer writer = LogStreaming.debugWriter
[jira] [Comment Edited] (LOG4J2-547) Update LoggerStream API
[
https://issues.apache.org/jira/browse/LOG4J2-547?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13917415#comment-13917415
]
Bruce Brouwer edited comment on LOG4J2-547 at 3/2/14 5:02 PM:
--
So, I've been thinking about the actual use case for this feature. I was
thinking it would be even more helpful if I grabbed a writer that actually
wrapped another writer. This way I could "spy" on the writer and send all its
content to a Logger and to the actual writer. I'm thinking of a method
signatures like this:
{code}
public Writer writer(Writer writer, Level level);
public Writer writer(Writer writer, Marker marker, Level level);
public OutputStream stream(OutputStream stream, Level level);
public OutputStream stream(OutputStream stream, Marker marker, Level level);
{code}
We could also provide ones basically like before:
{code}
public Writer writer(Level level);
public Writer writer(Marker marker, Level level);
public OutputStream stream(Level level);
public OutputStream stream(Marker marker, Level level);
{code}
Maybe we would keep the PrintWriter/PrintStream variants as well. But then why
stop there. We could spy on Readers as well:
{code}
public Reader reader(Reader reader, Level level);
public Reader reader(Reader reader, Marker marker, Level level);
public InputStream stream(InputStream stream, Level level);
public InputStream stream(InputStream stream, Marker marker, Level level);
{code}
Making this all work is not really any harder than what is already there.
But now we're starting to really pollute the Logger interface with stuff that
99% of the time I'll never want or use. What if we took it a different
direction and didn't put these reader/writer methods on Logger, but instead put
them on LogManager or even a new class LogStreaming. Now I could get one of
these things with code like this:
{code}
Writer writer = LogStreaming.writer(MyExample.class, myWriter, Level.WARNING);
{code}
We could go even further and add debug/warn/info/error methods on here as well:
{code}
Writer writer = LogStreaming.debugWriter(MyExample.class, myWriter);
{code}
This goes along with another idea I had in LOG4J-242 which would make a fluent
interface for log4j. I eventually decided that all that stuff doesn't belong in
Logger either, so instead I would want a FluentLogManager instead of LogManager.
Now back to the whole FQCN thing. If I do the hack I mentioned coupled with
these new methods I described, if I were to pass in a PrintWriter to this
writer method, then using the FQCN of PrintWriter will determine the caller is
the caller of the PrintWriter I passed in, and not the one being created by the
writer method.
Furthermore, the reason I would want one of these Writers is to pass to some
function that expects a Writer, and therefore the call stack will not be from
my code anyway and then the caller location probably won't be as useful to me
as I would want.
So, what are your thoughts on all of these ideas? Am I making this more
complicated than it should be?
was (Author: bruce.brouwer):
So, I've been thinking about the actual use case for this feature. I was
thinking it would be even more helpful if I grabbed a writer that actually
wrapped another writer. This way I could "spy" on the writer and send all its
content to a Logger and to the actual writer. I'm thinking of a method
signatures like this:
{code}
public Writer writer(Writer writer, Level level);
public Writer writer(Writer writer, Marker marker, Level level);
public OutputStream stream(OutputStream stream, Level level);
public OutputStream stream(OutputStream stream, Marker marker, Level level);
{code}
We could also provide ones basically like before:
{code}
public Writer writer(Level level);
public Writer writer(Marker marker, Level level);
public OutputStream stream(Level level);
public OutputStream stream(Marker marker, Level level);
{code}
Maybe we would keep the PrintWriter/PrintStream variants as well. But then why
stop there. We could spy on Readers as well:
{code}
public Reader reader(Reader reader, Level level);
public Reader reader(Reader reader, Marker marker, Level level);
public InputStream stream(InputStream stream, Level level);
public InputStream stream(InputStream stream, Marker marker, Level level);
{code}
Making this all work is not really any harder than what is already there.
But now we're starting to really pollute the Logger interface with stuff that
99% of the time I'll never want or use. What if we took it a different
direction and didn't put these reader/writer methods on Logger, but instead put
them on LogManager or even a new class LogStreaming. Now I could get one of
these things with code like this:
{code}
Writer writer = LogStreaming.writer(myWriter, Level.WARNING);
{code}
We could go even further and add debug/warn/info/error methods on here as well:
{code}
Writer writer = LogStre
[jira] [Comment Edited] (LOG4J2-547) Update LoggerStream API
[ https://issues.apache.org/jira/browse/LOG4J2-547?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13917268#comment-13917268 ] Bruce Brouwer edited comment on LOG4J2-547 at 3/2/14 2:15 AM: -- Thanks, I'll use those unit tests. Here's what I was thinking and a quick spike proved it works. If I pass the FQCN to the LoggerWriter and make the FQCN=java.io.PrintWriter, I get the correct call information. I don't have to decorate any classes. But is that too hacky to give PrintWriter as the FQCN to the LoggerWriter? Also, I didn't realize initially that you were using PrintStream, not PrintWriter. In some ways, I prefer your idea of using PrintStream. It is easier to turn a PrintStream into a PrintWriter than the other way around. Or do you think there is value in keeping both? was (Author: bruce.brouwer): Thanks, I'll use those unit tests. Here's what I was thinking and a quick spike proved it works. If I pass the FQCN to the LoggerWriter and make the FQCN=java.io.PrintWriter, I get the correct call information. I don't have to decorate any classes. But is that too hacky to give PrintWriter as the FQCN to the LoggerWriter? Also, I didn't realize initially that you were using PrintStream, not PrintWriter. In some ways, I prefer your idea of using PrintWriter. It is easier to turn a PrintStream into a PrintWriter than the other way around. Or do you think there is value in keeping both? > Update LoggerStream API > --- > > Key: LOG4J2-547 > URL: https://issues.apache.org/jira/browse/LOG4J2-547 > Project: Log4j 2 > Issue Type: Improvement > Components: API >Affects Versions: 2.0-rc1 >Reporter: Matt Sicker >Assignee: Ralph Goers > Fix For: 2.0 > > Attachments: 0001-PrintStream-API-update.patch, > Add_caller_info_tests.patch, log4j2-loggerStream.patch > > > I've got some ideas on how to improve the LoggerStream idea that I added a > little while ago. The main thing I'd like to do is extract an interface from > it, rename the default implementation to SimpleLoggerStream (part of the > SimpleLogger stuff), and allow log4j implementations to specify a different > implementation if desired. > In doing this, I'm not sure where specifically I'd prefer the getStream > methods to be. Right now, it's in Logger, but really, it could be in > LoggerContext instead. I don't think I should be required to get a Logger > just to get a LoggerStream. > Now if only the java.io package used interfaces instead of classes. This > would be so much easier to design! -- This message was sent by Atlassian JIRA (v6.1.5#6160) - To unsubscribe, e-mail: log4j-dev-unsubscr...@logging.apache.org For additional commands, e-mail: log4j-dev-h...@logging.apache.org
[jira] [Comment Edited] (LOG4J2-547) Update LoggerStream API
[ https://issues.apache.org/jira/browse/LOG4J2-547?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13917159#comment-13917159 ] Bruce Brouwer edited comment on LOG4J2-547 at 3/1/14 7:04 PM: -- In looking at LoggerStream, I can see how it could help me when shelling out to commands when all I want to do is log the output of that shelled command. There is some non-trivial stuff that LoggerStream is doing, so I don't want to get rid of that concept. But I agree that as it stands, the most helpful part of LoggerStream is actually the HelperStream. I'm including a patch that gets rid of LoggerStream and instead returns a plain old PrintWriter. This involved making a LoggerWriter which does basically what HelperStream did before. I don't see a whole lot of value adding this to the LoggerContext. In the end it needs an AbstractLogger anyway, so why not get it from the AbstractLogger. Oh, and I called the method .printWriter(...) instead of .getStream(...). First, the old .getStream didn't even return an OutputStream, it returned something that extended PrintWriter. And also, by removing the .get part, users might not expect to get the exact same PrintWriter instance each time they call it. In this case, I don't think I would want to get the same instance each time. was (Author: bruce.brouwer): In looking at LoggerStream, I can see how it could help me when shelling out to commands when all I want to do is log the output of that shelled command. There is some non-trivial stuff that LoggerStream is doing, so I don't want to get rid of that concept. But I agree that as it stands, the most helpful part of LoggerStream is actually the HelperStream. I'm including a patch that gets rid of LoggerStream and instead returns a plain old PrintWriter. This involved making a LoggerWriter which does basically what HelperStream did before. I don't see a whole lot of value adding this to the LoggerContext. In the end it needs an AbstractLogger anyway, so why not get it from the AbstractLogger. > Update LoggerStream API > --- > > Key: LOG4J2-547 > URL: https://issues.apache.org/jira/browse/LOG4J2-547 > Project: Log4j 2 > Issue Type: Improvement > Components: API >Affects Versions: 2.0-rc1 >Reporter: Matt Sicker > Fix For: 2.0 > > Attachments: 0001-PrintStream-API-update.patch, > log4j2-loggerStream.patch > > > I've got some ideas on how to improve the LoggerStream idea that I added a > little while ago. The main thing I'd like to do is extract an interface from > it, rename the default implementation to SimpleLoggerStream (part of the > SimpleLogger stuff), and allow log4j implementations to specify a different > implementation if desired. > In doing this, I'm not sure where specifically I'd prefer the getStream > methods to be. Right now, it's in Logger, but really, it could be in > LoggerContext instead. I don't think I should be required to get a Logger > just to get a LoggerStream. > Now if only the java.io package used interfaces instead of classes. This > would be so much easier to design! -- This message was sent by Atlassian JIRA (v6.1.5#6160) - To unsubscribe, e-mail: log4j-dev-unsubscr...@logging.apache.org For additional commands, e-mail: log4j-dev-h...@logging.apache.org
