fix-unit-test

open-telemetry · Sep 13, 2024 · 41b9c6e · 41b9c6e
1 parent 593db23
commit 41b9c6e
Show file tree

Hide file tree

Showing 23 changed files with 158 additions and 122 deletions.
diff --git a/docs/apidiffs/current_vs_latest/opentelemetry-instrumentation-api.txt b/docs/apidiffs/current_vs_latest/opentelemetry-instrumentation-api.txt
@@ -1,2 +1,2 @@
-Comparing source compatibility of opentelemetry-instrumentation-api-2.7.0-SNAPSHOT.jar against opentelemetry-instrumentation-api-2.6.0.jar
+Comparing source compatibility of opentelemetry-instrumentation-api-2.7.0-SNAPSHOT.jar against opentelemetry-instrumentation-api-2.7.0.jar
 No changes.
diff --git a/...ain/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcClientMetrics.java b/...ain/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcClientMetrics.java
@@ -73,9 +73,7 @@ public void onEnd(Context context, Attributes endAttributes, long endNanos) {
       return;
     }
     clientDurationHistogram.record(
-        (endNanos - state.startTimeNanos()) / NANOS_PER_MS,
-        state.startAttributes().toBuilder().putAll(endAttributes).build(),
-        context);
+        (endNanos - state.startTimeNanos()) / NANOS_PER_MS, endAttributes, context);
   }
 
   @AutoValue

diff --git a/...opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcCommonAttributesExtractor.java b/...opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcCommonAttributesExtractor.java
@@ -28,11 +28,7 @@ abstract class RpcCommonAttributesExtractor<REQUEST, RESPONSE>
   }
 
   @Override
-  public final void onStart(AttributesBuilder attributes, Context parentContext, REQUEST request) {
-    internalSet(attributes, RPC_SYSTEM, getter.getSystem(request));
-    internalSet(attributes, RPC_SERVICE, getter.getService(request));
-    internalSet(attributes, RPC_METHOD, getter.getMethod(request));
-  }
+  public final void onStart(AttributesBuilder attributes, Context parentContext, REQUEST request) {}
 
   @Override
   public final void onEnd(
@@ -42,5 +38,8 @@ public final void onEnd(
       @Nullable RESPONSE response,
       @Nullable Throwable error) {
     // No response attributes
+    internalSet(attributes, RPC_SYSTEM, getter.getSystem(request));
+    internalSet(attributes, RPC_SERVICE, getter.getService(request));
+    internalSet(attributes, RPC_METHOD, getter.getMethod(request));
   }
 }
diff --git a/...ain/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcServerMetrics.java b/...ain/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcServerMetrics.java
@@ -73,9 +73,7 @@ public void onEnd(Context context, Attributes endAttributes, long endNanos) {
       return;
     }
     serverDurationHistogram.record(
-        (endNanos - state.startTimeNanos()) / NANOS_PER_MS,
-        state.startAttributes().toBuilder().putAll(endAttributes).build(),
-        context);
+        (endNanos - state.startTimeNanos()) / NANOS_PER_MS, endAttributes, context);
   }
 
   @AutoValue

