Skip to content

Commit 016ad50

Browse files
jbachorikjbachorik
committed
Correct the mixup in compression algorithms on Oracle JDK 8
1 parent 9f47ca3 commit 016ad50

File tree

2 files changed

+57
-72
lines changed

2 files changed

+57
-72
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-smoke-tests/profiling-integration-tests/src/test/java/datadog/smoketest/JFRBasedProfilingIntegrationTest.java‎

Lines changed: 50 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

@@ -276,10 +250,9 @@ private void testContinuousRecording(
276250
assertEquals(InetAddress.getLocalHost().getHostName(), requestTags.get("host"));
277251

278252
assertFalse(logHasErrors(logFilePath));
253+
Files.copy(new ByteArrayInputStream(rawJfr.get()), Paths.get("/tmp/out.jfr"));
279254
InputStream eventStream = new ByteArrayInputStream(rawJfr.get());
280-
if (withZstd) {
281-
eventStream = new ZstdInputStream(eventStream);
282-
}
255+
eventStream = decompressStream(withCompression, eventStream);
283256
IItemCollection events = JfrLoaderToolkit.loadEvents(eventStream);
284257
assertTrue(events.hasItems());
285258
Pair<Instant, Instant> rangeStartAndEnd = getRangeStartAndEnd(events);
@@ -328,9 +301,7 @@ private void testContinuousRecording(
328301
() -> "Upload period = " + period + "ms, expected (0, " + upperLimit + "]ms");
329302

330303
eventStream = new ByteArrayInputStream(rawJfr.get());
331-
if (withZstd) {
332-
eventStream = new ZstdInputStream(eventStream);
333-
}
304+
eventStream = decompressStream(withCompression, eventStream);
334305
events = JfrLoaderToolkit.loadEvents(eventStream);
335306
assertTrue(events.hasItems());
336307
verifyDatadogEventsNotCorrupt(events);
@@ -372,6 +343,20 @@ private void testContinuousRecording(
372343
}
373344
}
374345

346+
private static InputStream decompressStream(String withCompression, InputStream eventStream) {
347+
if (!JavaVirtualMachine.isOracleJDK8()) {
348+
if ("zstd".equals(withCompression) || "on".equals(withCompression)) {
349+
eventStream = new ZstdInputStream(eventStream);
350+
} else {
351+
// nothing; jmc already handles lz4 and gzip
352+
}
353+
} else {
354+
// Oracle JDK 8 JFR writes files in GZIP format; jmc already can handle that
355+
}
356+
357+
return eventStream;
358+
}
359+
375360
private static void verifyJdkEventsDisabled(IItemCollection events) {
376361
assertFalse(events.apply(ItemFilters.type("jdk.ExecutionSample")).hasItems());
377362
assertFalse(events.apply(ItemFilters.type("jdk.ThreadPark")).hasItems());
@@ -466,7 +451,7 @@ void testBogusApiKey(final TestInfo testInfo) throws Exception {
466451
PROFILING_UPLOAD_PERIOD_SECONDS,
467452
ENDPOINT_COLLECTION_ENABLED,
468453
true,
469-
false,
454+
"off",
470455
exitDelay,
471456
logFilePath)
472457
.start();
@@ -524,7 +509,7 @@ void testShutdown(final TestInfo testInfo) throws Exception {
524509
PROFILING_UPLOAD_PERIOD_SECONDS,
525510
ENDPOINT_COLLECTION_ENABLED,
526511
true,
527-
false,
512+
"off",
528513
duration,
529514
logFilePath)
530515
.start();
@@ -752,7 +737,7 @@ private ProcessBuilder createDefaultProcessBuilder(
752737
final int jmxFetchDelay,
753738
final boolean endpointCollectionEnabled,
754739
final boolean asyncProfilerEnabled,
755-
final boolean withZstd,
740+
final String withCompression,
756741
final Path logFilePath) {
757742
return createProcessBuilder(
758743
VALID_API_KEY,
@@ -761,7 +746,7 @@ private ProcessBuilder createDefaultProcessBuilder(
761746
PROFILING_UPLOAD_PERIOD_SECONDS,
762747
endpointCollectionEnabled,
763748
asyncProfilerEnabled,
764-
withZstd,
749+
withCompression,
765750
0,
766751
logFilePath);
767752
}
@@ -773,7 +758,7 @@ private ProcessBuilder createProcessBuilder(
773758
final int profilingUploadPeriodSecs,
774759
final boolean endpointCollectionEnabled,
775760
final boolean asyncProfilerEnabled,
776-
final boolean withZstd,
761+
final String withCompression,
777762
final int exitDelay,
778763
final Path logFilePath) {
779764
return createProcessBuilder(
@@ -785,7 +770,7 @@ private ProcessBuilder createProcessBuilder(
785770
profilingUploadPeriodSecs,
786771
endpointCollectionEnabled,
787772
asyncProfilerEnabled,
788-
withZstd,
773+
withCompression,
789774
exitDelay,
790775
logFilePath);
791776
}
@@ -799,7 +784,7 @@ private static ProcessBuilder createProcessBuilder(
799784
final int profilingUploadPeriodSecs,
800785
final boolean endpointCollectionEnabled,
801786
final boolean asyncProfilerEnabled,
802-
final boolean withZstd,
787+
final String withCompression,
803788
final int exitDelay,
804789
final Path logFilePath) {
805790
final String templateOverride =
@@ -833,7 +818,7 @@ private static ProcessBuilder createProcessBuilder(
833818
"-Ddd.profiling.debug.dump_path=/tmp/dd-profiler",
834819
"-Ddd.profiling.queueing.time.enabled=true",
835820
"-Ddd.profiling.queueing.time.threshold.millis=0",
836-
"-Ddd.profiling.debug.upload.compression=" + (withZstd ? "zstd" : "on"),
821+
"-Ddd.profiling.debug.upload.compression=" + withCompression,
837822
"-Ddatadog.slf4j.simpleLogger.defaultLogLevel=debug",
838823
"-Ddd.profiling.context.attributes=foo,bar",
839824
"-Dorg.slf4j.simpleLogger.defaultLogLevel=debug",

0 commit comments

Comments
 (0)