Skip to content

Commit 80d50cc

Browse files
author
Mateusz Rzeszutek
authored
HTTP metrics: don't use seconds unless only stable spec is used (open-telemetry#8721)
1 parent 04704c7 commit 80d50cc

File tree

8 files changed

+84
-75
lines changed

8 files changed

+84
-75
lines changed

‎instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HistogramAdviceUtil.java

Lines changed: 0 additions & 33 deletions
This file was deleted.

‎instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientMetrics.java

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,20 +7,20 @@
77

88
import static io.opentelemetry.instrumentation.api.instrumenter.http.HttpMessageBodySizeUtil.getHttpRequestBodySize;
99
import static io.opentelemetry.instrumentation.api.instrumenter.http.HttpMessageBodySizeUtil.getHttpResponseBodySize;
10+
import static io.opentelemetry.instrumentation.api.instrumenter.http.HttpMetricsUtil.createDurationHistogram;
11+
import static io.opentelemetry.instrumentation.api.instrumenter.http.HttpMetricsUtil.nanosToUnit;
1012
import static io.opentelemetry.instrumentation.api.instrumenter.http.TemporaryMetricsView.applyClientDurationAndSizeView;
1113
import static java.util.logging.Level.FINE;
1214

1315
import com.google.auto.value.AutoValue;
1416
import io.opentelemetry.api.common.Attributes;
1517
import io.opentelemetry.api.metrics.DoubleHistogram;
16-
import io.opentelemetry.api.metrics.DoubleHistogramBuilder;
1718
import io.opentelemetry.api.metrics.LongHistogram;
1819
import io.opentelemetry.api.metrics.Meter;
1920
import io.opentelemetry.context.Context;
2021
import io.opentelemetry.context.ContextKey;
2122
import io.opentelemetry.instrumentation.api.instrumenter.OperationListener;
2223
import io.opentelemetry.instrumentation.api.instrumenter.OperationMetrics;
23-
import java.util.concurrent.TimeUnit;
2424
import java.util.logging.Logger;
2525

