Fix metrics cardinality (#3972)

* Fix metrics cardinality * Add test
open-telemetry · Aug 27, 2021 · 9a31559 · 9a31559
1 parent bd576f2
commit 9a31559
Show file tree

Hide file tree

Showing 5 changed files with 134 additions and 7 deletions.
diff --git a/...c/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientMetrics.java b/...c/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientMetrics.java
@@ -5,6 +5,8 @@
 
 package io.opentelemetry.instrumentation.api.instrumenter.http;
 
+import static io.opentelemetry.instrumentation.api.instrumenter.http.TemporaryMetricsView.applyDurationView;
+
 import com.google.auto.value.AutoValue;
 import io.opentelemetry.api.common.Attributes;
 import io.opentelemetry.api.metrics.DoubleHistogram;
@@ -75,7 +77,8 @@ public void end(Context context, Attributes responseAttributes) {
       return;
     }
     duration.record(
-        (System.nanoTime() - state.startTimeNanos()) / NANOS_PER_MS, state.startAttributes());
+        (System.nanoTime() - state.startTimeNanos()) / NANOS_PER_MS,
+        applyDurationView(state.startAttributes()));
   }
 
   @AutoValue

diff --git a/...c/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerMetrics.java b/...c/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerMetrics.java
@@ -5,6 +5,9 @@
 
 package io.opentelemetry.instrumentation.api.instrumenter.http;
 
+import static io.opentelemetry.instrumentation.api.instrumenter.http.TemporaryMetricsView.applyActiveRequestsView;
+import static io.opentelemetry.instrumentation.api.instrumenter.http.TemporaryMetricsView.applyDurationView;
+
 import com.google.auto.value.AutoValue;
 import io.opentelemetry.api.common.Attributes;
 import io.opentelemetry.api.metrics.DoubleHistogram;
@@ -69,7 +72,7 @@ private HttpServerMetrics(Meter meter) {
   @Override
   public Context start(Context context, Attributes requestAttributes) {
     long startTimeNanos = System.nanoTime();
-    activeRequests.add(1, requestAttributes);
+    activeRequests.add(1, applyActiveRequestsView(requestAttributes));
 
     return context.with(
         HTTP_SERVER_REQUEST_METRICS_STATE,
@@ -84,9 +87,10 @@ public void end(Context context, Attributes responseAttributes) {
           "No state present when ending context {}. Cannot reset HTTP request metrics.", context);
       return;
     }
-    activeRequests.add(-1, state.startAttributes());
+    activeRequests.add(-1, applyActiveRequestsView(state.startAttributes()));
     duration.record(
-        (System.nanoTime() - state.startTimeNanos()) / NANOS_PER_MS, state.startAttributes());
+        (System.nanoTime() - state.startTimeNanos()) / NANOS_PER_MS,
+        applyDurationView(state.startAttributes()));
   }
 
   @AutoValue

