Remove Attributes combanation from onEnd, move Attributes extract fro…

…m onStart to onEnd
open-telemetry · Aug 7, 2024 · e023041 · e023041
1 parent 593db23
commit e023041
Show file tree

Hide file tree

Showing 15 changed files with 45 additions and 46 deletions.
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
@@ -74,7 +74,7 @@ public void onEnd(Context context, Attributes endAttributes, long endNanos) {
     }
     clientDurationHistogram.record(
         (endNanos - state.startTimeNanos()) / NANOS_PER_MS,
-        state.startAttributes().toBuilder().putAll(endAttributes).build(),
+            endAttributes,
         context);
   }
 

diff --git a/...opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcCommonAttributesExtractor.java b/...opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcCommonAttributesExtractor.java
@@ -29,9 +29,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));
+
   }
 
   @Override
@@ -42,5 +40,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
@@ -74,7 +74,7 @@ public void onEnd(Context context, Attributes endAttributes, long endNanos) {
     }
     serverDurationHistogram.record(
         (endNanos - state.startTimeNanos()) / NANOS_PER_MS,
-        state.startAttributes().toBuilder().putAll(endAttributes).build(),
+            endAttributes,
         context);
   }
 

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);