From 0accdf311dd599717cdf1a74454bab697285c134 Mon Sep 17 00:00:00 2001 From: Ricardo Mestre Date: Tue, 19 Dec 2023 14:44:58 +0000 Subject: [PATCH 1/3] Adding UncheckedIOException to onFailure handler --- .../exporter/internal/http/HttpExporter.java | 9 ++++- exporters/sender/jdk/build.gradle.kts | 1 + .../jdk/internal/JdkHttpSenderTest.java | 34 +++++++++++++++++++ .../internal/AuthenticatingExporterTest.java | 1 + 4 files changed, 44 insertions(+), 1 deletion(-) diff --git a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/http/HttpExporter.java b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/http/HttpExporter.java index 6cb38d78ae4..46341702f9e 100644 --- a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/http/HttpExporter.java +++ b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/http/HttpExporter.java @@ -77,7 +77,14 @@ public CompletableResultCode export(T exportRequest, int numItems) { try { body = httpResponse.responseBody(); } catch (IOException ex) { - throw new IllegalStateException(ex); + logger.log( + Level.WARNING, + "Failed to export " + + type + + "s. Server responded with exception message " + + ex.getMessage()); + result.fail(); + return; } String status = extractErrorStatus(httpResponse.statusMessage(), body); diff --git a/exporters/sender/jdk/build.gradle.kts b/exporters/sender/jdk/build.gradle.kts index 13f77b14824..b5a075d5aba 100644 --- a/exporters/sender/jdk/build.gradle.kts +++ b/exporters/sender/jdk/build.gradle.kts @@ -11,6 +11,7 @@ dependencies { implementation(project(":sdk:common")) compileOnly("com.fasterxml.jackson.core:jackson-core") + testImplementation("com.linecorp.armeria:armeria-junit5") } tasks { diff --git a/exporters/sender/jdk/src/test/java/io/opentelemetry/exporter/sender/jdk/internal/JdkHttpSenderTest.java b/exporters/sender/jdk/src/test/java/io/opentelemetry/exporter/sender/jdk/internal/JdkHttpSenderTest.java index 8b29fc9d8d7..a9a65b7d088 100644 --- a/exporters/sender/jdk/src/test/java/io/opentelemetry/exporter/sender/jdk/internal/JdkHttpSenderTest.java +++ b/exporters/sender/jdk/src/test/java/io/opentelemetry/exporter/sender/jdk/internal/JdkHttpSenderTest.java @@ -14,20 +14,30 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import com.linecorp.armeria.common.HttpResponse; +import com.linecorp.armeria.common.HttpStatus; +import com.linecorp.armeria.testing.junit5.server.mock.MockWebServerExtension; +import io.opentelemetry.exporter.internal.http.HttpExporter; +import io.opentelemetry.exporter.internal.http.HttpExporterBuilder; import io.opentelemetry.exporter.internal.marshal.Marshaler; import io.opentelemetry.exporter.internal.marshal.Serializer; +import io.opentelemetry.sdk.common.CompletableResultCode; import io.opentelemetry.sdk.common.export.RetryPolicy; import java.io.IOException; import java.net.http.HttpClient; import java.net.http.HttpConnectTimeoutException; import java.time.Duration; import java.util.Collections; +import java.util.HashMap; +import java.util.Map; import java.util.concurrent.TimeUnit; import javax.net.ssl.SSLException; +import org.assertj.core.api.Assertions; import org.assertj.core.api.InstanceOfAssertFactories; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.extension.RegisterExtension; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import org.mockito.junit.jupiter.MockitoSettings; @@ -37,6 +47,8 @@ @MockitoSettings(strictness = Strictness.LENIENT) class JdkHttpSenderTest { + @RegisterExtension static final MockWebServerExtension server = new MockWebServerExtension(); + private final HttpClient realHttpClient = HttpClient.newBuilder().connectTimeout(Duration.ofMillis(10)).build(); @Mock private HttpClient mockHttpClient; @@ -66,6 +78,28 @@ void setup() throws IOException, InterruptedException { .build()); } + @SuppressWarnings("SystemOut") + @Test + void exportAsJson() { + HttpExporter exporter = + new HttpExporterBuilder<>("jdk", "test", server.httpUri().toASCIIString()) + .setAuthenticator( + () -> { + Map headers = new HashMap<>(); + headers.put("Authorization", "auth"); + return headers; + }) + .exportAsJson() + .build(); + + server.enqueue(HttpResponse.of(HttpStatus.OK)); + + CompletableResultCode result = exporter.export(new NoOpMarshaler(), 0); + result.join(1, TimeUnit.MINUTES); + exporter.shutdown().join(1, TimeUnit.MINUTES); + Assertions.assertThat(result.isSuccess()).isTrue(); + } + @Test void sendInternal_RetryableConnectTimeoutException() throws IOException, InterruptedException { assertThatThrownBy(() -> sender.sendInternal(new NoOpMarshaler())) diff --git a/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/AuthenticatingExporterTest.java b/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/AuthenticatingExporterTest.java index e1c948f550c..a028d9c085b 100644 --- a/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/AuthenticatingExporterTest.java +++ b/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/AuthenticatingExporterTest.java @@ -76,6 +76,7 @@ void export_giveup() { server.enqueue(HttpResponse.of(HttpStatus.UNAUTHORIZED)); return Collections.emptyMap(); }) + .exportAsJson() .build(); server.enqueue(HttpResponse.of(HttpStatus.UNAUTHORIZED)); assertThat(exporter.export(marshaler, 0).join(1, TimeUnit.MINUTES).isSuccess()).isFalse(); From 8c590e9781aef2a4f8e0bf6d9eb13653e86dfee8 Mon Sep 17 00:00:00 2001 From: Ricardo Mestre Date: Thu, 21 Dec 2023 07:36:27 +0100 Subject: [PATCH 2/3] Update exporters/common/src/main/java/io/opentelemetry/exporter/internal/http/HttpExporter.java Co-authored-by: jack-berg <34418638+jack-berg@users.noreply.github.com> --- .../exporter/internal/http/HttpExporter.java | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/http/HttpExporter.java b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/http/HttpExporter.java index 46341702f9e..6e358f35592 100644 --- a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/http/HttpExporter.java +++ b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/http/HttpExporter.java @@ -77,14 +77,7 @@ public CompletableResultCode export(T exportRequest, int numItems) { try { body = httpResponse.responseBody(); } catch (IOException ex) { - logger.log( - Level.WARNING, - "Failed to export " - + type - + "s. Server responded with exception message " - + ex.getMessage()); - result.fail(); - return; + logger.log(Level.FIND, "Unable to obtain response body", ex); } String status = extractErrorStatus(httpResponse.statusMessage(), body); From 4600eea343dfb9deebb2b149f7ba876848916ced Mon Sep 17 00:00:00 2001 From: Ricardo Mestre Date: Thu, 21 Dec 2023 01:54:43 -0500 Subject: [PATCH 3/3] Adding log message level and test changes --- .../exporter/internal/http/HttpExporter.java | 4 ++-- .../sender/jdk/internal/JdkHttpSenderTest.java | 10 ---------- 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/http/HttpExporter.java b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/http/HttpExporter.java index 6e358f35592..befb579d964 100644 --- a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/http/HttpExporter.java +++ b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/http/HttpExporter.java @@ -73,11 +73,11 @@ public CompletableResultCode export(T exportRequest, int numItems) { exporterMetrics.addFailed(numItems); - byte[] body; + byte[] body = null; try { body = httpResponse.responseBody(); } catch (IOException ex) { - logger.log(Level.FIND, "Unable to obtain response body", ex); + logger.log(Level.FINE, "Unable to obtain response body", ex); } String status = extractErrorStatus(httpResponse.statusMessage(), body); diff --git a/exporters/sender/jdk/src/test/java/io/opentelemetry/exporter/sender/jdk/internal/JdkHttpSenderTest.java b/exporters/sender/jdk/src/test/java/io/opentelemetry/exporter/sender/jdk/internal/JdkHttpSenderTest.java index a9a65b7d088..5e480ee49d8 100644 --- a/exporters/sender/jdk/src/test/java/io/opentelemetry/exporter/sender/jdk/internal/JdkHttpSenderTest.java +++ b/exporters/sender/jdk/src/test/java/io/opentelemetry/exporter/sender/jdk/internal/JdkHttpSenderTest.java @@ -28,8 +28,6 @@ import java.net.http.HttpConnectTimeoutException; import java.time.Duration; import java.util.Collections; -import java.util.HashMap; -import java.util.Map; import java.util.concurrent.TimeUnit; import javax.net.ssl.SSLException; import org.assertj.core.api.Assertions; @@ -78,17 +76,10 @@ void setup() throws IOException, InterruptedException { .build()); } - @SuppressWarnings("SystemOut") @Test void exportAsJson() { HttpExporter exporter = new HttpExporterBuilder<>("jdk", "test", server.httpUri().toASCIIString()) - .setAuthenticator( - () -> { - Map headers = new HashMap<>(); - headers.put("Authorization", "auth"); - return headers; - }) .exportAsJson() .build(); @@ -96,7 +87,6 @@ void exportAsJson() { CompletableResultCode result = exporter.export(new NoOpMarshaler(), 0); result.join(1, TimeUnit.MINUTES); - exporter.shutdown().join(1, TimeUnit.MINUTES); Assertions.assertThat(result.isSuccess()).isTrue(); }