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