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 <shawj...@gmail.com> 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 <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> >>>> >> >> >>