2626
/**
@@ -30,8 +30,6 @@
3030
*/
3131
public final class HttpClientMetrics implements OperationListener {
3232

33-
private static final double NANOS_PER_S = TimeUnit.SECONDS.toNanos(1);
34-
3533
private static final ContextKey<State> HTTP_CLIENT_REQUEST_METRICS_STATE =
3634
ContextKey.named("http-client-request-metrics-state");
3735

@@ -51,13 +49,9 @@ public static OperationMetrics get() {
5149
private final LongHistogram responseSize;
5250

5351
private HttpClientMetrics(Meter meter) {
54-
DoubleHistogramBuilder durationBuilder =
55-
meter
56-
.histogramBuilder("http.client.duration")
57-
.setUnit("s")
58-
.setDescription("The duration of the outbound HTTP request");
59-
HistogramAdviceUtil.setHttpDurationBuckets(durationBuilder);
60-
duration = durationBuilder.build();
52+
duration =
53+
createDurationHistogram(
54+
meter, "http.client.duration", "The duration of the outbound HTTP request");
6155
requestSize =
6256
meter
6357
.histogramBuilder("http.client.request.size")
@@ -95,7 +89,7 @@ public void onEnd(Context context, Attributes endAttributes, long endNanos) {
9589
Attributes durationAndSizeAttributes =
9690
applyClientDurationAndSizeView(state.startAttributes(), endAttributes);
9791
duration.record(
98-
(endNanos - state.startTimeNanos()) / NANOS_PER_S, durationAndSizeAttributes, context);
92+
nanosToUnit(endNanos - state.startTimeNanos()), durationAndSizeAttributes, context);
9993

10094
Long requestBodySize = getHttpRequestBodySize(endAttributes, state.startAttributes());
10195
if (requestBodySize != null) {

‎instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpMetricsUtil.java

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
/*
2+
* Copyright The OpenTelemetry Authors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package io.opentelemetry.instrumentation.api.instrumenter.http;
7+
8+
import static java.util.Arrays.asList;
9+
import static java.util.Collections.unmodifiableList;
10+
11+
import io.opentelemetry.api.metrics.DoubleHistogram;
12+
import io.opentelemetry.api.metrics.DoubleHistogramBuilder;
13+
import io.opentelemetry.api.metrics.Meter;
14+
import io.opentelemetry.extension.incubator.metrics.ExtendedDoubleHistogramBuilder;
15+
import io.opentelemetry.instrumentation.api.internal.SemconvStability;
16+
import java.util.List;
17+
import java.util.concurrent.TimeUnit;
18+
19+
final class HttpMetricsUtil {
20+
21+
// we'll use the old unit if the old semconv is in use
22+
private static final boolean useSeconds =
23+
SemconvStability.emitStableHttpSemconv() && !SemconvStability.emitOldHttpSemconv();
24+
25+
static final List<Double> DURATION_SECONDS_BUCKETS =
26+
unmodifiableList(
27+
asList(
28+
0.0, 0.005, 0.01, 0.025, 0.05, 0.075, 0.1, 0.25, 0.5, 0.75, 1.0, 2.5, 5.0, 7.5,
29+
10.0));
30+
31+
private static final double NANOS_PER_MS = TimeUnit.MILLISECONDS.toNanos(1);
32+
private static final double NANOS_PER_S = TimeUnit.SECONDS.toNanos(1);
33+
34+
static DoubleHistogram createDurationHistogram(Meter meter, String name, String description) {
35+
DoubleHistogramBuilder durationBuilder =
36+
meter.histogramBuilder(name).setUnit(useSeconds ? "s" : "ms").setDescription(description);
37+
// don't set custom buckets if milliseconds are still used
38+
if (useSeconds && durationBuilder instanceof ExtendedDoubleHistogramBuilder) {
39+
((ExtendedDoubleHistogramBuilder) durationBuilder)
40+
.setAdvice(advice -> advice.setExplicitBucketBoundaries(DURATION_SECONDS_BUCKETS));
41+
}
42+
return durationBuilder.build();
43+
}
44+
45+
static double nanosToUnit(long durationNanos) {
46+
return durationNanos / (useSeconds ? NANOS_PER_S : NANOS_PER_MS);
47+
}
48+
49+
private HttpMetricsUtil() {}
50+
}

‎instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerMetrics.java

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,22 +7,22 @@
77

88
import static io.opentelemetry.instrumentation.api.instrumenter.http.HttpMessageBodySizeUtil.getHttpRequestBodySize;
99
import static io.opentelemetry.instrumentation.api.instrumenter.http.HttpMessageBodySizeUtil.getHttpResponseBodySize;
10+
import static io.opentelemetry.instrumentation.api.instrumenter.http.HttpMetricsUtil.createDurationHistogram;
11+
import static io.opentelemetry.instrumentation.api.instrumenter.http.HttpMetricsUtil.nanosToUnit;
1012
import static io.opentelemetry.instrumentation.api.instrumenter.http.TemporaryMetricsView.applyActiveRequestsView;
1113
import static io.opentelemetry.instrumentation.api.instrumenter.http.TemporaryMetricsView.applyServerDurationAndSizeView;
1214
import static java.util.logging.Level.FINE;
1315

1416
import com.google.auto.value.AutoValue;
1517
import io.opentelemetry.api.common.Attributes;
1618
import io.opentelemetry.api.metrics.DoubleHistogram;
17-
import io.opentelemetry.api.metrics.DoubleHistogramBuilder;
1819
import io.opentelemetry.api.metrics.LongHistogram;
1920
import io.opentelemetry.api.metrics.LongUpDownCounter;
2021
import io.opentelemetry.api.metrics.Meter;
2122
import io.opentelemetry.context.Context;
2223
import io.opentelemetry.context.ContextKey;
2324
import io.opentelemetry.instrumentation.api.instrumenter.OperationListener;
2425
import io.opentelemetry.instrumentation.api.instrumenter.OperationMetrics;
25-
import java.util.concurrent.TimeUnit;
2626
import java.util.logging.Logger;
2727

2828
/**
@@ -32,8 +32,6 @@
3232
*/
3333
public final class HttpServerMetrics implements OperationListener {
3434

35-
private static final double NANOS_PER_S = TimeUnit.SECONDS.toNanos(1);
36-
3735
private static final ContextKey<State> HTTP_SERVER_REQUEST_METRICS_STATE =
3836
ContextKey.named("http-server-request-metrics-state");
3937

@@ -60,14 +58,9 @@ private HttpServerMetrics(Meter meter) {
6058
.setUnit("{requests}")
6159
.setDescription("The number of concurrent HTTP requests that are currently in-flight")
6260
.build();
63-
64-
DoubleHistogramBuilder durationBuilder =
65-
meter
66-
.histogramBuilder("http.server.duration")
67-
.setUnit("s")
68-
.setDescription("The duration of the inbound HTTP request");
69-
HistogramAdviceUtil.setHttpDurationBuckets(durationBuilder);
70-
duration = durationBuilder.build();
61+
duration =
62+
createDurationHistogram(
63+
meter, "http.server.duration", "The duration of the inbound HTTP request");
7164
requestSize =
7265
meter
7366
.histogramBuilder("http.server.request.size")
@@ -110,7 +103,7 @@ public void onEnd(Context context, Attributes endAttributes, long endNanos) {
110103
Attributes durationAndSizeAttributes =
111104
applyServerDurationAndSizeView(state.startAttributes(), endAttributes);
112105
duration.record(
113-
(endNanos - state.startTimeNanos()) / NANOS_PER_S, durationAndSizeAttributes, context);
106+
nanosToUnit(endNanos - state.startTimeNanos()), durationAndSizeAttributes, context);
114107

115108
Long requestBodySize = getHttpRequestBodySize(endAttributes, state.startAttributes());
116109
if (requestBodySize != null) {

‎instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientMetricsTest.java

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,18 @@
1717
import io.opentelemetry.instrumentation.api.instrumenter.OperationListener;
1818
import io.opentelemetry.instrumentation.api.instrumenter.net.internal.NetAttributes;
1919
import io.opentelemetry.sdk.metrics.SdkMeterProvider;
20+
import io.opentelemetry.sdk.metrics.internal.aggregator.ExplicitBucketHistogramUtils;
2021
import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader;
2122
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
2223
import java.util.concurrent.TimeUnit;
2324
import org.junit.jupiter.api.Test;
2425

2526
class HttpClientMetricsTest {
2627

27-
static final double[] DURATION_BUCKETS =
28-
HistogramAdviceUtil.DURATION_SECONDS_BUCKETS.stream().mapToDouble(d -> d).toArray();
28+
static final double[] DEFAULT_BUCKETS =
29+
ExplicitBucketHistogramUtils.DEFAULT_HISTOGRAM_BUCKET_BOUNDARIES.stream()
30+
.mapToDouble(d -> d)
31+
.toArray();
2932

3033
@Test
3134
void collectsMetrics() {
@@ -82,13 +85,13 @@ void collectsMetrics() {
8285
metric ->
8386
assertThat(metric)
8487
.hasName("http.client.duration")
85-
.hasUnit("s")
88+
.hasUnit("ms")
8689
.hasHistogramSatisfying(
8790
histogram ->
8891
histogram.hasPointsSatisfying(
8992
point ->
9093
point
91-
.hasSum(0.15 /* seconds */)
94+
.hasSum(150 /* millis */)
9295
.hasAttributesSatisfying(
9396
equalTo(SemanticAttributes.HTTP_METHOD, "GET"),
9497
equalTo(SemanticAttributes.HTTP_STATUS_CODE, 200),
@@ -103,7 +106,7 @@ void collectsMetrics() {
103106
exemplar
104107
.hasTraceId("ff01020304050600ff0a0b0c0d0e0f00")
105108
.hasSpanId("090a0b0c0d0e0f00"))
106-
.hasBucketBoundaries(DURATION_BUCKETS))),
109+
.hasBucketBoundaries(DEFAULT_BUCKETS))),
107110
metric ->
108111
assertThat(metric)
109112
.hasName("http.client.request.size")
@@ -162,8 +165,7 @@ void collectsMetrics() {
162165
.hasName("http.client.duration")
163166
.hasHistogramSatisfying(
164167
histogram ->
165-
histogram.hasPointsSatisfying(
166-
point -> point.hasSum(0.3 /* seconds */))),
168+
histogram.hasPointsSatisfying(point -> point.hasSum(300 /* millis */))),
167169
metric ->
168170
assertThat(metric)
169171
.hasName("http.client.request.size")

‎instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerMetricsTest.java

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,18 @@
1818
import io.opentelemetry.instrumentation.api.instrumenter.OperationListener;
1919
import io.opentelemetry.instrumentation.api.instrumenter.net.internal.NetAttributes;
2020
import io.opentelemetry.sdk.metrics.SdkMeterProvider;
21+
import io.opentelemetry.sdk.metrics.internal.aggregator.ExplicitBucketHistogramUtils;
2122
import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader;
2223
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
2324
import java.util.concurrent.TimeUnit;
2425
import org.junit.jupiter.api.Test;
2526

2627
class HttpServerMetricsTest {
2728

28-
static final double[] DURATION_BUCKETS =
29-
HistogramAdviceUtil.DURATION_SECONDS_BUCKETS.stream().mapToDouble(d -> d).toArray();
29+
static final double[] DEFAULT_BUCKETS =
30+
ExplicitBucketHistogramUtils.DEFAULT_HISTOGRAM_BUCKET_BOUNDARIES.stream()
31+
.mapToDouble(d -> d)
32+
.toArray();
3033

3134
@Test
3235
void collectsMetrics() {
@@ -152,13 +155,13 @@ void collectsMetrics() {
152155
metric ->
153156
assertThat(metric)
154157
.hasName("http.server.duration")
155-
.hasUnit("s")
158+
.hasUnit("ms")
156159
.hasHistogramSatisfying(
157160
histogram ->
158161
histogram.hasPointsSatisfying(
159162
point ->
160163
point
161-
.hasSum(0.15 /* seconds */)
164+
.hasSum(150 /* millis */)
162165
.hasAttributesSatisfying(
163166
equalTo(SemanticAttributes.HTTP_METHOD, "GET"),
164167
equalTo(SemanticAttributes.HTTP_STATUS_CODE, 200),
@@ -172,7 +175,7 @@ void collectsMetrics() {
172175
exemplar
173176
.hasTraceId(spanContext1.getTraceId())
174177
.hasSpanId(spanContext1.getSpanId()))
175-
.hasBucketBoundaries(DURATION_BUCKETS))),
178+
.hasBucketBoundaries(DEFAULT_BUCKETS))),
176179
metric ->
177180
assertThat(metric)
178181
.hasName("http.server.request.size")
@@ -246,7 +249,7 @@ void collectsMetrics() {
246249
histogram.hasPointsSatisfying(
247250
point ->
248251
point
249-
.hasSum(0.3 /* seconds */)
252+
.hasSum(300 /* millis */)
250253
.hasExemplarsSatisfying(
251254
exemplar ->
252255
exemplar
@@ -308,13 +311,13 @@ void collectsHttpRouteFromEndAttributes() {
308311
metric ->
309312
assertThat(metric)
310313
.hasName("http.server.duration")
311-
.hasUnit("s")
314+
.hasUnit("ms")
312315
.hasHistogramSatisfying(
313316
histogram ->
314317
histogram.hasPointsSatisfying(
315318
point ->
316319
point
317-
.hasSum(0.100 /* seconds */)
320+
.hasSum(100 /* millis */)
318321
.hasAttributesSatisfying(
319322
equalTo(SemanticAttributes.HTTP_SCHEME, "https"),
320323
equalTo(SemanticAttributes.NET_HOST_NAME, "host"),

‎instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientMetricsStableSemconvTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
class HttpClientMetricsStableSemconvTest {
2626

2727
static final double[] DURATION_BUCKETS =
28-
HistogramAdviceUtil.DURATION_SECONDS_BUCKETS.stream().mapToDouble(d -> d).toArray();
28+
HttpMetricsUtil.DURATION_SECONDS_BUCKETS.stream().mapToDouble(d -> d).toArray();
2929

3030
@Test
3131
void collectsMetrics() {

‎instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerMetricsStableSemconvTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
class HttpServerMetricsStableSemconvTest {
2727

2828
static final double[] DURATION_BUCKETS =
29-
HistogramAdviceUtil.DURATION_SECONDS_BUCKETS.stream().mapToDouble(d -> d).toArray();
29+
HttpMetricsUtil.DURATION_SECONDS_BUCKETS.stream().mapToDouble(d -> d).toArray();
3030

3131
@Test
3232
void collectsMetrics() {

0 commit comments

Comments
 (0)