Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable retry by default for OTLP exporters #6588

Merged
merged 3 commits into from
Jul 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading