Re: Monitoring discards from async logging

2024-04-11 Thread Volkan Yazıcı
*Short answer:* I would suggest using reflection for the time being. We
don't anticipate any changes in that part of the code base in the
foreseeable future.

*Slightly longer answer:* Log4j has never had a holistic observability view
to its internals. We need something new (if not, from scratch), and
preferably, applicable to not only async. logging, but all components
wherever this can be needed. This is time consuming hard work.
Piotr and I, sort of, the only active Log4j maintainers nowadays, are
swamped with other priorities. All that said, you can contract a maintainer
 to get this
rolling.

On Thu, Apr 11, 2024 at 6:36 PM Thomas, Adam 
wrote:

> Volkan,
>
> Not frustrating at all, I'm coming into this without any real experience
> with how log4j approaches API design.
>
> > Thinking naively: Assuming you already thought about it, why doesn't the
> following work for you?
> > 1. Cast the `LoggerContext` to `AsyncLoggerContext`
> > 2. Reflectively extract the `AsyncLoggerContext#loggerDisruptor`
> > 3. Reflectively extract the `AsyncLoggerDisruptor#asyncQueueFullPolicy`
> > 4. Call `DiscardingAsyncQueueFullPolicy.getDiscardCount()` on the policy
> instance
>
> It's something we considered a last resort. I'm always hesitant to couple
> to a non-public API (such as reflecting into private fields), and doubly so
> when it is code I do not control.
>
> -Adam
>
>
> From: Volkan Yazıcı 
> Date: Thursday, April 11, 2024 at 4:09 AM
> To: Adam Thomas 
> Cc: "dev@logging.apache.org" 
> Subject: RE: Monitoring discards from async logging
>
>
> Hello Adam,
>
> I am very sorry for the frustrating experience. I can certainly see you do
> an excellent job in getting engaged with the project to get this feature
> request rolling. Thank you so much for that.
>
> I read your conversation with Piotr and I get your point – yes, neither
> migration to Log4j 3 (which still has an unknown release date), nor
> augmenting the configuration is an option for you. I agree with your
> assessments there.
>
> Thinking naively: Assuming you already thought about it, why doesn't the
> following work for you?
> 1. Cast the `LoggerContext` to `AsyncLoggerContext`
> 2. Reflectively extract the `AsyncLoggerContext#loggerDisruptor`
> 3. Reflectively extract the `AsyncLoggerDisruptor#asyncQueueFullPolicy`
> 4. Call `DiscardingAsyncQueueFullPolicy.getDiscardCount()` on the policy
> instance
> Kind regards.
>
> On Wed, Apr 10, 2024 at 12:11 AM Thomas, Adam 
> wrote:
> I created a pull request[1] late last year for this, and was encouraged to
> start a discussion on the dev list at the time.
>
> We run log4j2 on a lot of hosts that are operated by different teams. By
> default, our users use async logging and use the Discard
> asyncQueueFullPolicy. As far as I can tell, the status logger gets one WARN
> message the first time an event is discarded, and during shutdown a TRACE
> message is emitted if any discards happened. This leaves us with
> essentially no insight as to whether or not events are actually being
> discarded, when they are being discarded, or how many we are discarding.
>
> My pull request is pretty simple; the discarding policy already tracks
> discards, and I expose a getter for this counter from
> LoggerContext/AsyncLoggerContext. Polling the counter and pushing It to our
> telemetry system is an exercise left up to the user (me). I don’t have any
> real concerns about the safety of the change, but I have no idea how
> acceptable it is as a change to the API surface.
>
> This also spawned a metrics proof-of-concept that was recently closed [2].
> I’m fine with a more complete metrics solution, but I’m not sure of the
> timeline of such a solution, if any.
>
> How can I move forward on this pull request to make it acceptable, or is
> there another way to accomplish this that would be more palatable?
>
> -Adam
>
> [1] https://github.com/apache/logging-log4j2/pull/1927
> [2] https://github.com/apache/logging-log4j2/pull/1943
>
>


Re: Monitoring discards from async logging

2024-04-11 Thread Thomas, Adam
Volkan,

Not frustrating at all, I'm coming into this without any real experience with 
how log4j approaches API design.

> Thinking naively: Assuming you already thought about it, why doesn't the 
> following work for you?
> 1. Cast the `LoggerContext` to `AsyncLoggerContext`
> 2. Reflectively extract the `AsyncLoggerContext#loggerDisruptor`
> 3. Reflectively extract the `AsyncLoggerDisruptor#asyncQueueFullPolicy`
> 4. Call `DiscardingAsyncQueueFullPolicy.getDiscardCount()` on the policy 
> instance

It's something we considered a last resort. I'm always hesitant to couple to a 
non-public API (such as reflecting into private fields), and doubly so when it 
is code I do not control.

