logging-log4j2 icon indicating copy to clipboard operation
logging-log4j2 copied to clipboard

Expose disruptor discard count in AsyncLoggerContext

Open adamthom-amzn opened this issue 2 years ago • 2 comments

I would like to monitor the disruptor's discard count on a periodic basis so that I can tell if events are being discarded due to the queue being full, and how often it is happening if so. As it stands, this count is not accessible outside of the AsyncLoggerDisruptor, and the discard count is only published in a trace log when the logger is stopped.

I considered adding this to RingBufferAdminMBean but since it currently monitors the backing ring buffer, and not the disruptor itself, it did not seem to fit.

This is my first contribution so I'm unsure if this meets the bar for a changelog entry, and whether or not this needs an issue to be cut beforehand, and if I incremented the log4j-core version correctly (it was required for mvnw verify to work, otherwise the baseline plugin failed the build).

Checklist

  • Base your changes on 2.x branch if you are targeting Log4j 2; use main otherwise
  • ./mvnw verify succeeds (if it fails due to code formatting issues reported by Spotless, simply run ./mvnw spotless:apply and retry)
  • Non-trivial changes contain an entry file in the src/changelog/.2.x.x directory
  • Tests for the changes are provided
  • Commits are signed (optional, but highly recommended)

adamthom-amzn avatar Oct 30 '23 22:10 adamthom-amzn

Is this unfeasible to add for monitoring in the interim, and I should just track the metrics API PR?

adamthom-amzn avatar Nov 15 '23 22:11 adamthom-amzn

@adamthom-amzn, my personal opinion (with my PMC hat on) is as follows: Your changes touch parts of the code that

  1. a majority of the active maintainers don't have expertise on
  2. have quite some abstraction leakage already in place, and hence maintainers are hesitant to punch another hole

You can try pitching your idea in the [email protected] mailing list.

vy avatar Nov 16 '23 10:11 vy

I am closing this PR, since #2469 seems to have a larger consensus.

The latter unfortunately is stack at the end of a long TODO list.

ppkarwasz avatar Aug 16 '24 09:08 ppkarwasz