Skip to content

Commit 44e5822

Browse files
jbachorikjbachorik
authored
Correct the mixup in compression algorithms on Oracle JDK 8 (#9546)
1 parent 4e3332a commit 44e5822

File tree

5 files changed

+62
-78
lines changed

5 files changed

+62
-78
lines changed

‎dd-java-agent/agent-profiling/profiling-uploader/src/main/java/com/datadog/profiling/uploader/CompressingRequestBody.java‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -85,9 +85,9 @@ interface RetryBackoff {
8585

8686
// https://github.com/lz4/lz4/blob/dev/doc/lz4_Frame_format.md#general-structure-of-lz4-frame-format
8787
private static final int[] LZ4_MAGIC = new int[] {0x04, 0x22, 0x4D, 0x18};
88-
private static final int ZIP_MAGIC[] = new int[] {80, 75, 3, 4};
89-
private static final int GZ_MAGIC[] = new int[] {31, 139};
90-
private static final int ZSTD_MAGIC[] = new int[] {0x28, 0xB5, 0x2F, 0xFD};
88+
private static final int[] ZIP_MAGIC = new int[] {80, 75, 3, 4};
89+
private static final int[] GZ_MAGIC = new int[] {31, 139};
90+
private static final int[] ZSTD_MAGIC = new int[] {0x28, 0xB5, 0x2F, 0xFD};
9191

9292
private final InputStreamSupplier inputStreamSupplier;
9393
private final OutputStreamMappingFunction outputStreamMapper;
@@ -364,15 +364,15 @@ private static OutputStreamMappingFunction getOutputStreamMapper(
364364
{
365365
return out -> out;
366366
}
367-
case ZSTD:
367+
case LZ4:
368368
{
369-
return CompressingRequestBody::toZstdStream;
369+
return CompressingRequestBody::toLz4Stream;
370370
}
371371
case ON:
372-
case LZ4:
372+
case ZSTD:
373373
default:
374374
{
375-
return CompressingRequestBody::toLz4Stream;
375+
return CompressingRequestBody::toZstdStream;
376376
}
377377
}
378378
}

‎dd-java-agent/agent-profiling/profiling-uploader/src/main/java/com/datadog/profiling/uploader/CompressionType.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ static CompressionType of(String type) {
3535
case "zstd":
3636
return ZSTD;
3737
default:
38-
log.warn("Unrecognizable compression type: {}. Defaulting to 'lz4'.", type);
38+
log.warn("Unrecognizable compression type: {}. Defaulting to '{}'.", type, ZSTD);
3939
return ON;
4040
}
4141
}

