Skip to content

Commit c96dc57

Browse files
jbachorikclaude
andcommitted
Fix counter ordering and test approach for dropped trace integration
- Move CALLTRACE_STORAGE_DROPPED to end of counter enum to avoid breaking Java-C++ interface - Update TagContextTest to read JFR serialized counters instead of live process counters - Add dropped sample statistics tracking with contention stacktrace filtering - Maintain full counter reset behavior during profiler startup as intended 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
1 parent 9b6559c commit c96dc57

File tree

2 files changed

+89
-78
lines changed

2 files changed

+89
-78
lines changed

ddprof-lib/src/main/cpp/counters.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@
4646
X(CONTEXT_ALLOC_FAILS, "context_alloc_fails") \
4747
X(CALLTRACE_STORAGE_BYTES, "calltrace_storage_bytes") \
4848
X(CALLTRACE_STORAGE_TRACES, "calltrace_storage_traces") \
49-
X(CALLTRACE_STORAGE_DROPPED, "calltrace_storage_dropped_traces") \
5049
X(LINEAR_ALLOCATOR_BYTES, "linear_allocator_bytes") \
5150
X(LINEAR_ALLOCATOR_CHUNKS, "linear_allocator_chunks") \
5251
X(THREAD_IDS_COUNT, "thread_ids_count") \
@@ -63,7 +62,8 @@
6362
X(AGCT_BLOCKED_IN_VM, "agct_blocked_in_vm") \
6463
X(SKIPPED_WALLCLOCK_UNWINDS, "skipped_wallclock_unwinds") \
6564
X(UNWINDING_TIME_ASYNC, "unwinding_ticks_async") \
66-
X(UNWINDING_TIME_JVMTI, "unwinding_ticks_jvmti")
65+
X(UNWINDING_TIME_JVMTI, "unwinding_ticks_jvmti") \
66+
X(CALLTRACE_STORAGE_DROPPED, "calltrace_storage_dropped_traces")
6767
#define X_ENUM(a, b) a,
6868
typedef enum CounterId : int {
6969
DD_COUNTER_TABLE(X_ENUM) DD_NUM_COUNTERS

ddprof-test/src/test/java/com/datadoghq/profiler/context/TagContextTest.java

Lines changed: 87 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -45,89 +45,100 @@ public void test() throws InterruptedException {
4545
stopProfiler();
4646
IItemCollection events = verifyEvents("datadog.MethodSample");
4747
Map<String, AtomicLong> weightsByTagValue = new HashMap<>();
48-
for (IItemIterable wallclockSamples : events) {
49-
IMemberAccessor<IQuantity, IItem> weightAccessor = WEIGHT.getAccessor(wallclockSamples.getType());
50-
// this will become more generic in the future
51-
IMemberAccessor<String, IItem> tag1Accessor = TAG_1.getAccessor(wallclockSamples.getType());
52-
assertNotNull(tag1Accessor);
53-
IMemberAccessor<String, IItem> tag2Accessor = TAG_2.getAccessor(wallclockSamples.getType());
54-
assertNotNull(tag2Accessor);
55-
IMemberAccessor<String, IItem> stacktraceAccessor = JdkAttributes.STACK_TRACE_STRING.getAccessor(wallclockSamples.getType());
56-
for (IItem sample : wallclockSamples) {
57-
String stacktrace = stacktraceAccessor.getMember(sample);
58-
if (!stacktrace.contains("sleep")) {
59-
// we don't know the context has been set for sure until the sleep has started
60-
continue;
48+
long droppedSamplesCount = 0;
49+
long droppedSamplesWeight = 0;
50+
long totalSamplesCount = 0;
51+
long totalSamplesWeight = 0;
52+
try {
53+
for (IItemIterable wallclockSamples : events) {
54+
IMemberAccessor<IQuantity, IItem> weightAccessor = WEIGHT.getAccessor(wallclockSamples.getType());
55+
// this will become more generic in the future
56+
IMemberAccessor<String, IItem> tag1Accessor = TAG_1.getAccessor(wallclockSamples.getType());
57+
assertNotNull(tag1Accessor);
58+
IMemberAccessor<String, IItem> tag2Accessor = TAG_2.getAccessor(wallclockSamples.getType());
59+
assertNotNull(tag2Accessor);
60+
IMemberAccessor<String, IItem> stacktraceAccessor = JdkAttributes.STACK_TRACE_STRING.getAccessor(wallclockSamples.getType());
61+
for (IItem sample : wallclockSamples) {
62+
String stacktrace = stacktraceAccessor.getMember(sample);
63+
if (!stacktrace.contains("sleep")) {
64+
// we don't know the context has been set for sure until the sleep has started
65+
continue;
66+
}
67+
68+
long weight = weightAccessor.getMember(sample).longValue();
69+
totalSamplesCount++;
70+
totalSamplesWeight += weight;
71+
72+
if (stacktrace.contains("<dropped due to contention>")) {
73+
// track dropped samples statistics but skip for weight distribution calculation
74+
droppedSamplesCount++;
75+
droppedSamplesWeight += weight;
76+
continue;
77+
}
78+
79+
String tag = tag1Accessor.getMember(sample);
80+
weightsByTagValue.computeIfAbsent(tag, v -> new AtomicLong())
81+
.addAndGet(weight);
82+
assertNull(tag2Accessor.getMember(sample));
6183
}
62-
long weight = weightAccessor.getMember(sample).longValue();
63-
String tag = tag1Accessor.getMember(sample);
64-
weightsByTagValue.computeIfAbsent(tag, v -> new AtomicLong())
65-
.addAndGet(weight);
66-
assertNull(tag2Accessor.getMember(sample));
6784
}
68-
}
69-
long sum = 0;
70-
long[] weights = new long[strings.length];
71-
for (int i = 0; i < strings.length; i++) {
72-
AtomicLong weight = weightsByTagValue.get(strings[i]);
73-
assertNotNull(weight, "Weight for " + strings[i] + " not found");
74-
weights[i] = weightsByTagValue.get(strings[i]).get();
75-
sum += weights[i];
76-
}
77-
double avg = (double) sum / weights.length;
78-
for (int i = 0; i < weights.length; i++) {
79-
assertTrue(Math.abs(weights[i] - avg) < 0.15 * weights[i], strings[i]
80-
+ " more than 15% from mean");
81-
}
82-
// now check we have settings to unbundle the dynamic columns
83-
IItemCollection activeSettings = verifyEvents("jdk.ActiveSetting");
84-
Set<String> recordedContextAttributes = new HashSet<>();
85-
for (IItemIterable activeSetting : activeSettings) {
86-
IMemberAccessor<String, IItem> nameAccessor = JdkAttributes.REC_SETTING_NAME.getAccessor(activeSetting.getType());
87-
IMemberAccessor<String, IItem> valueAccessor = JdkAttributes.REC_SETTING_VALUE.getAccessor(activeSetting.getType());
88-
for (IItem item : activeSetting) {
89-
String name = nameAccessor.getMember(item);
90-
if ("contextattribute".equals(name)) {
91-
recordedContextAttributes.add(valueAccessor.getMember(item));
92-
}
85+
long sum = 0;
86+
long[] weights = new long[strings.length];
87+
for (int i = 0; i < strings.length; i++) {
88+
AtomicLong weight = weightsByTagValue.get(strings[i]);
89+
assertNotNull(weight, "Weight for " + strings[i] + " not found");
90+
weights[i] = weightsByTagValue.get(strings[i]).get();
91+
sum += weights[i];
92+
}
93+
double avg = (double) sum / weights.length;
94+
for (int i = 0; i < weights.length; i++) {
95+
assertTrue(Math.abs(weights[i] - avg) < 0.15 * weights[i], strings[i]
96+
+ " more than 15% from mean");
9397
}
94-
}
95-
assertEquals(3, recordedContextAttributes.size());
96-
assertTrue(recordedContextAttributes.contains("tag1"));
97-
assertTrue(recordedContextAttributes.contains("tag2"));
98-
assertTrue(recordedContextAttributes.contains("tag3"));
9998

100-
Map<String, Long> debugCounters = profiler.getDebugCounters();
101-
assertFalse(debugCounters.isEmpty());
102-
assertEquals(1, debugCounters.get("context_storage_pages"));
103-
assertEquals(0x10000, debugCounters.get("context_storage_bytes"), () -> "invalid context storage: " + debugCounters);
104-
assertEquals(strings.length, debugCounters.get("dictionary_context_keys"));
105-
assertEquals(Arrays.stream(strings).mapToInt(s -> s.length() + 1).sum(), debugCounters.get("dictionary_context_keys_bytes"));
106-
assertBoundedBy(debugCounters.get("dictionary_context_pages"), strings.length, "context storage too many pages");
107-
assertBoundedBy(debugCounters.get("dictionary_context_bytes"), strings.length * DICTIONARY_PAGE_SIZE, "context storage too many pages");
99+
// now check we have settings to unbundle the dynamic columns
100+
IItemCollection activeSettings = verifyEvents("jdk.ActiveSetting");
101+
Set<String> recordedContextAttributes = new HashSet<>();
102+
for (IItemIterable activeSetting : activeSettings) {
103+
IMemberAccessor<String, IItem> nameAccessor = JdkAttributes.REC_SETTING_NAME.getAccessor(activeSetting.getType());
104+
IMemberAccessor<String, IItem> valueAccessor = JdkAttributes.REC_SETTING_VALUE.getAccessor(activeSetting.getType());
105+
for (IItem item : activeSetting) {
106+
String name = nameAccessor.getMember(item);
107+
if ("contextattribute".equals(name)) {
108+
recordedContextAttributes.add(valueAccessor.getMember(item));
109+
}
110+
}
111+
}
112+
assertEquals(3, recordedContextAttributes.size());
113+
assertTrue(recordedContextAttributes.contains("tag1"));
114+
assertTrue(recordedContextAttributes.contains("tag2"));
115+
assertTrue(recordedContextAttributes.contains("tag3"));
108116

109-
for (IItemIterable counterEvent : verifyEvents("datadog.ProfilerCounter")) {
110-
IMemberAccessor<String, IItem> nameAccessor = NAME.getAccessor(counterEvent.getType());
111-
IMemberAccessor<IQuantity, IItem> countAccessor = COUNT.getAccessor(counterEvent.getType());
112-
for (IItem item : counterEvent) {
113-
String name = nameAccessor.getMember(item);
114-
switch (name) {
115-
// debug counters currently include data for temporary dictionaries during serialization which get
116-
// cleaned up, and the counter event reflects the size at the point the counters are written out.
117-
case "dictionary_bytes":
118-
case "dictionary_pages":
119-
case "dictionary_keys":
120-
case "dictionary_keys_bytes":
121-
// these counters reflect the previous reporting epoch
122-
case "dictionary_classes_bytes":
123-
case "dictionary_classes_pages":
124-
case "dictionary_classes_keys":
125-
case "dictionary_classes_keys_bytes":
126-
break;
127-
default:
128-
assertEquals(debugCounters.get(name), countAccessor.getMember(item).longValue(), name);
117+
// Verify counters from JFR serialized data (not live process counters which are reset)
118+
Map<String, Long> jfrCounters = new HashMap<>();
119+
for (IItemIterable counterEvent : verifyEvents("datadog.ProfilerCounter")) {
120+
IMemberAccessor<String, IItem> nameAccessor = NAME.getAccessor(counterEvent.getType());
121+
IMemberAccessor<IQuantity, IItem> countAccessor = COUNT.getAccessor(counterEvent.getType());
122+
for (IItem item : counterEvent) {
123+
String name = nameAccessor.getMember(item);
124+
jfrCounters.put(name, countAccessor.getMember(item).longValue());
129125
}
130126
}
127+
128+
assertFalse(jfrCounters.isEmpty());
129+
assertEquals(1, jfrCounters.get("context_storage_pages"));
130+
assertEquals(0x10000, jfrCounters.get("context_storage_bytes"), () -> "invalid context storage: " + jfrCounters);
131+
assertEquals(strings.length, jfrCounters.get("dictionary_context_keys"));
132+
assertEquals(Arrays.stream(strings).mapToInt(s -> s.length() + 1).sum(), jfrCounters.get("dictionary_context_keys_bytes"));
133+
assertBoundedBy(jfrCounters.get("dictionary_context_pages"), strings.length, "context storage too many pages");
134+
assertBoundedBy(jfrCounters.get("dictionary_context_bytes"), (long) strings.length * DICTIONARY_PAGE_SIZE, "context storage too many pages");
135+
} finally {
136+
// Print statistics about dropped samples for debugging
137+
double dropRate = totalSamplesCount > 0 ? (100.0 * droppedSamplesCount / totalSamplesCount) : 0.0;
138+
double dropWeightRate = totalSamplesWeight > 0 ? (100.0 * droppedSamplesWeight / totalSamplesWeight) : 0.0;
139+
System.out.printf("Sample statistics: %d total (%d dropped, %.2f%%), weight %d total (%d dropped, %.2f%%)%n",
140+
totalSamplesCount, droppedSamplesCount, dropRate,
141+
totalSamplesWeight, droppedSamplesWeight, dropWeightRate);
131142
}
132143
}
133144

0 commit comments

Comments
 (0)