diff --git a/dropwizard-metrics4/src/test/java/feign/metrics4/Metrics4CapabilityTest.java b/dropwizard-metrics4/src/test/java/feign/metrics4/Metrics4CapabilityTest.java index 64374126d..31a824728 100644 --- a/dropwizard-metrics4/src/test/java/feign/metrics4/Metrics4CapabilityTest.java +++ b/dropwizard-metrics4/src/test/java/feign/metrics4/Metrics4CapabilityTest.java @@ -71,7 +71,8 @@ protected Metric getMetric(String suffix, String... tags) { } for (int i = 0; i < tags.length; i += 2) { - if (!name.contains(tags[i]) && !name.contains(tags[i] + 1)) { + // metrics 4 doesn't support tags, for that reason we don't include tag name + if (!name.contains(tags[i + 1])) { return false; } } diff --git a/micrometer/src/main/java/feign/micrometer/MeteredInvocationHandleFactory.java b/micrometer/src/main/java/feign/micrometer/MeteredInvocationHandleFactory.java index 61489e931..07fd2e608 100644 --- a/micrometer/src/main/java/feign/micrometer/MeteredInvocationHandleFactory.java +++ b/micrometer/src/main/java/feign/micrometer/MeteredInvocationHandleFactory.java @@ -67,22 +67,15 @@ public InvocationHandler create(Target target, Map dispat final Timer.Sample sample = Timer.start(meterRegistry); Timer timer = null; try { - try { - final Object invoke = invocationHandle.invoke(proxy, method, args); - timer = createTimer(target, method, args, null); - return invoke; - } catch (Exception e) { - throw e; - } catch (Throwable e) { - throw new Exception(e); - } + final Object invoke = invocationHandle.invoke(proxy, method, args); + timer = createTimer(target, method, args, null); + return invoke; } catch (final FeignException e) { timer = createTimer(target, method, args, e); createFeignExceptionCounter(target, method, args, e).increment(); throw e; } catch (final Throwable e) { timer = createTimer(target, method, args, e); - createExceptionCounter(target, method, args, e).increment(); throw e; } finally { if (timer == null) { @@ -99,15 +92,6 @@ protected Timer createTimer(Target target, Method method, Object[] args, Throwab return meterRegistry.timer(metricName.name(e), allTags); } - protected Counter createExceptionCounter(Target target, - Method method, - Object[] args, - Throwable e) { - final Tag[] extraTags = extraTags(target, method, args, e); - final Tags allTags = metricTagResolver.tag(target.type(), method, target.url(), e, extraTags); - return meterRegistry.counter(metricName.name(e), allTags); - } - protected Counter createFeignExceptionCounter(Target target, Method method, Object[] args, diff --git a/micrometer/src/test/java/feign/micrometer/AbstractMetricsTestBase.java b/micrometer/src/test/java/feign/micrometer/AbstractMetricsTestBase.java index 038a235ea..2f2c3453f 100644 --- a/micrometer/src/test/java/feign/micrometer/AbstractMetricsTestBase.java +++ b/micrometer/src/test/java/feign/micrometer/AbstractMetricsTestBase.java @@ -15,12 +15,13 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.aMapWithSize; +import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.notNullValue; import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.fail; import java.util.Map; import java.util.concurrent.atomic.AtomicReference; -import org.hamcrest.Matchers; import org.junit.Before; import org.junit.Test; import feign.Capability; @@ -123,12 +124,30 @@ public void decoderPropagatesUncheckedException() { .addCapability(createMetricCapability()) .target(new MockTarget<>(MicrometerCapabilityTest.SimpleSource.class)); - try { - source.get("0x3456789"); - fail("Should throw NotFound exception"); - } catch (FeignException.NotFound e) { - assertSame(notFound.get(), e); - } + FeignException.NotFound thrown = + assertThrows(FeignException.NotFound.class, () -> source.get("0x3456789")); + assertSame(notFound.get(), thrown); + } + + @Test + public void shouldMetricCollectionWithCustomException() { + final SimpleSource source = Feign.builder() + .client((request, options) -> { + throw new RuntimeException("Test error"); + }) + .addCapability(createMetricCapability()) + .target(new MockTarget<>(MicrometerCapabilityTest.SimpleSource.class)); + + RuntimeException thrown = assertThrows(RuntimeException.class, () -> source.get("0x3456789")); + assertThat(thrown.getMessage(), equalTo("Test error")); + + assertThat( + getMetric("exception", "exception_name", "RuntimeException", "method", "get"), + notNullValue()); + } + + + }