Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Aggregate Micrometer "uri" label values under uri template #1493

Merged
merged 5 commits into from
Sep 1, 2021
Merged

Aggregate Micrometer "uri" label values under uri template #1493

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
dd2fa71
Use path expression rather than explicit value for Micrometer "uri" l…
martinacat Aug 26, 2021
bc6171c
Make assertions more self explanatory
martinacat Aug 26, 2021
a976caf
Test Micrometer Decoder uri label
martinacat Aug 26, 2021
c214eef
Merge branch 'master' into uri-label-value-as-expression
velo Sep 1, 2021
ecbccf1
Move tests to AbstractMetricsTestBase, add uri label to dropwizard-me…
martinacat Sep 1, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 9 additions & 4 deletions dropwizard-metrics4/src/main/java/feign/metrics4/MeteredClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,23 +43,28 @@ public Response execute(Request request, Options options) throws IOException {
final RequestTemplate template = request.requestTemplate();
try (final Timer.Context classTimer =
metricRegistry.timer(
metricName.metricName(template.methodMetadata(), template.feignTarget()),
MetricRegistry.name(
metricName.metricName(template.methodMetadata(), template.feignTarget()),
"uri", template.methodMetadata().template().path()),
metricSuppliers.timers()).time()) {
Response response = client.execute(request, options);
metricRegistry.meter(
MetricRegistry.name(
metricName.metricName(template.methodMetadata(), template.feignTarget(),
"http_response_code"),
"status_group", response.status() / 100 + "xx", "http_status",
String.valueOf(response.status())),
"status_group", response.status() / 100 + "xx",
"http_status", String.valueOf(response.status()),
"uri", template.methodMetadata().template().path()),
metricSuppliers.meters()).mark();
return response;
} catch (FeignException e) {
metricRegistry.meter(
MetricRegistry.name(
metricName.metricName(template.methodMetadata(), template.feignTarget(),
"http_response_code"),
"status_group", e.status() / 100 + "xx", "http_status", String.valueOf(e.status())),
"status_group", e.status() / 100 + "xx",
"http_status", String.valueOf(e.status()),
"uri", template.methodMetadata().template().path()),
metricSuppliers.meters()).mark();
throw e;
} catch (IOException | RuntimeException e) {
Expand Down
7 changes: 5 additions & 2 deletions dropwizard-metrics4/src/main/java/feign/metrics4/MeteredDecoder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Copyright 2012-2020 The Feign Authors
* Copyright 2012-2021 The Feign Authors
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
* in compliance with the License. You may obtain a copy of the License at
Expand Down Expand Up @@ -55,7 +55,10 @@ public Object decode(Response response, Type type)
final Object decoded;
try (final Timer.Context classTimer =
metricRegistry
.timer(metricName.metricName(template.methodMetadata(), template.feignTarget()),
.timer(
MetricRegistry.name(
metricName.metricName(template.methodMetadata(), template.feignTarget()),
"uri", template.methodMetadata().template().path()),
metricSuppliers.timers())
.time()) {
decoded = decoder.decode(response, type);
Expand Down
20 changes: 17 additions & 3 deletions dropwizard-metrics4/src/test/java/feign/metrics4/Metrics4CapabilityTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,14 @@
*/
package feign.metrics4;

import java.util.Arrays;
import java.util.Map;
import java.util.Map.Entry;
import com.codahale.metrics.Metric;
import com.codahale.metrics.MetricRegistry;
import feign.Capability;
import feign.Util;
import feign.micrometer.AbstractMetricsTestBase;
import java.util.Arrays;
import java.util.Map;
import java.util.Map.Entry;

public class Metrics4CapabilityTest
extends AbstractMetricsTestBase<MetricRegistry, String, Metric> {
Expand Down Expand Up @@ -84,5 +84,19 @@ protected Metric getMetric(String suffix, String... tags) {
.orElse(null);
}

@Override
protected boolean isClientMetric(String metricId) {
return metricId.startsWith("feign.Client");
}

@Override
protected boolean isDecoderMetric(String metricId) {
return metricId.startsWith("feign.codec.Decoder");
}

@Override
protected boolean doesMetricIncludeUri(String metricId, String uri) {
return metricId.contains(uri);
}

}
10 changes: 7 additions & 3 deletions dropwizard-metrics5/src/main/java/feign/metrics5/MeteredClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,22 +43,26 @@ public Response execute(Request request, Options options) throws IOException {
final RequestTemplate template = request.requestTemplate();
try (final Context classTimer =
metricRegistry.timer(
metricName.metricName(template.methodMetadata(), template.feignTarget()),
metricName.metricName(template.methodMetadata(),
template.feignTarget())
.tagged("uri", template.methodMetadata().template().path()),
metricSuppliers.timers()).time()) {
Response response = client.execute(request, options);
metricRegistry.counter(
metricName
.metricName(template.methodMetadata(), template.feignTarget(), "http_response_code")
.tagged("http_status", String.valueOf(response.status()))
.tagged("status_group", response.status() / 100 + "xx"))
.tagged("status_group", response.status() / 100 + "xx")
.tagged("uri", template.methodMetadata().template().path()))
.inc();
return response;
} catch (FeignException e) {
metricRegistry.counter(
metricName
.metricName(template.methodMetadata(), template.feignTarget(), "http_response_code")
.tagged("http_status", String.valueOf(e.status()))
.tagged("status_group", e.status() / 100 + "xx"))
.tagged("status_group", e.status() / 100 + "xx")
.tagged("uri", template.methodMetadata().template().path()))
.inc();
throw e;
} catch (IOException | RuntimeException e) {
Expand Down
14 changes: 9 additions & 5 deletions dropwizard-metrics5/src/main/java/feign/metrics5/MeteredDecoder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Copyright 2012-2020 The Feign Authors
* Copyright 2012-2021 The Feign Authors
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
* in compliance with the License. You may obtain a copy of the License at
Expand Down Expand Up @@ -55,28 +55,32 @@ public Object decode(Response response, Type type)
final Object decoded;
try (final Context classTimer =
metricRegistry
.timer(metricName.metricName(template.methodMetadata(), template.feignTarget()),
.timer(metricName.metricName(template.methodMetadata(), template.feignTarget())
.tagged("uri", template.methodMetadata().template().path()),
metricSuppliers.timers())
.time()) {
decoded = decoder.decode(response, type);
} catch (IOException | RuntimeException e) {
metricRegistry.meter(
metricName.metricName(template.methodMetadata(), template.feignTarget(), "error_count")
.tagged("exception_name", e.getClass().getSimpleName()),
.tagged("exception_name", e.getClass().getSimpleName())
.tagged("uri", template.methodMetadata().template().path()),
metricSuppliers.meters()).mark();
throw e;
} catch (Exception e) {
metricRegistry.meter(
metricName.metricName(template.methodMetadata(), template.feignTarget(), "error_count")
.tagged("exception_name", e.getClass().getSimpleName()),
.tagged("exception_name", e.getClass().getSimpleName())
.tagged("uri", template.methodMetadata().template().path()),
metricSuppliers.meters()).mark();
throw new IOException(e);
}

if (body != null) {
metricRegistry.histogram(
metricName.metricName(template.methodMetadata(), template.feignTarget(),
"response_size"),
"response_size")
.tagged("uri", template.methodMetadata().template().path()),
metricSuppliers.histograms()).update(body.count());
}

Expand Down
15 changes: 15 additions & 0 deletions dropwizard-metrics5/src/test/java/feign/metrics5/Metrics5CapabilityTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,4 +88,19 @@ protected Metric getMetric(String suffix, String... tags) {
.orElse(null);
}

@Override
protected boolean isClientMetric(MetricName metricId) {
return metricId.getKey().startsWith("feign.Client");
}

@Override
protected boolean isDecoderMetric(MetricName metricId) {
return metricId.getKey().startsWith("feign.codec.Decoder");
}

@Override
protected boolean doesMetricIncludeUri(MetricName metricId, String uri) {
return uri.equals(metricId.getTags().get("uri"));
}

}
9 changes: 5 additions & 4 deletions micrometer/src/main/java/feign/micrometer/MeteredClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,9 @@ protected void countResponseCode(Request request,
final RequestTemplate template = request.requestTemplate();
final Tags allTags = metricTagResolver
.tag(template.methodMetadata(), template.feignTarget(), e,
new Tag[] {Tag.of("http_status", String.valueOf(responseStatus)),
Tag.of("status_group", responseStatus / 100 + "xx"),
Tag.of("uri", template.path())})
Tag.of("http_status", String.valueOf(responseStatus)),
Tag.of("status_group", responseStatus / 100 + "xx"),
Tag.of("uri", template.methodMetadata().template().path()))
martinacat marked this conversation as resolved.
Show resolved Hide resolved
.and(extraTags);
meterRegistry.counter(
metricName.name("http_response_code"),
Expand All @@ -99,7 +99,8 @@ protected Timer createTimer(Request request,
Exception e) {
final RequestTemplate template = request.requestTemplate();
final Tags allTags = metricTagResolver
.tag(template.methodMetadata(), template.feignTarget(), e, Tag.of("uri", template.path()))
.tag(template.methodMetadata(), template.feignTarget(), e,
Tag.of("uri", template.methodMetadata().template().path()))
martinacat marked this conversation as resolved.
Show resolved Hide resolved
.and(extraTags(request, response, options, e));
return meterRegistry.timer(metricName.name(e), allTags);
}
Expand Down
5 changes: 3 additions & 2 deletions micrometer/src/main/java/feign/micrometer/MeteredDecoder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ protected Counter createExceptionCounter(Response response, Type type, Exception
final Tag[] extraTags = extraTags(response, type, e);
final RequestTemplate template = response.request().requestTemplate();
final Tags allTags = metricTagResolver.tag(template.methodMetadata(), template.feignTarget(),
Tag.of("uri", template.path()),
Tag.of("uri", template.methodMetadata().template().path()),
martinacat marked this conversation as resolved.
Show resolved Hide resolved
Tag.of("exception_name", e.getClass().getSimpleName()))
.and(extraTags);
return meterRegistry.counter(metricName.name("error_count"), allTags);
Expand All @@ -110,6 +110,7 @@ protected DistributionSummary createSummary(Response response, Type type) {
}

protected Tag[] extraTags(Response response, Type type, Exception e) {
return EMPTY_TAGS_ARRAY;
RequestTemplate template = response.request().requestTemplate();
return new Tag[] {Tag.of("uri", template.methodMetadata().template().path())};
}
}
101 changes: 86 additions & 15 deletions micrometer/src/test/java/feign/micrometer/AbstractMetricsTestBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,24 +13,18 @@
*/
package feign.micrometer;

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.junit.Before;
import org.junit.Test;
import feign.Capability;
import feign.Feign;
import feign.FeignException;
import feign.RequestLine;
import feign.*;
import feign.mock.HttpMethod;
import feign.mock.MockClient;
import feign.mock.MockTarget;
import org.junit.Before;
import org.junit.Test;
import java.util.Map;
import java.util.concurrent.atomic.AtomicReference;
import java.util.stream.Collectors;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.*;
import static org.junit.Assert.*;

public abstract class AbstractMetricsTestBase<MR, METRIC_ID, METRIC> {

Expand Down Expand Up @@ -149,5 +143,82 @@ public void shouldMetricCollectionWithCustomException() {
}


@Test
public void clientMetricsHaveUriLabel() {
final SimpleSource source = Feign.builder()
.client(new MockClient()
.ok(HttpMethod.GET, "/get", "1234567890abcde"))
.addCapability(createMetricCapability())
.target(new MockTarget<>(SimpleSource.class));

source.get("0x3456789");

final Map<METRIC_ID, METRIC> clientMetrics = getFeignMetrics().entrySet().stream()
.filter(entry -> isClientMetric(entry.getKey()))
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));

clientMetrics.keySet().forEach(metricId -> assertThat(
"Expect all Client metric names to include uri:" + metricId,
doesMetricIncludeUri(metricId, "/get")));
}

public interface SourceWithPathExpressions {

@RequestLine("GET /get/{id}")
String get(@Param("id") String id, String body);

}

@Test
public void clientMetricsHaveUriLabelWithPathExpression() {
final SourceWithPathExpressions source = Feign.builder()
.client(new MockClient()
.ok(HttpMethod.GET, "/get/123", "1234567890abcde"))
.addCapability(createMetricCapability())
.target(new MockTarget<>(SourceWithPathExpressions.class));

source.get("123", "0x3456789");

final Map<METRIC_ID, METRIC> clientMetrics = getFeignMetrics().entrySet().stream()
.filter(entry -> isClientMetric(entry.getKey()))
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));

clientMetrics.keySet().forEach(metricId -> assertThat(
"Expect all Client metric names to include uri as aggregated path expression:" + metricId,
doesMetricIncludeUri(metricId, "/get/{id}")));
}