diff --git a/...ain/java/io/opentelemetry/instrumentation/api/instrumenter/http/TemporaryMetricsView.java b/...ain/java/io/opentelemetry/instrumentation/api/instrumenter/http/TemporaryMetricsView.java
@@ -0,0 +1,77 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+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.semconv.trace.attributes.SemanticAttributes;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.function.BiConsumer;
+
+// this is temporary, see
+// https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/3962#issuecomment-906606325
+@SuppressWarnings("rawtypes")
+final class TemporaryMetricsView {
+
+  private static final Set<AttributeKey> durationView = buildDurationView();
+
+  private static final Set<AttributeKey> activeRequestsView = buildActiveRequestsView();
+
+  private static Set<AttributeKey> buildDurationView() {
+    // the list of included metrics is from
+    // https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/semantic_conventions/http-metrics.md#attributes
+    Set<AttributeKey> view = new HashSet<>();
+    view.add(SemanticAttributes.HTTP_METHOD);
+    view.add(SemanticAttributes.HTTP_HOST);
+    view.add(SemanticAttributes.HTTP_SCHEME);
+    view.add(SemanticAttributes.HTTP_STATUS_CODE);
+    view.add(SemanticAttributes.HTTP_FLAVOR);
+    view.add(SemanticAttributes.NET_PEER_NAME);
+    view.add(SemanticAttributes.NET_PEER_PORT);
+    view.add(SemanticAttributes.NET_PEER_IP);
+    view.add(SemanticAttributes.HTTP_SERVER_NAME);
+    view.add(SemanticAttributes.NET_HOST_NAME);
+    view.add(SemanticAttributes.NET_HOST_PORT);
+    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
+    Set<AttributeKey> view = new HashSet<>();
+    view.add(SemanticAttributes.HTTP_METHOD);
+    view.add(SemanticAttributes.HTTP_HOST);
+    view.add(SemanticAttributes.HTTP_SCHEME);
+    view.add(SemanticAttributes.HTTP_FLAVOR);
+    view.add(SemanticAttributes.HTTP_SERVER_NAME);
+    return view;
+  }
+
+  static Attributes applyDurationView(Attributes attributes) {
+    return applyView(attributes, durationView);
+  }
+
+  static Attributes applyActiveRequestsView(Attributes attributes) {
+    return applyView(attributes, activeRequestsView);
+  }
+
+  @SuppressWarnings("unchecked")
+  private static Attributes applyView(Attributes attributes, Set<AttributeKey> view) {
+    AttributesBuilder filtered = Attributes.builder();
+    attributes.forEach(
+        (BiConsumer<AttributeKey, Object>)
+            (key, value) -> {
+              if (view.contains(key)) {
+                filtered.put(key, value);
+              }
+            });
+    return filtered.build();
+  }
+
+  private TemporaryMetricsView() {}
+}
diff --git a/...st/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerMetricsTest.java b/...st/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerMetricsTest.java
@@ -63,9 +63,7 @@ void collectsMetrics() {
                                 .containsOnly(
                                     attributeEntry("http.host", "host"),
                                     attributeEntry("http.method", "GET"),
-                                    attributeEntry("http.scheme", "https"),
-                                    attributeEntry("net.host.name", "localhost"),
-                                    attributeEntry("net.host.port", 1234L))));
+                                    attributeEntry("http.scheme", "https"))));
 
     Context context2 = listener.start(Context.current(), requestAttributes);
 

diff --git a/...java/io/opentelemetry/instrumentation/api/instrumenter/http/TemporaryMetricsViewTest.java b/...java/io/opentelemetry/instrumentation/api/instrumenter/http/TemporaryMetricsViewTest.java
@@ -0,0 +1,45 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.api.instrumenter.http;
+
+import static io.opentelemetry.instrumentation.api.instrumenter.http.TemporaryMetricsView.applyActiveRequestsView;
+import static io.opentelemetry.instrumentation.api.instrumenter.http.TemporaryMetricsView.applyDurationView;
+import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.attributeEntry;
+
+import io.opentelemetry.api.common.Attributes;
+import io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions;
+import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
+import org.junit.jupiter.api.Test;
+
+public class TemporaryMetricsViewTest {
+
+  @Test
+  public void shouldApplyDurationView() {
+    Attributes attributes =
+        Attributes.builder()
+            .put(SemanticAttributes.HTTP_METHOD, "GET")
+            .put(SemanticAttributes.HTTP_URL, "http://somehost/high/cardinality/12345")
+            .put(SemanticAttributes.NET_PEER_NAME, "somehost")
+            .build();
+
+    OpenTelemetryAssertions.assertThat(applyDurationView(attributes))
+        .containsOnly(
+            attributeEntry("http.method", "GET"), attributeEntry("net.peer.name", "somehost"));
+  }
+
+  @Test
+  public void shouldApplyActiveRequestsView() {
+    Attributes attributes =
+        Attributes.builder()
+            .put(SemanticAttributes.HTTP_METHOD, "GET")
+            .put(SemanticAttributes.HTTP_URL, "/high/cardinality/12345")
+            .put(SemanticAttributes.NET_PEER_NAME, "somehost")
+            .build();
+
+    OpenTelemetryAssertions.assertThat(applyActiveRequestsView(attributes))
+        .containsOnly(attributeEntry("http.method", "GET"));
+  }
+}