move return value attribute to separate annotation

open-telemetry · Aug 14, 2024 · 0010367 · 0010367
1 parent 6ba7771
commit 0010367
Show file tree

Hide file tree

Showing 9 changed files with 57 additions and 39 deletions.
diff --git a/...cubator/src/main/java/io/opentelemetry/instrumentation/annotations/incubator/Counted.java b/...cubator/src/main/java/io/opentelemetry/instrumentation/annotations/incubator/Counted.java
@@ -61,17 +61,4 @@
    * <p>This property would not take effect if the value is not specified.
    */
   String unit() default "{invocation}";
-
-  /**
-   * Attribute name for the return value.
-   *
-   * <p>The name of the attribute for the return value of the method call. {@link Object#toString()}
-   * may be called on the return value to convert it to a String.
-   *
-   * <p>By default, the instrument will not record an attribute with the return value.
-   *
-   * <p>Warning: be careful using this because it might cause an explosion of the cardinality on
-   * your metric
-   */
-  String returnValueAttribute() default "";
 }
diff --git a/...src/main/java/io/opentelemetry/instrumentation/annotations/incubator/MetricAttribute.java b/...src/main/java/io/opentelemetry/instrumentation/annotations/incubator/MetricAttribute.java
@@ -27,6 +27,7 @@
 @Target(ElementType.PARAMETER)
 @Retention(RetentionPolicy.RUNTIME)
 public @interface MetricAttribute {
+
   /**
    * Optional name of the attribute.
    *

diff --git a/...io/opentelemetry/instrumentation/annotations/incubator/MetricAttributeForReturnValue.java b/...io/opentelemetry/instrumentation/annotations/incubator/MetricAttributeForReturnValue.java
@@ -0,0 +1,38 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.annotations.incubator;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+/**
+ * This annotation allows for adding method return value as attribute to the metrics recorded using
+ * {@link Timed} and {@link Counted} annotations.
+ *
+ * <p>Application developers can use this annotation to signal OpenTelemetry auto-instrumentation
+ * that the attribute should be created.
+ *
+ * <p>If you are a library developer, then probably you should NOT use this annotation, because it
+ * is non-functional without the OpenTelemetry auto-instrumentation agent, or some other annotation
+ * processor.
+ *
+ * <p>Warning: be careful using this because it might cause an explosion of the cardinality on your
+ * metric.
+ */
+@Target(ElementType.METHOD)
+@Retention(RetentionPolicy.RUNTIME)
+public @interface MetricAttributeForReturnValue {
+
+  /**
+   * Attribute name for the return value.
+   *
+   * <p>The name of the attribute for the return value of the method call. {@link Object#toString()}
+   * may be called on the return value to convert it to a String.
+   */
+  String value();
+}
diff --git a/...incubator/src/main/java/io/opentelemetry/instrumentation/annotations/incubator/Timed.java b/...incubator/src/main/java/io/opentelemetry/instrumentation/annotations/incubator/Timed.java
@@ -57,17 +57,4 @@
    * <p>Default is seconds.
    */
   TimeUnit unit() default TimeUnit.SECONDS;
-
-  /**
-   * Attribute name for the return value.
-   *
-   * <p>The name of the attribute for the return value of the method call. {@link Object#toString()}
-   * may be called on the return value to convert it to a String.
-   *
-   * <p>By default, the instrument will not record an attribute with the return value.
-   *
-   * <p>Warning: be careful using this because it might cause an explosion of the cardinality on
-   * your metric
-   */
-  String returnValueAttribute() default "";
 }
