Skip to content

Commit

Permalink
Issue ReactiveX#806: Switch resilience4j.retry.calls from Gauge to Co…
Browse files Browse the repository at this point in the history
…unter (ReactiveX#812)

In fact under the hood it is a change from Gauge
to FunctionCounter which better represents this metric
as it constantly grows.
  • Loading branch information
wyhasany authored and RobWin committed Jan 20, 2020
1 parent 097e9c9 commit bb72046
Show file tree
Hide file tree
Showing 14 changed files with 121 additions and 130 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
package io.github.resilience4j.micrometer.tagged;

import io.github.resilience4j.retry.Retry;
import io.micrometer.core.instrument.Gauge;
import io.micrometer.core.instrument.FunctionCounter;
import io.micrometer.core.instrument.Meter;
import io.micrometer.core.instrument.MeterRegistry;
import io.micrometer.core.instrument.Tag;
Expand All @@ -44,28 +44,28 @@ protected void addMetrics(MeterRegistry meterRegistry, Retry retry) {

private void registerMetrics(MeterRegistry meterRegistry, Retry retry, List<Tag> customTags) {
Set<Meter.Id> idSet = new HashSet<>();
idSet.add(Gauge.builder(names.getCallsMetricName(), retry,
idSet.add(FunctionCounter.builder(names.getCallsMetricName(), retry,
rt -> rt.getMetrics().getNumberOfSuccessfulCallsWithoutRetryAttempt())
.description("The number of successful calls without a retry attempt")
.tag(TagNames.NAME, retry.getName())
.tag(TagNames.KIND, "successful_without_retry")
.tags(customTags)
.register(meterRegistry).getId());
idSet.add(Gauge.builder(names.getCallsMetricName(), retry,
idSet.add(FunctionCounter.builder(names.getCallsMetricName(), retry,
rt -> rt.getMetrics().getNumberOfSuccessfulCallsWithRetryAttempt())
.description("The number of successful calls after a retry attempt")
.tag(TagNames.NAME, retry.getName())
.tag(TagNames.KIND, "successful_with_retry")
.tags(customTags)
.register(meterRegistry).getId());
idSet.add(Gauge.builder(names.getCallsMetricName(), retry,
idSet.add(FunctionCounter.builder(names.getCallsMetricName(), retry,
rt -> rt.getMetrics().getNumberOfFailedCallsWithoutRetryAttempt())
.description("The number of failed calls without a retry attempt")
.tag(TagNames.NAME, retry.getName())
.tag(TagNames.KIND, "failed_without_retry")
.tags(customTags)
.register(meterRegistry).getId());
idSet.add(Gauge.builder(names.getCallsMetricName(), retry,
idSet.add(FunctionCounter.builder(names.getCallsMetricName(), retry,
rt -> rt.getMetrics().getNumberOfFailedCallsWithRetryAttempt())
.description("The number of failed calls after a retry attempt")
.tag(TagNames.NAME, retry.getName())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,47 +15,23 @@
*/
package io.github.resilience4j.micrometer.tagged;

import io.micrometer.core.instrument.Counter;
import io.micrometer.core.instrument.Gauge;
import io.micrometer.core.instrument.Timer;
import io.micrometer.core.instrument.Meter;

import java.util.Collection;
import java.util.Optional;

final class MetricsTestHelper {

static Optional<Gauge> findGaugeByKindAndNameTags(Collection<Gauge> gauges, String kind,
static <T extends Meter> Optional<T> findMeterByKindAndNameTags(Collection<T> meters, String kind,
String name) {
return gauges.stream()
return meters.stream()
.filter(g -> kind.equals(g.getId().getTag(TagNames.KIND)))
.filter(g -> name.equals(g.getId().getTag(TagNames.NAME)))
.findAny();
}

static Optional<Timer> findTimerByKindAndNameTags(Collection<Timer> timers, String kind,
String name) {
return timers.stream()
.filter(g -> kind.equals(g.getId().getTag(TagNames.KIND)))
.filter(g -> name.equals(g.getId().getTag(TagNames.NAME)))
.findAny();
}

static Optional<Counter> findCounterByKindAndNameTags(Collection<Counter> counters, String kind,
String name) {
return counters.stream()
.filter(g -> kind.equals(g.getId().getTag(TagNames.KIND)))
.filter(g -> name.equals(g.getId().getTag(TagNames.NAME)))
.findAny();
}

static Optional<Counter> findCounterByNamesTag(Collection<Counter> gauges, String name) {
return gauges.stream()
.filter(g -> name.equals(g.getId().getTag(TagNames.NAME)))
.findAny();
}

static Optional<Gauge> findGaugeByNamesTag(Collection<Gauge> gauges, String name) {
return gauges.stream()
static <T extends Meter> Optional<T> findMeterByNamesTag(Collection<T> meters, String name) {
return meters.stream()
.filter(g -> name.equals(g.getId().getTag(TagNames.NAME)))
.findAny();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@

import static io.github.resilience4j.micrometer.tagged.AbstractBulkheadMetrics.MetricNames.DEFAULT_BULKHEAD_AVAILABLE_CONCURRENT_CALLS_METRIC_NAME;
import static io.github.resilience4j.micrometer.tagged.AbstractBulkheadMetrics.MetricNames.DEFAULT_BULKHEAD_MAX_ALLOWED_CONCURRENT_CALLS_METRIC_NAME;
import static io.github.resilience4j.micrometer.tagged.MetricsTestHelper.findGaugeByNamesTag;
import static io.github.resilience4j.micrometer.tagged.MetricsTestHelper.findMeterByNamesTag;
import static org.assertj.core.api.Assertions.assertThat;

public class TaggedBulkheadMetricsPublisherTest {
Expand Down Expand Up @@ -69,7 +69,7 @@ public void shouldAddMetricsForANewlyCreatedRetry() {
Collection<Gauge> gauges = meterRegistry
.get(DEFAULT_BULKHEAD_MAX_ALLOWED_CONCURRENT_CALLS_METRIC_NAME).gauges();

Optional<Gauge> successful = findGaugeByNamesTag(gauges, newBulkhead.getName());
Optional<Gauge> successful = findMeterByNamesTag(gauges, newBulkhead.getName());
assertThat(successful).isPresent();
assertThat(successful.get().value())
.isEqualTo(newBulkhead.getMetrics().getMaxAllowedConcurrentCalls());
Expand All @@ -94,7 +94,7 @@ public void shouldReplaceMetrics() {
Collection<Gauge> gauges = meterRegistry
.get(DEFAULT_BULKHEAD_MAX_ALLOWED_CONCURRENT_CALLS_METRIC_NAME).gauges();

Optional<Gauge> successful = findGaugeByNamesTag(gauges, bulkhead.getName());
Optional<Gauge> successful = findMeterByNamesTag(gauges, bulkhead.getName());
assertThat(successful).isPresent();
assertThat(successful.get().value())
.isEqualTo(bulkhead.getMetrics().getMaxAllowedConcurrentCalls());
Expand All @@ -107,7 +107,7 @@ public void shouldReplaceMetrics() {
gauges = meterRegistry.get(DEFAULT_BULKHEAD_MAX_ALLOWED_CONCURRENT_CALLS_METRIC_NAME)
.gauges();

successful = findGaugeByNamesTag(gauges, newBulkhead.getName());
successful = findMeterByNamesTag(gauges, newBulkhead.getName());
assertThat(successful).isPresent();
assertThat(successful.get().value())
.isEqualTo(newBulkhead.getMetrics().getMaxAllowedConcurrentCalls());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@

import static io.github.resilience4j.micrometer.tagged.AbstractBulkheadMetrics.MetricNames.DEFAULT_BULKHEAD_AVAILABLE_CONCURRENT_CALLS_METRIC_NAME;
import static io.github.resilience4j.micrometer.tagged.AbstractBulkheadMetrics.MetricNames.DEFAULT_BULKHEAD_MAX_ALLOWED_CONCURRENT_CALLS_METRIC_NAME;
import static io.github.resilience4j.micrometer.tagged.MetricsTestHelper.findGaugeByNamesTag;
import static io.github.resilience4j.micrometer.tagged.MetricsTestHelper.findMeterByNamesTag;
import static org.assertj.core.api.Assertions.assertThat;

public class TaggedBulkheadMetricsTest {
Expand Down Expand Up @@ -69,7 +69,7 @@ public void shouldAddMetricsForANewlyCreatedRetry() {
Collection<Gauge> gauges = meterRegistry
.get(DEFAULT_BULKHEAD_MAX_ALLOWED_CONCURRENT_CALLS_METRIC_NAME).gauges();

Optional<Gauge> successful = findGaugeByNamesTag(gauges, newBulkhead.getName());
Optional<Gauge> successful = findMeterByNamesTag(gauges, newBulkhead.getName());
assertThat(successful).isPresent();
assertThat(successful.get().value())
.isEqualTo(newBulkhead.getMetrics().getMaxAllowedConcurrentCalls());
Expand All @@ -94,7 +94,7 @@ public void shouldReplaceMetrics() {
Collection<Gauge> gauges = meterRegistry
.get(DEFAULT_BULKHEAD_MAX_ALLOWED_CONCURRENT_CALLS_METRIC_NAME).gauges();

Optional<Gauge> successful = findGaugeByNamesTag(gauges, bulkhead.getName());
Optional<Gauge> successful = findMeterByNamesTag(gauges, bulkhead.getName());
assertThat(successful).isPresent();
assertThat(successful.get().value())
.isEqualTo(bulkhead.getMetrics().getMaxAllowedConcurrentCalls());
Expand All @@ -107,7 +107,7 @@ public void shouldReplaceMetrics() {
gauges = meterRegistry.get(DEFAULT_BULKHEAD_MAX_ALLOWED_CONCURRENT_CALLS_METRIC_NAME)
.gauges();

successful = findGaugeByNamesTag(gauges, newBulkhead.getName());
successful = findMeterByNamesTag(gauges, newBulkhead.getName());
assertThat(successful).isPresent();
assertThat(successful.get().value())
.isEqualTo(newBulkhead.getMetrics().getMaxAllowedConcurrentCalls());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@
import java.util.stream.Collectors;

import static io.github.resilience4j.micrometer.tagged.AbstractCircuitBreakerMetrics.MetricNames.*;
import static io.github.resilience4j.micrometer.tagged.MetricsTestHelper.findCounterByKindAndNameTags;
import static io.github.resilience4j.micrometer.tagged.MetricsTestHelper.findGaugeByKindAndNameTags;
import static io.github.resilience4j.micrometer.tagged.MetricsTestHelper.findMeterByKindAndNameTags;
import static org.assertj.core.api.Assertions.assertThat;

public class TaggedCircuitBreakerMetricsPublisherTest {
Expand Down Expand Up @@ -82,7 +81,8 @@ public void shouldAddMetricsForANewlyCreatedCircuitBreaker() {
Collection<Gauge> gauges = meterRegistry.get(DEFAULT_CIRCUIT_BREAKER_BUFFERED_CALLS)
.gauges();

Optional<Gauge> successful = findGaugeByKindAndNameTags(gauges, "successful",
Optional<Gauge> successful = MetricsTestHelper
.findMeterByKindAndNameTags(gauges, "successful",
newCircuitBreaker.getName());
assertThat(successful).isPresent();
assertThat(successful.get().value())
Expand Down Expand Up @@ -110,7 +110,7 @@ public void notPermittedCallsCounterReportsCorrespondingValue() {

Collection<Counter> counters = meterRegistry.get(DEFAULT_CIRCUIT_BREAKER_CALLS).counters();

Optional<Counter> notPermitted = findCounterByKindAndNameTags(counters, "not_permitted",
Optional<Counter> notPermitted = findMeterByKindAndNameTags(counters, "not_permitted",
circuitBreaker.getName());
assertThat(notPermitted).isPresent();
assertThat(notPermitted.get().count())
Expand All @@ -122,7 +122,7 @@ public void failedCallsGaugeReportsCorrespondingValue() {
Collection<Gauge> gauges = meterRegistry.get(DEFAULT_CIRCUIT_BREAKER_BUFFERED_CALLS)
.gauges();

Optional<Gauge> failed = findGaugeByKindAndNameTags(gauges, "failed",
Optional<Gauge> failed = MetricsTestHelper.findMeterByKindAndNameTags(gauges, "failed",
circuitBreaker.getName());
assertThat(failed).isPresent();
assertThat(failed.get().value())
Expand All @@ -134,7 +134,8 @@ public void successfulCallsGaugeReportsCorrespondingValue() {
Collection<Gauge> gauges = meterRegistry.get(DEFAULT_CIRCUIT_BREAKER_BUFFERED_CALLS)
.gauges();

Optional<Gauge> successful = findGaugeByKindAndNameTags(gauges, "successful",
Optional<Gauge> successful = MetricsTestHelper
.findMeterByKindAndNameTags(gauges, "successful",
circuitBreaker.getName());
assertThat(successful).isPresent();
assertThat(successful.get().value())
Expand All @@ -145,7 +146,7 @@ public void successfulCallsGaugeReportsCorrespondingValue() {
public void slowSuccessFulCallsGaugeReportsCorrespondingValue() {
Collection<Gauge> gauges = meterRegistry.get(DEFAULT_CIRCUIT_BREAKER_SLOW_CALLS).gauges();

Optional<Gauge> slow = findGaugeByKindAndNameTags(gauges, "successful",
Optional<Gauge> slow = MetricsTestHelper.findMeterByKindAndNameTags(gauges, "successful",
circuitBreaker.getName());
assertThat(slow).isPresent();
assertThat(slow.get().value())
Expand All @@ -156,7 +157,7 @@ public void slowSuccessFulCallsGaugeReportsCorrespondingValue() {
public void slowFailedCallsGaugeReportsCorrespondingValue() {
Collection<Gauge> gauges = meterRegistry.get(DEFAULT_CIRCUIT_BREAKER_SLOW_CALLS).gauges();

Optional<Gauge> slow = findGaugeByKindAndNameTags(gauges, "failed",
Optional<Gauge> slow = MetricsTestHelper.findMeterByKindAndNameTags(gauges, "failed",
circuitBreaker.getName());
assertThat(slow).isPresent();
assertThat(slow.get().value())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@
import java.util.stream.Collectors;

import static io.github.resilience4j.micrometer.tagged.AbstractCircuitBreakerMetrics.MetricNames.*;
import static io.github.resilience4j.micrometer.tagged.MetricsTestHelper.findCounterByKindAndNameTags;
import static io.github.resilience4j.micrometer.tagged.MetricsTestHelper.findGaugeByKindAndNameTags;
import static io.github.resilience4j.micrometer.tagged.MetricsTestHelper.findMeterByKindAndNameTags;
import static org.assertj.core.api.Assertions.assertThat;

public class TaggedCircuitBreakerMetricsTest {
Expand Down Expand Up @@ -80,7 +79,8 @@ public void shouldAddMetricsForANewlyCreatedCircuitBreaker() {
Collection<Gauge> gauges = meterRegistry.get(DEFAULT_CIRCUIT_BREAKER_BUFFERED_CALLS)
.gauges();

Optional<Gauge> successful = findGaugeByKindAndNameTags(gauges, "successful",
Optional<Gauge> successful = MetricsTestHelper
.findMeterByKindAndNameTags(gauges, "successful",
newCircuitBreaker.getName());
assertThat(successful).isPresent();
assertThat(successful.get().value())
Expand Down Expand Up @@ -120,7 +120,7 @@ public void notPermittedCallsCounterReportsCorrespondingValue() {

Collection<Counter> counters = meterRegistry.get(DEFAULT_CIRCUIT_BREAKER_CALLS).counters();

Optional<Counter> notPermitted = findCounterByKindAndNameTags(counters, "not_permitted",
Optional<Counter> notPermitted = findMeterByKindAndNameTags(counters, "not_permitted",
circuitBreaker.getName());
assertThat(notPermitted).isPresent();
assertThat(notPermitted.get().count())
Expand All @@ -132,7 +132,7 @@ public void failedCallsGaugeReportsCorrespondingValue() {
Collection<Gauge> gauges = meterRegistry.get(DEFAULT_CIRCUIT_BREAKER_BUFFERED_CALLS)
.gauges();

Optional<Gauge> failed = findGaugeByKindAndNameTags(gauges, "failed",
Optional<Gauge> failed = MetricsTestHelper.findMeterByKindAndNameTags(gauges, "failed",
circuitBreaker.getName());
assertThat(failed).isPresent();
assertThat(failed.get().value())
Expand All @@ -144,7 +144,8 @@ public void successfulCallsGaugeReportsCorrespondingValue() {
Collection<Gauge> gauges = meterRegistry.get(DEFAULT_CIRCUIT_BREAKER_BUFFERED_CALLS)
.gauges();

Optional<Gauge> successful = findGaugeByKindAndNameTags(gauges, "successful",
Optional<Gauge> successful = MetricsTestHelper
.findMeterByKindAndNameTags(gauges, "successful",
circuitBreaker.getName());
assertThat(successful).isPresent();
assertThat(successful.get().value())
Expand All @@ -156,7 +157,7 @@ public void successfulCallsGaugeReportsCorrespondingValue() {
public void slowSuccessfulGaugeReportsCorrespondingValue() {
Collection<Gauge> gauges = meterRegistry.get(DEFAULT_CIRCUIT_BREAKER_SLOW_CALLS).gauges();

Optional<Gauge> slow = findGaugeByKindAndNameTags(gauges, "successful",
Optional<Gauge> slow = MetricsTestHelper.findMeterByKindAndNameTags(gauges, "successful",
circuitBreaker.getName());
assertThat(slow).isPresent();
assertThat(slow.get().value())
Expand All @@ -168,7 +169,7 @@ public void slowSuccessfulGaugeReportsCorrespondingValue() {
public void slowFailedCallsGaugeReportsCorrespondingValue() {
Collection<Gauge> gauges = meterRegistry.get(DEFAULT_CIRCUIT_BREAKER_SLOW_CALLS).gauges();

Optional<Gauge> slow = findGaugeByKindAndNameTags(gauges, "failed",
Optional<Gauge> slow = MetricsTestHelper.findMeterByKindAndNameTags(gauges, "failed",
circuitBreaker.getName());
assertThat(slow).isPresent();
assertThat(slow.get().value())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@

import static io.github.resilience4j.micrometer.tagged.AbstractRateLimiterMetrics.MetricNames.DEFAULT_AVAILABLE_PERMISSIONS_METRIC_NAME;
import static io.github.resilience4j.micrometer.tagged.AbstractRateLimiterMetrics.MetricNames.DEFAULT_WAITING_THREADS_METRIC_NAME;
import static io.github.resilience4j.micrometer.tagged.MetricsTestHelper.findGaugeByNamesTag;
import static io.github.resilience4j.micrometer.tagged.MetricsTestHelper.findMeterByNamesTag;
import static org.assertj.core.api.Assertions.assertThat;

public class TaggedRateLimiterMetricsPublisherTest {
Expand Down Expand Up @@ -66,7 +66,7 @@ public void shouldAddMetricsForANewlyCreatedRateLimiter() {
Collection<Gauge> gauges = meterRegistry.get(DEFAULT_AVAILABLE_PERMISSIONS_METRIC_NAME)
.gauges();

Optional<Gauge> successful = findGaugeByNamesTag(gauges, newRateLimiter.getName());
Optional<Gauge> successful = findMeterByNamesTag(gauges, newRateLimiter.getName());
assertThat(successful).isPresent();
assertThat(successful.get().value())
.isEqualTo(newRateLimiter.getMetrics().getAvailablePermissions());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@

import static io.github.resilience4j.micrometer.tagged.AbstractRateLimiterMetrics.MetricNames.DEFAULT_AVAILABLE_PERMISSIONS_METRIC_NAME;
import static io.github.resilience4j.micrometer.tagged.AbstractRateLimiterMetrics.MetricNames.DEFAULT_WAITING_THREADS_METRIC_NAME;
import static io.github.resilience4j.micrometer.tagged.MetricsTestHelper.findGaugeByNamesTag;
import static io.github.resilience4j.micrometer.tagged.MetricsTestHelper.findMeterByNamesTag;
import static org.assertj.core.api.Assertions.assertThat;

public class TaggedRateLimiterMetricsTest {
Expand Down Expand Up @@ -65,7 +65,7 @@ public void shouldAddMetricsForANewlyCreatedRateLimiter() {
Collection<Gauge> gauges = meterRegistry.get(DEFAULT_AVAILABLE_PERMISSIONS_METRIC_NAME)
.gauges();

Optional<Gauge> successful = findGaugeByNamesTag(gauges, newRateLimiter.getName());
Optional<Gauge> successful = findMeterByNamesTag(gauges, newRateLimiter.getName());
assertThat(successful).isPresent();
assertThat(successful.get().value())
.isEqualTo(newRateLimiter.getMetrics().getAvailablePermissions());
Expand Down
Loading

0 comments on commit bb72046

Please sign in to comment.