Skip to content

Commit

Permalink
Fix Stork metrics with Micrometer 1.13
Browse files Browse the repository at this point in the history
- Eagerly create failures counter
- Avoid decrementing the number of found instances when there is a service discovery failure
  • Loading branch information
cescoffier committed Aug 22, 2024
1 parent e27abb8 commit 373e7c6
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,14 @@ private void assertStorkMetricsInMicrometerRegistry(String serviceName) {
Util.assertTags(Tag.of("service-name", serviceName), instanceCounter, serviceDiscoveryDuration,
serviceSelectionDuration);

Assertions.assertThat(instanceCounter).isNotNull();
Assertions.assertThat(serviceDiscoveryDuration).isNotNull();
Assertions.assertThat(serviceSelectionDuration).isNotNull();
Assertions.assertThat(serviceDiscoveryFailures).isNotNull();
Assertions.assertThat(loadBalancerFailures).isNotNull();
Assertions.assertThat(instanceCounter.count()).isEqualTo(1);
Assertions.assertThat(loadBalancerFailures.count()).isEqualTo(1);
Assertions.assertThat(serviceDiscoveryFailures).isNull();// FIXME should this be null?
Assertions.assertThat(serviceDiscoveryFailures.count()).isEqualTo(0);
Assertions.assertThat(serviceDiscoveryDuration.totalTime(TimeUnit.NANOSECONDS)).isGreaterThan(0);
Assertions.assertThat(serviceSelectionDuration.totalTime(TimeUnit.NANOSECONDS)).isGreaterThan(0);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,13 @@ private void assertStorkMetricsInMicrometerRegistry(String serviceName) {
Util.assertTags(Tag.of("service-name", serviceName), instanceCounter, serviceDiscoveryDuration,
serviceSelectionDuration);

Assertions.assertThat(instanceCounter.count()).isEqualTo(-1); // same as initial value
Assertions.assertThat(loadBalancerFailures).isNull(); // FIXME it probably shouldn't be null
Assertions.assertThat(instanceCounter).isNotNull();
Assertions.assertThat(serviceDiscoveryDuration).isNotNull();
Assertions.assertThat(serviceSelectionDuration).isNotNull();
Assertions.assertThat(serviceDiscoveryFailures).isNotNull();
Assertions.assertThat(loadBalancerFailures).isNotNull();
Assertions.assertThat(instanceCounter.count()).isEqualTo(0);
Assertions.assertThat(loadBalancerFailures.count()).isEqualTo(0);
Assertions.assertThat(serviceDiscoveryFailures.count()).isEqualTo(1);
Assertions.assertThat(serviceDiscoveryDuration.totalTime(TimeUnit.NANOSECONDS)).isGreaterThan(0);
Assertions.assertThat(serviceSelectionDuration.totalTime(TimeUnit.NANOSECONDS)).isGreaterThan(0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ static void removeRegistry() {
}

@Test
public void shouldGetStorkMetricsForTwoServicesWhenEverythingSucceded() {
public void shouldGetStorkMetricsForTwoServicesWhenEverythingSucceeded() {
when().get("/ping/one").then().statusCode(200);
when().get("greeting/hola").then().statusCode(200);

Expand Down Expand Up @@ -86,12 +86,16 @@ private void assertStorkMetricsInMicrometerRegistry(String serviceName) {
Util.assertTags(Tag.of("service-name", serviceName), instanceCounter, serviceDiscoveryDuration,
serviceSelectionDuration);

Assertions.assertThat(instanceCounter).isNotNull();
Assertions.assertThat(serviceDiscoveryDuration).isNotNull();
Assertions.assertThat(serviceSelectionDuration).isNotNull();
Assertions.assertThat(serviceDiscoveryFailures).isNotNull();
Assertions.assertThat(loadBalancerFailures).isNotNull();
Assertions.assertThat(instanceCounter.count()).isEqualTo(1);
Assertions.assertThat(serviceDiscoveryDuration.totalTime(TimeUnit.NANOSECONDS)).isGreaterThan(0);
Assertions.assertThat(serviceSelectionDuration.totalTime(TimeUnit.NANOSECONDS)).isGreaterThan(0);
// FIXME not why this is null.
Assertions.assertThat(serviceDiscoveryFailures).isNull();
Assertions.assertThat(loadBalancerFailures).isNull();
Assertions.assertThat(serviceDiscoveryFailures.count()).isEqualTo(0);
Assertions.assertThat(loadBalancerFailures.count()).isEqualTo(0);
}

public static void assertStorkMetrics(String serviceName) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public StorkObservationCollectorBean(final MeterRegistry registry) {

this.serviceSelectionTimer = Timer
.builder("stork.service-selection.duration")
.description("The duration of the selection operation ")
.description("The duration of the selection operation")
.withRegistry(registry);

this.serviceDiscoveryFailures = Counter
Expand All @@ -53,7 +53,7 @@ public StorkObservationCollectorBean(final MeterRegistry registry) {

this.serviceSelectionFailures = Counter
.builder("stork.service-selection.failures")
.description("The number of failures during service selection.")
.description("The number of failures during service selection")
.withRegistry(registry);
}

Expand All @@ -69,18 +69,27 @@ public StorkObservation create(String serviceName, String serviceDiscoveryType,
public void complete(StorkObservation observation) {
Tags tags = Tags.of(Tag.of("service-name", observation.getServiceName()));

this.instanceCounter.withTags(tags).increment(observation.getDiscoveredInstancesCount());
int count = observation.getDiscoveredInstancesCount();
this.instanceCounter.withTags(tags).increment(Math.max(count, 0));
this.serviceDiscoveryTimer.withTags(tags).record(observation.getServiceDiscoveryDuration().getNano(),
TimeUnit.NANOSECONDS);
this.serviceSelectionTimer.withTags(tags).record(observation.getServiceSelectionDuration().getNano(),
TimeUnit.NANOSECONDS);

Counter ssf = this.serviceSelectionFailures.withTags(tags);
Counter sdf = this.serviceDiscoveryFailures.withTags(tags);
if (observation.failure() != null) {
if (observation.isServiceDiscoverySuccessful()) {
this.serviceSelectionFailures.withTags(tags).increment();
ssf.increment();
} else {// SD failure
this.serviceDiscoveryFailures.withTags(tags).increment();
sdf.increment();
// This forces the creation of the counter if it does not exist.
ssf.increment(0);
}
} else {
// This forces the creation of the counters if they do not exist.
ssf.increment(0);
sdf.increment(0);
}

}
Expand Down

0 comments on commit 373e7c6

Please sign in to comment.