@Test
public void decoderExceptionCounterHasUriLabelWithPathExpression() {
final AtomicReference<FeignException.NotFound> notFound = new AtomicReference<>();

final SourceWithPathExpressions source = Feign.builder()
.client(new MockClient()
.ok(HttpMethod.GET, "/get/123", "1234567890abcde"))
.decoder((response, type) -> {
notFound.set(new FeignException.NotFound("test", response.request(), null, null));
throw notFound.get();
})
.addCapability(createMetricCapability())
.target(new MockTarget<>(MicrometerCapabilityTest.SourceWithPathExpressions.class));

FeignException.NotFound thrown =
assertThrows(FeignException.NotFound.class, () -> source.get("123", "0x3456789"));
assertSame(notFound.get(), thrown);

final Map<METRIC_ID, METRIC> decoderMetrics = getFeignMetrics().entrySet().stream()
.filter(entry -> isDecoderMetric(entry.getKey()))
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));

decoderMetrics.keySet().forEach(metricId -> assertThat(
"Expect all Decoder metric names to include uri as aggregated path expression:" + metricId,
doesMetricIncludeUri(metricId, "/get/{id}")));
}

protected abstract boolean isClientMetric(METRIC_ID metricId);

protected abstract boolean isDecoderMetric(METRIC_ID metricId);

