Skip to content

Commit 3605d1a

Browse files
jbachorikclaude
jbachorik
and
claude
committed
Address muse review findings for PROF-10633 unified config gates
- Deprecate misleading PROFILING_ALLOCATION_ENABLED_DEFAULT constant - Update PROFILING_HEAP_ENABLED_DEFAULT javadoc to explain dynamic default - Fix PROFILING_DIRECT_ALLOCATION_ENABLED missing @deprecated javadoc - Drop JavaMonitorWait/ThreadPark/ThreadSleep from wall gate (timeline events) - Add startup-only/no-RC comment to feature gate block in OpenJdkController - Add BiasedLock noop comment for JDK 18+ awareness - Add lock sampler note to DatadogProfilerConfig - Extract DirectMemoryProfilingHelper shared utility - Add gate tests: CPU, exception, I/O, lock, thread, wall-timeline - Set metadata defaults to false (safe fallback for dynamic defaults) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 91581f2 commit 3605d1a

File tree

9 files changed

+170
-42
lines changed

9 files changed

+170
-42
lines changed

‎dd-java-agent/agent-profiling/profiling-controller-openjdk/src/main/java/com/datadog/profiling/controller/openjdk/OpenJdkController.java‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,6 @@
3636
import static datadog.trace.api.config.ProfilingConfig.PROFILING_THREAD_ENABLED;
3737
import static datadog.trace.api.config.ProfilingConfig.PROFILING_THREAD_ENABLED_DEFAULT;
3838
import static datadog.trace.api.config.ProfilingConfig.PROFILING_ULTRA_MINIMAL;
39-
import static datadog.trace.api.config.ProfilingConfig.PROFILING_WALL_ENABLED;
40-
import static datadog.trace.api.config.ProfilingConfig.PROFILING_WALL_ENABLED_DEFAULT;
4139

4240
import com.datadog.profiling.controller.ConfigurationException;
4341
import com.datadog.profiling.controller.Controller;
@@ -276,7 +274,9 @@ ProfilingConfig.PROFILING_ALLOCATION_ENABLED, isObjectAllocationSampleAvailable(
276274
"Aggregated smaps collection is disabled in the config");
277275
}
278276

279-
// Feature gates — unified profiling.*.enabled keys
277+
// Feature gates — unified profiling.*.enabled keys.
278+
// These gates are evaluated at recording creation time only; profiling does not use Remote
279+
// Config, so events disabled here remain disabled for the lifetime of the recording.
280280

281281
if (!configProvider.getBoolean(PROFILING_CPU_ENABLED, PROFILING_CPU_ENABLED_DEFAULT)) {
282282
disableEvent(recordingSettings, "jdk.ExecutionSample", "CPU profiling is disabled");
@@ -285,11 +285,9 @@ ProfilingConfig.PROFILING_ALLOCATION_ENABLED, isObjectAllocationSampleAvailable(
285285
disableEvent(recordingSettings, "jdk.CPUTimeSamplesLost", "CPU profiling is disabled");
286286
}
287287

288-
if (!configProvider.getBoolean(PROFILING_WALL_ENABLED, PROFILING_WALL_ENABLED_DEFAULT)) {
289-
disableEvent(recordingSettings, "jdk.JavaMonitorWait", "wall-clock profiling is disabled");
290-
disableEvent(recordingSettings, "jdk.ThreadPark", "wall-clock profiling is disabled");
291-
disableEvent(recordingSettings, "jdk.ThreadSleep", "wall-clock profiling is disabled");
292-
}
288+
// jdk.JavaMonitorWait, jdk.ThreadPark, and jdk.ThreadSleep are intentionally NOT gated by
289+
// PROFILING_WALL_ENABLED: they serve purposes beyond wall-clock profile construction
290+
// (timeline blocking events, queueing time) and must remain enabled independently.
293291

294292
if (!configProvider.getBoolean(
295293
PROFILING_EXCEPTION_ENABLED, PROFILING_EXCEPTION_ENABLED_DEFAULT)) {
@@ -307,6 +305,8 @@ ProfilingConfig.PROFILING_ALLOCATION_ENABLED, isObjectAllocationSampleAvailable(
307305

308306
if (!configProvider.getBoolean(PROFILING_LOCK_ENABLED, PROFILING_LOCK_ENABLED_DEFAULT)) {
309307
disableEvent(recordingSettings, "jdk.JavaMonitorEnter", "lock profiling is disabled");
308+
// BiasedLock events are no-ops on JDK 18+ (biased locking removed via JEP 374); disabling
309+
// them here is harmless on modern JDKs.
310310
disableEvent(recordingSettings, "jdk.BiasedLockRevocation", "lock profiling is disabled");
311311
disableEvent(recordingSettings, "jdk.BiasedLockSelfRevocation", "lock profiling is disabled");
312312
disableEvent(

‎dd-java-agent/agent-profiling/profiling-controller-openjdk/src/test/java/com/datadog/profiling/controller/openjdk/OpenJdkControllerTest.java‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,15 @@
66
import static datadog.trace.api.config.ProfilingConfig.PROFILING_ALLOCATION_ENABLED;
77
import static datadog.trace.api.config.ProfilingConfig.PROFILING_AUXILIARY_TYPE;
88
import static datadog.trace.api.config.ProfilingConfig.PROFILING_AUXILIARY_TYPE_DEFAULT;
9+
import static datadog.trace.api.config.ProfilingConfig.PROFILING_CPU_ENABLED;
910
import static datadog.trace.api.config.ProfilingConfig.PROFILING_DATADOG_PROFILER_ENABLED;
11+
import static datadog.trace.api.config.ProfilingConfig.PROFILING_EXCEPTION_ENABLED;
1012
import static datadog.trace.api.config.ProfilingConfig.PROFILING_HEAP_ENABLED;
13+
import static datadog.trace.api.config.ProfilingConfig.PROFILING_IO_ENABLED;
14+
import static datadog.trace.api.config.ProfilingConfig.PROFILING_LOCK_ENABLED;
1115
import static datadog.trace.api.config.ProfilingConfig.PROFILING_TEMPLATE_OVERRIDE_FILE;
16+
import static datadog.trace.api.config.ProfilingConfig.PROFILING_THREAD_ENABLED;
17+
import static datadog.trace.api.config.ProfilingConfig.PROFILING_WALL_ENABLED;
1218
import static org.junit.jupiter.api.Assertions.assertEquals;
1319
import static org.junit.jupiter.api.Assertions.assertFalse;
1420
import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -276,6 +282,123 @@ public void testUnifiedFlagDisabledTurnsOffOldObjectSample() throws Exception {
276282
}
277283
}
278284

285+
@Test
286+
public void testCpuGateDisablesCpuEvents() throws Exception {
287+
Properties props = getConfigProperties();
288+
props.put(PROFILING_CPU_ENABLED, "false");
289+
290+
ConfigProvider configProvider = ConfigProvider.withPropertiesOverride(props);
291+
OpenJdkController controller = new OpenJdkController(configProvider);
292+
try (final Recording recording =
293+
((OpenJdkRecordingData)
294+
controller.createRecording(TEST_NAME, new ControllerContext().snapshot()).stop())
295+
.getRecording()) {
296+
assertFalse(
297+
Boolean.parseBoolean(recording.getSettings().get("jdk.ExecutionSample#enabled")),
298+
"ExecutionSample must be disabled when CPU profiling is disabled");
299+
}
300+
}
301+
302+
@Test
303+
public void testExceptionGateDisablesExceptionEvents() throws Exception {
304+
Properties props = getConfigProperties();
305+
props.put(PROFILING_EXCEPTION_ENABLED, "false");
306+
307+
ConfigProvider configProvider = ConfigProvider.withPropertiesOverride(props);
308+
OpenJdkController controller = new OpenJdkController(configProvider);
309+
try (final Recording recording =
310+
((OpenJdkRecordingData)
311+
controller.createRecording(TEST_NAME, new ControllerContext().snapshot()).stop())
312+
.getRecording()) {
313+
assertFalse(
314+
Boolean.parseBoolean(recording.getSettings().get("datadog.ExceptionSample#enabled")),
315+
"ExceptionSample must be disabled when exception profiling is disabled");
316+
assertFalse(
317+
Boolean.parseBoolean(recording.getSettings().get("datadog.ExceptionCount#enabled")),
318+
"ExceptionCount must be disabled when exception profiling is disabled");
319+
}
320+
}
321+
322+
@Test
323+
public void testIoGateDisablesIoEvents() throws Exception {
324+
Properties props = getConfigProperties();
325+
props.put(PROFILING_IO_ENABLED, "false");
326+
327+
ConfigProvider configProvider = ConfigProvider.withPropertiesOverride(props);
328+
OpenJdkController controller = new OpenJdkController(configProvider);
329+
try (final Recording recording =
330+
((OpenJdkRecordingData)
331+
controller.createRecording(TEST_NAME, new ControllerContext().snapshot()).stop())
332+
.getRecording()) {
333+
assertFalse(
334+
Boolean.parseBoolean(recording.getSettings().get("jdk.FileRead#enabled")),
335+
"FileRead must be disabled when I/O profiling is disabled");
336+
assertFalse(
337+
Boolean.parseBoolean(recording.getSettings().get("jdk.SocketRead#enabled")),
338+
"SocketRead must be disabled when I/O profiling is disabled");
339+
}
340+
}
341+
342+
@Test
343+
public void testLockGateDisablesLockEvents() throws Exception {
344+
Properties props = getConfigProperties();
345+
props.put(PROFILING_LOCK_ENABLED, "false");
346+
347+
ConfigProvider configProvider = ConfigProvider.withPropertiesOverride(props);
348+
OpenJdkController controller = new OpenJdkController(configProvider);
349+
try (final Recording recording =
350+
((OpenJdkRecordingData)
351+
controller.createRecording(TEST_NAME, new ControllerContext().snapshot()).stop())
352+
.getRecording()) {
353+
assertFalse(
354+
Boolean.parseBoolean(recording.getSettings().get("jdk.JavaMonitorEnter#enabled")),
355+
"JavaMonitorEnter must be disabled when lock profiling is disabled");
356+
}
357+
}
358+
359+
@Test
360+
public void testThreadGateDisablesThreadEvents() throws Exception {
361+
Properties props = getConfigProperties();
362+
props.put(PROFILING_THREAD_ENABLED, "false");
363+
364+
ConfigProvider configProvider = ConfigProvider.withPropertiesOverride(props);
365+
OpenJdkController controller = new OpenJdkController(configProvider);
366+
try (final Recording recording =
367+
((OpenJdkRecordingData)
368+
controller.createRecording(TEST_NAME, new ControllerContext().snapshot()).stop())
369+
.getRecording()) {
370+
assertFalse(
371+
Boolean.parseBoolean(recording.getSettings().get("jdk.ThreadStart#enabled")),
372+
"ThreadStart must be disabled when thread profiling is disabled");
373+
assertFalse(
374+
Boolean.parseBoolean(recording.getSettings().get("jdk.ThreadEnd#enabled")),
375+
"ThreadEnd must be disabled when thread profiling is disabled");
376+
}
377+
}
378+
379+
@Test
380+
public void testWallGateDoesNotDisableTimelineEvents() throws Exception {
381+
Properties props = getConfigProperties();
382+
props.put(PROFILING_WALL_ENABLED, "false");
383+
384+
ConfigProvider configProvider = ConfigProvider.withPropertiesOverride(props);
385+
OpenJdkController controller = new OpenJdkController(configProvider);
386+
try (final Recording recording =
387+
((OpenJdkRecordingData)
388+
controller.createRecording(TEST_NAME, new ControllerContext().snapshot()).stop())
389+
.getRecording()) {
390+
// JavaMonitorWait and ThreadPark serve timeline purposes beyond wall-clock profiling and must
391+
// NOT be disabled when only wall profiling is disabled. (ThreadSleep is disabled by the
392+
// dd.jfp template by default and is therefore not checked here.)
393+
assertTrue(
394+
Boolean.parseBoolean(recording.getSettings().get("jdk.JavaMonitorWait#enabled")),
395+
"JavaMonitorWait must remain enabled when only wall profiling is disabled");
396+
assertTrue(
397+
Boolean.parseBoolean(recording.getSettings().get("jdk.ThreadPark#enabled")),
398+
"ThreadPark must remain enabled when only wall profiling is disabled");
399+
}
400+
}
401+
279402
private static Properties getConfigProperties() {
280403
Properties props = new Properties();
281404
// make sure the async profiler is not force-enabled

‎dd-java-agent/agent-profiling/profiling-ddprof/src/main/java/com/datadog/profiling/ddprof/DatadogProfilerConfig.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,11 @@ public static boolean isMemoryLeakProfilingEnabled() {
252252
return isMemoryLeakProfilingEnabled(ConfigProvider.getInstance());
253253
}
254254

255+
// Note: ddprof does not currently implement a native lock sampler. The unified gate
256+
// dd.profiling.lock.enabled (PROFILING_LOCK_ENABLED) is honored by the JFR controller
257+
// for jdk.JavaMonitorEnter. If a ddprof-based lock profiler is added in the future,
258+
// it should check PROFILING_LOCK_ENABLED here.
259+
255260
public static boolean isLiveHeapSizeTrackingEnabled(ConfigProvider configProvider) {
256261
return getBoolean(
257262
configProvider,

‎dd-java-agent/instrumentation/java/java-nio-1.8/src/main/java/datadog/trace/instrumentation/directbytebuffer/ByteBufferInstrumentation.java‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,6 @@
11
package datadog.trace.instrumentation.directbytebuffer;
22

33
import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named;
4-
import static datadog.trace.api.config.ProfilingConfig.PROFILING_DIRECT_ALLOCATION_ENABLED;
5-
import static datadog.trace.api.config.ProfilingConfig.PROFILING_DIRECT_MEMORY_ENABLED;
6-
import static datadog.trace.api.config.ProfilingConfig.PROFILING_DIRECT_MEMORY_ENABLED_DEFAULT;
74
import static net.bytebuddy.matcher.ElementMatchers.isMethod;
85
import static net.bytebuddy.matcher.ElementMatchers.isStatic;
96
import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
@@ -27,14 +24,9 @@ public ByteBufferInstrumentation() {
2724
@Override
2825
public boolean isEnabled() {
2926
ConfigProvider cp = ConfigProvider.getInstance();
30-
boolean enabled =
31-
cp.getBoolean(
32-
PROFILING_DIRECT_MEMORY_ENABLED,
33-
cp.getBoolean(
34-
PROFILING_DIRECT_ALLOCATION_ENABLED, PROFILING_DIRECT_MEMORY_ENABLED_DEFAULT));
3527
return JavaVirtualMachine.isJavaVersionAtLeast(11)
3628
&& super.isEnabled()
37-
&& enabled
29+
&& DirectMemoryProfilingHelper.isEnabled(cp)
3830
&& Platform.hasJfr();
3931
}
4032

‎dd-java-agent/instrumentation/java/java-nio-1.8/src/main/java/datadog/trace/instrumentation/directbytebuffer/DirectByteBufferInstrumentation.java‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,5 @@
11
package datadog.trace.instrumentation.directbytebuffer;
22

3-
import static datadog.trace.api.config.ProfilingConfig.PROFILING_DIRECT_ALLOCATION_ENABLED;
4-
import static datadog.trace.api.config.ProfilingConfig.PROFILING_DIRECT_MEMORY_ENABLED;
5-
import static datadog.trace.api.config.ProfilingConfig.PROFILING_DIRECT_MEMORY_ENABLED_DEFAULT;
63
import static net.bytebuddy.matcher.ElementMatchers.isConstructor;
74
import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
85
import static net.bytebuddy.matcher.ElementMatchers.takesArguments;
@@ -25,14 +22,9 @@ public DirectByteBufferInstrumentation() {
2522
@Override
2623
public boolean isEnabled() {
2724
ConfigProvider cp = ConfigProvider.getInstance();
28-
boolean enabled =
29-
cp.getBoolean(
30-
PROFILING_DIRECT_MEMORY_ENABLED,
31-
cp.getBoolean(
32-
PROFILING_DIRECT_ALLOCATION_ENABLED, PROFILING_DIRECT_MEMORY_ENABLED_DEFAULT));
3325
return JavaVirtualMachine.isJavaVersionAtLeast(11)
3426
&& super.isEnabled()
35-
&& enabled
27+
&& DirectMemoryProfilingHelper.isEnabled(cp)
3628
&& Platform.hasJfr();
3729
}
3830

‎dd-java-agent/instrumentation/java/java-nio-1.8/src/main/java/datadog/trace/instrumentation/directbytebuffer/DirectMemoryProfilingHelper.java‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
package datadog.trace.instrumentation.directbytebuffer;
2+
3+
import static datadog.trace.api.config.ProfilingConfig.PROFILING_DIRECT_ALLOCATION_ENABLED;
4+
import static datadog.trace.api.config.ProfilingConfig.PROFILING_DIRECT_MEMORY_ENABLED;
5+
import static datadog.trace.api.config.ProfilingConfig.PROFILING_DIRECT_MEMORY_ENABLED_DEFAULT;
6+
7+
import datadog.trace.bootstrap.config.provider.ConfigProvider;
8+
9+
final class DirectMemoryProfilingHelper {
10+
11+
static boolean isEnabled(ConfigProvider cp) {
12+
return cp.getBoolean(
13+
PROFILING_DIRECT_MEMORY_ENABLED,
14+
cp.getBoolean(
15+
PROFILING_DIRECT_ALLOCATION_ENABLED, PROFILING_DIRECT_MEMORY_ENABLED_DEFAULT));
16+
}
17+
18+
private DirectMemoryProfilingHelper() {}
19+
}

‎dd-java-agent/instrumentation/java/java-nio-1.8/src/main/java/datadog/trace/instrumentation/directbytebuffer/FileChannelImplInstrumentation.java‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,6 @@
11
package datadog.trace.instrumentation.directbytebuffer;
22

33
import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named;
4-
import static datadog.trace.api.config.ProfilingConfig.PROFILING_DIRECT_ALLOCATION_ENABLED;
5-
import static datadog.trace.api.config.ProfilingConfig.PROFILING_DIRECT_MEMORY_ENABLED;
6-
import static datadog.trace.api.config.ProfilingConfig.PROFILING_DIRECT_MEMORY_ENABLED_DEFAULT;
74
import static net.bytebuddy.matcher.ElementMatchers.isMethod;
85
import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
96
import static net.bytebuddy.matcher.ElementMatchers.takesArguments;
@@ -26,14 +23,9 @@ public FileChannelImplInstrumentation() {
2623
@Override
2724
public boolean isEnabled() {
2825
ConfigProvider cp = ConfigProvider.getInstance();
29-
boolean enabled =
30-
cp.getBoolean(
31-
PROFILING_DIRECT_MEMORY_ENABLED,
32-
cp.getBoolean(
33-
PROFILING_DIRECT_ALLOCATION_ENABLED, PROFILING_DIRECT_MEMORY_ENABLED_DEFAULT));
3426
return JavaVirtualMachine.isJavaVersionAtLeast(11)
3527
&& super.isEnabled()
36-
&& enabled
28+
&& DirectMemoryProfilingHelper.isEnabled(cp)
3729
&& Platform.hasJfr();
3830
}
3931

‎dd-trace-api/src/main/java/datadog/trace/api/config/ProfilingConfig.java‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,17 @@ public final class ProfilingConfig {
1010
public static final String PROFILING_ENABLED = "profiling.enabled";
1111
public static final boolean PROFILING_ENABLED_DEFAULT = false;
1212
public static final String PROFILING_ALLOCATION_ENABLED = "profiling.allocation.enabled";
13-
public static final boolean PROFILING_ALLOCATION_ENABLED_DEFAULT = true;
13+
14+
/**
15+
* @deprecated Not used. The actual default is computed dynamically by {@link
16+
* datadog.trace.api.profiling.ProfilingSupport#isObjectAllocationSampleAvailable()}.
17+
*/
18+
@Deprecated public static final boolean PROFILING_ALLOCATION_ENABLED_DEFAULT = true;
1419
public static final String PROFILING_HEAP_ENABLED = "profiling.heap.enabled";
1520

1621
/**
17-
* @deprecated The default is now computed dynamically based on JVM capabilities.
22+
* @deprecated The old value was {@code false}. The default is now computed dynamically via
23+
* {@link datadog.trace.api.profiling.ProfilingSupport#isLiveHeapProfilingSafe()}.
1824
*/
1925
@Deprecated public static final boolean PROFILING_HEAP_ENABLED_DEFAULT = false;
2026

@@ -32,9 +38,8 @@ public final class ProfilingConfig {
3238
public static final boolean PROFILING_THREAD_ENABLED_DEFAULT = true;
3339

3440
/**
35-
* Unified gate for direct memory (native ByteBuffer / FileChannel) profiling.
36-
*
37-
* @see #PROFILING_DIRECT_ALLOCATION_ENABLED
41+
* Unified gate for direct memory (native ByteBuffer / FileChannel) profiling. Replaces the
42+
* deprecated {@link #PROFILING_DIRECT_ALLOCATION_ENABLED}.
3843
*/
3944
public static final String PROFILING_DIRECT_MEMORY_ENABLED = "profiling.direct.memory.enabled";
4045

‎metadata/supported-configurations.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2461,7 +2461,7 @@
24612461
{
24622462
"version": "A",
24632463
"type": "boolean",
2464-
"default": "true",
2464+
"default": "false",
24652465
"aliases": ["DD_PROFILING_DDPROF_ALLOC_ENABLED"]
24662466
}
24672467
],
@@ -3069,7 +3069,7 @@
30693069
{
30703070
"version": "A",
30713071
"type": "boolean",
3072-
"default": "true",
3072+
"default": "false",
30733073
"aliases": []
30743074
}
30753075
],

0 commit comments

Comments
 (0)