-Adam


From: Volkan Yazıcı 
Date: Thursday, April 11, 2024 at 4:09 AM
To: Adam Thomas 
Cc: "dev@logging.apache.org" 
Subject: RE: Monitoring discards from async logging


Hello Adam, 

I am very sorry for the frustrating experience. I can certainly see you do an 
excellent job in getting engaged with the project to get this feature request 
rolling. Thank you so much for that.

I read your conversation with Piotr and I get your point – yes, neither 
migration to Log4j 3 (which still has an unknown release date), nor augmenting 
the configuration is an option for you. I agree with your assessments there.

Thinking naively: Assuming you already thought about it, why doesn't the 
following work for you?
1. Cast the `LoggerContext` to `AsyncLoggerContext`
2. Reflectively extract the `AsyncLoggerContext#loggerDisruptor`
3. Reflectively extract the `AsyncLoggerDisruptor#asyncQueueFullPolicy`
4. Call `DiscardingAsyncQueueFullPolicy.getDiscardCount()` on the policy 
instance
Kind regards.

On Wed, Apr 10, 2024 at 12:11 AM Thomas, Adam  
wrote:
I created a pull request[1] late last year for this, and was encouraged to 
start a discussion on the dev list at the time.

We run log4j2 on a lot of hosts that are operated by different teams. By 
default, our users use async logging and use the Discard asyncQueueFullPolicy. 
As far as I can tell, the status logger gets one WARN message the first time an 
event is discarded, and during shutdown a TRACE message is emitted if any 
discards happened. This leaves us with essentially no insight as to whether or 
not events are actually being discarded, when they are being discarded, or how 
many we are discarding.

My pull request is pretty simple; the discarding policy already tracks 
discards, and I expose a getter for this counter from 
LoggerContext/AsyncLoggerContext. Polling the counter and pushing It to our 
telemetry system is an exercise left up to the user (me). I don’t have any real 
concerns about the safety of the change, but I have no idea how acceptable it 
is as a change to the API surface.

This also spawned a metrics proof-of-concept that was recently closed [2]. I’m 
fine with a more complete metrics solution, but I’m not sure of the timeline of 
such a solution, if any.

How can I move forward on this pull request to make it acceptable, or is there 
another way to accomplish this that would be more palatable?

-Adam

[1] https://github.com/apache/logging-log4j2/pull/1927
[2] https://github.com/apache/logging-log4j2/pull/1943



Re: Monitoring discards from async logging

2024-04-11 Thread Volkan Yazıcı
Hello Adam,

I am very sorry for the frustrating experience. I can certainly see you do
an excellent job in getting engaged with the project to get this feature
request rolling. Thank you so much for that.

I read your conversation with Piotr and I get your point – yes, neither
migration to Log4j 3 (which still has an unknown release date), nor
augmenting the configuration is an option for you. I agree with your
assessments there.

Thinking naively: Assuming you already thought about it, why doesn't the
following work for you?

   1. Cast the `LoggerContext` to `AsyncLoggerContext`
   2. Reflectively extract the `AsyncLoggerContext#loggerDisruptor`
   3. Reflectively extract the `AsyncLoggerDisruptor#asyncQueueFullPolicy`
   4. Call `DiscardingAsyncQueueFullPolicy.getDiscardCount()` on the policy
   instance

Kind regards.

On Wed, Apr 10, 2024 at 12:11 AM Thomas, Adam 
wrote:

> I created a pull request[1] late last year for this, and was encouraged to
> start a discussion on the dev list at the time.
>
> We run log4j2 on a lot of hosts that are operated by different teams. By
> default, our users use async logging and use the Discard
> asyncQueueFullPolicy. As far as I can tell, the status logger gets one WARN
> message the first time an event is discarded, and during shutdown a TRACE
> message is emitted if any discards happened. This leaves us with
> essentially no insight as to whether or not events are actually being
> discarded, when they are being discarded, or how many we are discarding.
>
> My pull request is pretty simple; the discarding policy already tracks
> discards, and I expose a getter for this counter from
> LoggerContext/AsyncLoggerContext. Polling the counter and pushing It to our
> telemetry system is an exercise left up to the user (me). I don’t have any
> real concerns about the safety of the change, but I have no idea how
> acceptable it is as a change to the API surface.
>
> This also spawned a metrics proof-of-concept that was recently closed [2].
> I’m fine with a more complete metrics solution, but I’m not sure of the
> timeline of such a solution, if any.
>
> How can I move forward on this pull request to make it acceptable, or is
> there another way to accomplish this that would be more palatable?
>
> -Adam
>
> [1] https://github.com/apache/logging-log4j2/pull/1927
> [2] https://github.com/apache/logging-log4j2/pull/1943
>