protected abstract boolean doesMetricIncludeUri(METRIC_ID metricId, String uri);

}
29 changes: 22 additions & 7 deletions micrometer/src/test/java/feign/micrometer/MicrometerCapabilityTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,24 +13,25 @@
*/
package feign.micrometer;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.function.Function;
import java.util.stream.Collectors;
import feign.Capability;
import feign.Util;
import io.micrometer.core.instrument.Meter;
import io.micrometer.core.instrument.Meter.Id;
import io.micrometer.core.instrument.MockClock;
import io.micrometer.core.instrument.simple.SimpleConfig;
import io.micrometer.core.instrument.simple.SimpleMeterRegistry;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.function.Function;
import java.util.stream.Collectors;

public class MicrometerCapabilityTest
extends AbstractMetricsTestBase<SimpleMeterRegistry, Id, Meter> {


@Override
protected SimpleMeterRegistry createMetricsRegistry() {
return new SimpleMeterRegistry(SimpleConfig.DEFAULT, new MockClock());
Expand Down Expand Up @@ -97,5 +98,19 @@ protected Meter getMetric(String suffix, String... tags) {
.orElse(null);
}

@Override
protected boolean isClientMetric(Id metricId) {
return metricId.getName().startsWith("feign.Client");
}

@Override
protected boolean isDecoderMetric(Id metricId) {
return metricId.getName().startsWith("feign.codec.Decoder");
}

@Override
protected boolean doesMetricIncludeUri(Id metricId, String uri) {
return uri.equals(metricId.getTag("uri"));
}

}