Skip to content

Commit

Permalink
Enable retry by default for OTLP exporters (#6588)
Browse files Browse the repository at this point in the history
  • Loading branch information
jack-berg authored Jul 19, 2024
1 parent fa215ff commit 8fd45a7
Show file tree
Hide file tree
Showing 27 changed files with 256 additions and 233 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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))) {
Expand All @@ -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);
}
Expand Down Expand Up @@ -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);
Expand All @@ -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))) {
Expand All @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.getMemoryMode()).isEqualTo(MemoryMode.IMMUTABLE_DATA);
}
Mockito.verifyNoInteractions(httpBuilder);
Expand All @@ -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 (MetricExporter exporter =
provider.createExporter(DefaultConfigProperties.createFromMap(config))) {
Expand All @@ -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);
}
Expand Down Expand Up @@ -211,7 +211,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.getMemoryMode()).isEqualTo(MemoryMode.IMMUTABLE_DATA);
}
Mockito.verifyNoInteractions(grpcBuilder);
Expand All @@ -228,7 +228,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 (MetricExporter exporter =
provider.createExporter(DefaultConfigProperties.createFromMap(config))) {
Expand All @@ -241,7 +241,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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,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);
Expand All @@ -144,7 +144,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 (SpanExporter exporter =
provider.createExporter(DefaultConfigProperties.createFromMap(config))) {
Expand All @@ -157,7 +157,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);
}
Expand Down Expand Up @@ -211,7 +211,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);
Expand All @@ -229,7 +229,7 @@ void createExporter_HttpWithGeneralConfiguration() throws CertificateEncodingExc
"otel.exporter.otlp.headers", "header-key1=header%20value1,header-key2=header value2");
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 (SpanExporter exporter =
provider.createExporter(DefaultConfigProperties.createFromMap(config))) {
Expand All @@ -243,7 +243,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();
assertThat(exporter).extracting("memoryMode").isEqualTo(MemoryMode.IMMUTABLE_DATA);
}
Mockito.verifyNoInteractions(grpcBuilder);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ void stringRepresentation() {
+ ", "
+ "exportAsJson=false, "
+ "headers=Headers\\{User-Agent=OBFUSCATED\\}, "
+ "retryPolicy=RetryPolicy\\{.*\\}, "
+ "aggregationTemporalitySelector=AggregationTemporalitySelector\\{.*\\}, "
+ "defaultAggregationSelector=DefaultAggregationSelector\\{.*\\}, "
+ "memoryMode=IMMUTABLE_DATA"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ void stringRepresentation() {
+ ", "
+ "exportAsJson=false, "
+ "headers=Headers\\{User-Agent=OBFUSCATED\\}, "
+ "retryPolicy=RetryPolicy\\{.*\\}, "
+ "memoryMode=IMMUTABLE_DATA"
+ "\\}");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ void stringRepresentation() {
+ ", "
+ "compressorEncoding=null, "
+ "headers=Headers\\{User-Agent=OBFUSCATED\\}, "
+ "retryPolicy=RetryPolicy\\{.*\\}, "
+ "aggregationTemporalitySelector=AggregationTemporalitySelector\\{.*\\}, "
+ "defaultAggregationSelector=DefaultAggregationSelector\\{.*\\}, "
+ "memoryMode=IMMUTABLE_DATA"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ void stringRepresentation() {
+ ", "
+ "compressorEncoding=null, "
+ "headers=Headers\\{User-Agent=OBFUSCATED\\}, "
+ "retryPolicy=RetryPolicy\\{.*\\}, "
+ "memoryMode=IMMUTABLE_DATA"
+ "\\}");
}
Expand Down
Loading

0 comments on commit 8fd45a7

Please sign in to comment.