Skip to content

Commit

Permalink
bugfix: add newline in stdout exporter (#6848)
Browse files Browse the repository at this point in the history
  • Loading branch information
zeitlinger authored Nov 5, 2024
1 parent 98fa296 commit a6b3302
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,14 @@ public final void writeJsonTo(JsonGenerator output) throws IOException {
}
}

/** 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();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public StreamJsonWriter(OutputStream originalStream, String type) {
@Override
public CompletableResultCode write(Marshaler exportRequest) {
try {
exportRequest.writeJsonTo(
exportRequest.writeJsonWithNewline(
JSON_FACTORY
.createGenerator(outputStream)
.disable(JsonGenerator.Feature.AUTO_CLOSE_TARGET));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
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;
Expand All @@ -57,6 +56,7 @@ abstract class AbstractOtlpStdoutExporterTest<T> {
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<? super T> testDataExporter;
protected final Class<?> exporterClass;
Expand Down Expand Up @@ -87,21 +87,22 @@ protected abstract T createExporter(
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();
}

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);
}
Expand Down Expand Up @@ -204,8 +205,7 @@ private static Arguments testCase(
@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()) {
Expand Down Expand Up @@ -247,6 +247,26 @@ void exportWithProgrammaticConfig(String name, TestCase testCase)
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<T> 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,9 @@ void testToString() throws IOException {
@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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -616,7 +616,7 @@ private static void assertExponentialHistogramComplete(
+ " value: 7.0\n"
+ " timestamp {\n"
+ " seconds: <timestamp>\n"
+ " nanos: <timestamp>\n"
+ " <maybeNanos>\n"
+ " }\n"
+ " }\n"
+ " }\n"
Expand Down Expand Up @@ -661,7 +661,7 @@ private static void assertExponentialHistogramComplete(
+ " value: 3.0\n"
+ " timestamp {\n"
+ " seconds: <timestamp>\n"
+ " nanos: <timestamp>\n"
+ " <maybeNanos>\n"
+ " }\n"
+ " }\n"
+ " }\n"
Expand Down Expand Up @@ -1115,7 +1115,9 @@ private static void assertMatches(String expected, String actual) {
*/
private static String toPattern(String expected) {
Map<String, String> 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\\.]*");
Expand Down

0 comments on commit a6b3302

Please sign in to comment.