You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by GitBox <gi...@apache.org> on 2020/06/27 18:27:44 UTC
[GitHub] [logging-log4j2] shawjef3 opened a new pull request #366: Allow clients to set the source manually.
shawjef3 opened a new pull request #366:
URL: https://github.com/apache/logging-log4j2/pull/366
The purpose of this is to make it possible to improve performance. Getting the StackTraceElement for a message is somewhat expensive. Creating it manually is tedious and no one should do it, but the Scala plugin can provide the source location at compile time. Possibly other tools could do the same thing. This enables that to be possible.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] garydgregory commented on pull request #366: Allow clients to set the source manually.
Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #366:
URL: https://github.com/apache/logging-log4j2/pull/366#issuecomment-650743832
Yeah, I do not have a use case for that, I just wanted to mention it for
completeness sake as well as guidance for contributing: don't break binary
compatibility of the public API, also small PRs are better than larger ones.
Gary
On Sat, Jun 27, 2020, 20:11 Ralph Goers <no...@github.com> wrote:
> That is true, but it seems odd to add the StackTraceElement to things like
> traceEntry & traceExit and then ALSO add them to Messages. The location
> information isn't logically part of a Message.
>
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/logging-log4j2/pull/366#issuecomment-650652949>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAJB6N25WAKKFSGK35IHB43RY2DCDANCNFSM4OKEQ7KA>
> .
>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] shawjef3 closed pull request #366: Allow clients to set the source manually.
Posted by GitBox <gi...@apache.org>.
shawjef3 closed pull request #366:
URL: https://github.com/apache/logging-log4j2/pull/366
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] garydgregory commented on pull request #366: Allow clients to set the source manually.
Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #366:
URL: https://github.com/apache/logging-log4j2/pull/366#issuecomment-650650605
The breaking could be less using default methods FWIW...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] shawjef3 commented on pull request #366: Allow clients to set the source manually.
Posted by GitBox <gi...@apache.org>.
shawjef3 commented on pull request #366:
URL: https://github.com/apache/logging-log4j2/pull/366#issuecomment-667584092
@rgoers thanks for the pointer to `Logger#atLevel`. It looks like that will work for most of it, if not all. I'll have to copy implementations of parts of AbstractLogger, like `throwing`, since I won't be able to use the public method, but it'll work.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] rgoers commented on pull request #366: Allow clients to set the source manually.
Posted by GitBox <gi...@apache.org>.
rgoers commented on pull request #366:
URL: https://github.com/apache/logging-log4j2/pull/366#issuecomment-650652949
That is true, but it seems odd to add the StackTraceElement to things like traceEntry & traceExit and then ALSO add them to Messages. The location information isn't logically part of a Message.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] rgoers commented on pull request #366: Allow clients to set the source manually.
Posted by GitBox <gi...@apache.org>.
rgoers commented on pull request #366:
URL: https://github.com/apache/logging-log4j2/pull/366#issuecomment-650650281
This change is modifying the signatures of virtually all the Messages, which will break some existing users. What's more, clients who want to set the source manually already can. Just use one of Logger's "at" methods to get a LogBuilder and then use withLocation(stackTraceElement).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] rgoers edited a comment on pull request #366: Allow clients to set the source manually.
Posted by GitBox <gi...@apache.org>.
rgoers edited a comment on pull request #366:
URL: https://github.com/apache/logging-log4j2/pull/366#issuecomment-650652949
That is true, but it seems odd to add the StackTraceElement to things like traceEntry & traceExit and then ALSO add them to Messages. The location information isn't logically part of a Message. What would it mean to use LogBuilder that specifies withLocation(element) and also includes a StackTraceElement in the Message, especially if they aren't the same?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] rgoers commented on pull request #366: Allow clients to set the source manually.
Posted by GitBox <gi...@apache.org>.
rgoers commented on pull request #366:
URL: https://github.com/apache/logging-log4j2/pull/366#issuecomment-664793002
@shawjef3 Based on our comments, what do you want to do with this PR?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org