Use `http.route` in `HttpServerMetrics` (#5266)

* Use http.route in HttpServerMetrics

* remove http.target fallback
This commit is contained in:
Mateusz Rzeszutek 2022-01-31 19:33:59 +01:00 committed by GitHub
parent c54a823bae
commit 666c22eb2a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 65 additions and 70 deletions

View File

@ -146,7 +146,6 @@ public final class HttpRouteHolder {
return name.length() > routeLength;
}
// TODO: use that in HttpServerMetrics
/**
* Returns the {@code http.route} attribute value that's stored in the passed {@code context}, or
* null if it was not set before.

View File

@ -18,7 +18,9 @@ import io.opentelemetry.context.ContextKey;
import io.opentelemetry.instrumentation.api.annotations.UnstableApi;
import io.opentelemetry.instrumentation.api.instrumenter.RequestListener;
import io.opentelemetry.instrumentation.api.instrumenter.RequestMetrics;
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
import java.util.concurrent.TimeUnit;
import java.util.function.Function;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@ -49,8 +51,14 @@ public final class HttpServerMetrics implements RequestListener {
private final LongUpDownCounter activeRequests;
private final DoubleHistogram duration;
private final Function<Context, String> httpRouteHolderGetter;
private HttpServerMetrics(Meter meter) {
this(meter, HttpRouteHolder::getRoute);
}
// visible for tests
HttpServerMetrics(Meter meter, Function<Context, String> httpRouteHolderGetter) {
activeRequests =
meter
.upDownCounterBuilder("http.server.active_requests")
@ -64,6 +72,8 @@ public final class HttpServerMetrics implements RequestListener {
.setUnit("ms")
.setDescription("The duration of the inbound HTTP request")
.build();
this.httpRouteHolderGetter = httpRouteHolderGetter;
}
@Override
@ -86,10 +96,19 @@ public final class HttpServerMetrics implements RequestListener {
activeRequests.add(-1, applyActiveRequestsView(state.startAttributes()));
duration.record(
(endNanos - state.startTimeNanos()) / NANOS_PER_MS,
applyServerDurationView(state.startAttributes(), endAttributes),
applyServerDurationView(state.startAttributes(), addHttpRoute(context, endAttributes)),
context);
}
// TODO: the http.route should be extracted by the AttributesExtractor, HttpServerMetrics should
// not access the context to get it
private Attributes addHttpRoute(Context context, Attributes endAttributes) {
String route = httpRouteHolderGetter.apply(context);
return route == null
? endAttributes
: endAttributes.toBuilder().put(SemanticAttributes.HTTP_ROUTE, route).build();
}
@AutoValue
abstract static class State {

View File

@ -8,7 +8,6 @@ package io.opentelemetry.instrumentation.api.instrumenter.http;
import io.opentelemetry.api.common.AttributeKey;
import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.api.common.AttributesBuilder;
import io.opentelemetry.instrumentation.api.config.Config;
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
import java.util.HashSet;
import java.util.Set;
@ -19,19 +18,9 @@ import java.util.function.BiConsumer;
@SuppressWarnings("rawtypes")
final class TemporaryMetricsView {
// TODO (trask) remove this once http.route is captured consistently
//
// this is not enabled by default because it falls back to http.target (which can be high
// cardinality) when http.route is not available
private static final boolean USE_HTTP_TARGET_FALLBACK =
Config.get()
.getBoolean("otel.instrumentation.metrics.experimental.use-http-target-fallback", false);
private static final Set<AttributeKey> durationAlwaysInclude = buildDurationAlwaysInclude();
private static final Set<AttributeKey> durationClientView = buildDurationClientView();
private static final Set<AttributeKey> durationServerView = buildDurationServerView();
private static final Set<AttributeKey> durationServerFallbackView =
buildDurationServerFallbackView();
private static final Set<AttributeKey> activeRequestsView = buildActiveRequestsView();
private static Set<AttributeKey> buildDurationAlwaysInclude() {
@ -65,19 +54,6 @@ final class TemporaryMetricsView {
return view;
}
private static Set<AttributeKey> buildDurationServerFallbackView() {
// We pull identifying attributes according to:
// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/semantic_conventions/http-metrics.md#attribute-alternatives
// With the following caveat:
// - we always rely on http.route + http.host in this repository.
// - we prefer http.route (which is scrubbed) over http.target (which is not scrubbed).
Set<AttributeKey> view = new HashSet<>(durationAlwaysInclude);
view.add(SemanticAttributes.HTTP_SCHEME);
view.add(SemanticAttributes.HTTP_HOST);
view.add(SemanticAttributes.HTTP_TARGET);
return view;
}
private static Set<AttributeKey> buildActiveRequestsView() {
// the list of included metrics is from
// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/semantic_conventions/http-metrics.md#attributes
@ -97,21 +73,10 @@ final class TemporaryMetricsView {
return filtered.build();
}
private static <T> boolean containsAttribute(
AttributeKey<T> key, Attributes startAttributes, Attributes endAttributes) {
return startAttributes.get(key) != null || endAttributes.get(key) != null;
}
static Attributes applyServerDurationView(Attributes startAttributes, Attributes endAttributes) {
Set<AttributeKey> fullSet = durationServerView;
// Use http.target when http.route is not available.
if (USE_HTTP_TARGET_FALLBACK
&& !containsAttribute(SemanticAttributes.HTTP_ROUTE, startAttributes, endAttributes)) {
fullSet = durationServerFallbackView;
}
AttributesBuilder filtered = Attributes.builder();
applyView(filtered, startAttributes, fullSet);
applyView(filtered, endAttributes, fullSet);
applyView(filtered, startAttributes, durationServerView);
applyView(filtered, endAttributes, durationServerView);
return filtered.build();
}

View File

@ -156,6 +156,49 @@ class HttpServerMetricsTest {
.satisfiesExactly(point -> assertThat(point).hasSum(300 /* millis */)));
}
@Test
void collectsHttpRouteFromContext() {
// given
InMemoryMetricReader metricReader = InMemoryMetricReader.create();
SdkMeterProvider meterProvider =
SdkMeterProvider.builder()
.registerMetricReader(metricReader)
.setMinimumCollectionInterval(Duration.ZERO)
.build();
RequestListener listener = new HttpServerMetrics(meterProvider.get("test"), c -> "/test/{id}");
Attributes requestAttributes =
Attributes.builder().put("http.host", "host").put("http.scheme", "https").build();
Attributes responseAttributes = Attributes.empty();
Context parentContext = Context.root();
// when
Context context = listener.start(parentContext, requestAttributes, nanos(100));
listener.end(context, responseAttributes, nanos(200));
// then
assertThat(metricReader.collectAllMetrics())
.anySatisfy(
metric ->
assertThat(metric)
.hasName("http.server.duration")
.hasUnit("ms")
.hasDoubleHistogram()
.points()
.satisfiesExactly(
point ->
assertThat(point)
.hasSum(100 /* millis */)
.attributes()
.containsOnly(
attributeEntry("http.scheme", "https"),
attributeEntry("http.host", "host"),
attributeEntry("http.route", "/test/{id}"))));
}
private static long nanos(int millis) {
return TimeUnit.MILLISECONDS.toNanos(millis);
}

