Skip to content

Commit a5197b9

Browse files
jbachorikclaude
jbachorik
and
claude
committed
fix(profiling): Fix timestamp validation and make profcheck mandatory
Fixed profcheck timestamp validation errors and made profcheck validation mandatory to pass alongside protoc validation. Timestamp Issues Fixed: - Removed manual startTime field assignments in all test JFR events - Manual timestamps were being interpreted as JFR ticks (not epoch nanos) - Let JFR recording system automatically assign correct timestamps - JFR auto-timestamps are properly converted via chunkInfo.asInstant() Validation Changes: - Made profcheck validation mandatory (previously only protoc was required) - Updated validation script to require both protoc AND profcheck to pass - Removed special handling for "known attribute_indices bug" (now fixed) - Updated test assertions to verify both validators pass - Both validators now cleanly pass for all test profiles Result: Complete OTLP profiles spec compliance with both: - protoc (official Protocol Buffers compiler) - structural validation - profcheck (OpenTelemetry conformance checker) - semantic validation All tests passing: empty, CPU, allocation, and mixed profiles. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
1 parent d5c78c2 commit a5197b9

File tree

2 files changed

+31
-40
lines changed

2 files changed

+31
-40
lines changed

‎dd-java-agent/agent-profiling/profiling-otel/src/test/java/com/datadog/profiling/otel/ProfcheckValidationTest.java‎

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ public void testEmptyProfile() throws Exception {
8989
public void testCpuProfile() throws Exception {
9090
// Generate JFR recording with CPU samples
9191
Path jfrFile = tempDir.resolve("cpu.jfr");
92+
9293
try (Recording recording = Recordings.newRecording(jfrFile)) {
9394
Types types = recording.getTypes();
9495

@@ -102,7 +103,6 @@ public void testCpuProfile() throws Exception {
102103

103104
// Add 100 CPU samples with various stack traces
104105
for (int i = 0; i < 100; i++) {
105-
final int index = i;
106106
final long spanId = 10000L + i;
107107
final long rootSpanId = 20000L + (i % 10);
108108

@@ -116,7 +116,6 @@ public void testCpuProfile() throws Exception {
116116
recording.writeEvent(
117117
executionSampleType.asValue(
118118
valueBuilder -> {
119-
valueBuilder.putField("startTime", System.nanoTime() + index * 1000000L);
120119
valueBuilder.putField("spanId", spanId);
121120
valueBuilder.putField("localRootSpanId", rootSpanId);
122121
valueBuilder.putField(
@@ -131,24 +130,23 @@ public void testCpuProfile() throws Exception {
131130
byte[] otlpData = convertJfrToOtlp(jfrFile);
132131
Files.write(otlpFile, otlpData);
133132

134-
// Validate with profcheck (now includes protoc validation)
133+
// Validate with profcheck (includes both protoc and profcheck validation)
135134
String result = validateWithProfcheck(otlpFile);
136135

137-
// Check for protoc validation success (authoritative)
136+
// Both validators must pass
138137
assertTrue(
139138
result.contains("protoc validation PASSED"),
140139
"CPU profile should pass protoc validation (spec-compliant). Output: " + result);
141-
142-
// Profcheck failures are expected due to known bug, just log them
143-
if (result.contains("profcheck") && result.contains("WARNING")) {
144-
System.out.println("Note: profcheck reported warnings (known attribute_indices parsing bug)");
145-
}
140+
assertTrue(
141+
result.contains("profcheck validation PASSED"),
142+
"CPU profile should pass profcheck validation. Output: " + result);
146143
}
147144

148145
@Test
149146
public void testAllocationProfile() throws Exception {
150147
// Generate JFR recording with allocation samples
151148
Path jfrFile = tempDir.resolve("alloc.jfr");
149+
152150
try (Recording recording = Recordings.newRecording(jfrFile)) {
153151
Types types = recording.getTypes();
154152

@@ -164,7 +162,6 @@ public void testAllocationProfile() throws Exception {
164162

165163
// Add 50 allocation samples
166164
for (int i = 0; i < 50; i++) {
167-
final int index = i;
168165
final long weight = 1024L * (i + 1);
169166
final long spanId = 30000L + i;
170167
final long rootSpanId = 40000L + (i % 5);
@@ -178,7 +175,6 @@ public void testAllocationProfile() throws Exception {
178175
recording.writeEvent(
179176
objectSampleType.asValue(
180177
valueBuilder -> {
181-
valueBuilder.putField("startTime", System.nanoTime() + index * 2000000L);
182178
valueBuilder.putField("size", weight);
183179
valueBuilder.putField("weight", 0.9f);
184180
valueBuilder.putField("spanId", spanId);
@@ -195,18 +191,23 @@ public void testAllocationProfile() throws Exception {
195191
byte[] otlpData = convertJfrToOtlp(jfrFile);
196192
Files.write(otlpFile, otlpData);
197193

198-
// Validate with profcheck (now includes protoc validation)
194+
// Validate with profcheck (includes both protoc and profcheck validation)
199195
String result = validateWithProfcheck(otlpFile);
200196

197+
// Both validators must pass
201198
assertTrue(
202199
result.contains("protoc validation PASSED"),
203200
"Allocation profile should pass protoc validation (spec-compliant). Output: " + result);
201+
assertTrue(
202+
result.contains("profcheck validation PASSED"),
203+
"Allocation profile should pass profcheck validation. Output: " + result);
204204
}
205205

206206
@Test
207207
public void testMixedProfile() throws Exception {
208208
// Generate JFR recording with multiple event types
209209
Path jfrFile = tempDir.resolve("mixed.jfr");
210+
210211
try (Recording recording = Recordings.newRecording(jfrFile)) {
211212
Types types = recording.getTypes();
212213

@@ -235,15 +236,13 @@ public void testMixedProfile() throws Exception {
235236

236237
// Add mix of events
237238
for (int i = 0; i < 20; i++) {
238-
final int index = i;
239239
final long spanId = 50000L + i;
240240
final long rootSpanId = 60000L;
241241

242242
// CPU sample
243243
recording.writeEvent(
244244
executionSampleType.asValue(
245245
valueBuilder -> {
246-
valueBuilder.putField("startTime", System.nanoTime() + index * 1000000L);
247246
valueBuilder.putField("spanId", spanId);
248247
valueBuilder.putField("localRootSpanId", rootSpanId);
249248
valueBuilder.putField(
@@ -255,8 +254,6 @@ public void testMixedProfile() throws Exception {
255254
recording.writeEvent(
256255
methodSampleType.asValue(
257256
valueBuilder -> {
258-
valueBuilder.putField(
259-
"startTime", System.nanoTime() + index * 1000000L + 500000L);
260257
valueBuilder.putField("spanId", spanId);
261258
valueBuilder.putField("localRootSpanId", rootSpanId);
262259
valueBuilder.putField(
@@ -271,12 +268,16 @@ public void testMixedProfile() throws Exception {
271268
byte[] otlpData = convertJfrToOtlp(jfrFile);
272269
Files.write(otlpFile, otlpData);
273270

274-
// Validate with profcheck (now includes protoc validation)
271+
// Validate with profcheck (includes both protoc and profcheck validation)
275272
String result = validateWithProfcheck(otlpFile);
276273

274+
// Both validators must pass
277275
assertTrue(
278276
result.contains("protoc validation PASSED"),
279277
"Mixed profile should pass protoc validation (spec-compliant). Output: " + result);
278+
assertTrue(
279+
result.contains("profcheck validation PASSED"),
280+
"Mixed profile should pass profcheck validation. Output: " + result);
280281
}
281282

282283
private byte[] convertJfrToOtlp(Path jfrFile) throws IOException {

‎docker/Dockerfile.profcheck‎

Lines changed: 13 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -77,45 +77,35 @@ fi
7777

7878
echo ""
7979
echo "[2/2] Running profcheck (OpenTelemetry conformance checker)..."
80-
echo "Note: profcheck currently has known issues with attribute_indices parsing"
81-
echo " and may report false positives. protoc validation is authoritative."
8280
echo ""
8381

8482
if profcheck "$FILE" 2>&1 | tee /tmp/profcheck.txt; then
8583
echo "✓ profcheck validation PASSED"
8684
PROFCHECK_STATUS=0
8785
else
8886
PROFCHECK_STATUS=1
89-
# Check if it's the known attribute_indices bug
90-
if grep -q "attribute_indices.*out of range" /tmp/profcheck.txt; then
91-
echo ""
92-
echo "⚠ KNOWN ISSUE: profcheck reports attribute_indices errors"
93-
echo " This is a known bug in profcheck PR #12 where it misreads"
94-
echo " link_index values as attribute_indices values."
95-
echo " Since protoc validation passed, the profile is correct."
96-
echo ""
97-
echo "Full profcheck output saved for inspection:"
98-
echo "--- BEGIN PROFCHECK OUTPUT ---"
99-
cat /tmp/profcheck.txt
100-
echo "--- END PROFCHECK OUTPUT ---"
101-
else
102-
# Unknown profcheck error - show the full output
103-
echo ""
104-
echo "⚠ Unexpected profcheck failure:"
105-
cat /tmp/profcheck.txt
106-
fi
87+
echo "✗ profcheck validation FAILED"
88+
echo ""
89+
echo "Full profcheck output:"
90+
echo "--- BEGIN PROFCHECK OUTPUT ---"
91+
cat /tmp/profcheck.txt
92+
echo "--- END PROFCHECK OUTPUT ---"
10793
fi
10894

10995
echo ""
11096
echo "================================"
11197
echo "Validation Summary"
11298
echo "================================"
11399
echo "protoc: $([ $PROTOC_STATUS -eq 0 ] && echo 'PASS ✓' || echo 'FAIL ✗')"
114-
echo "profcheck: $([ $PROFCHECK_STATUS -eq 0 ] && echo 'PASS ✓' || echo 'WARNING ⚠')"
100+
echo "profcheck: $([ $PROFCHECK_STATUS -eq 0 ] && echo 'PASS ✓' || echo 'FAIL ✗')"
115101
echo ""
116102

117-
# Return success if protoc passed (it's the authoritative validator)
118-
exit $PROTOC_STATUS
103+
# Both validators must pass
104+
if [ $PROTOC_STATUS -eq 0 ] && [ $PROFCHECK_STATUS -eq 0 ]; then
105+
exit 0
106+
else
107+
exit 1
108+
fi
119109
EOF
120110

121111
RUN chmod +x /usr/local/bin/validate-profile

0 commit comments

Comments
 (0)