Expand OkHttp retry exception predicate

exporters/sender/okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/RetryInterceptor.java

@@ -11,7 +11,7 @@
 import java.io.IOException;
 import java.net.ConnectException;
 import java.net.SocketTimeoutException;
-import java.util.Locale;
+import java.net.UnknownHostException;
 import java.util.StringJoiner;
 import java.util.concurrent.ThreadLocalRandom;
 import java.util.concurrent.TimeUnit;
@@ -154,14 +154,15 @@ boolean shouldRetryOnException(IOException e) {
 
   // Visible for testing
   static boolean isRetryableException(IOException e) {
+    // Known retryable SocketTimeoutException messages: null, "connect timed out", "timeout"
+    // Known retryable ConnectTimeout messages: "Failed to connect to
+    // localhost/[0:0:0:0:0:0:0:1]:62611"
+    // Known retryable UnknownHostException messages: "xxxxxx.com"
     if (e instanceof SocketTimeoutException) {
-      String message = e.getMessage();
-      // Connect timeouts can produce SocketTimeoutExceptions with no message, or with "connect
-      // timed out"
-      return message == null || message.toLowerCase(Locale.ROOT).contains("connect timed out");
+      return true;
     } else if (e instanceof ConnectException) {
-      // Exceptions resemble: java.net.ConnectException: Failed to connect to
-      // localhost/[0:0:0:0:0:0:0:1]:62611
+      return true;
+    } else if (e instanceof UnknownHostException) {
       return true;
     }
     return false;

exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/RetryInterceptorTest.java

@@ -27,11 +27,13 @@
 import java.net.HttpRetryException;
 import java.net.ServerSocket;
 import java.net.SocketTimeoutException;
+import java.net.UnknownHostException;
 import java.time.Duration;
 import java.util.concurrent.TimeUnit;
 import java.util.function.Predicate;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import java.util.stream.Stream;
 import okhttp3.OkHttpClient;
 import okhttp3.Request;
 import okhttp3.Response;
@@ -40,6 +42,8 @@
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.junit.jupiter.api.extension.RegisterExtension;
 import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 import org.junit.jupiter.params.provider.ValueSource;
 import org.mockito.Mock;
 import org.mockito.junit.jupiter.MockitoExtension;
@@ -217,26 +221,30 @@ private OkHttpClient connectTimeoutClient() {
         .build();
   }
 
-  @Test
-  void isRetryableException() {
-    // Should retry on connection timeouts, where error message is "Connect timed out" or "connect
-    // timed out"
-    assertThat(retrier.shouldRetryOnException(new SocketTimeoutException("Connect timed out")))
-        .isTrue();
-    assertThat(retrier.shouldRetryOnException(new SocketTimeoutException("connect timed out")))
-        .isTrue();
-    // Shouldn't retry on read timeouts, where error message is "Read timed out"
-    assertThat(retrier.shouldRetryOnException(new SocketTimeoutException("Read timed out")))
-        .isFalse();
-    // Shouldn't retry on write timeouts or other IOException
-    assertThat(retrier.shouldRetryOnException(new SocketTimeoutException("timeout"))).isFalse();
-    assertThat(retrier.shouldRetryOnException(new SocketTimeoutException())).isTrue();
-    assertThat(retrier.shouldRetryOnException(new IOException("error"))).isFalse();
-
-    // Testing configured predicate
-    assertThat(retrier.shouldRetryOnException(new HttpRetryException("error", 400))).isFalse();
-    assertThat(retrier.shouldRetryOnException(new HttpRetryException("timeout retry", 400)))
-        .isTrue();
+  @ParameterizedTest
+  @MethodSource("isRetryableExceptionArgs")
+  void isRetryableException(IOException exception, boolean expectedRetryResult) {
+    assertThat(retrier.shouldRetryOnException(exception)).isEqualTo(expectedRetryResult);
+  }
+
+  private static Stream<Arguments> isRetryableExceptionArgs() {
+    return Stream.of(
+        // Should retry on SocketTimeoutExceptions
+        Arguments.of(new SocketTimeoutException("Connect timed out"), true),
+        Arguments.of(new SocketTimeoutException("connect timed out"), true),
+        Arguments.of(new SocketTimeoutException("timeout"), true),
+        Arguments.of(new SocketTimeoutException("Read timed out"), true),
+        Arguments.of(new SocketTimeoutException(), true),
+        // Should retry on UnknownHostExceptions
+        Arguments.of(new UnknownHostException("host"), true),
+        // Should retry on ConnectException
+        Arguments.of(
+            new ConnectException("Failed to connect to localhost/[0:0:0:0:0:0:0:1]:62611"), true),
+        // Shouldn't retry other IOException
+        Arguments.of(new IOException("error"), false),
+        // Testing configured predicate
+        Arguments.of(new HttpRetryException("error", 400), false),
+        Arguments.of(new HttpRetryException("timeout retry", 400), true));
   }
 
   @Test

sdk/common/src/main/java/io/opentelemetry/sdk/common/export/RetryPolicy.java

@@ -70,8 +70,8 @@ public static RetryPolicyBuilder builder() {
   public abstract double getBackoffMultiplier();
 
   /**
-   * Returns the predicate used to determine if thrown exception is retryableor {@code null} if no
-   * predicate was set.
+   * Returns the predicate used to determine if an attempt which failed exceptionally should be
+   * retried, or {@code null} if the exporter specific default predicate should be used.
    */
   @Nullable
   public abstract Predicate<IOException> getRetryExceptionPredicate();
@@ -106,7 +106,10 @@ public abstract static class RetryPolicyBuilder {
      */
     public abstract RetryPolicyBuilder setBackoffMultiplier(double backoffMultiplier);
 
-    /** Set the predicate to determine if retry should happen based on exception. */
+    /**
+     * Set the predicate used to determine if an attempt which failed exceptionally should be
+     * retried. By default, an exporter specific default predicate should be used.
+     */
     public abstract RetryPolicyBuilder setRetryExceptionPredicate(
         Predicate<IOException> retryExceptionPredicate);