Refactor AttributesExtractor so that it extracts route from Context (o…

…pen-telemetry#5288) * Refactor AttributesExtractor so that it extracts route from Context * typo * fix tests * Update instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpRouteHolder.java Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com> * fix all AttributesExtractors Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>
RashmiRam · May 23, 2022 · 0de15a1 · 0de15a1
1 parent 93a8d86
commit 0de15a1
Show file tree

Hide file tree

Showing 72 changed files with 458 additions and 209 deletions.
diff --git a/...o/opentelemetry/instrumentation/api/annotation/support/MethodSpanAttributesExtractor.java b/...o/opentelemetry/instrumentation/api/annotation/support/MethodSpanAttributesExtractor.java
@@ -6,6 +6,7 @@
 package io.opentelemetry.instrumentation.api.annotation.support;
 
 import io.opentelemetry.api.common.AttributesBuilder;
+import io.opentelemetry.context.Context;
 import io.opentelemetry.instrumentation.api.cache.Cache;
 import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor;
 import java.lang.reflect.Method;
@@ -45,7 +46,7 @@ public static <REQUEST, RESPONSE> MethodSpanAttributesExtractor<REQUEST, RESPONS
   }
 
   @Override
-  public void onStart(AttributesBuilder attributes, REQUEST request) {
+  public void onStart(AttributesBuilder attributes, Context parentContext, REQUEST request) {
     Method method = methodExtractor.extract(request);
     AttributeBindings bindings = cache.computeIfAbsent(method, this::bind);
     if (!bindings.isEmpty()) {
@@ -57,6 +58,7 @@ public void onStart(AttributesBuilder attributes, REQUEST request) {
   @Override
   public void onEnd(
       AttributesBuilder attributes,
+      Context context,
       REQUEST request,
       @Nullable RESPONSE response,
       @Nullable Throwable error) {}

diff --git a/...telemetry/instrumentation/api/annotation/support/MethodSpanAttributesExtractorTest.groovy b/...telemetry/instrumentation/api/annotation/support/MethodSpanAttributesExtractorTest.groovy
@@ -6,6 +6,7 @@
 package io.opentelemetry.instrumentation.api.annotation.support
 
 import io.opentelemetry.api.common.AttributesBuilder
+import io.opentelemetry.context.Context
 import io.opentelemetry.instrumentation.api.cache.Cache
 import spock.lang.Specification
 
@@ -17,6 +18,7 @@ class MethodSpanAttributesExtractorTest extends Specification {
   def "extracts attributes for method with attribute names"() {
     given:
     def request = new Object()
+    def context = Context.root()
     def method = TestClass.getDeclaredMethod("method", String, String, String)
     AttributesBuilder builder = Mock()
 
@@ -32,7 +34,7 @@ class MethodSpanAttributesExtractorTest extends Specification {
     )
 
     when:
-    extractor.onStart(builder, request)
+    extractor.onStart(builder, context, request)
 
     then:
     1 * builder.put({ it.getKey() == "x" }, "a")
@@ -43,6 +45,7 @@ class MethodSpanAttributesExtractorTest extends Specification {
   def "does not extract attributes for empty attribute name array"() {
     given:
     def request = new Object()
+    def context = Context.root()
     def method = TestClass.getDeclaredMethod("method", String, String, String)
     AttributesBuilder builder = Mock()
 
@@ -58,7 +61,7 @@ class MethodSpanAttributesExtractorTest extends Specification {
     )
 
     when:
-    extractor.onStart(builder, request)
+    extractor.onStart(builder, context, request)
 
     then:
     0 * builder.put(*_)
@@ -67,6 +70,7 @@ class MethodSpanAttributesExtractorTest extends Specification {
   def "does not extract attributes for method with attribute names array with fewer elements than parameters"() {
     given:
     def request = new Object()
+    def context = Context.root()
     def method = TestClass.getDeclaredMethod("method", String, String, String)
     AttributesBuilder builder = Mock()
 
@@ -82,7 +86,7 @@ class MethodSpanAttributesExtractorTest extends Specification {
     )
 
     when:
-    extractor.onStart(builder, request)
+    extractor.onStart(builder, context, request)
 
     then:
     0 * builder.put(*_)
@@ -91,6 +95,7 @@ class MethodSpanAttributesExtractorTest extends Specification {
   def "extracts attributes for method with attribute names array with null element"() {
     given:
     def request = new Object()
+    def context = Context.root()
     def method = TestClass.getDeclaredMethod("method", String, String, String)
     AttributesBuilder builder = Mock()
 
@@ -106,7 +111,7 @@ class MethodSpanAttributesExtractorTest extends Specification {
     )
 
     when:
-    extractor.onStart(builder, request)
+    extractor.onStart(builder, context, request)
 
     then:
     1 * builder.put({ it.getKey() == "x" }, "a")
@@ -117,6 +122,7 @@ class MethodSpanAttributesExtractorTest extends Specification {
   def "does not extracts attribute for method with null argument"() {
     given:
     def request = new Object()
+    def context = Context.root()
     def method = TestClass.getDeclaredMethod("method", String, String, String)
     AttributesBuilder builder = Mock()
 
@@ -132,7 +138,7 @@ class MethodSpanAttributesExtractorTest extends Specification {
     )
 
     when:
-    extractor.onStart(builder, request)
+    extractor.onStart(builder, context, request)
 
     then:
     1 * builder.put({ it.getKey() == "x" }, "a")
@@ -143,6 +149,7 @@ class MethodSpanAttributesExtractorTest extends Specification {
   def "applies cached bindings"() {
     given:
     def request = new Object()
+    def context = Context.root()
     def method = TestClass.getDeclaredMethod("method", String, String, String)
     AttributesBuilder builder = Mock()
 
@@ -161,7 +168,7 @@ class MethodSpanAttributesExtractorTest extends Specification {
     )
 
     when:
-    extractor.onStart(builder, request)
+    extractor.onStart(builder, context, request)
 
     then:
     1 * bindings.apply(_, ["a", "b", "c"])
@@ -170,6 +177,7 @@ class MethodSpanAttributesExtractorTest extends Specification {
   def "does not apply cached empty bindings"() {
     given:
     def request = new Object()
+    def context = Context.root()
     def method = TestClass.getDeclaredMethod("method", String, String, String)
     AttributesBuilder builder = Mock()
 
@@ -188,13 +196,14 @@ class MethodSpanAttributesExtractorTest extends Specification {
     )
 
     when:
-    extractor.onStart(builder, request)
+    extractor.onStart(builder, context, request)
 
     then:
     0 * bindings.apply(_, _)
   }
 
   class TestClass {
+    @SuppressWarnings("unused")
     void method(String x, String y, String z) {}
   }
 }
diff --git a/.../src/main/java/io/opentelemetry/instrumentation/api/instrumenter/AttributesExtractor.java b/.../src/main/java/io/opentelemetry/instrumentation/api/instrumenter/AttributesExtractor.java
@@ -7,39 +7,74 @@
 
 import io.opentelemetry.api.common.AttributeKey;
 import io.opentelemetry.api.common.AttributesBuilder;
+import io.opentelemetry.context.Context;
 import io.opentelemetry.instrumentation.api.instrumenter.db.DbAttributesExtractor;
 import io.opentelemetry.instrumentation.api.instrumenter.http.HttpClientAttributesExtractor;
 import io.opentelemetry.instrumentation.api.instrumenter.net.NetServerAttributesExtractor;
 import javax.annotation.Nullable;
 
 /**
  * Extractor of {@link io.opentelemetry.api.common.Attributes} for a given request and response.
- * Will be called {@linkplain #onStart(AttributesBuilder, Object) on start} with just the {@link
- * REQUEST} and again {@linkplain #onEnd(AttributesBuilder, Object, Object, Throwable) on end} with
- * both {@link REQUEST} and {@link RESPONSE} to allow populating attributes at each stage of a
- * request's lifecycle. It is best to populate as much as possible in {@link
- * #onStart(AttributesBuilder, Object)} to have it available during sampling.
+ * Will be called {@linkplain #onStart(AttributesBuilder, Context, Object) on start} with just the
+ * {@link REQUEST} and again {@linkplain #onEnd(AttributesBuilder, Context, Object, Object,
+ * Throwable) on end} with both {@link REQUEST} and {@link RESPONSE} to allow populating attributes
+ * at each stage of a request's lifecycle. It is best to populate as much as possible in {@link
+ * #onStart(AttributesBuilder, Context, Object)} to have it available during sampling.
  *
  * @see DbAttributesExtractor
  * @see HttpClientAttributesExtractor
  * @see NetServerAttributesExtractor
  */
 public interface AttributesExtractor<REQUEST, RESPONSE> {
+
+  /**
+   * Extracts attributes from the {@link Context} and the {@link REQUEST} into the {@link
+   * AttributesBuilder} at the beginning of a request.
+   */
+  default void onStart(AttributesBuilder attributes, Context parentContext, REQUEST request) {
+    onStart(attributes, request);
+  }
+
   /**
    * Extracts attributes from the {@link REQUEST} into the {@link AttributesBuilder} at the
    * beginning of a request.
+   *
+   * @deprecated Use {@link #onStart(AttributesBuilder, Context, Object)} instead.
+   */
+  @Deprecated
+  default void onStart(AttributesBuilder attributes, REQUEST request) {
+    throw new UnsupportedOperationException(
+        "This method variant is deprecated and will be removed in the next minor release.");
+  }
+
+  /**
+   * Extracts attributes from the {@link Context}, the {@link REQUEST} and either {@link RESPONSE}
+   * or {@code error} into the {@link AttributesBuilder} at the end of a request.
    */
-  void onStart(AttributesBuilder attributes, REQUEST request);
+  default void onEnd(
+      AttributesBuilder attributes,
+      Context context,
+      REQUEST request,
+      @Nullable RESPONSE response,
+      @Nullable Throwable error) {
+    onEnd(attributes, request, response, error);
+  }
 
   /**
    * Extracts attributes from the {@link REQUEST} and either {@link RESPONSE} or {@code error} into
    * the {@link AttributesBuilder} at the end of a request.
+   *
+   * @deprecated Use {@link #onEnd(AttributesBuilder, Context, Object, Object, Throwable)} instead.
    */
-  void onEnd(
+  @Deprecated
+  default void onEnd(
       AttributesBuilder attributes,
       REQUEST request,
       @Nullable RESPONSE response,
-      @Nullable Throwable error);
+      @Nullable Throwable error) {
+    throw new UnsupportedOperationException(
+        "This method variant is deprecated and will be removed in the next minor release.");
+  }
 
   /**
    * Sets the {@code value} with the given {@code key} to the {@link AttributesBuilder} if {@code

diff --git a/...n/java/io/opentelemetry/instrumentation/api/instrumenter/ConstantAttributesExtractor.java b/...n/java/io/opentelemetry/instrumentation/api/instrumenter/ConstantAttributesExtractor.java
@@ -7,6 +7,7 @@
 
 import io.opentelemetry.api.common.AttributeKey;
 import io.opentelemetry.api.common.AttributesBuilder;
+import io.opentelemetry.context.Context;
 import javax.annotation.Nullable;
 
 final class ConstantAttributesExtractor<REQUEST, RESPONSE, T>
@@ -21,13 +22,14 @@ final class ConstantAttributesExtractor<REQUEST, RESPONSE, T>
   }
 
   @Override
-  public void onStart(AttributesBuilder attributes, REQUEST request) {
+  public void onStart(AttributesBuilder attributes, Context parentContext, REQUEST request) {
     attributes.put(attributeKey, attributeValue);
   }
 
   @Override
   public void onEnd(
       AttributesBuilder attributes,
+      Context context,
       REQUEST request,
       @Nullable RESPONSE response,
       @Nullable Throwable error) {}

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
@@ -171,7 +171,7 @@ public Context start(Context parentContext, REQUEST request) {
 
     UnsafeAttributes attributesBuilder = new UnsafeAttributes();
     for (AttributesExtractor<? super REQUEST, ? super RESPONSE> extractor : attributesExtractors) {
-      extractor.onStart(attributesBuilder, request);
+      extractor.onStart(attributesBuilder, parentContext, request);
     }
     Attributes attributes = attributesBuilder;
 
@@ -221,7 +221,7 @@ public void end(
 
     UnsafeAttributes attributes = new UnsafeAttributes();
     for (AttributesExtractor<? super REQUEST, ? super RESPONSE> extractor : attributesExtractors) {
-      extractor.onEnd(attributes, request, response, error);
+      extractor.onEnd(attributes, context, request, response, error);
     }
     span.setAllAttributes(attributes);
 

diff --git a/...ava/io/opentelemetry/instrumentation/api/instrumenter/PeerServiceAttributesExtractor.java b/...ava/io/opentelemetry/instrumentation/api/instrumenter/PeerServiceAttributesExtractor.java
@@ -8,6 +8,7 @@
 import static java.util.Collections.emptyMap;
 
 import io.opentelemetry.api.common.AttributesBuilder;
+import io.opentelemetry.context.Context;
 import io.opentelemetry.instrumentation.api.config.Config;
 import io.opentelemetry.instrumentation.api.instrumenter.net.NetClientAttributesGetter;
 import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
@@ -49,11 +50,12 @@ public static <REQUEST, RESPONSE> PeerServiceAttributesExtractor<REQUEST, RESPON
   }
 
   @Override
-  public void onStart(AttributesBuilder attributes, REQUEST request) {}
+  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) {

diff --git a/.../java/io/opentelemetry/instrumentation/api/instrumenter/code/CodeAttributesExtractor.java b/.../java/io/opentelemetry/instrumentation/api/instrumenter/code/CodeAttributesExtractor.java
@@ -6,6 +6,7 @@
 package io.opentelemetry.instrumentation.api.instrumenter.code;
 
 import io.opentelemetry.api.common.AttributesBuilder;
+import io.opentelemetry.context.Context;
 import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor;
 import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
 import javax.annotation.Nullable;
@@ -19,7 +20,7 @@ public abstract class CodeAttributesExtractor<REQUEST, RESPONSE>
     implements AttributesExtractor<REQUEST, RESPONSE> {
 
   @Override
-  public final void onStart(AttributesBuilder attributes, REQUEST request) {
+  public final void onStart(AttributesBuilder attributes, Context parentContext, REQUEST request) {
     Class<?> cls = codeClass(request);
     if (cls != null) {
       set(attributes, SemanticAttributes.CODE_NAMESPACE, cls.getName());
@@ -32,6 +33,7 @@ public final void onStart(AttributesBuilder attributes, REQUEST request) {
   @Override
   public final void onEnd(
       AttributesBuilder attributes,
+      Context context,
       REQUEST request,
       @Nullable RESPONSE response,
       @Nullable Throwable error) {}

diff --git a/...main/java/io/opentelemetry/instrumentation/api/instrumenter/db/DbAttributesExtractor.java b/...main/java/io/opentelemetry/instrumentation/api/instrumenter/db/DbAttributesExtractor.java
@@ -6,6 +6,7 @@
 package io.opentelemetry.instrumentation.api.instrumenter.db;
 
 import io.opentelemetry.api.common.AttributesBuilder;
+import io.opentelemetry.context.Context;
 import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor;
 import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
 import javax.annotation.Nullable;
@@ -21,8 +22,9 @@
  */
 public abstract class DbAttributesExtractor<REQUEST, RESPONSE>
     implements AttributesExtractor<REQUEST, RESPONSE> {
+
   @Override
-  public void onStart(AttributesBuilder attributes, REQUEST request) {
+  public void onStart(AttributesBuilder attributes, Context parentContext, REQUEST request) {
     set(attributes, SemanticAttributes.DB_SYSTEM, system(request));
     set(attributes, SemanticAttributes.DB_USER, user(request));
     set(attributes, SemanticAttributes.DB_NAME, name(request));
@@ -34,6 +36,7 @@ public void onStart(AttributesBuilder attributes, REQUEST request) {
   @Override
   public final void onEnd(
       AttributesBuilder attributes,
+      Context context,
       REQUEST request,
       @Nullable RESPONSE response,
       @Nullable Throwable error) {}

diff --git a/...ain/java/io/opentelemetry/instrumentation/api/instrumenter/db/SqlAttributesExtractor.java b/...ain/java/io/opentelemetry/instrumentation/api/instrumenter/db/SqlAttributesExtractor.java
@@ -7,6 +7,7 @@
 
 import io.opentelemetry.api.common.AttributeKey;
 import io.opentelemetry.api.common.AttributesBuilder;
+import io.opentelemetry.context.Context;
 import io.opentelemetry.instrumentation.api.db.SqlStatementInfo;
 import io.opentelemetry.instrumentation.api.db.SqlStatementSanitizer;
 import javax.annotation.Nullable;
@@ -26,8 +27,8 @@ public abstract class SqlAttributesExtractor<REQUEST, RESPONSE>
     extends DbAttributesExtractor<REQUEST, RESPONSE> {
 
   @Override
-  public final void onStart(AttributesBuilder attributes, REQUEST request) {
-    super.onStart(attributes, request);
+  public final void onStart(AttributesBuilder attributes, Context parentContext, REQUEST request) {
+    super.onStart(attributes, parentContext, request);
     AttributeKey<String> dbTable = dbTableAttribute();
     if (dbTable != null) {
       set(attributes, dbTable, table(request));