Create metrics API proof of concept

[jvz]

Related to #1344, this demonstrates a proof of concept metrics API to begin using. This implements the metric added in #1927, though I think we should be defining several more metrics beyond that. Before going too far into this idea, here's the gist of what I've come up with so far. API is loosely based on Micrometer which will be added as an implementation (probably its own module; this is effectively a plugin).

[rgoers]

This PR has me very confused. I do see the new metrics package but the vast majority of this PR seems to have nothing to do with metrics. Could you please separate out the builder improvements into their own PR?

[jvz]

The builder updates are related to adding dependency-injected values. I did not include any concrete implementations of the metrics API yet, but I updated some areas that are supposed to export metrics.

[jvz]

Or to be more specific, instead of creating yet another global variable, I had to refactor some code to apply inversion of control a little bit. If we were adapting Log4j to work via Spring beans, we'd still have to do the exact same thing.

[vy]

Imagine a future where we appropriately measure almost every activity. If we stick to the current strategy drafted here, we would be passing measurement objects (tags, metric registries, etc.) at every method call instantiating a component. This sounds pretty invasive to me. Can't we rather have a listener concept instead? That is, say, DiscardingAsyncQueueFullPolicy accepting a List<DiscardingAsyncQueueFullPolicyListener> in its constructor. Then measurement probes will simply be classes extending such listeners. This way we can avoid polluting the policy code with measurement logic.

Mine is just a suggestion, maybe there are better ones. But my point stands: we shouldn't be passing metric objects at every method call.

[jvz]

I did this here so I could customize the particular tags being used by an otherwise super-generic counter (if I skip the tags, then the discard policy metrics don't distinguish the various ways you can configure it). If we do a listener concept, we'd still need to define some metadata in the event to indicate things like tags. Some of this would be more straightforward to hard-code if we didn't use so much inheritance in the classes, but I digress.

Unless you've got an idea on how to use an event listener concept here that isn't replicating the same information except via a class constructor instead of a method parameter? I'd love to implement this in the most simple and straightforward way possible, but I'm not sure on how to rectify that (unless we define different event classes for different metrics?)

[vy]

I smell a Dropwizard Metrics, Micrometer, etc. rewrite here. I doubt if such a big undertaking is necessary. If we stick to the listener interface concept I outlined above, maybe we can provide vendor-specific – one for Micrometer, one for Dropwizard Metrics, etc. – modules and avoid rolling out our own implementation.

[jvz]

I don't want to rewrite anything; I was developing interfaces that could easily delegate to Micrometer APIs. If we go for an event listener API, then we have to define a bunch of rules about event interpretation which should be logically equivalent to this proposal.

[jvz]

I don't see how that's going to be possible without either adding third-party dependencies or implementing facades for all the various OpenTelemetry APIs for internal use.

[ppkarwasz]

I was also thinking more about an API that allows a metrics implementation to subscribe to the events it is interested.

Of course in this proposal a metrics implementation can always return no-op counters (or null?) for events is it not interested, so it is just a question of point of view.

Remark that in some cases (e.g. the ring buffer), we should not push every change in the buffer's free slots to the metrics implementation, but rather let the metrics implementation to pull the current value at some intervals. If we don't do that, we might lose a lot of performance.

[jvz]

What sort of event would be published here that could correspond to a counter? That's a push-based mechanism itself.

[jvz]

Looking through https://opentelemetry.io/docs/specs/otel/logs/ and OTel in general, it seems like we should have support for some of this directly. I don't think we need to implement all the APIs (like metrics and traces, though they might be natural logging APIs to include in the future). The alternative is a dependency stack larger than Log4j 2.x with optional dependencies in practice.

[ppkarwasz]

What sort of event would be published here that could correspond to a counter? That's a push-based mechanism itself.

For me an event is just any kind of object: in this case long is an event. ;-)
So stripped to the bone we need methods that return long and methods that consume Supplier<Long>.

[ppkarwasz]

@jvz,

I think we could choose an approach similar to RxJavaHooks to provide instrumentation hooks without affecting the performance of our application.

For 2.x we could create an InstrumentationService to give users the possibility to wrap our most important services:

public interface InstrumentationService {

    ReliabilityStrategy instrumentReliabilityStrategy(ReliabilityStrategy strategy);

    MessageFactory instrumentMessageFactory(MessageFactory factory);

    LogEventFactory instrumentEventFactory(LogEventFactory factory);

    AsyncQueueFullPolicy instrumentQueueFullPolicy(AsyncQueueFullPolicy policy);

    Appender instrumentAppender(Appender appender);
}

The default implementation would of course return the argument unchanged. What do you think?

For 3.x I believe that users can use the DI system to post-process these services.

Can we close this PR?

[jvz]

I think that's a good idea.