The counter indicator increases by 2 each time (on happy path) (OpenFeign#1548)

vitalijr2 · web-flow · commit 24635e6e85f4 · 2021-12-13T21:07:08.000Z
* The counter indicator increases by 2 each time (on happy path).

* Add test coverage

* Make test beaty again

* Fix license header

* Move new tests to the common abstract class
diff --git a/dropwizard-metrics4/src/test/java/feign/metrics4/Metrics4CapabilityTest.java b/dropwizard-metrics4/src/test/java/feign/metrics4/Metrics4CapabilityTest.java
@@ -13,6 +13,7 @@
  */
 package feign.metrics4;
 
+import com.codahale.metrics.Metered;
 import com.codahale.metrics.Metric;
 import com.codahale.metrics.MetricRegistry;
 import feign.Capability;
@@ -99,4 +100,14 @@ protected boolean doesMetricIncludeUri(String metricId, String uri) {
     return metricId.contains(uri);
   }
 
+  @Override
+  protected boolean doesMetricHasCounter(Metric metric) {
+    return metric instanceof Metered;
+  }
+
+  @Override
+  protected long getMetricCounter(Metric metric) {
+    return ((Metered) metric).getCount();
+  }
+
 }
diff --git a/dropwizard-metrics5/src/test/java/feign/metrics5/Metrics5CapabilityTest.java b/dropwizard-metrics5/src/test/java/feign/metrics5/Metrics5CapabilityTest.java
@@ -20,6 +20,7 @@
 import feign.Capability;
 import feign.Util;
 import feign.micrometer.AbstractMetricsTestBase;
+import io.dropwizard.metrics5.Metered;
 import io.dropwizard.metrics5.Metric;
 import io.dropwizard.metrics5.MetricName;
 import io.dropwizard.metrics5.MetricRegistry;
@@ -103,4 +104,14 @@ protected boolean doesMetricIncludeUri(MetricName metricId, String uri) {
     return uri.equals(metricId.getTags().get("uri"));
   }
 
+  @Override
+  protected boolean doesMetricHasCounter(Metric metric) {
+    return metric instanceof Metered;
+  }
+
+  @Override
+  protected long getMetricCounter(Metric metric) {
+    return ((Metered) metric).getCount();
+  }
+
 }
diff --git a/micrometer/pom.xml b/micrometer/pom.xml
@@ -44,6 +44,12 @@
       <artifactId>micrometer-core</artifactId>
       <version>1.3.5</version>
     </dependency>
+    <dependency>
+      <groupId>org.mockito</groupId>
+      <artifactId>mockito-core</artifactId>
+      <version>${mockito.version}</version>
+      <scope>test</scope>
+    </dependency>
     <dependency>
       <groupId>org.hamcrest</groupId>
       <artifactId>hamcrest</artifactId>
diff --git a/micrometer/src/main/java/feign/micrometer/MeteredClient.java b/micrometer/src/main/java/feign/micrometer/MeteredClient.java
@@ -54,7 +54,6 @@ public Response execute(Request request, Options options) throws IOException {
       final Response response = client.execute(request, options);
       countResponseCode(request, response, options, response.status(), null);
       timer = createTimer(request, response, options, null);
-      sample.stop(timer);
       return response;
     } catch (FeignException e) {
       timer = createTimer(request, null, options, e);
diff --git a/micrometer/src/test/java/feign/micrometer/AbstractMetricsTestBase.java b/micrometer/src/test/java/feign/micrometer/AbstractMetricsTestBase.java
@@ -65,6 +65,15 @@ public final void addMetricsCapability() {
     metrics.keySet().forEach(metricId -> assertThat(
         "Expect all metric names to include host name:" + metricId,
         doesMetricIncludeHost(metricId)));
+
+    final Map<METRIC_ID, METRIC> clientMetrics = getFeignMetrics().entrySet().stream()
+        .filter(entry -> isClientMetric(entry.getKey()))
+        .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
+
+    clientMetrics.values().stream()
+        .filter(this::doesMetricHasCounter)
+        .forEach(metric -> assertEquals(1, getMetricCounter(metric)));
+
   }
 
   protected abstract boolean doesMetricIncludeHost(METRIC_ID metricId);
@@ -221,4 +230,8 @@ public void decoderExceptionCounterHasUriLabelWithPathExpression() {
 
   protected abstract boolean doesMetricIncludeUri(METRIC_ID metricId, String uri);
 
+  protected abstract boolean doesMetricHasCounter(METRIC metric);
+
+  protected abstract long getMetricCounter(METRIC metric);
+
 }
diff --git a/micrometer/src/test/java/feign/micrometer/MicrometerCapabilityTest.java b/micrometer/src/test/java/feign/micrometer/MicrometerCapabilityTest.java
@@ -15,6 +15,7 @@
 
 import feign.Capability;
 import feign.Util;
+import io.micrometer.core.instrument.Measurement;
 import io.micrometer.core.instrument.Meter;
 import io.micrometer.core.instrument.Meter.Id;
 import io.micrometer.core.instrument.MockClock;
@@ -113,4 +114,24 @@ protected boolean doesMetricIncludeUri(Id metricId, String uri) {
     return uri.equals(metricId.getTag("uri"));
   }
 
+  @Override
+  protected boolean doesMetricHasCounter(Meter meter) {
+    for (Measurement measurement : meter.measure()) {
+      if ("COUNT".equals(measurement.getStatistic().name())) {
+        return true;
+      }
+    }
+    return false;
+  }
+
+  @Override
+  protected long getMetricCounter(Meter meter) {
+    for (Measurement measurement : meter.measure()) {
+      if ("COUNT".equals(measurement.getStatistic().name())) {
+        return (long) measurement.getValue();
+      }
+    }
+    return 0;
+  }
+
 }
