Consolidate to single ExceptionAttributeResolver method

Author: jack-berg

sdk/common/src/main/java/io/opentelemetry/sdk/internal/AttributeUtil.java

@@ -5,17 +5,12 @@
 
 package io.opentelemetry.sdk.internal;
 
-import static io.opentelemetry.sdk.internal.ExceptionAttributeResolver.EXCEPTION_MESSAGE;
-import static io.opentelemetry.sdk.internal.ExceptionAttributeResolver.EXCEPTION_STACKTRACE;
-import static io.opentelemetry.sdk.internal.ExceptionAttributeResolver.EXCEPTION_TYPE;
-
 import io.opentelemetry.api.common.AttributeKey;
 import io.opentelemetry.api.common.Attributes;
 import io.opentelemetry.api.common.AttributesBuilder;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
-import java.util.function.BiConsumer;
 import java.util.function.Predicate;
 
 /**
@@ -100,28 +95,4 @@ public static Object applyAttributeLengthLimit(Object value, int lengthLimit) {
     }
     return value;
   }
-
-  public static void addExceptionAttributes(
-      ExceptionAttributeResolver exceptionAttributeResolver,
-      int maxAttributeLength,
-      Throwable exception,
-      BiConsumer<AttributeKey<String>, String> attributeConsumer) {
-    String exceptionType =
-        exceptionAttributeResolver.getExceptionType(exception, maxAttributeLength);
-    if (exceptionType != null) {
-      attributeConsumer.accept(EXCEPTION_TYPE, exceptionType);
-    }
-
-    String exceptionMessage =
-        exceptionAttributeResolver.getExceptionMessage(exception, maxAttributeLength);
-    if (exceptionMessage != null) {
-      attributeConsumer.accept(EXCEPTION_MESSAGE, exceptionMessage);
-    }
-
-    String stackTrace =
-        exceptionAttributeResolver.getExceptionStacktrace(exception, maxAttributeLength);
-    if (stackTrace != null) {
-      attributeConsumer.accept(EXCEPTION_STACKTRACE, stackTrace);
-    }
-  }
 }

sdk/common/src/main/java/io/opentelemetry/sdk/internal/AttributesMap.java

@@ -21,8 +21,8 @@
  * <p>WARNING: In order to reduce memory allocation, this class extends {@link HashMap} when it
  * would be more appropriate to delegate. The problem with extending is that we don't enforce that
  * all {@link HashMap} methods for reading / writing data conform to the configured attribute
- * limits. Therefore, it's easy to accidentally call something like {@link Map#putAll(Map)} or
- * {@link Map#put(Object, Object)} and bypass the restrictions (see <a
+ * limits. Therefore, it's easy to accidentally call something like {@link Map#putAll(Map)} and
+ * bypass the restrictions (see <a
  * href="https://github.com/open-telemetry/opentelemetry-java/issues/7135">#7135</a>). Callers MUST
  * take care to only call methods from {@link AttributesMap}, and not {@link HashMap}.
  *
@@ -58,14 +58,22 @@ public static AttributesMap create(long capacity, int lengthLimit) {
    */
   @Override
   @Nullable
