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]> >
