Is there no mangled method name generated? For example, inside a lambda, the function might be called Foo$1 like with Java 8 lambdas.
On 22 December 2017 at 14:18, Ralph Goers <[email protected]> wrote: > I am concerned because users can create customer Appenders and Layouts > that will be affected by a change to the signature of getSource() in the > LogEvent. I suspect that there are other things that use the LogEvent (the > Logging Server? Lillith?) that would be impacted by this. > > Ralph > > > On Dec 22, 2017, at 12:26 PM, Jeffrey Shaw <[email protected]> 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]> > >>>> > >> > >> > >> > > > -- Matt Sicker <[email protected]>