diff --git a/...lemetry/javaagent/instrumentation/instrumentationannotations/incubator/CountedHelper.java b/...lemetry/javaagent/instrumentation/instrumentationannotations/incubator/CountedHelper.java
@@ -59,9 +59,7 @@ private static class MethodCounter {
               .setDescription(countedAnnotation.description())
               .setUnit(countedAnnotation.unit())
               .build();
-
-      String returnValueAttribute = countedAnnotation.returnValueAttribute();
-      attributeHelper = new MetricAttributeHelper(method, returnValueAttribute);
+      attributeHelper = new MetricAttributeHelper(method);
     }
 
     void record(Object returnValue, Object[] arguments, Throwable throwable) {

diff --git a/...vaagent/instrumentation/instrumentationannotations/incubator/MetricsAnnotationHelper.java b/...vaagent/instrumentation/instrumentationannotations/incubator/MetricsAnnotationHelper.java
@@ -6,6 +6,7 @@
 package io.opentelemetry.javaagent.instrumentation.instrumentationannotations.incubator;
 
 import application.io.opentelemetry.instrumentation.annotations.incubator.MetricAttribute;
+import application.io.opentelemetry.instrumentation.annotations.incubator.MetricAttributeForReturnValue;
 import application.io.opentelemetry.instrumentation.annotations.incubator.StaticMetricAttribute;
 import io.opentelemetry.api.GlobalOpenTelemetry;
 import io.opentelemetry.api.common.Attributes;
@@ -65,11 +66,13 @@ static class MetricAttributeHelper {
     private final BiConsumer<AttributesBuilder, Object> bindReturn;
     private final Attributes staticAttributes;
 
-    MetricAttributeHelper(Method method, String returnValueAttribute) {
+    MetricAttributeHelper(Method method) {
       bindParameters = MethodBinder.bindParameters(method, PARAMETER_ATTRIBUTE_NAMES_EXTRACTOR);
+      MetricAttributeForReturnValue returnValueAttribute =
+          method.getAnnotation(MetricAttributeForReturnValue.class);
       bindReturn =
-          !returnValueAttribute.isEmpty()
-              ? MethodBinder.bindReturnValue(method, returnValueAttribute)
+          returnValueAttribute != null
+              ? MethodBinder.bindReturnValue(method, returnValueAttribute.value())
               : null;
 
       AttributesBuilder attributesBuilder = Attributes.builder();

diff --git a/...telemetry/javaagent/instrumentation/instrumentationannotations/incubator/TimedHelper.java b/...telemetry/javaagent/instrumentation/instrumentationannotations/incubator/TimedHelper.java
@@ -96,9 +96,7 @@ private static class MethodTimer {
               .setDescription(timedAnnotation.description())
               .setUnit(timeUnitToString(unit))
               .build();
-
-      String returnValueAttribute = timedAnnotation.returnValueAttribute();
-      attributeHelper = new MetricAttributeHelper(method, returnValueAttribute);
+      attributeHelper = new MetricAttributeHelper(method);
     }
 
     void record(Object returnValue, Object[] arguments, Throwable throwable, long startNanoTime) {

diff --git a/...avaagent/instrumentation/instrumentationannotations/incubator/counted/CountedExample.java b/...avaagent/instrumentation/instrumentationannotations/incubator/counted/CountedExample.java
@@ -6,6 +6,7 @@
 package io.opentelemetry.javaagent.instrumentation.instrumentationannotations.incubator.counted;
 
 import io.opentelemetry.instrumentation.annotations.incubator.Counted;
+import io.opentelemetry.instrumentation.annotations.incubator.MetricAttributeForReturnValue;
 import io.opentelemetry.instrumentation.annotations.incubator.StaticMetricAttribute;
 import java.util.concurrent.CompletableFuture;
 
@@ -31,7 +32,8 @@ public void exampleWithUnit() {}
   @StaticMetricAttribute(name = "key2", value = "value2")
   public void exampleWithAdditionalAttributes1() {}
 
-  @Counted(value = "example.with.return.count", returnValueAttribute = "returnValue")
+  @Counted(value = "example.with.return.count")
+  @MetricAttributeForReturnValue("returnValue")
   public ReturnObject exampleWithReturnValueAttribute() {
     return new ReturnObject();
   }
@@ -44,7 +46,8 @@ public void exampleWithException() {
   @Counted("example.ignore.count")
   public void exampleIgnore() {}
 
-  @Counted(value = "example.completable.future.count", returnValueAttribute = "returnValue")
+  @Counted(value = "example.completable.future.count")
+  @MetricAttributeForReturnValue("returnValue")
   public CompletableFuture<String> completableFuture(CompletableFuture<String> future) {
     return future;
   }

diff --git a/...ry/javaagent/instrumentation/instrumentationannotations/incubator/timed/TimedExample.java b/...ry/javaagent/instrumentation/instrumentationannotations/incubator/timed/TimedExample.java
@@ -5,6 +5,7 @@
 
 package io.opentelemetry.javaagent.instrumentation.instrumentationannotations.incubator.timed;
 
+import io.opentelemetry.instrumentation.annotations.incubator.MetricAttributeForReturnValue;
 import io.opentelemetry.instrumentation.annotations.incubator.StaticMetricAttribute;
 import io.opentelemetry.instrumentation.annotations.incubator.Timed;
 import java.util.concurrent.CompletableFuture;
@@ -39,12 +40,14 @@ public void exampleWithException() {
     throw new IllegalStateException("test");
   }
 
-  @Timed(value = "example.with.return.duration", returnValueAttribute = "returnValue")
+  @Timed(value = "example.with.return.duration")
+  @MetricAttributeForReturnValue("returnValue")
   public ReturnObject exampleWithReturnValueAttribute() {
     return new ReturnObject();
   }
 
-  @Timed(value = "example.completable.future.duration", returnValueAttribute = "returnValue")
+  @Timed(value = "example.completable.future.duration")
+  @MetricAttributeForReturnValue("returnValue")
   public CompletableFuture<String> completableFuture(CompletableFuture<String> future) {
     return future;
   }