Skip to content

Commit f3d04f4

Browse files
YuriyHolinkoYuriyHolinko
committed
User can use default predicate, override it or extend
1 parent aac988d commit f3d04f4

File tree

6 files changed

+71
-30
lines changed

6 files changed

+71
-30
lines changed

‎exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/AbstractGrpcTelemetryExporterTest.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -1000,7 +1000,7 @@ void stringRepresentation() throws IOException, CertificateEncodingException {
10001000
+ ", "
10011001
+ "compressorEncoding=gzip, "
10021002
+ "headers=Headers\\{.*foo=OBFUSCATED.*\\}, "
1003-
+ "retryPolicy=RetryPolicy\\{maxAttempts=2, initialBackoff=PT0\\.05S, maxBackoff=PT3S, backoffMultiplier=1\\.3, retryExceptionPredicate=io.opentelemetry.sdk.common.export.RetryPolicy.*\\}"
1003+
+ "retryPolicy=RetryPolicy\\{maxAttempts=2, initialBackoff=PT0\\.05S, maxBackoff=PT3S, backoffMultiplier=1\\.3, retryExceptionPredicate=io.opentelemetry.sdk.common.export.DefaultRetryExceptionPredicate.*\\}"
10041004
+ ".*" // Maybe additional grpcChannel field, signal specific fields
10051005
+ "\\}");
10061006
} finally {

‎exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/AbstractHttpTelemetryExporterTest.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -904,7 +904,7 @@ void stringRepresentation() throws IOException, CertificateEncodingException {
904904
+ ", "
905905
+ "exportAsJson=false, "
906906
+ "headers=Headers\\{.*foo=OBFUSCATED.*\\}, "
907-
+ "retryPolicy=RetryPolicy\\{maxAttempts=2, initialBackoff=PT0\\.05S, maxBackoff=PT3S, backoffMultiplier=1\\.3, retryExceptionPredicate=io.opentelemetry.sdk.common.export.RetryPolicy.*\\}"
907+
+ "retryPolicy=RetryPolicy\\{maxAttempts=2, initialBackoff=PT0\\.05S, maxBackoff=PT3S, backoffMultiplier=1\\.3, retryExceptionPredicate=io.opentelemetry.sdk.common.export.DefaultRetryExceptionPredicate.*\\}"
908908
+ ".*" // Maybe additional signal specific fields
909909
+ "\\}");
910910
} finally {

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

+1-3
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,7 @@ public RetryInterceptor(RetryPolicy retryPolicy, Function<Response, Boolean> isR
4242
this(
4343
retryPolicy,
4444
isRetryable,
45-
e ->
46-
retryPolicy.getRetryExceptionPredicate().test(e)
47-
|| RetryInterceptor.isRetryableException(e),
45+
e -> retryPolicy.getRetryExceptionPredicate().test(e),
4846
TimeUnit.NANOSECONDS::sleep,
4947
bound -> ThreadLocalRandom.current().nextLong(bound));
5048
}

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

+31-24
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import com.linecorp.armeria.common.HttpResponse;
2222
import com.linecorp.armeria.common.HttpStatus;
2323
import com.linecorp.armeria.testing.junit5.server.mock.MockWebServerExtension;
24+
import io.opentelemetry.sdk.common.export.DefaultRetryExceptionPredicate;
2425
import io.opentelemetry.sdk.common.export.RetryPolicy;
2526
import java.io.IOException;
2627
import java.net.ConnectException;
@@ -29,7 +30,7 @@
2930
import java.net.SocketTimeoutException;
3031
import java.time.Duration;
3132
import java.util.concurrent.TimeUnit;
32-
import java.util.function.Function;
33+
import java.util.function.Predicate;
3334
import okhttp3.OkHttpClient;
3435
import okhttp3.Request;
3536
import okhttp3.Response;
@@ -49,34 +50,40 @@ class RetryInterceptorTest {
4950

5051
@Mock private RetryInterceptor.Sleeper sleeper;
5152
@Mock private RetryInterceptor.BoundedLongGenerator random;
52-
private Function<IOException, Boolean> isRetryableException;
53+
private Predicate<IOException> retryPredicate;
5354

5455
private RetryInterceptor retrier;
5556
private OkHttpClient client;
5657

5758
@BeforeEach
5859
void setUp() {
59-
// Note: cannot replace this with lambda or method reference because we need to spy on it
60-
isRetryableException =
60+
DefaultRetryExceptionPredicate defaultRetryExceptionPredicate =
61+
new DefaultRetryExceptionPredicate();
62+
retryPredicate =
6163
spy(
62-
new Function<IOException, Boolean>() {
64+
new Predicate<IOException>() {
6365
@Override
64-
public Boolean apply(IOException exception) {
65-
return RetryInterceptor.isRetryableException(exception)
66-
|| (exception instanceof HttpRetryException
67-
&& exception.getMessage().contains("timeout retry"));
66+
public boolean test(IOException e) {
67+
return defaultRetryExceptionPredicate.test(e)
68+
|| (e instanceof HttpRetryException
69+
&& e.getMessage().contains("timeout retry"));
6870
}
6971
});
72+
73+
RetryPolicy retryPolicy =
74+
RetryPolicy.builder()
75+
.setBackoffMultiplier(1.6)
76+
.setInitialBackoff(Duration.ofSeconds(1))
77+
.setMaxBackoff(Duration.ofSeconds(2))
78+
.setMaxAttempts(5)
79+
.setRetryExceptionPredicate(retryPredicate)
80+
.build();
81+
7082
retrier =
7183
new RetryInterceptor(
72-
RetryPolicy.builder()
73-
.setBackoffMultiplier(1.6)
74-
.setInitialBackoff(Duration.ofSeconds(1))
75-
.setMaxBackoff(Duration.ofSeconds(2))
76-
.setMaxAttempts(5)
77-
.build(),
84+
retryPolicy,
7885
r -> !r.isSuccessful(),
79-
isRetryableException,
86+
e -> retryPolicy.getRetryExceptionPredicate().test(e),
8087
sleeper,
8188
random);
8289
client = new OkHttpClient.Builder().addInterceptor(retrier).build();
@@ -157,7 +164,7 @@ void connectTimeout() throws Exception {
157164
client.newCall(new Request.Builder().url("http://10.255.255.1").build()).execute())
158165
.isInstanceOf(SocketTimeoutException.class);
159166

160-
verify(isRetryableException, times(5)).apply(any());
167+
verify(retryPredicate, times(5)).test(any());
161168
// Should retry maxAttempts, and sleep maxAttempts - 1 times
162169
verify(sleeper, times(4)).sleep(anyLong());
163170
}
@@ -177,7 +184,7 @@ void connectException() throws Exception {
177184
.execute())
178185
.isInstanceOfAny(ConnectException.class, SocketTimeoutException.class);
179186

180-
verify(isRetryableException, times(5)).apply(any());
187+
verify(retryPredicate, times(5)).test(any());
181188
// Should retry maxAttempts, and sleep maxAttempts - 1 times
182189
verify(sleeper, times(4)).sleep(anyLong());
183190
}
@@ -193,16 +200,16 @@ private static int freePort() {
193200
@Test
194201
void nonRetryableException() throws InterruptedException {
195202
client = connectTimeoutClient();
196-
// Override isRetryableException so that no exception is retryable
197-
when(isRetryableException.apply(any())).thenReturn(false);
203+
// Override retryPredicate so that no exception is retryable
204+
when(retryPredicate.test(any())).thenReturn(false);
198205

199206
// Connecting to a non-routable IP address to trigger connection timeout
200207
assertThatThrownBy(
201208
() ->
202209
client.newCall(new Request.Builder().url("http://10.255.255.1").build()).execute())
203210
.isInstanceOf(SocketTimeoutException.class);
204211

205-
verify(isRetryableException, times(1)).apply(any());
212+
verify(retryPredicate, times(1)).test(any());
206213
verify(sleeper, never()).sleep(anyLong());
207214
}
208215

@@ -224,8 +231,8 @@ void isRetryableException() {
224231
// Shouldn't retry on read timeouts, where error message is "Read timed out"
225232
assertThat(retrier.shouldRetryOnException(new SocketTimeoutException("Read timed out")))
226233
.isFalse();
227-
// Shouldn't retry on write timeouts, where error message is "timeout", or other IOException
228-
assertThat(retrier.shouldRetryOnException(new SocketTimeoutException("timeout"))).isFalse();
234+
// Shouldn't retry on write timeouts or other IOException
235+
assertThat(retrier.shouldRetryOnException(new SocketTimeoutException("timeout"))).isTrue();
229236
assertThat(retrier.shouldRetryOnException(new SocketTimeoutException())).isTrue();
230237
assertThat(retrier.shouldRetryOnException(new IOException("error"))).isFalse();
231238

@@ -261,7 +268,7 @@ void isRetryableExceptionCustomRetryPredicate() {
261268
assertThat(
262269
retryInterceptor.shouldRetryOnException(
263270
new SocketTimeoutException("Connect timed out")))
264-
.isTrue();
271+
.isFalse();
265272
}
266273

267274
private Response sendRequest() throws IOException {

‎sdk/common/src/main/java/io/opentelemetry/sdk/common/export/DefaultRetryExceptionPredicate.java

+33
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
/*
2+
* Copyright The OpenTelemetry Authors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package io.opentelemetry.sdk.common.export;
7+
8+
import java.io.IOException;
9+
import java.net.ConnectException;
10+
import java.net.SocketTimeoutException;
11+
import java.util.Locale;
12+
import java.util.function.Predicate;
13+
14+
public class DefaultRetryExceptionPredicate implements Predicate<IOException> {
15+
@Override
16+
public boolean test(IOException e) {
17+
if (e instanceof SocketTimeoutException) {
18+
String message = e.getMessage();
19+
// Connect timeouts can produce SocketTimeoutExceptions with no message, or with "connect
20+
// timed out", or timeout
21+
if (message == null) {
22+
return true;
23+
}
24+
message = message.toLowerCase(Locale.ROOT);
25+
return message.contains("connect timed out") || message.contains("timeout");
26+
} else if (e instanceof ConnectException) {
27+
// Exceptions resemble: java.net.ConnectException: Failed to connect to
28+
// localhost/[0:0:0:0:0:0:0:1]:62611
29+
return true;
30+
}
31+
return false;
32+
}
33+
}

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

+4-1
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@ public abstract class RetryPolicy {
3030

3131
private static final double DEFAULT_BACKOFF_MULTIPLIER = 1.5;
3232

33+
private static final Predicate<IOException> DEFAULT_RETRY_PREDICATE =
34+
new DefaultRetryExceptionPredicate();
35+
3336
private static final RetryPolicy DEFAULT = RetryPolicy.builder().build();
3437

3538
RetryPolicy() {}
@@ -46,7 +49,7 @@ public static RetryPolicyBuilder builder() {
4649
.setInitialBackoff(Duration.ofSeconds(DEFAULT_INITIAL_BACKOFF_SECONDS))
4750
.setMaxBackoff(Duration.ofSeconds(DEFAULT_MAX_BACKOFF_SECONDS))
4851
.setBackoffMultiplier(DEFAULT_BACKOFF_MULTIPLIER)
49-
.setRetryExceptionPredicate((e) -> false);
52+
.setRetryExceptionPredicate(DEFAULT_RETRY_PREDICATE);
5053
}
5154

5255
/**

0 commit comments

Comments
 (0)