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 <ralph.go...@dslextreme.com> 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 <shawj...@gmail.com> 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 <boa...@gmail.com> wrote: > > > >> I think that should be configurable in the layout options. > >> > >> On 20 December 2017 at 13:04, Dominik Psenner <dpsen...@gmail.com> > wrote: > >> > >>> Could a compile time environment variable like SrcRootDirectory do the > >> job? > >>> > >>> On 20 Dec 2017 7:49 p.m., "Jeffrey Shaw" <shawj...@gmail.com> 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 <boa...@gmail.com> > >> 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 <shawj...@gmail.com> wrote: > >>>>> > >>>>>> I should add that manually testing it works. > >>>>>> > >>>>>> Sent from my phone > >>>>>> > >>>>>>> On Dec 20, 2017, at 12:45 AM, Jeffrey Shaw <shawj...@gmail.com> > >>>> 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 < > >> shawj...@gmail.com > >>>> > >>>>>> 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 <boa...@gmail.com > >>> > >>>>> 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 <shawj...@gmail.com > >>> > >>>>> 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 < > >> boa...@gmail.com > >>>> > >>>>>> wrote: > >>>>>>>>>> > >>>>>>>>>>> This sounds like it'd make a great addition to the Scala > >> API! > >>>>>>>>>>> > >>>>>>>>>>> On 9 December 2017 at 15:36, Jeffrey Shaw < > >>> shawj...@gmail.com> > >>>>>> 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 < > >>>>>>>>>> ralph.go...@dslextreme.com> > >>>>>>>>>>>> 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 < > >>>>> shawj...@gmail.com > >>>>>>> > >>>>>>>>>> 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 < > >>>>>> mi...@apache.org> > >>>>>>>>>>>>> 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 <boa...@gmail.com> > >>>>>>>>>>> > >>>>>>>>>> > >>>>>>>>> > >>>>>>>>> > >>>>>>>>> > >>>>>>>>> -- > >>>>>>>>> Matt Sicker <boa...@gmail.com> > >>>>>>>> > >>>>>>> > >>>>>> > >>>>> > >>>>> > >>>>> > >>>>> -- > >>>>> Matt Sicker <boa...@gmail.com> > >>>>> > >>>> > >>> > >> > >> > >> > >> -- > >> Matt Sicker <boa...@gmail.com> > >> > > >