View File

@ -109,37 +109,6 @@ public class TemporaryMetricsViewTest {
attributeEntry(SemanticAttributes.HTTP_STATUS_CODE.getKey(), 500));
}
@Test
public void shouldApplyServerDurationView_withTarget() {
Attributes startAttributes =
Attributes.builder()
.put(SemanticAttributes.HTTP_METHOD, "GET")
.put(SemanticAttributes.HTTP_URL, "https://somehost/high/cardinality/12345")
.put(SemanticAttributes.HTTP_SCHEME, "https")
.put(SemanticAttributes.HTTP_HOST, "somehost")
.put(SemanticAttributes.HTTP_SERVER_NAME, "somehost")
.put(
SemanticAttributes.HTTP_TARGET, "/somehost/high/cardinality/12345;jsessionId=12145")
.put(SemanticAttributes.NET_HOST_NAME, "somehost")
.put(SemanticAttributes.NET_HOST_PORT, 443)
.build();
Attributes endAttributes =
Attributes.builder()
.put(SemanticAttributes.HTTP_STATUS_CODE, 500)
.put(SemanticAttributes.NET_PEER_NAME, "somehost2")
.put(SemanticAttributes.NET_PEER_IP, "127.0.0.1")
.put(SemanticAttributes.NET_PEER_PORT, 443)
.build();
OpenTelemetryAssertions.assertThat(applyServerDurationView(startAttributes, endAttributes))
.containsOnly(
attributeEntry(SemanticAttributes.HTTP_SCHEME.getKey(), "https"),
attributeEntry(SemanticAttributes.HTTP_HOST.getKey(), "somehost"),
attributeEntry(SemanticAttributes.HTTP_METHOD.getKey(), "GET"),
attributeEntry(SemanticAttributes.HTTP_STATUS_CODE.getKey(), 500));
}
@Test
public void shouldApplyActiveRequestsView() {
Attributes attributes =