From a6b33029be2287ea459ed93edbc02298a3515905 Mon Sep 17 00:00:00 2001 From: Gregor Zeitlinger Date: Tue, 5 Nov 2024 19:37:40 +0100 Subject: [PATCH] bugfix: add newline in stdout exporter (#6848) --- .../exporter/internal/marshal/Marshaler.java | 8 +++++ .../internal/writer/StreamJsonWriter.java | 2 +- .../otlp/AbstractOtlpStdoutExporterTest.java | 30 +++++++++++++++---- .../internal/writer/StreamJsonWriterTest.java | 4 ++- .../PrometheusMetricReaderTest.java | 8 +++-- 5 files changed, 42 insertions(+), 10 deletions(-) diff --git a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/marshal/Marshaler.java b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/marshal/Marshaler.java index 2e6fba4644..6201481c02 100644 --- a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/marshal/Marshaler.java +++ b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/marshal/Marshaler.java @@ -38,6 +38,14 @@ public abstract class Marshaler { } } + /** Marshals into the {@link JsonGenerator} in proto JSON format and adds a newline. */ + public final void writeJsonWithNewline(JsonGenerator output) throws IOException { + try (JsonSerializer serializer = new JsonSerializer(output)) { + serializer.writeMessageValue(this); + output.writeRaw('\n'); + } + } + /** Returns the number of bytes this Marshaler will write in proto binary format. */ public abstract int getBinarySerializedSize(); diff --git a/exporters/logging-otlp/src/main/java/io/opentelemetry/exporter/logging/otlp/internal/writer/StreamJsonWriter.java b/exporters/logging-otlp/src/main/java/io/opentelemetry/exporter/logging/otlp/internal/writer/StreamJsonWriter.java index 0674810fa5..0f47ce20e0 100644 --- a/exporters/logging-otlp/src/main/java/io/opentelemetry/exporter/logging/otlp/internal/writer/StreamJsonWriter.java +++ b/exporters/logging-otlp/src/main/java/io/opentelemetry/exporter/logging/otlp/internal/writer/StreamJsonWriter.java @@ -38,7 +38,7 @@ public class StreamJsonWriter implements JsonWriter { @Override public CompletableResultCode write(Marshaler exportRequest) { try { - exportRequest.writeJsonTo( + exportRequest.writeJsonWithNewline( JSON_FACTORY .createGenerator(outputStream) .disable(JsonGenerator.Feature.AUTO_CLOSE_TARGET)); diff --git a/exporters/logging-otlp/src/test/java/io/opentelemetry/exporter/logging/otlp/AbstractOtlpStdoutExporterTest.java b/exporters/logging-otlp/src/test/java/io/opentelemetry/exporter/logging/otlp/AbstractOtlpStdoutExporterTest.java index 3988fae332..54835720d3 100644 --- a/exporters/logging-otlp/src/test/java/io/opentelemetry/exporter/logging/otlp/AbstractOtlpStdoutExporterTest.java +++ b/exporters/logging-otlp/src/test/java/io/opentelemetry/exporter/logging/otlp/AbstractOtlpStdoutExporterTest.java @@ -34,7 +34,6 @@ import java.util.concurrent.TimeUnit; import java.util.function.Supplier; import java.util.stream.Stream; import javax.annotation.Nullable; -import org.json.JSONException; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; @@ -57,6 +56,7 @@ abstract class AbstractOtlpStdoutExporterTest { private static final PrintStream SYSTEM_OUT_PRINT_STREAM = new PrintStream(SYSTEM_OUT_STREAM); @RegisterExtension LogCapturer logs; + private int skipLogs; private final String defaultConfigString; private final TestDataExporter testDataExporter; protected final Class exporterClass; @@ -87,6 +87,7 @@ abstract class AbstractOtlpStdoutExporterTest { private String output(@Nullable OutputStream outputStream, @Nullable Path file) { if (outputStream == null) { return logs.getEvents().stream() + .skip(skipLogs) .map(LoggingEvent::getMessage) .reduce("", (a, b) -> a + b + "\n") .trim(); @@ -94,14 +95,14 @@ abstract class AbstractOtlpStdoutExporterTest { if (file != null) { try { - return new String(Files.readAllBytes(file), StandardCharsets.UTF_8); + return new String(Files.readAllBytes(file), StandardCharsets.UTF_8).trim(); } catch (IOException e) { throw new RuntimeException(e); } } try { - return SYSTEM_OUT_STREAM.toString(StandardCharsets.UTF_8.name()); + return SYSTEM_OUT_STREAM.toString(StandardCharsets.UTF_8.name()).trim(); } catch (UnsupportedEncodingException e) { throw new RuntimeException(e); } @@ -204,8 +205,7 @@ abstract class AbstractOtlpStdoutExporterTest { @SuppressWarnings("SystemOut") @ParameterizedTest(name = "{0}") @MethodSource("exportTestCases") - void exportWithProgrammaticConfig(String name, TestCase testCase) - throws JSONException, IOException { + void exportWithProgrammaticConfig(String name, TestCase testCase) throws Exception { OutputStream outputStream; Path file = null; switch (testCase.getOutputType()) { @@ -247,6 +247,26 @@ abstract class AbstractOtlpStdoutExporterTest { if (testCase.isWrapperJsonObject()) { assertThat(output).doesNotContain("\n"); } + + if (file == null) { + // no need to test again for file - and it's not working with files + assertDoubleOutput(exporter, expectedJson, outputStream); + } + } + + private void assertDoubleOutput( + Supplier exporter, String expectedJson, @Nullable OutputStream outputStream) + throws Exception { + SYSTEM_OUT_STREAM.reset(); + skipLogs = logs.getEvents().size(); + testDataExporter.export(exporter.get()); + testDataExporter.export(exporter.get()); + + String[] lines = output(outputStream, null).split("\n"); + assertThat(lines).hasSize(2); + for (String line : lines) { + JSONAssert.assertEquals("Got \n" + line, expectedJson, line, false); + } } @Test diff --git a/exporters/logging-otlp/src/test/java/io/opentelemetry/exporter/logging/otlp/internal/writer/StreamJsonWriterTest.java b/exporters/logging-otlp/src/test/java/io/opentelemetry/exporter/logging/otlp/internal/writer/StreamJsonWriterTest.java index 2245a15e0c..eabfbc5351 100644 --- a/exporters/logging-otlp/src/test/java/io/opentelemetry/exporter/logging/otlp/internal/writer/StreamJsonWriterTest.java +++ b/exporters/logging-otlp/src/test/java/io/opentelemetry/exporter/logging/otlp/internal/writer/StreamJsonWriterTest.java @@ -46,7 +46,9 @@ class StreamJsonWriterTest { @Test void errorWriting() throws IOException { Marshaler marshaler = mock(Marshaler.class); - Mockito.doThrow(new IOException("test")).when(marshaler).writeJsonTo(any(JsonGenerator.class)); + Mockito.doThrow(new IOException("test")) + .when(marshaler) + .writeJsonWithNewline(any(JsonGenerator.class)); StreamJsonWriter writer = new StreamJsonWriter(System.out, "type"); writer.write(marshaler); diff --git a/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/PrometheusMetricReaderTest.java b/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/PrometheusMetricReaderTest.java index 1a4f9d4288..20076cc9c9 100644 --- a/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/PrometheusMetricReaderTest.java +++ b/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/PrometheusMetricReaderTest.java @@ -616,7 +616,7 @@ class PrometheusMetricReaderTest { + " value: 7.0\n" + " timestamp {\n" + " seconds: \n" - + " nanos: \n" + + " \n" + " }\n" + " }\n" + " }\n" @@ -661,7 +661,7 @@ class PrometheusMetricReaderTest { + " value: 3.0\n" + " timestamp {\n" + " seconds: \n" - + " nanos: \n" + + " \n" + " }\n" + " }\n" + " }\n" @@ -1115,7 +1115,9 @@ class PrometheusMetricReaderTest { */ private static String toPattern(String expected) { Map replacePatterns = new HashMap<>(); - replacePatterns.put("timestamp", "[0-9]+(\\.[0-9]+)?"); + String timestampPattern = "[0-9]+(\\.[0-9]+)?"; + replacePatterns.put("timestamp", timestampPattern); + replacePatterns.put("maybeNanos", String.format("(nanos: %s)?", timestampPattern)); replacePatterns.put("spanId", "[a-z0-9]*"); replacePatterns.put("traceId", "[a-z0-9]*"); replacePatterns.put("measurement", "[0-9\\.]*");