Use HTTP error bodies in HttpExporter warnings (#8428)

Author: ADITYA-CODE-SOURCE

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

@@ -18,6 +18,7 @@
 import io.opentelemetry.sdk.common.internal.ThrottlingLogger;
 import java.io.IOException;
 import java.net.URI;
+import java.nio.charset.StandardCharsets;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.function.Supplier;
 import java.util.logging.Level;
@@ -32,6 +33,8 @@
  */
 @SuppressWarnings("checkstyle:JavadocMethod")
 public final class HttpExporter {
+  // Limit logged response body text to avoid flooding warnings with large payloads.
+  private static final int MAX_RESPONSE_BODY_LOG_LENGTH = 1024;
 
   private static final Logger internalLogger = Logger.getLogger(HttpExporter.class.getName());
 
@@ -132,10 +135,23 @@ private static String extractErrorStatus(String statusMessage, @Nullable byte[]
     if (responseBody == null) {
       return "Response body missing, HTTP status message: " + statusMessage;
     }
+    if (responseBody.length == 0) {
+      return "Response body has 0 length, HTTP status message: " + statusMessage;
+    }
     try {
       return GrpcExporterUtil.getStatusMessage(responseBody);
     } catch (IOException e) {
-      return "Unable to parse response body, HTTP status message: " + statusMessage;
+      return extractResponseBodyMessage(responseBody, statusMessage);
+    }
+  }
+
+  private static String extractResponseBodyMessage(byte[] responseBody, String statusMessage) {
+    int lengthToRead = Math.min(responseBody.length, MAX_RESPONSE_BODY_LOG_LENGTH);
+    String responseBodyText =
+        new String(responseBody, 0, lengthToRead, StandardCharsets.UTF_8).trim();
+    if (responseBodyText.isEmpty()) {
+      return "HTTP status message: " + statusMessage;
     }
+    return "Response body: " + responseBodyText + ", HTTP status message: " + statusMessage;
   }
 }

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

@@ -9,7 +9,9 @@
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.doAnswer;
 
+import io.github.netmikey.logunit.api.LogCapturer;
 import io.opentelemetry.api.common.Attributes;
+import io.opentelemetry.api.metrics.MeterProvider;
 import io.opentelemetry.exporter.internal.marshal.Marshaler;
 import io.opentelemetry.internal.testing.slf4j.SuppressLogger;
 import io.opentelemetry.sdk.common.InternalTelemetryVersion;
@@ -22,13 +24,20 @@
 import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader;
 import java.io.IOException;
 import java.net.URI;
+import java.nio.charset.StandardCharsets;
+import java.util.concurrent.TimeUnit;
 import java.util.function.Consumer;
+import javax.annotation.Nullable;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.EnumSource;
 import org.mockito.Mockito;
 
 class HttpExporterTest {
 
+  @RegisterExtension LogCapturer logs = LogCapturer.create().captureForType(HttpExporter.class);
+
   @ParameterizedTest
   @EnumSource
   @SuppressLogger(HttpExporter.class)
@@ -197,14 +206,112 @@ void testInternalTelemetry(StandardComponentId.ExporterType exporterType) {
     }
   }
 
+  @Test
+  @SuppressLogger(HttpExporter.class)
+  void export_httpJsonErrorBodyUsesBodyTextWithoutGrpcParseWarning() {
+    HttpSender mockSender = Mockito.mock(HttpSender.class);
+    Marshaler mockMarshaller = Mockito.mock(Marshaler.class);
+    HttpExporter exporter =
+        new HttpExporter(
+            ComponentId.generateLazy(StandardComponentId.ExporterType.OTLP_HTTP_SPAN_EXPORTER),
+            mockSender,
+            MeterProvider::noop,
+            InternalTelemetryVersion.LATEST,
+            URI.create("http://testing:1234"),
+            false);
+
+    doAnswer(
+            invoc -> {
+              Consumer<HttpResponse> onResponse = invoc.getArgument(1);
+              onResponse.accept(
+                  new FakeHttpResponse(
+                      500,
+                      "Internal Server Error",
+                      "{\"error\":\"grpc not supported\"}".getBytes(StandardCharsets.UTF_8)));
+              return null;
+            })
+        .when(mockSender)
+        .send(any(), any(), any());
+
+    assertThat(exporter.export(mockMarshaller, 1).join(10, TimeUnit.SECONDS).isSuccess()).isFalse();
+
+    logs.assertContains("Response body: {\"error\":\"grpc not supported\"}");
+    logs.assertDoesNotContain("Unable to parse response body");
+  }
+
+  @Test
+  @SuppressLogger(HttpExporter.class)
+  void export_nullErrorBodyUsesMissingBodyMessage() {
+    HttpSender mockSender = Mockito.mock(HttpSender.class);
+    Marshaler mockMarshaller = Mockito.mock(Marshaler.class);
+    HttpExporter exporter =
+        new HttpExporter(
+            ComponentId.generateLazy(StandardComponentId.ExporterType.OTLP_HTTP_SPAN_EXPORTER),
+            mockSender,
+            MeterProvider::noop,
+            InternalTelemetryVersion.LATEST,
+            URI.create("http://testing:1234"),
+            false);
+
+    doAnswer(
+            invoc -> {
+              Consumer<HttpResponse> onResponse = invoc.getArgument(1);
+              onResponse.accept(new FakeHttpResponse(500, "Internal Server Error", null));
+              return null;
+            })
+        .when(mockSender)
+        .send(any(), any(), any());
+
+    assertThat(exporter.export(mockMarshaller, 1).join(10, TimeUnit.SECONDS).isSuccess()).isFalse();
+
+    logs.assertContains("Response body missing, HTTP status message: Internal Server Error");
+  }
+
+  @Test
+  @SuppressLogger(HttpExporter.class)
+  void export_whitespaceErrorBodyFallsBackToStatusMessage() {
+    HttpSender mockSender = Mockito.mock(HttpSender.class);
+    Marshaler mockMarshaller = Mockito.mock(Marshaler.class);
+    HttpExporter exporter =
+        new HttpExporter(
+            ComponentId.generateLazy(StandardComponentId.ExporterType.OTLP_HTTP_SPAN_EXPORTER),
+            mockSender,
+            MeterProvider::noop,
+            InternalTelemetryVersion.LATEST,
+            URI.create("http://testing:1234"),
+            false);
+
+    doAnswer(
+            invoc -> {
+              Consumer<HttpResponse> onResponse = invoc.getArgument(1);
+              onResponse.accept(
+                  new FakeHttpResponse(
+                      500, "Internal Server Error", "   ".getBytes(StandardCharsets.UTF_8)));
+              return null;
+            })
+        .when(mockSender)
+        .send(any(), any(), any());
+
+    assertThat(exporter.export(mockMarshaller, 1).join(10, TimeUnit.SECONDS).isSuccess()).isFalse();
+
+    logs.assertContains("HTTP status message: Internal Server Error");
+    logs.assertDoesNotContain("Response body:");
+  }
+
   private static class FakeHttpResponse implements HttpResponse {
 
     final int statusCode;
     final String statusMessage;
+    @Nullable final byte[] responseBody;
 
     FakeHttpResponse(int statusCode, String statusMessage) {
+      this(statusCode, statusMessage, new byte[0]);
+    }
+
+    FakeHttpResponse(int statusCode, String statusMessage, @Nullable byte[] responseBody) {
       this.statusCode = statusCode;
       this.statusMessage = statusMessage;
+      this.responseBody = responseBody;
     }
 
     @Override
@@ -218,8 +325,9 @@ public String getStatusMessage() {
     }
 
     @Override
+    @Nullable
     public byte[] getResponseBody() {
-      return new byte[0];
+      return responseBody;
     }
   }
 }