Enable retry by default for OTLP exporters (#6588)

Author: jack-berg

exporters/common/src/main/java/io/opentelemetry/exporter/internal/grpc/GrpcExporterBuilder.java

@@ -60,7 +60,7 @@ public class GrpcExporterBuilder<T extends Marshaler> {
   private final Map<String, String> constantHeaders = new HashMap<>();
   private Supplier<Map<String, String>> headerSupplier = Collections::emptyMap;
   private TlsConfigHelper tlsConfigHelper = new TlsConfigHelper();
-  @Nullable private RetryPolicy retryPolicy;
+  @Nullable private RetryPolicy retryPolicy = RetryPolicy.getDefault();
   private Supplier<MeterProvider> meterProviderSupplier = GlobalOpenTelemetry::getMeterProvider;
 
   // Use Object type since gRPC may not be on the classpath.
@@ -137,7 +137,7 @@ public GrpcExporterBuilder<T> setHeadersSupplier(Supplier<Map<String, String>> h
     return this;
   }
 
-  public GrpcExporterBuilder<T> setRetryPolicy(RetryPolicy retryPolicy) {
+  public GrpcExporterBuilder<T> setRetryPolicy(@Nullable RetryPolicy retryPolicy) {
     this.retryPolicy = retryPolicy;
     return this;
   }

exporters/common/src/main/java/io/opentelemetry/exporter/internal/http/HttpExporterBuilder.java

@@ -59,7 +59,7 @@ public final class HttpExporterBuilder<T extends Marshaler> {
   private Supplier<Map<String, String>> headerSupplier = Collections::emptyMap;
 
   private TlsConfigHelper tlsConfigHelper = new TlsConfigHelper();
-  @Nullable private RetryPolicy retryPolicy;
+  @Nullable private RetryPolicy retryPolicy = RetryPolicy.getDefault();
   private Supplier<MeterProvider> meterProviderSupplier = GlobalOpenTelemetry::getMeterProvider;
   @Nullable private Authenticator authenticator;
 
@@ -128,7 +128,7 @@ public HttpExporterBuilder<T> setMeterProvider(Supplier<MeterProvider> meterProv
     return this;
   }
 
-  public HttpExporterBuilder<T> setRetryPolicy(RetryPolicy retryPolicy) {
+  public HttpExporterBuilder<T> setRetryPolicy(@Nullable RetryPolicy retryPolicy) {
     this.retryPolicy = retryPolicy;
     return this;
   }

exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/http/logs/OtlpHttpLogRecordExporterBuilder.java

@@ -23,6 +23,7 @@
 import java.util.Map;
 import java.util.concurrent.TimeUnit;
 import java.util.function.Supplier;
+import javax.annotation.Nullable;
 import javax.net.ssl.SSLContext;
 import javax.net.ssl.X509TrustManager;
 
@@ -166,12 +167,12 @@ public OtlpHttpLogRecordExporterBuilder setSslContext(
   }
 
   /**
-   * Set the retry policy. Retry is disabled by default.
+   * Set the retry policy, or {@code null} to disable retry. Retry policy is {@link
+   * RetryPolicy#getDefault()} by default
    *
    * @since 1.28.0
    */
-  public OtlpHttpLogRecordExporterBuilder setRetryPolicy(RetryPolicy retryPolicy) {
-    requireNonNull(retryPolicy, "retryPolicy");
+  public OtlpHttpLogRecordExporterBuilder setRetryPolicy(@Nullable RetryPolicy retryPolicy) {
     delegate.setRetryPolicy(retryPolicy);
     return this;
   }

exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/http/metrics/OtlpHttpMetricExporterBuilder.java

@@ -28,6 +28,7 @@
 import java.util.Map;
 import java.util.concurrent.TimeUnit;
 import java.util.function.Supplier;
+import javax.annotation.Nullable;
 import javax.net.ssl.SSLContext;
 import javax.net.ssl.X509TrustManager;
 
@@ -212,12 +213,12 @@ public OtlpHttpMetricExporterBuilder setDefaultAggregationSelector(
   }
 
   /**
-   * Set the retry policy. Retry is disabled by default.
+   * Set the retry policy, or {@code null} to disable retry. Retry policy is {@link
+   * RetryPolicy#getDefault()} by default
    *
    * @since 1.28.0
    */
-  public OtlpHttpMetricExporterBuilder setRetryPolicy(RetryPolicy retryPolicy) {
-    requireNonNull(retryPolicy, "retryPolicy");
+  public OtlpHttpMetricExporterBuilder setRetryPolicy(@Nullable RetryPolicy retryPolicy) {
     delegate.setRetryPolicy(retryPolicy);
     return this;
   }

exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/http/trace/OtlpHttpSpanExporterBuilder.java

@@ -23,6 +23,7 @@
 import java.util.Map;
 import java.util.concurrent.TimeUnit;
 import java.util.function.Supplier;
+import javax.annotation.Nullable;
 import javax.net.ssl.SSLContext;
 import javax.net.ssl.X509TrustManager;
 
@@ -167,12 +168,12 @@ public OtlpHttpSpanExporterBuilder setSslContext(
   }
 
   /**
-   * Set the retry policy. Retry is disabled by default.
+   * Set the retry policy, or {@code null} to disable retry. Retry policy is {@link
+   * RetryPolicy#getDefault()} by default
    *
    * @since 1.28.0
    */
-  public OtlpHttpSpanExporterBuilder setRetryPolicy(RetryPolicy retryPolicy) {
-    requireNonNull(retryPolicy, "retryPolicy");
+  public OtlpHttpSpanExporterBuilder setRetryPolicy(@Nullable RetryPolicy retryPolicy) {
     delegate.setRetryPolicy(retryPolicy);
     return this;
   }

exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/internal/OtlpConfigUtil.java

@@ -151,10 +151,16 @@ public static void configureOtlpExporterBuilder(
       setClientTls.accept(clientKeyBytes, clientKeyChainBytes);
     }
 
-    boolean retryEnabled =
-        config.getBoolean("otel.experimental.exporter.otlp.retry.enabled", false);
-    if (retryEnabled) {
-      setRetryPolicy.accept(RetryPolicy.getDefault());
+    Boolean retryDisabled = config.getBoolean("otel.java.exporter.otlp.retry.disabled");
+    if (retryDisabled == null) {
+      Boolean experimentalRetryEnabled =
+          config.getBoolean("otel.experimental.exporter.otlp.retry.enabled");
+      if (experimentalRetryEnabled != null) {
+        retryDisabled = !experimentalRetryEnabled;
+      }
+    }
+    if (retryDisabled != null && retryDisabled) {
+      setRetryPolicy.accept(null);
     }
 
     ExporterBuilderUtil.configureExporterMemoryMode(config, setMemoryMode);

exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/logs/OtlpGrpcLogRecordExporterBuilder.java

@@ -24,6 +24,7 @@
 import java.util.Map;
 import java.util.concurrent.TimeUnit;
 import java.util.function.Supplier;
+import javax.annotation.Nullable;
 import javax.net.ssl.SSLContext;
 import javax.net.ssl.X509TrustManager;
 
@@ -210,12 +211,12 @@ public OtlpGrpcLogRecordExporterBuilder setHeaders(Supplier<Map<String, String>>
   }
 
   /**
-   * Set the retry policy. Retry is disabled by default.
+   * Set the retry policy, or {@code null} to disable retry. Retry policy is {@link
+   * RetryPolicy#getDefault()} by default
    *
    * @since 1.28.0
    */
-  public OtlpGrpcLogRecordExporterBuilder setRetryPolicy(RetryPolicy retryPolicy) {
-    requireNonNull(retryPolicy, "retryPolicy");
+  public OtlpGrpcLogRecordExporterBuilder setRetryPolicy(@Nullable RetryPolicy retryPolicy) {
     delegate.setRetryPolicy(retryPolicy);
     return this;
   }

exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/metrics/OtlpGrpcMetricExporterBuilder.java

@@ -29,6 +29,7 @@
 import java.util.Map;
 import java.util.concurrent.TimeUnit;
 import java.util.function.Supplier;
+import javax.annotation.Nullable;
 import javax.net.ssl.SSLContext;
 import javax.net.ssl.X509TrustManager;
 
@@ -256,12 +257,12 @@ public OtlpGrpcMetricExporterBuilder setDefaultAggregationSelector(
   }
 
   /**
-   * Set the retry policy. Retry is disabled by default.
+   * Set the retry policy, or {@code null} to disable retry. Retry policy is {@link
+   * RetryPolicy#getDefault()} by default
    *
    * @since 1.28.0
    */
-  public OtlpGrpcMetricExporterBuilder setRetryPolicy(RetryPolicy retryPolicy) {
-    requireNonNull(retryPolicy, "retryPolicy");
+  public OtlpGrpcMetricExporterBuilder setRetryPolicy(@Nullable RetryPolicy retryPolicy) {
     delegate.setRetryPolicy(retryPolicy);
     return this;
   }

exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/trace/OtlpGrpcSpanExporterBuilder.java

@@ -24,6 +24,7 @@
 import java.util.Map;
 import java.util.concurrent.TimeUnit;
 import java.util.function.Supplier;
+import javax.annotation.Nullable;
 import javax.net.ssl.SSLContext;
 import javax.net.ssl.X509TrustManager;
 
@@ -207,12 +208,12 @@ public OtlpGrpcSpanExporterBuilder setHeaders(Supplier<Map<String, String>> head
   }
 
   /**
-   * Set the retry policy. Retry is disabled by default.
+   * Set the retry policy, or {@code null} to disable retry. Retry policy is {@link
+   * RetryPolicy#getDefault()} by default
    *
    * @since 1.28.0
    */
-  public OtlpGrpcSpanExporterBuilder setRetryPolicy(RetryPolicy retryPolicy) {
-    requireNonNull(retryPolicy, "retryPolicy");
+  public OtlpGrpcSpanExporterBuilder setRetryPolicy(@Nullable RetryPolicy retryPolicy) {
     delegate.setRetryPolicy(retryPolicy);
     return this;
   }

exporters/otlp/all/src/test/java/io/opentelemetry/exporter/otlp/internal/OtlpLogRecordExporterProviderTest.java

@@ -127,7 +127,7 @@ void createExporter_GrpcDefaults() {
       verify(grpcBuilder, never()).setTimeout(any());
       verify(grpcBuilder, never()).setTrustedCertificates(any());
       verify(grpcBuilder, never()).setClientTls(any(), any());
-      assertThat(grpcBuilder).extracting("delegate").extracting("retryPolicy").isNull();
+      assertThat(grpcBuilder).extracting("delegate").extracting("retryPolicy").isNotNull();
       assertThat(exporter).extracting("memoryMode").isEqualTo(MemoryMode.IMMUTABLE_DATA);
     }
     Mockito.verifyNoInteractions(httpBuilder);
@@ -143,7 +143,7 @@ void createExporter_GrpcWithGeneralConfiguration() throws CertificateEncodingExc
     config.put("otel.exporter.otlp.headers", "header-key=header-value");
     config.put("otel.exporter.otlp.compression", "gzip");
     config.put("otel.exporter.otlp.timeout", "15s");
-    config.put("otel.experimental.exporter.otlp.retry.enabled", "true");
+    config.put("otel.java.exporter.otlp.retry.disabled", "true");
 
     try (LogRecordExporter exporter =
         provider.createExporter(DefaultConfigProperties.createFromMap(config))) {
@@ -156,7 +156,7 @@ void createExporter_GrpcWithGeneralConfiguration() throws CertificateEncodingExc
       verify(grpcBuilder).setTrustedCertificates(serverTls.certificate().getEncoded());
       verify(grpcBuilder)
           .setClientTls(clientTls.privateKey().getEncoded(), clientTls.certificate().getEncoded());
-      assertThat(grpcBuilder).extracting("delegate").extracting("retryPolicy").isNotNull();
+      assertThat(grpcBuilder).extracting("delegate").extracting("retryPolicy").isNull();
     }
     Mockito.verifyNoInteractions(httpBuilder);
   }
@@ -210,7 +210,7 @@ void createExporter_HttpDefaults() {
       verify(httpBuilder, never()).setTimeout(any());
       verify(httpBuilder, never()).setTrustedCertificates(any());
       verify(httpBuilder, never()).setClientTls(any(), any());
-      assertThat(httpBuilder).extracting("delegate").extracting("retryPolicy").isNull();
+      assertThat(httpBuilder).extracting("delegate").extracting("retryPolicy").isNotNull();
       assertThat(exporter).extracting("memoryMode").isEqualTo(MemoryMode.IMMUTABLE_DATA);
     }
     Mockito.verifyNoInteractions(grpcBuilder);
@@ -227,7 +227,7 @@ void createExporter_HttpWithGeneralConfiguration() throws CertificateEncodingExc
     config.put("otel.exporter.otlp.headers", "header-key=header-value");
     config.put("otel.exporter.otlp.compression", "gzip");
     config.put("otel.exporter.otlp.timeout", "15s");
-    config.put("otel.experimental.exporter.otlp.retry.enabled", "true");
+    config.put("otel.java.exporter.otlp.retry.disabled", "true");
 
     try (LogRecordExporter exporter =
         provider.createExporter(DefaultConfigProperties.createFromMap(config))) {
@@ -240,7 +240,7 @@ void createExporter_HttpWithGeneralConfiguration() throws CertificateEncodingExc
       verify(httpBuilder).setTrustedCertificates(serverTls.certificate().getEncoded());
       verify(httpBuilder)
           .setClientTls(clientTls.privateKey().getEncoded(), clientTls.certificate().getEncoded());
-      assertThat(httpBuilder).extracting("delegate").extracting("retryPolicy").isNotNull();
+      assertThat(httpBuilder).extracting("delegate").extracting("retryPolicy").isNull();
     }
     Mockito.verifyNoInteractions(grpcBuilder);
   }