Re: [PR] Bump org.springframework:spring-framework-bom from 6.1.0 to 6.1.1 [logging-log4j-jakarta]
github-actions[bot] closed pull request #5: Bump org.springframework:spring-framework-bom from 6.1.0 to 6.1.1 URL: https://github.com/apache/logging-log4j-jakarta/pull/5 -- 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. To unsubscribe, e-mail: dev-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Bump org.springframework:spring-framework-bom from 6.1.0 to 6.1.1 [logging-log4j-jakarta]
github-actions[bot] commented on PR #5: URL: https://github.com/apache/logging-log4j-jakarta/pull/5#issuecomment-1824937518 Changes are applied by CI in 14a91d49c6fe432e9df0e0b8429d01dbf036bf13 -- 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. To unsubscribe, e-mail: dev-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Bump org.springframework:spring-framework-bom from 6.1.0 to 6.1.1 [logging-log4j-jakarta]
ppkarwasz commented on PR #5: URL: https://github.com/apache/logging-log4j-jakarta/pull/5#issuecomment-1824930468 @dependabot rebase -- 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. To unsubscribe, e-mail: dev-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: Change semantics of `Throwable` parameters
I agree with everything Remko said plus these extra comments.
Throwing is a convenience method to standardize the output when an application
will be throwing an exception. That, of course, means it isn’t a required
method. I doubt it is used very much. Of course, catching is its companion but
I would bet it is used even less.
We use the entry/exit stuff everywhere. I have not seen a replacement for it
that provides the same functionality so I would not want it removed.
I agree that AbstractLogger should have as few methods as possible that need to
be overridden by the implementation.
Ralph
> On Nov 23, 2023, at 4:31 AM, Piotr P. Karwasz wrote:
> Hi Matt,
>
> On Tue, 21 Nov 2023 at 23:22, Matt Sicker wrote:
>>
>> This sounds like it might be a good basis for figuring out a parallel v3 API
>> for a “hard to mis-use” style API. However, once you go that route, you
>> start to wonder how useful templated log messages are when you can capture a
>> lambda instead. Parameterized log messages might work better as structured
>> log messages, something that is awkward to use in the API at the moment.
>
> If we'll create a separate `v3.Logger` interface I would clean it up
> from many methods, e.g.:
>
> * getLevel() and getName(): how are these useful for the user? An `if
> (logger.getLevel() == Level.INFO)` should be replaced by `isEnabled`,
> * getMessageFactory() and getFlowMessageFactory() (the latter is my
> fault): again these are not useful to the user. If I need a message
> factory, it will be a different message factory,
> * printf(): a better approach is to use StringFormatterMessageFactory
> for the logger,
> * catching(Throwable): can be replaced with `error(Object)` and the
> semantics described in this thread,
> * throwing: does anybody use it? Maybe it could stay,
> * entry/exit, traceEntry/traceExit: I can not imagine using these on
> each method (or important method). I'd rather use AspectJ pointcuts
> instead (or a @LogTrace annotation),
> * methods that use `MessageSupplier` like `info(MessageSupplier)`:
> couldn't these be integrated into the logic of `info(Supplier)`?
> * the `is*Enabled` methods are prone to misuse: a snippet like:
>
> if (logger.isDebugEnabled()) {
> logger.debug(MARKER, "Hello world!");
> }
>
> will not print any message if the level of the logger is less specific
> than DEBUG, even if the user asks for **all** MARKER messages to be
> printed.
>
> IMHO opinion `v3.AbstractLogger` should only have 2 abstract methods:
> * logMessage(Level level, Marker marker, String fqcn,
> StackTraceElement location, Message message, Throwable throwable)
> * isEnabled(Level level, Marker marker, String fqcn,
> StackTraceElement location, Message message, Throwable throwable)
>
> Piotr
Re: Change semantics of `Throwable` parameters
> On Nov 23, 2023, at 18:31, Piotr P. Karwasz wrote:
>
> Hi Matt,
>
>> On Tue, 21 Nov 2023 at 23:22, Matt Sicker wrote:
>>
>> This sounds like it might be a good basis for figuring out a parallel v3 API
>> for a “hard to mis-use” style API. However, once you go that route, you
>> start to wonder how useful templated log messages are when you can capture a
>> lambda instead. Parameterized log messages might work better as structured
>> log messages, something that is awkward to use in the API at the moment.
>
> If we'll create a separate `v3.Logger` interface I would clean it up
> from many methods, e.g.:
>
> * getLevel() and getName(): how are these useful for the user? An `if
> (logger.getLevel() == Level.INFO)` should be replaced by `isEnabled`,
Sorry but I disagree.
The getLevel method is useful when the level to compare to is not known in
advance.
> * getMessageFactory() and getFlowMessageFactory() (the latter is my
> fault): again these are not useful to the user. If I need a message
> factory, it will be a different message factory,
Agreed.
> * printf(): a better approach is to use StringFormatterMessageFactory
> for the logger,
I see your point, but in my experience the printf method is rarely used, only
for specific use cases; most logging in the same class would use the info(),
debug() etc methods.
Without printf users would need to declare a separate logger.
The printf method is a nice convenience.
> * catching(Throwable): can be replaced with `error(Object)` and the
> semantics described in this thread,
Agreed.
> * throwing: does anybody use it? Maybe it could stay,
I never use it.
> * entry/exit, traceEntry/traceExit: I can not imagine using these on
> each method (or important method). I'd rather use AspectJ pointcuts
> instead (or a @LogTrace annotation),
Agreed.
> * methods that use `MessageSupplier` like `info(MessageSupplier)`:
> couldn't these be integrated into the logic of `info(Supplier)`?
Yes.
> * the `is*Enabled` methods are prone to misuse: a snippet like:
>
> if (logger.isDebugEnabled()) {
> logger.debug(MARKER, "Hello world!");
> }
>
> will not print any message if the level of the logger is less specific
> than DEBUG, even if the user asks for **all** MARKER messages to be
> printed.
Interesting.
By the way; isn’t that behavior the same even if there’s no “if” condition
around the logger.debug(MARKER, "Hello world!");
statement?
>
> IMHO opinion `v3.AbstractLogger` should only have 2 abstract methods:
> * logMessage(Level level, Marker marker, String fqcn,
> StackTraceElement location, Message message, Throwable throwable)
> * isEnabled(Level level, Marker marker, String fqcn,
> StackTraceElement location, Message message, Throwable throwable)
>
> Piotr
Re: Change semantics of `Throwable` parameters
Hi Matt,
On Tue, 21 Nov 2023 at 23:22, Matt Sicker wrote:
>
> This sounds like it might be a good basis for figuring out a parallel v3 API
> for a “hard to mis-use” style API. However, once you go that route, you start
> to wonder how useful templated log messages are when you can capture a lambda
> instead. Parameterized log messages might work better as structured log
> messages, something that is awkward to use in the API at the moment.
If we'll create a separate `v3.Logger` interface I would clean it up
from many methods, e.g.:
* getLevel() and getName(): how are these useful for the user? An `if
(logger.getLevel() == Level.INFO)` should be replaced by `isEnabled`,
* getMessageFactory() and getFlowMessageFactory() (the latter is my
fault): again these are not useful to the user. If I need a message
factory, it will be a different message factory,
* printf(): a better approach is to use StringFormatterMessageFactory
for the logger,
* catching(Throwable): can be replaced with `error(Object)` and the
semantics described in this thread,
* throwing: does anybody use it? Maybe it could stay,
* entry/exit, traceEntry/traceExit: I can not imagine using these on
each method (or important method). I'd rather use AspectJ pointcuts
instead (or a @LogTrace annotation),
* methods that use `MessageSupplier` like `info(MessageSupplier)`:
couldn't these be integrated into the logic of `info(Supplier)`?
* the `is*Enabled` methods are prone to misuse: a snippet like:
if (logger.isDebugEnabled()) {
logger.debug(MARKER, "Hello world!");
}
will not print any message if the level of the logger is less specific
than DEBUG, even if the user asks for **all** MARKER messages to be
printed.
IMHO opinion `v3.AbstractLogger` should only have 2 abstract methods:
* logMessage(Level level, Marker marker, String fqcn,
StackTraceElement location, Message message, Throwable throwable)
* isEnabled(Level level, Marker marker, String fqcn,
StackTraceElement location, Message message, Throwable throwable)
Piotr
