You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by "ppkarwasz (via GitHub)" <gi...@apache.org> on 2024/04/12 18:11:53 UTC
[PR] Add service to attach metrics to Log4j Core (logging-log4j2)
ppkarwasz opened a new pull request, #2469:
URL: https://github.com/apache/logging-log4j2/pull/2469
This PR is an evolution of #1943, that adds an easy to use way to add metrics and other instrumentation to Log4j Core.
It introduces an `InstrumentationService` class that is used each time a component on the "logging path" is created or added to the `Configuration`. Most notably:
- `instrumentMessageFactory` is called each time a logger is created,
- each time a logger configuration is added `instrumentLogEventFactory`, `instrumentReliabilityStrategy` and `instrumentLoggerConfig` are called,
- each time an asynchronous element is created (`AsyncLoggerDisruptor`, `AsyncLoggerConfigDisruptor` or `AsyncAppender) the `instrumentQueueFullPolicy` method is called,
- the startup of a disruptor-based element additionally causes an `instrumentRingBuffer` call,
- finally each time an appender is added to the configuration an `instrumentAppender` call happens.
Most of these methods can be use twofold:
- they can return a wrapped component, so that "push" metrics can be implemented,
- they can return the parameter of the method itself.
What do you think? cc/ @adamthom-amzn, @jvz
Looking at the currently existing solutions, `micrometer-metrics` (cf. [`Log4j2Metrics`](https://github.com/micrometer-metrics/micrometer/blob/main/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/logging/Log4j2Metrics.java)) could replace their filter-based solution to one based on `instrumentLogEventFactory`.
Which of the other method's could be useful? Are the additional parameters enough to define all the dimensions of the metrics?
**Note**: this PR is a draft for now, since it lacks documentation and tests.
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add service to attach metrics to Log4j Core (logging-log4j2)
Posted by "adamthom-amzn (via GitHub)" <gi...@apache.org>.
adamthom-amzn commented on PR #2469:
URL: https://github.com/apache/logging-log4j2/pull/2469#issuecomment-2052616884
At first glance, I quite like this approach, and it would help with some other instrumentation I've hacked in - I observe configuration changes to modify all attached appenders to add a filter that tracks appender throughput, and since I was already reflectively pulling out AsyncQueueFullPolicies, I was also grabbing RingBuffers to monitor them as well. Being able to decorate these components is appealing.
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add service to attach metrics to Log4j Core (logging-log4j2)
Posted by "rgoers (via GitHub)" <gi...@apache.org>.
rgoers commented on code in PR #2469:
URL: https://github.com/apache/logging-log4j2/pull/2469#discussion_r1564869924
##########
log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java:
##########
@@ -800,6 +802,12 @@ private void initApiModule() {
// LOG4J2-151: changed visibility from private to protected
protected Logger newInstance(final LoggerContext ctx, final String name, final MessageFactory messageFactory) {
- return new Logger(ctx, name, messageFactory);
+ // TODO: Adapt after
+ // https://github.com/apache/logging-log4j2/issues/2379
+ // is fixed.
+ final MessageFactory actualMessageFactory = InstrumentationService.getInstance()
Review Comment:
I am not sure I agree with this approach. It is basically making InstrumentationService a factory for wrappers. I would rather see things that should be instrumented implement a well defined interface for providing the instrumented data such as https://github.com/apache/logging-flume/tree/trunk/flume-ng-core/src/main/java/org/apache/flume/instrumentation does. I think the temptation to do more than instrumentation is way too great with the current approach. It also means that InstrumentationServcie has to be aware of every type of thing that can be instrumented.
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add service to attach metrics to Log4j Core (logging-log4j2)
Posted by "rgoers (via GitHub)" <gi...@apache.org>.
rgoers commented on code in PR #2469:
URL: https://github.com/apache/logging-log4j2/pull/2469#discussion_r1564872577
##########
log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java:
##########
@@ -800,6 +802,12 @@ private void initApiModule() {
// LOG4J2-151: changed visibility from private to protected
protected Logger newInstance(final LoggerContext ctx, final String name, final MessageFactory messageFactory) {
- return new Logger(ctx, name, messageFactory);
+ // TODO: Adapt after
+ // https://github.com/apache/logging-log4j2/issues/2379
+ // is fixed.
+ final MessageFactory actualMessageFactory = InstrumentationService.getInstance()
Review Comment:
@vy I would NOT object to _"intercepting"_ instantiation so long as a) it can't wrap the object being returned and b) it can only modify the created object if that object permits it. In my mind instrumentation means the ability to capture statistics or perform an allowed operation, not completely replace the thing being instrumented.
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org