On Fri, Feb 23, 2018 at 7:02 PM, Jeffrey Shaw <[email protected]> wrote:
> Hi All, > I spent some more time on this tonight. I've brought my code up to date for > log4j versions 2 and 3. version 2 > <https://github.com/shawjef3/logging-log4j2/tree/message- > location-release-2.x> > version > 3 <https://github.com/shawjef3/logging-log4j2/tree/message-location> > Per Ralph Goers and some others I removed SourceLocation and reverted back > to StackTraceElement. > Hello, Can you create pull requests so we can see better what you are proposing? Thank you! Gary > > In case this thread is lost in your email, the changes I worked on expose > the location part of the Message to loggers, so that if the source location > is known at compile time, it can be added to the log with no runtime cost. > An example use is the scala loggers > <https://github.com/shawjef3/logging-log4j-scala> which are implemented > with macros, and therefore have that information available at no cost to > the developer, either. > > On Sat, Dec 23, 2017 at 2:14 PM, Mikael Ståldal <[email protected]> wrote: > > > Yes, StackTraceElement does not allow null methodName. > > > > But what about using the empty string, or a string like "<unknown>" if it > > is not possible to get? > > > > > > On 2017-12-22 20:26, Jeffrey Shaw wrote: > > > >> I didn't know about clirr, but I can try running that. I tried to not > >> break > >> any existing public methods. > >> > >> I created SourceLocation because I think for Scala macros, the method > name > >> can not exist, but StackTraceElement requires a method name. I'll double > >> check this. > >> > >> On Thu, Dec 21, 2017 at 12:40 AM, Ralph Goers < > [email protected] > >> > > >> wrote: > >> > >> I’m not sure I would be comfortable applying the patch this way. Have > you > >>> run a clirr report on your changes? I am concerned that this could > break > >>> customizations that users might have made. What is the reason > >>> SourceLocation had to be used instead of StackTraceElement? > >>> > >>> Ralph > >>> > >>> On Dec 20, 2017, at 9:58 PM, Jeffrey Shaw <[email protected]> wrote: > >>>> > >>>> It looks like the location only ever gets the file name, because > that's > >>>> what StackTraceElement gives. So that's fine. > >>>> > >>>> I think I'm ready for a formal review. Should I create pull requests > on > >>>> github for https://github.com/shawjef3/logging-log4j2/tree/message- > >>>> > >>> location > >>> > >>>> and https://github.com/shawjef3/logging-log4j-scala/tree/ > >>>> > >>> message-location, > >>> > >>>> or is there another process? > >>>> > >>>> > >>>> On Wed, Dec 20, 2017 at 2:11 PM, Matt Sicker <[email protected]> > wrote: > >>>> > >>>> I think that should be configurable in the layout options. > >>>>> > >>>>> On 20 December 2017 at 13:04, Dominik Psenner <[email protected]> > >>>>> > >>>> wrote: > >>> > >>>> > >>>>> Could a compile time environment variable like SrcRootDirectory do > the > >>>>>> > >>>>> job? > >>>>> > >>>>>> > >>>>>> On 20 Dec 2017 7:49 p.m., "Jeffrey Shaw" <[email protected]> > wrote: > >>>>>> > >>>>>> I got it working using a custom ExtendedLogger instead of mocking. > >>>>>>> > >>>>>>> It looks like for file name, there are only two options. We can > have > >>>>>>> > >>>>>> the > >>>>> > >>>>>> file name, or the full path to the file. The path relative to the > >>>>>>> > >>>>>> project > >>>>> > >>>>>> root looks impossible to get, unless we look for magic names like > >>>>>>> > >>>>>> "src". > >>>>> > >>>>>> Any opinion as to which to use? I'm tempted to use just the file > name, > >>>>>>> since the full path to the file seems intrusive, and is dependent > on > >>>>>>> > >>>>>> the > >>>>> > >>>>>> build machine. > >>>>>>> > >>>>>>> On Wed, Dec 20, 2017 at 11:35 AM, Matt Sicker <[email protected]> > >>>>>>> > >>>>>> wrote: > >>>>> > >>>>>> > >>>>>>> It's possible that macros and mocks don't work well together, > though > >>>>>>>> > >>>>>>> that's > >>>>>>> > >>>>>>>> just a guess. > >>>>>>>> > >>>>>>>> On 20 December 2017 at 00:00, Jeff Shaw <[email protected]> > wrote: > >>>>>>>> > >>>>>>>> I should add that manually testing it works. > >>>>>>>>> > >>>>>>>>> Sent from my phone > >>>>>>>>> > >>>>>>>>> On Dec 20, 2017, at 12:45 AM, Jeffrey Shaw <[email protected]> > >>>>>>>>>> > >>>>>>>>> wrote: > >>>>>>> > >>>>>>>> > >>>>>>>>>> I added some tests for traced, but they don't pass. The mocks > >>>>>>>>>> > >>>>>>>>> say, > >>>>> > >>>>>> "Actually, there were zero interactions with this mock." I could > >>>>>>>>> > >>>>>>>> use > >>>>> > >>>>>> some > >>>>>>> > >>>>>>>> help getting these two tests to work. > >>>>>>>>> > >>>>>>>>>> > >>>>>>>>>> https://github.com/shawjef3/logging-log4j-scala/blob/ > >>>>>>>>>> > >>>>>>>>> message-location/log4j-api-scala_2.12/src/test/scala/org/ > >>>>>>>>> apache/logging/log4j/scala/LoggerTest.scala#L574 > >>>>>>>>> > >>>>>>>>>> > >>>>>>>>>> On Sun, Dec 17, 2017 at 8:50 PM, Jeffrey Shaw < > >>>>>>>>>>> > >>>>>>>>>> [email protected] > >>>>> > >>>>>> > >>>>>>> wrote: > >>>>>>>>> > >>>>>>>>>> Thanks for the encouragement everyone! I never worked on an > >>>>>>>>>>> > >>>>>>>>>> Apache > >>>>> > >>>>>> project before and had no idea what to expect from the community. > >>>>>>>>> > >>>>>>>>>> > >>>>>>>>>>> I've made some progress. One cool thing I added was a `traced` > >>>>>>>>>>> > >>>>>>>>>> method > >>>>>>> > >>>>>>>> (source), which does the work you'd want for traceEntry, > traceExit, > >>>>>>>>> > >>>>>>>> and > >>>>>> > >>>>>>> throwing. It would be cool to add catching as well, but that would > >>>>>>>>> > >>>>>>>> require > >>>>>>>> > >>>>>>>>> tree traversal, which is beyond me at the moment. I also haven't > >>>>>>>>> > >>>>>>>> figured > >>>>>>> > >>>>>>>> out how to add the parameter lists. Anyway, an example: > >>>>>>>>> > >>>>>>>>>> > >>>>>>>>>>> before: > >>>>>>>>>>> def f() = {...} > >>>>>>>>>>> > >>>>>>>>>>> after: > >>>>>>>>>>> def f() = logger.traced(Level.INFO) {...} > >>>>>>>>>>> > >>>>>>>>>>> On Mon, Dec 11, 2017 at 9:55 PM, Matt Sicker <[email protected] > >>>>>>>>>>>> > >>>>>>>>>>> > >>>>>> wrote: > >>>>>>>> > >>>>>>>>> From the little I've worked with macros (worked more with > >>>>>>>>>>>> > >>>>>>>>>>> scalameta > >>>>>> > >>>>>>> and > >>>>>>>> > >>>>>>>>> shapeless), that looks good so far. If you can add some unit > >>>>>>>>>>>> > >>>>>>>>>>> tests, > >>>>>> > >>>>>>> then > >>>>>>>>> > >>>>>>>>>> we'd be happy to merge! > >>>>>>>>>>>> > >>>>>>>>>>>> On 11 December 2017 at 20:41, Jeffrey Shaw < > [email protected] > >>>>>>>>>>>> > >>>>>>>>>>> > >>>>>> wrote: > >>>>>>>> > >>>>>>>>> > >>>>>>>>>>>> Great news! I'm able to run LoggingApp in the scala api repo > >>>>>>>>>>>>> > >>>>>>>>>>>> without > >>>>>>>> > >>>>>>>>> it > >>>>>>>>> > >>>>>>>>>> calling StackLocatorUtil.calcLocation, but it prints the same > >>>>>>>>>>>>> > >>>>>>>>>>>> messages as > >>>>>>>>> > >>>>>>>>>> before. I have to use my patch to log4j of course. > >>>>>>>>>>>>> > >>>>>>>>>>>>> See https://github.com/shawjef3/logging-log4j-scala/commits/ > >>>>>>>>>>>>> message-location > >>>>>>>>>>>>> > >>>>>>>>>>>>> On Sun, Dec 10, 2017 at 3:56 PM, Matt Sicker < > >>>>>>>>>>>>> > >>>>>>>>>>>> [email protected] > >>>>> > >>>>>> > >>>>>>> wrote: > >>>>>>>>> > >>>>>>>>>> > >>>>>>>>>>>>> This sounds like it'd make a great addition to the Scala > >>>>>>>>>>>>>> > >>>>>>>>>>>>> API! > >>>>> > >>>>>> > >>>>>>>>>>>>>> On 9 December 2017 at 15:36, Jeffrey Shaw < > >>>>>>>>>>>>>> > >>>>>>>>>>>>> [email protected]> > >>>>>> > >>>>>>> wrote: > >>>>>>>>> > >>>>>>>>>> > >>>>>>>>>>>>>> Ralph, I agree with you entirely. My intent for these new > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>> log > >>>>>> > >>>>>>> methods > >>>>>>>>> > >>>>>>>>>> is > >>>>>>>>>>>>> > >>>>>>>>>>>>>> that they only be called from compile-time generated > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>> code. > >>>>> > >>>>>> > >>>>>>>>>>>>>>> On Sat, Dec 9, 2017 at 4:30 PM, Ralph Goers < > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>> [email protected]> > >>>>>>>>>>>>> > >>>>>>>>>>>>>> wrote: > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> I don’t understand how this is a good idea. To use this > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> you > >>>>>> > >>>>>>> would > >>>>>>>>> > >>>>>>>>>> need > >>>>>>>>>>>>> > >>>>>>>>>>>>>> to > >>>>>>>>>>>>>> > >>>>>>>>>>>>>>> do something like: > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> Message msg = new StringMessage(getCaller(), “My > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Message”); > >>>>>> > >>>>>>> logger.debug(msg); > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> Unfortunately the line number would point to the line > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> where > >>>>>> > >>>>>>> getCaller() > >>>>>>>>>>>>> > >>>>>>>>>>>>>> is > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> called not to the logger statement. > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> I had thought about modifying AbstractLogger to do > >>>>>>>>>>>>>>>> public void debug(final Marker marker, final String > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> message) > >>>>>>> > >>>>>>>> { > >>>>>>>> > >>>>>>>>> logIfEnabled(getCaller(), Level.DEBUG, marker, > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> message, > >>>>>> > >>>>>>> (Throwable) > >>>>>>>>>>>>> > >>>>>>>>>>>>>> null); > >>>>>>>>>>>>>>>> } > >>>>>>>>>>>>>>>> instead of the current > >>>>>>>>>>>>>>>> public void debug(final Marker marker, final String > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> message) > >>>>>>> > >>>>>>>> { > >>>>>>>> > >>>>>>>>> logIfEnabled(FQCN, Level.DEBUG, marker, message, > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> (Throwable) > >>>>>>>>> > >>>>>>>>>> null); > >>>>>>>>>>>>> > >>>>>>>>>>>>>> } > >>>>>>>>>>>>>>>> But the amount of changes required to get it into the > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> LogEvent > >>>>>>>> > >>>>>>>>> was > >>>>>>>>> > >>>>>>>>>> large. > >>>>>>>>>>>>>> > >>>>>>>>>>>>>>> OTOH, if we create a CallerLocationMessage that > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> contains > >>>>> > >>>>>> the > >>>>>>> > >>>>>>>> StackTraceElement and then have existing Messages > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> extend > >>>>> > >>>>>> that > >>>>>>> > >>>>>>>> then we > >>>>>>>>> > >>>>>>>>>> could > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> store the location in the Message if it is a > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> CallerLocationMessage. > >>>>>>>>> > >>>>>>>>>> Calling > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> getCaller() in this way would be much better since it > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> is > >>>>> > >>>>>> at a > >>>>>>> > >>>>>>>> fixed > >>>>>>>>> > >>>>>>>>>> depth > >>>>>>>>>>>>>> > >>>>>>>>>>>>>>> from the caller. > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> With Java 9 this could become: > >>>>>>>>>>>>>>>> public void debug(final Marker marker, final String > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> message) > >>>>>>> > >>>>>>>> { > >>>>>>>> > >>>>>>>>> logIfEnabled(stackWalker.walk( > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> s->s.skip(1).findFirst(), > >>>>>>> > >>>>>>>> Level.DEBUG, > >>>>>>>>>>>>>> > >>>>>>>>>>>>>>> marker, message, (Throwable) null); > >>>>>>>>>>>>>>>> } > >>>>>>>>>>>>>>>> although that would pass a StackFrame instead of a > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> StackTraceElement. > >>>>>>>>> > >>>>>>>>>> The > >>>>>>>>>>>>>> > >>>>>>>>>>>>>>> only problems with this is that there is still some > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> overhead > >>>>>>> > >>>>>>>> in > >>>>>>>> > >>>>>>>>> calling > >>>>>>>>>>>>> > >>>>>>>>>>>>>> StackWalker like this. Also, if this is called from a > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> facade, > >>>>>>> > >>>>>>>> such as > >>>>>>>>> > >>>>>>>>>> log4j-slf4j-impl then the number of levels that have to > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> be > >>>>>> > >>>>>>> skipped > >>>>>>>>> > >>>>>>>>>> would > >>>>>>>>>>>>>> > >>>>>>>>>>>>>>> be > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> different. > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> I would really prefer if there was some way to capture > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> the > >>>>>> > >>>>>>> line > >>>>>>>> > >>>>>>>>> number > >>>>>>>>>>>>> > >>>>>>>>>>>>>> information for the various loggers when the annotation > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> processor > >>>>>>>>> > >>>>>>>>>> runs > >>>>>>>>>>>>> > >>>>>>>>>>>>>> at > >>>>>>>>>>>>>> > >>>>>>>>>>>>>>> compile time. > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> Ralph > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> On Dec 9, 2017, at 1:22 PM, Jeffrey Shaw < > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> [email protected] > >>>>>>>> > >>>>>>>>> > >>>>>>>>>> wrote: > >>>>>>>>>>>>> > >>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> Thanks for the link, Mikael. I'll take a look at it. > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> I added some plumbing to core to allow clients to > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> pass > >>>>> > >>>>>> a > >>>>>> > >>>>>>> StackTraceElement > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> to loggers. I'd like a code review. I'm happy to try > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> other > >>>>>>> > >>>>>>>> methods. > >>>>>>>>> > >>>>>>>>>> See > >>>>>>>>>>>>>> > >>>>>>>>>>>>>>> the > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> following commit. > >>>>>>>>>>>>>>>>> https://github.com/shawjef3/logging-log4j2/commit/ > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> 9c42073e9ca4f25a2f4075b1791eee2893534c54 > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> On Sat, Dec 9, 2017 at 10:09 AM, Mikael Ståldal < > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> [email protected]> > >>>>>>>>> > >>>>>>>>>> wrote: > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> Have you tried the Log4j Scala API? > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> http://logging.apache.org/ > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> log4j/2.x/manual/scala-api. > >>>>> > >>>>>> html > >>>>>>> > >>>>>>>> > >>>>>>>>>>>>>>>>>> It does currently not support this, but it uses > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> Scala > >>>>> > >>>>>> macros, and > >>>>>>>>> > >>>>>>>>>> this > >>>>>>>>>>>>>> > >>>>>>>>>>>>>>> could be added there. But log4j-api and/or > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> log4j-core > >>>>> > >>>>>> probably > >>>>>>>>> > >>>>>>>>>> needs > >>>>>>>>>>>>> > >>>>>>>>>>>>>> to > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> adapted as well. > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> On 2017-12-09 07:30, Jeffrey Shaw wrote: > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> Hello, > >>>>>>>>>>>>>>>>>>> I've found that I am able to use Scala macros to > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> provide > >>>>>>> > >>>>>>>> compile-time > >>>>>>>>>>>>>> > >>>>>>>>>>>>>>> source information for log messages. However, I > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> don't > >>>>> > >>>>>> see > >>>>>>> > >>>>>>>> a way > >>>>>>>>> > >>>>>>>>>> to > >>>>>>>>>>>>> > >>>>>>>>>>>>>> inject > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> this into log4j's logging mechanism. > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> I'm wondering if there is something I'm missing, or > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> if > >>>>>> > >>>>>>> LogEvent's > >>>>>>>>> > >>>>>>>>>> getSource > >>>>>>>>>>>>>>>>>>> method could be duplicated in Message. > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> We could then have zero-overhead location > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> information > >>>>> > >>>>>> in > >>>>>>> > >>>>>>>> logs. > >>>>>>>>> > >>>>>>>>>> I'm > >>>>>>>>>>>>> > >>>>>>>>>>>>>> thinking > >>>>>>>>>>>>>>>>>>> that tools other than Scala could also take > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> advantage > >>>>> > >>>>>> of > >>>>>>> > >>>>>>>> this. > >>>>>>>>> > >>>>>>>>>> > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> -- > >>>>>>>>>>>>>> Matt Sicker <[email protected]> > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>>> -- > >>>>>>>>>>>> Matt Sicker <[email protected]> > >>>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>> > >>>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>>>> -- > >>>>>>>> Matt Sicker <[email protected]> > >>>>>>>> > >>>>>>>> > >>>>>>> > >>>>>> > >>>>> > >>>>> > >>>>> -- > >>>>> Matt Sicker <[email protected]> > >>>>> > >>>>> > >>> > >>> > >>> > >> > > >