diff --git a/...ion-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/Instrumenter.java b/...ion-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/Instrumenter.java
@@ -250,6 +250,7 @@ private void doEnd(
     if (operationListeners.length != 0) {
       long endNanos = getNanos(endTime);
       for (int i = operationListeners.length - 1; i >= 0; i--) {
+
         operationListeners[i].onEnd(context, attributes, endNanos);
       }
     }

diff --git a/...java/io/opentelemetry/instrumentation/api/semconv/http/HttpClientAttributesExtractor.java b/...java/io/opentelemetry/instrumentation/api/semconv/http/HttpClientAttributesExtractor.java
@@ -69,17 +69,7 @@ public static <REQUEST, RESPONSE> HttpClientAttributesExtractorBuilder<REQUEST,
 
   @Override
   public void onStart(AttributesBuilder attributes, Context parentContext, REQUEST request) {
-    super.onStart(attributes, parentContext, request);
 
-    internalServerExtractor.onStart(attributes, request);
-
-    String fullUrl = stripSensitiveData(getter.getUrlFull(request));
-    internalSet(attributes, UrlAttributes.URL_FULL, fullUrl);
-
-    int resendCount = resendCountIncrementer.applyAsInt(parentContext);
-    if (resendCount > 0) {
-      attributes.put(HttpAttributes.HTTP_REQUEST_RESEND_COUNT, resendCount);
-    }
   }
 
   @Override
@@ -90,7 +80,15 @@ public void onEnd(
       @Nullable RESPONSE response,
       @Nullable Throwable error) {
     super.onEnd(attributes, context, request, response, error);
+    internalServerExtractor.onEnd(attributes, request);
 
+    String fullUrl = stripSensitiveData(getter.getUrlFull(request));
+    internalSet(attributes, UrlAttributes.URL_FULL, fullUrl);
+
+    int resendCount = resendCountIncrementer.applyAsInt(context);
+    if (resendCount > 0) {
+      attributes.put(HttpAttributes.HTTP_REQUEST_RESEND_COUNT, resendCount);
+    }
     internalNetworkExtractor.onEnd(attributes, request, response);
   }
 

diff --git a/...pi/src/main/java/io/opentelemetry/instrumentation/api/semconv/http/HttpClientMetrics.java b/...pi/src/main/java/io/opentelemetry/instrumentation/api/semconv/http/HttpClientMetrics.java
@@ -78,9 +78,7 @@ public void onEnd(Context context, Attributes endAttributes, long endNanos) {
       return;
     }
 
-    Attributes attributes = state.startAttributes().toBuilder().putAll(endAttributes).build();
-
-    duration.record((endNanos - state.startTimeNanos()) / NANOS_PER_S, attributes, context);
+    duration.record((endNanos - state.startTimeNanos()) / NANOS_PER_S, endAttributes, context);
   }
 
   @AutoValue

diff --git a/...java/io/opentelemetry/instrumentation/api/semconv/http/HttpCommonAttributesExtractor.java b/...java/io/opentelemetry/instrumentation/api/semconv/http/HttpCommonAttributesExtractor.java
@@ -58,6 +58,16 @@ abstract class HttpCommonAttributesExtractor<
 
   @Override
   public void onStart(AttributesBuilder attributes, Context parentContext, REQUEST request) {
+
+  }
+
+  @Override
+  public void onEnd(
+      AttributesBuilder attributes,
+      Context context,
+      REQUEST request,
+      @Nullable RESPONSE response,
+      @Nullable Throwable error) {
     String method = getter.getHttpRequestMethod(request);
     if (method == null || knownMethods.contains(method)) {
       internalSet(attributes, HttpAttributes.HTTP_REQUEST_METHOD, method);
@@ -72,15 +82,6 @@ public void onStart(AttributesBuilder attributes, Context parentContext, REQUEST
         internalSet(attributes, requestAttributeKey(name), values);
       }
     }
-  }
-
-  @Override
-  public void onEnd(
-      AttributesBuilder attributes,
-      Context context,
-      REQUEST request,
-      @Nullable RESPONSE response,
-      @Nullable Throwable error) {
 
     Integer statusCode = null;
     if (response != null) {

diff --git a/...java/io/opentelemetry/instrumentation/api/semconv/http/HttpServerAttributesExtractor.java b/...java/io/opentelemetry/instrumentation/api/semconv/http/HttpServerAttributesExtractor.java
@@ -75,14 +75,7 @@ public static <REQUEST, RESPONSE> HttpServerAttributesExtractorBuilder<REQUEST,
 
   @Override
   public void onStart(AttributesBuilder attributes, Context parentContext, REQUEST request) {
-    super.onStart(attributes, parentContext, request);
 
-    internalUrlExtractor.onStart(attributes, request);
-    internalServerExtractor.onStart(attributes, request);
-    internalClientExtractor.onStart(attributes, request);
-
-    internalSet(attributes, HttpAttributes.HTTP_ROUTE, getter.getHttpRoute(request));
-    internalSet(attributes, UserAgentAttributes.USER_AGENT_ORIGINAL, userAgent(request));
   }
 
   @Override
@@ -94,7 +87,13 @@ public void onEnd(
       @Nullable Throwable error) {
 
     super.onEnd(attributes, context, request, response, error);
+    internalSet(attributes, UserAgentAttributes.USER_AGENT_ORIGINAL, userAgent(request));
+
+    internalUrlExtractor.onEnd(attributes, request);
+    internalServerExtractor.onEnd(attributes, request);
+    internalClientExtractor.onEnd(attributes, request);
 
+    internalSet(attributes, HttpAttributes.HTTP_ROUTE, getter.getHttpRoute(request));
     internalNetworkExtractor.onEnd(attributes, request, response);
 
     internalSet(attributes, HttpAttributes.HTTP_ROUTE, httpRouteGetter.apply(context));

diff --git a/...pi/src/main/java/io/opentelemetry/instrumentation/api/semconv/http/HttpServerMetrics.java b/...pi/src/main/java/io/opentelemetry/instrumentation/api/semconv/http/HttpServerMetrics.java
@@ -78,9 +78,7 @@ public void onEnd(Context context, Attributes endAttributes, long endNanos) {
       return;
     }
 
-    Attributes attributes = state.startAttributes().toBuilder().putAll(endAttributes).build();
-
-    duration.record((endNanos - state.startTimeNanos()) / NANOS_PER_S, attributes, context);
+    duration.record((endNanos - state.startTimeNanos()) / NANOS_PER_S, endAttributes, context);
   }
 
   @AutoValue

diff --git a/.../java/io/opentelemetry/instrumentation/api/semconv/network/ClientAttributesExtractor.java b/.../java/io/opentelemetry/instrumentation/api/semconv/network/ClientAttributesExtractor.java
@@ -45,7 +45,6 @@ public static <REQUEST, RESPONSE> ClientAttributesExtractor<REQUEST, RESPONSE> c
 
   @Override
   public void onStart(AttributesBuilder attributes, Context parentContext, REQUEST request) {
-    internalExtractor.onStart(attributes, request);
   }
 
   @Override
@@ -54,5 +53,7 @@ public void onEnd(
       Context context,
       REQUEST request,
       @Nullable RESPONSE response,
-      @Nullable Throwable error) {}
+      @Nullable Throwable error) {
+    internalExtractor.onEnd(attributes, request);
+  }
 }
