armeria icon indicating copy to clipboard operation
armeria copied to clipboard

Allow customizing the `MeterIdPrefix` of `CircuitBreaker` metrics.

Open trustin opened this issue 2 years ago • 1 comments

Currently, the metric name and tags of circuit breaker metrics are determined by MetricCollectingCircuitBreakerListener.metricsOf(String circuitBreakerName). The resulting meter ID always contains the name of the circuit breaker.

When a user uses a CircuitBreakerMapping such as perHost(), each circuit breaker will have its own metrics under its own meter ID prefix, which may or may not be something a user desires.

We could allow a user to specify a function that allows a user to choose the MeterIdPrefix of the circuit breaker so that a user can use the same MeterIdPrefix while using per-host circuit breakers:

MeterIdPrefixFunction func = ...;
var listener = CircuitBreakerListener.metricCollecting(registry, func);
var mapping = CircuitBreakerMapping.perHost(host -> {
    return CircuitBreaker.builder(host)
                         .listener(listener)
                         .build();
});

(Requested by @delegacy)

trustin avatar Nov 29 '23 05:11 trustin

I'll take this task..!

Kyurenpoto avatar Dec 07 '23 05:12 Kyurenpoto

While reviewing #5385, I've realized that we can't utilize MeterIdPrefixFunction due to metricsOf being called before setting even a RequestHeaders. https://github.com/line/armeria/blob/598bf747cc2116a0b9079cdb0c925d6aa42f5621/core/src/main/java/com/linecorp/armeria/common/metric/MeterIdPrefixFunction.java#L95 Additionally, MeterIdPrefixFunction does not accept the circuitBreakerName.

Instead, we need a Function like Function<String, MeterIdPrefix> or a similar one. Do you have any suggestions? @line/dx

minwoox avatar May 16 '24 09:05 minwoox

@minwoox, @Kyurenpoto I think we should go with Micrometer Observation as described at #5736. Sorry but let me close this. :bow:

trustin avatar Jun 06 '24 14:06 trustin

Sounds good. 👍

minwoox avatar Jun 07 '24 00:06 minwoox