‎dd-java-agent/agent-profiling/profiling-uploader/src/test/java/com/datadog/profiling/uploader/CompressingRequestBodyTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,6 @@ void writeTo(CompressionType compressionType) throws IOException {
170170
break;
171171
}
172172
case LZ4:
173-
case ON:
174173
{
175174
assertTrue(CompressingRequestBody.isLz4(compressedStream));
176175
byte[] uncompressed = IOUtils.toByteArray(new LZ4FrameInputStream(compressedStream));
@@ -188,6 +187,7 @@ void writeTo(CompressionType compressionType) throws IOException {
188187
assertEquals(compressed.length, instance.getWrittenBytes());
189188
break;
190189
}
190+
case ON:
191191
case ZSTD:
192192
{
193193
assertTrue(CompressingRequestBody.isZstd(compressedStream));

‎dd-java-agent/agent-profiling/profiling-uploader/src/test/java/com/datadog/profiling/uploader/ProfileUploaderTest.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -373,11 +373,11 @@ public void testCompression(final String compression) throws Exception {
373373
byte[] uploadedBytes = rawJfr.get();
374374
if (compression.equals("gzip")) {
375375
uploadedBytes = unGzip(uploadedBytes);
376-
} else if (compression.equals("zstd")) {
377-
uploadedBytes = unZstd(uploadedBytes);
378-
} else if (compression.equals("on")
379-
|| compression.equals("lz4")
376+
} else if (compression.equals("zstd")
377+
|| compression.equals("on")
380378
|| compression.equals("invalid")) {
379+
uploadedBytes = unZstd(uploadedBytes);
380+
} else if (compression.equals("lz4")) {
381381
uploadedBytes = unLz4(uploadedBytes);
382382
}
383383
assertArrayEquals(expectedBytes, uploadedBytes);

‎dd-smoke-tests/profiling-integration-tests/src/test/java/datadog/smoketest/JFRBasedProfilingIntegrationTest.java‎

Lines changed: 49 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import java.util.concurrent.TimeUnit;
3939
import java.util.concurrent.TimeoutException;
4040
import java.util.function.Predicate;
41+
import java.util.stream.Stream;
4142
import okhttp3.mockwebserver.Dispatcher;
4243
import okhttp3.mockwebserver.MockResponse;
4344
import okhttp3.mockwebserver.MockWebServer;
@@ -53,7 +54,8 @@
5354
import org.junit.jupiter.api.TestInfo;
5455
import org.junit.jupiter.api.condition.DisabledIfSystemProperty;
5556
import org.junit.jupiter.params.ParameterizedTest;
56-
import org.junit.jupiter.params.provider.ValueSource;
57+
import org.junit.jupiter.params.provider.Arguments;
58+
import org.junit.jupiter.params.provider.MethodSource;
5759
import org.openjdk.jmc.common.IMCStackTrace;
5860
import org.openjdk.jmc.common.item.Aggregators;
5961
import org.openjdk.jmc.common.item.IAttribute;
@@ -154,57 +156,29 @@ void teardown() throws Exception {
154156
}
155157
}
156158

157-
@ParameterizedTest
158-
@ValueSource(strings = {"jfr", "ddprof"})
159-
@DisplayName("Test continuous recording - no jmx delay, default compression")
160-
public void testContinuousRecording_no_jmx_delay(String profiler, final TestInfo testInfo)
161-
throws Exception {
162-
Assumptions.assumeTrue("jfr".equals(profiler) || OperatingSystem.isLinux());
163-
testWithRetry(
164-
() ->
165-
testContinuousRecording(
166-
0, ENDPOINT_COLLECTION_ENABLED, "ddprof".equals(profiler), false),
167-
testInfo,
168-
5);
169-
}
170-
171-
@ParameterizedTest
172-
@ValueSource(strings = {"jfr", "ddprof"})
173-
@DisplayName("Test continuous recording - no jmx delay, zstd compression")
174-
public void testContinuousRecording_no_jmx_delay_zstd(String profiler, final TestInfo testInfo)
175-
throws Exception {
176-
Assumptions.assumeTrue("jfr".equals(profiler) || OperatingSystem.isLinux());
159+
private static Stream<Arguments> testArguments() {
160+
return Stream.of(
161+
Arguments.of(0, "on", "jfr"),
162+
Arguments.of(0, "on", "ddprof"),
163+
Arguments.of(0, "lz4", "jfr"),
164+
Arguments.of(0, "lz4", "ddprof"),
165+
Arguments.of(1, "on", "jfr"),
166+
Arguments.of(1, "on", "ddprof"),
167+
Arguments.of(1, "lz4", "jfr"),
168+
Arguments.of(1, "lz4", "ddprof"));
169+
}
170+
171+
@ParameterizedTest(name = "Continuous recording [jmx delay: {0}, compression: {1}, mode: {2}]")
172+
@MethodSource("testArguments")
173+
public void testContinuousRecording_with_params(
174+
int jmxDelay, String compression, String mode, final TestInfo testInfo) throws Exception {
175+
Assumptions.assumeTrue("jfr".equals(mode) || OperatingSystem.isLinux());
176+
// Do not test compressions for Oracle JDK 8 - it will always be GZIP
177+
Assumptions.assumeTrue(!JavaVirtualMachine.isOracleJDK8() || "on".equals(mode));
177178
testWithRetry(
178179
() ->
179180
testContinuousRecording(
180-
0, ENDPOINT_COLLECTION_ENABLED, "ddprof".equals(profiler), true),
181-
testInfo,
182-
5);
183-
}
184-
185-
@ParameterizedTest
186-
@ValueSource(strings = {"jfr", "ddprof"})
187-
@DisplayName("Test continuous recording - 1 sec jmx delay, default compression")
188-
public void testContinuousRecording(String profiler, final TestInfo testInfo) throws Exception {
189-
Assumptions.assumeTrue("jfr".equals(profiler) || OperatingSystem.isLinux());
190-
testWithRetry(
191-
() ->
192-
testContinuousRecording(
193-
1, ENDPOINT_COLLECTION_ENABLED, "ddprof".equals(profiler), false),
194-
testInfo,
195-
5);
196-
}
197-
198-
@ParameterizedTest
199-
@ValueSource(strings = {"jfr", "ddprof"})
200-
@DisplayName("Test continuous recording - 1 sec jmx delay, zstd compression")
201-
public void testContinuousRecording_zstd(String profiler, final TestInfo testInfo)
202-
throws Exception {
203-
Assumptions.assumeTrue("jfr".equals(profiler) || OperatingSystem.isLinux());
204-
testWithRetry(
205-
() ->
206-
testContinuousRecording(
207-
1, ENDPOINT_COLLECTION_ENABLED, "ddprof".equals(profiler), true),
181+
jmxDelay, ENDPOINT_COLLECTION_ENABLED, "ddprof".equals(mode), compression),
208182
testInfo,
209183
5);
210184
}
@@ -213,7 +187,7 @@ private void testContinuousRecording(
213187
final int jmxFetchDelay,
214188
final boolean endpointCollectionEnabled,
215189
final boolean asyncProfilerEnabled,
216-
final boolean withZstd)
190+
final String withCompression)
217191
throws Exception {
218192
final ObjectMapper mapper = new ObjectMapper();
219193
try {
@@ -222,7 +196,7 @@ private void testContinuousRecording(
222196
jmxFetchDelay,
223197
endpointCollectionEnabled,
224198
asyncProfilerEnabled,
225-
withZstd,
199+
withCompression,
226200
logFilePath)
227201
.start();
228202

@@ -277,9 +251,7 @@ private void testContinuousRecording(
277251

278252
assertFalse(logHasErrors(logFilePath));
279253
InputStream eventStream = new ByteArrayInputStream(rawJfr.get());
280-
if (withZstd) {
281-
eventStream = new ZstdInputStream(eventStream);
282-
}
254+
eventStream = decompressStream(withCompression, eventStream);
283255
IItemCollection events = JfrLoaderToolkit.loadEvents(eventStream);
284256
assertTrue(events.hasItems());
285257
Pair<Instant, Instant> rangeStartAndEnd = getRangeStartAndEnd(events);
@@ -328,9 +300,7 @@ private void testContinuousRecording(
328300
() -> "Upload period = " + period + "ms, expected (0, " + upperLimit + "]ms");
329301

330302
eventStream = new ByteArrayInputStream(rawJfr.get());
331-
if (withZstd) {
332-
eventStream = new ZstdInputStream(eventStream);
333-
}
303+
eventStream = decompressStream(withCompression, eventStream);
334304
events = JfrLoaderToolkit.loadEvents(eventStream);
335305
assertTrue(events.hasItems());
336306
verifyDatadogEventsNotCorrupt(events);
@@ -372,6 +342,20 @@ private void testContinuousRecording(
372342
}
373343
}
374344

345+
private static InputStream decompressStream(String withCompression, InputStream eventStream) {
346+
if (!JavaVirtualMachine.isOracleJDK8()) {
347+
if ("zstd".equals(withCompression) || "on".equals(withCompression)) {
348+
eventStream = new ZstdInputStream(eventStream);
349+
} else {
350+
// nothing; jmc already handles lz4 and gzip
351+
}
352+
} else {
353+
// Oracle JDK 8 JFR writes files in GZIP format; jmc already can handle that
354+
}
355+
356+
return eventStream;
357+
}
358+
375359
private static void verifyJdkEventsDisabled(IItemCollection events) {
376360
assertFalse(events.apply(ItemFilters.type("jdk.ExecutionSample")).hasItems());
377361
assertFalse(events.apply(ItemFilters.type("jdk.ThreadPark")).hasItems());
@@ -466,7 +450,7 @@ void testBogusApiKey(final TestInfo testInfo) throws Exception {
466450
PROFILING_UPLOAD_PERIOD_SECONDS,
467451
ENDPOINT_COLLECTION_ENABLED,
468452
true,
469-
false,
453+
"off",
470454
exitDelay,
471455
logFilePath)
472456
.start();
@@ -524,7 +508,7 @@ void testShutdown(final TestInfo testInfo) throws Exception {
524508
PROFILING_UPLOAD_PERIOD_SECONDS,
525509
ENDPOINT_COLLECTION_ENABLED,
526510
true,
527-
false,
511+
"off",
528512
duration,
529513
logFilePath)
530514
.start();
@@ -752,7 +736,7 @@ private ProcessBuilder createDefaultProcessBuilder(
752736
final int jmxFetchDelay,
753737
final boolean endpointCollectionEnabled,
754738
final boolean asyncProfilerEnabled,
755-
final boolean withZstd,
739+
final String withCompression,
756740
final Path logFilePath) {
757741
return createProcessBuilder(
758742
VALID_API_KEY,
@@ -761,7 +745,7 @@ private ProcessBuilder createDefaultProcessBuilder(
761745
PROFILING_UPLOAD_PERIOD_SECONDS,
762746
endpointCollectionEnabled,
763747
asyncProfilerEnabled,
764-
withZstd,
748+
withCompression,
765749
0,
766750
logFilePath);
767751
}
@@ -773,7 +757,7 @@ private ProcessBuilder createProcessBuilder(
773757
final int profilingUploadPeriodSecs,
774758
final boolean endpointCollectionEnabled,
775759
final boolean asyncProfilerEnabled,
776-
final boolean withZstd,
760+
final String withCompression,
777761
final int exitDelay,
778762
final Path logFilePath) {
779763
return createProcessBuilder(
@@ -785,7 +769,7 @@ private ProcessBuilder createProcessBuilder(
785769
profilingUploadPeriodSecs,
786770
endpointCollectionEnabled,
787771
asyncProfilerEnabled,
788-
withZstd,
772+
withCompression,
789773
exitDelay,
790774
logFilePath);
791775
}
@@ -799,7 +783,7 @@ private static ProcessBuilder createProcessBuilder(
799783
final int profilingUploadPeriodSecs,
800784
final boolean endpointCollectionEnabled,
801785
final boolean asyncProfilerEnabled,
802-
final boolean withZstd,
786+
final String withCompression,
803787
final int exitDelay,
804788
final Path logFilePath) {
805789
final String templateOverride =
@@ -833,7 +817,7 @@ private static ProcessBuilder createProcessBuilder(
833817
"-Ddd.profiling.debug.dump_path=/tmp/dd-profiler",
834818
"-Ddd.profiling.queueing.time.enabled=true",
835819
"-Ddd.profiling.queueing.time.threshold.millis=0",
836-
"-Ddd.profiling.debug.upload.compression=" + (withZstd ? "zstd" : "on"),
820+
"-Ddd.profiling.debug.upload.compression=" + withCompression,
837821
"-Ddatadog.slf4j.simpleLogger.defaultLogLevel=debug",
838822
"-Ddd.profiling.context.attributes=foo,bar",
839823
"-Dorg.slf4j.simpleLogger.defaultLogLevel=debug",

0 commit comments

Comments
 (0)