-  public Object put(AttributeKey<?> key, Object value) {
+  public Object put(AttributeKey<?> key, @Nullable Object value) {
+    if (value == null) {
+      return null;
+    }
     totalAddedValues++;
     if (size() >= capacity && !containsKey(key)) {
       return null;
     }
     return super.put(key, AttributeUtil.applyAttributeLengthLimit(value, lengthLimit));
   }
 
+  /** Generic overload of {@link #put(AttributeKey, Object)}. */
+  public <T> void putIfCapacity(AttributeKey<T> key, @Nullable T value) {
+    put(key, value);
+  }
+
   /** Get the total number of attributes added, including those dropped for capacity limits. */
   public int getTotalAddedValues() {
     return totalAddedValues;

sdk/common/src/main/java/io/opentelemetry/sdk/internal/DefaultExceptionAttributeResolver.java

@@ -7,7 +7,6 @@
 
 import java.io.PrintWriter;
 import java.io.StringWriter;
-import javax.annotation.Nullable;
 
 /**
  * This class is internal and experimental. Its APIs are unstable and can change at any time. Its
@@ -26,24 +25,19 @@ public static ExceptionAttributeResolver getInstance() {
   }
 
   @Override
-  @Nullable
-  public String getExceptionType(Throwable throwable, int maxAttributeLength) {
-    return throwable.getClass().getCanonicalName();
-  }
-
-  @Override
-  @Nullable
-  public String getExceptionMessage(Throwable throwable, int maxAttributeLength) {
-    return throwable.getMessage();
-  }
-
-  @Override
-  @Nullable
-  public String getExceptionStacktrace(Throwable throwable, int maxAttributeLength) {
+  public void setExceptionAttributes(
+      AttributeSetter attributeSetter, Throwable throwable, int maxAttributeLength) {
+    attributeSetter.setAttribute(
+        ExceptionAttributeResolver.EXCEPTION_TYPE, throwable.getClass().getCanonicalName());
+    String message = throwable.getMessage();
+    if (message != null) {
+      attributeSetter.setAttribute(ExceptionAttributeResolver.EXCEPTION_MESSAGE, message);
+    }
     StringWriter stringWriter = new StringWriter();
     try (PrintWriter printWriter = new PrintWriter(stringWriter)) {
       throwable.printStackTrace(printWriter);
     }
-    return stringWriter.toString();
+    attributeSetter.setAttribute(
+        ExceptionAttributeResolver.EXCEPTION_STACKTRACE, stringWriter.toString());
   }
 }

sdk/common/src/main/java/io/opentelemetry/sdk/internal/ExceptionAttributeResolver.java

@@ -21,39 +21,17 @@ public interface ExceptionAttributeResolver {
   AttributeKey<String> EXCEPTION_MESSAGE = AttributeKey.stringKey("exception.message");
   AttributeKey<String> EXCEPTION_STACKTRACE = AttributeKey.stringKey("exception.stacktrace");
 
-  /**
-   * Resolve the {@link #EXCEPTION_TYPE} attribute from the {@code throwable}, or {@code null} if no
-   * value should be set.
-   *
-   * @param throwable the throwable
-   * @param maxAttributeLength the max attribute length that will be retained by the SDK. Responses
-   *     are not required to conform to this limit, but implementations may incorporate this limit
-   *     to avoid unnecessary compute.
-   */
-  @Nullable
-  String getExceptionType(Throwable throwable, int maxAttributeLength);
-
-  /**
-   * Resolve the {@link #EXCEPTION_MESSAGE} attribute from the {@code throwable}, or {@code null} if
-   * no value should be set.
-   *
-   * @param throwable the throwable
-   * @param maxAttributeLength the max attribute length that will be retained by the SDK. Responses
-   *     are not required to conform to this limit, but implementations may incorporate this limit
-   *     to avoid unnecessary compute.
-   */
-  @Nullable
-  String getExceptionMessage(Throwable throwable, int maxAttributeLength);
+  void setExceptionAttributes(
+      AttributeSetter attributeSetter, Throwable throwable, int maxAttributeLength);
 
   /**
-   * Resolve the {@link #EXCEPTION_STACKTRACE} attribute from the {@code throwable}, or {@code null}
-   * if no value should be set.
-   *
-   * @param throwable the throwable
-   * @param maxAttributeLength the max attribute length that will be retained by the SDK. Responses
-   *     are not required to conform to this limit, but implementations may incorporate this limit
-   *     to avoid unnecessary compute.
+   * This class is internal and experimental. Its APIs are unstable and can change at any time. Its
+   * APIs (or a version of them) may be promoted to the public stable API in the future, but no
+   * guarantees are made.
    */
-  @Nullable
-  String getExceptionStacktrace(Throwable throwable, int maxAttributeLength);
+  // TODO(jack-berg): Consider promoting to opentelemetry and extending with Span, LogRecordBuilder,
+  // AttributeBuilder, AttributesMap etc.
+  interface AttributeSetter {
+    <T> void setAttribute(AttributeKey<T> key, @Nullable T value);
+  }
 }

sdk/common/src/main/java/io/opentelemetry/sdk/internal/ExtendedAttributesMap.java

@@ -49,14 +49,23 @@ public static ExtendedAttributesMap create(long capacity, int lengthLimit) {
   }
 
   /** Add the attribute key value pair, applying capacity and length limits. */
-  public <T> void put(ExtendedAttributeKey<T> key, T value) {
+  @Override
+  @Nullable
+  public Object put(ExtendedAttributeKey<?> key, @Nullable Object value) {
+    if (value == null) {
+      return null;
+    }
     totalAddedValues++;
-    // TODO(jack-berg): apply capcity to nested entries
+    // TODO(jack-berg): apply capacity to nested entries
     if (size() >= capacity && !containsKey(key)) {
-      return;
+      return null;
     }
     // TODO(jack-berg): apply limits to nested entries
-    super.put(key, AttributeUtil.applyAttributeLengthLimit(value, lengthLimit));
+    return super.put(key, AttributeUtil.applyAttributeLengthLimit(value, lengthLimit));
+  }
+
+  public <T> void putIfCapacity(ExtendedAttributeKey<T> key, @Nullable T value) {
+    put(key, value);
   }
 
   /**

sdk/common/src/test/java/io/opentelemetry/sdk/internal/AttributesMapTest.java

@@ -16,8 +16,8 @@ class AttributesMapTest {
   @Test
   void asMap() {
     AttributesMap attributesMap = AttributesMap.create(2, Integer.MAX_VALUE);
-    attributesMap.put(longKey("one"), 1L);
-    attributesMap.put(longKey("two"), 2L);
+    attributesMap.putIfCapacity(longKey("one"), 1L);
+    attributesMap.putIfCapacity(longKey("two"), 2L);
 
     assertThat(attributesMap.asMap())
         .containsOnly(entry(longKey("one"), 1L), entry(longKey("two"), 2L));

sdk/logs/src/main/java/io/opentelemetry/sdk/logs/ExtendedSdkLogRecordBuilder.java

@@ -13,7 +13,6 @@
 import io.opentelemetry.api.trace.Span;
 import io.opentelemetry.context.Context;
 import io.opentelemetry.sdk.common.InstrumentationScopeInfo;
-import io.opentelemetry.sdk.internal.AttributeUtil;
 import io.opentelemetry.sdk.internal.ExtendedAttributesMap;
 import java.time.Instant;
 import java.util.concurrent.TimeUnit;
@@ -42,11 +41,12 @@ public ExtendedSdkLogRecordBuilder setException(Throwable throwable) {
       return this;
     }
 
-    AttributeUtil.addExceptionAttributes(
-        loggerSharedState.getExceptionAttributeResolver(),
-        loggerSharedState.getLogLimits().getMaxAttributeValueLength(),
-        throwable,
-        this::setAttribute);
+    loggerSharedState
+        .getExceptionAttributeResolver()
+        .setExceptionAttributes(
+            this::setAttribute,
+            throwable,
+            loggerSharedState.getLogLimits().getMaxAttributeValueLength());
 
     return this;
   }
@@ -115,7 +115,7 @@ public <T> ExtendedSdkLogRecordBuilder setAttribute(ExtendedAttributeKey<T> key,
           ExtendedAttributesMap.create(
               logLimits.getMaxNumberOfAttributes(), logLimits.getMaxAttributeValueLength());
     }
-    this.extendedAttributes.put(key, value);
+    this.extendedAttributes.putIfCapacity(key, value);
     return this;
   }
 

sdk/logs/src/main/java/io/opentelemetry/sdk/logs/ExtendedSdkReadWriteLogRecord.java

@@ -105,7 +105,7 @@ public <T> ExtendedSdkReadWriteLogRecord setAttribute(ExtendedAttributeKey<T> ke
             ExtendedAttributesMap.create(
                 logLimits.getMaxNumberOfAttributes(), logLimits.getMaxAttributeValueLength());
       }
-      extendedAttributes.put(key, value);
+      extendedAttributes.putIfCapacity(key, value);
     }
     return this;
   }

sdk/logs/src/main/java/io/opentelemetry/sdk/logs/SdkLogRecordBuilder.java

@@ -111,7 +111,7 @@ public <T> SdkLogRecordBuilder setAttribute(AttributeKey<T> key, @Nullable T val
           AttributesMap.create(
               logLimits.getMaxNumberOfAttributes(), logLimits.getMaxAttributeValueLength());
     }
-    this.attributes.put(key, value);
+    this.attributes.putIfCapacity(key, value);
     return this;
   }
 

sdk/logs/src/main/java/io/opentelemetry/sdk/logs/SdkReadWriteLogRecord.java

@@ -100,7 +100,7 @@ public <T> ReadWriteLogRecord setAttribute(AttributeKey<T> key, T value) {
             AttributesMap.create(
                 logLimits.getMaxNumberOfAttributes(), logLimits.getMaxAttributeValueLength());
       }
-      attributes.put(key, value);
+      attributes.putIfCapacity(key, value);
     }
     return this;
   }