diff --git a/.../java/io/opentelemetry/instrumentation/api/semconv/network/ServerAttributesExtractor.java b/.../java/io/opentelemetry/instrumentation/api/semconv/network/ServerAttributesExtractor.java
@@ -44,7 +44,6 @@ public static <REQUEST, RESPONSE> ServerAttributesExtractor<REQUEST, RESPONSE> c
 
   @Override
   public void onStart(AttributesBuilder attributes, Context parentContext, REQUEST request) {
-    internalExtractor.onStart(attributes, request);
   }
 
   @Override
@@ -53,5 +52,7 @@ public void onEnd(
       Context context,
       REQUEST request,
       @Nullable RESPONSE response,
-      @Nullable Throwable error) {}
+      @Nullable Throwable error) {
+    internalExtractor.onEnd(attributes, request);
+  }
 }
diff --git a/...metry/instrumentation/api/semconv/network/internal/InternalClientAttributesExtractor.java b/...metry/instrumentation/api/semconv/network/internal/InternalClientAttributesExtractor.java
@@ -25,7 +25,7 @@ public InternalClientAttributesExtractor(
     this.capturePort = capturePort;
   }
 
-  public void onStart(AttributesBuilder attributes, REQUEST request) {
+  public void onEnd(AttributesBuilder attributes, REQUEST request) {
     AddressAndPort clientAddressAndPort = addressAndPortExtractor.extract(request);
 
     if (clientAddressAndPort.address != null) {

diff --git a/...metry/instrumentation/api/semconv/network/internal/InternalServerAttributesExtractor.java b/...metry/instrumentation/api/semconv/network/internal/InternalServerAttributesExtractor.java
@@ -23,7 +23,7 @@ public InternalServerAttributesExtractor(
     this.addressAndPortExtractor = addressAndPortExtractor;
   }
 
-  public void onStart(AttributesBuilder attributes, REQUEST request) {
+  public void onEnd(AttributesBuilder attributes, REQUEST request) {
     AddressAndPort serverAddressAndPort = addressAndPortExtractor.extract(request);
 
     if (serverAddressAndPort.address != null) {

diff --git a/...rc/main/java/io/opentelemetry/instrumentation/api/semconv/url/UrlAttributesExtractor.java b/...rc/main/java/io/opentelemetry/instrumentation/api/semconv/url/UrlAttributesExtractor.java
@@ -43,7 +43,6 @@ public static <REQUEST, RESPONSE> UrlAttributesExtractor<REQUEST, RESPONSE> crea
 
   @Override
   public void onStart(AttributesBuilder attributes, Context parentContext, REQUEST request) {
-    internalExtractor.onStart(attributes, request);
   }
 
   @Override
@@ -52,5 +51,7 @@ public void onEnd(
       Context context,
       REQUEST request,
       @Nullable RESPONSE response,
-      @Nullable Throwable error) {}
+      @Nullable Throwable error) {
+    internalExtractor.onEnd(attributes, request);
+  }
 }
diff --git a/...pentelemetry/instrumentation/api/semconv/url/internal/InternalUrlAttributesExtractor.java b/...pentelemetry/instrumentation/api/semconv/url/internal/InternalUrlAttributesExtractor.java
@@ -27,7 +27,7 @@ public InternalUrlAttributesExtractor(
     this.alternateSchemeProvider = alternateSchemeProvider;
   }
 
-  public void onStart(AttributesBuilder attributes, REQUEST request) {
+  public void onEnd(AttributesBuilder attributes, REQUEST request) {
     String urlScheme = getUrlScheme(request);
     String urlPath = getter.getUrlPath(request);
     String urlQuery = getter.getUrlQuery(request);

diff --git a/.../io/opentelemetry/instrumentation/api/semconv/http/HttpClientAttributesExtractorTest.java b/.../io/opentelemetry/instrumentation/api/semconv/http/HttpClientAttributesExtractorTest.java
@@ -169,16 +169,16 @@ void normal() {
 
     AttributesBuilder startAttributes = Attributes.builder();
     extractor.onStart(startAttributes, Context.root(), request);
-    assertThat(startAttributes.build())
-        .containsOnly(
-            entry(HttpAttributes.HTTP_REQUEST_METHOD, "POST"),
-            entry(UrlAttributes.URL_FULL, "http://github.com"),
-            entry(
-                AttributeKey.stringArrayKey("http.request.header.custom-request-header"),
-                asList("123", "456")),
-            entry(ServerAttributes.SERVER_ADDRESS, "github.com"),
-            entry(ServerAttributes.SERVER_PORT, 80L),
-            entry(HttpAttributes.HTTP_REQUEST_RESEND_COUNT, 2L));
+//    assertThat(startAttributes.build())
+//        .containsOnly(
+//            entry(HttpAttributes.HTTP_REQUEST_METHOD, "POST"),
+//            entry(UrlAttributes.URL_FULL, "http://github.com"),
+//            entry(
+//                AttributeKey.stringArrayKey("http.request.header.custom-request-header"),
+//                asList("123", "456")),
+//            entry(ServerAttributes.SERVER_ADDRESS, "github.com"),
+//            entry(ServerAttributes.SERVER_PORT, 80L),
+//            entry(HttpAttributes.HTTP_REQUEST_RESEND_COUNT, 2L));
 
     AttributesBuilder endAttributes = Attributes.builder();
     extractor.onEnd(endAttributes, Context.root(), request, response, null);
@@ -190,6 +190,14 @@ void normal() {
                 asList("654", "321")),
             entry(NetworkAttributes.NETWORK_PROTOCOL_VERSION, "1.1"),
             entry(NetworkAttributes.NETWORK_PEER_ADDRESS, "4.3.2.1"),
+                            entry(HttpAttributes.HTTP_REQUEST_METHOD, "POST"),
+            entry(UrlAttributes.URL_FULL, "http://github.com"),
+            entry(
+                AttributeKey.stringArrayKey("http.request.header.custom-request-header"),
+                asList("123", "456")),
+            entry(ServerAttributes.SERVER_ADDRESS, "github.com"),
+            entry(ServerAttributes.SERVER_PORT, 80L),
+                entry(HttpAttributes.HTTP_REQUEST_RESEND_COUNT, 2L),
             entry(NetworkAttributes.NETWORK_PEER_PORT, 456L));
   }
 
@@ -358,15 +366,17 @@ void shouldExtractServerAddressAndPortFromHostHeader() {
 
     AttributesBuilder startAttributes = Attributes.builder();
     extractor.onStart(startAttributes, Context.root(), request);
-    assertThat(startAttributes.build())
-        .containsOnly(
-            entry(ServerAttributes.SERVER_ADDRESS, "github.com"),
-            entry(ServerAttributes.SERVER_PORT, 123L));
+//    assertThat(startAttributes.build())
+//        .containsOnly(
+//            entry(ServerAttributes.SERVER_ADDRESS, "github.com"),
+//            entry(ServerAttributes.SERVER_PORT, 123L));
 
     AttributesBuilder endAttributes = Attributes.builder();
     extractor.onEnd(endAttributes, Context.root(), request, response, null);
     assertThat(endAttributes.build())
-        .containsOnly(entry(HttpAttributes.HTTP_RESPONSE_STATUS_CODE, 200L));
+        .containsOnly(entry(HttpAttributes.HTTP_RESPONSE_STATUS_CODE, 200L),
+                entry(ServerAttributes.SERVER_ADDRESS, "github.com"),
+                entry(ServerAttributes.SERVER_PORT, 123L));
   }
 
   @Test
@@ -385,18 +395,20 @@ void shouldExtractPeerAddressEvenIfItDuplicatesServerAddress() {
 
     AttributesBuilder startAttributes = Attributes.builder();
     extractor.onStart(startAttributes, Context.root(), request);
-    assertThat(startAttributes.build())
-        .containsOnly(
-            entry(ServerAttributes.SERVER_ADDRESS, "1.2.3.4"),
-            entry(ServerAttributes.SERVER_PORT, 123L));
+//    assertThat(startAttributes.build())
+//        .containsOnly(
+//            entry(ServerAttributes.SERVER_ADDRESS, "1.2.3.4"),
+//            entry(ServerAttributes.SERVER_PORT, 123L));
 
     AttributesBuilder endAttributes = Attributes.builder();
     extractor.onEnd(endAttributes, Context.root(), request, response, null);
     assertThat(endAttributes.build())
         .containsOnly(
             entry(HttpAttributes.HTTP_RESPONSE_STATUS_CODE, 200L),
             entry(NetworkAttributes.NETWORK_PEER_ADDRESS, "1.2.3.4"),
-            entry(NetworkAttributes.NETWORK_PEER_PORT, 456L));
+            entry(NetworkAttributes.NETWORK_PEER_PORT, 456L),
+                entry(ServerAttributes.SERVER_ADDRESS, "1.2.3.4"),
+                entry(ServerAttributes.SERVER_PORT, 123L));
   }
 
   @Test

diff --git a/...rc/test/java/io/opentelemetry/instrumentation/api/semconv/http/HttpClientMetricsTest.java b/...rc/test/java/io/opentelemetry/instrumentation/api/semconv/http/HttpClientMetricsTest.java
@@ -51,7 +51,10 @@ void collectsMetrics() {
 
     Attributes responseAttributes =
         Attributes.builder()
-            .put(HttpAttributes.HTTP_RESPONSE_STATUS_CODE, 200)
+                .put(HttpAttributes.HTTP_REQUEST_METHOD, "GET")
+                .put(ServerAttributes.SERVER_PORT, 1234)
+                .put(ServerAttributes.SERVER_ADDRESS, "localhost")
+                .put(HttpAttributes.HTTP_RESPONSE_STATUS_CODE, 200)
             .put(ErrorAttributes.ERROR_TYPE, "400")
             .put(HttpIncubatingAttributes.HTTP_REQUEST_BODY_SIZE, 100)
             .put(HttpIncubatingAttributes.HTTP_RESPONSE_BODY_SIZE, 200)