Skip to content

Commit 8bba34a

Browse files
jbachorikclaude
jbachorik
and
claude
committed
fix(profiling): Fix OTLP dictionary index 0 sentinel encoding
Dictionary tables (location, function, link, stack, attribute) were omitting their required index 0 sentinel entries from the wire format, causing profcheck validation failures. Root cause: 1. Dictionary loops started at i=1 instead of i=0, skipping sentinels 2. ProtobufEncoder.writeNestedMessage() had an if (length > 0) check that completely skipped writing empty messages 3. Sentinel entries encode as empty messages (all fields are 0/empty) 4. Result: Index 0 was not present in wire format, causing off-by-one array indexing errors in profcheck validation Fix: - Changed ProtobufEncoder.writeNestedMessage() to always write tag+length even for empty messages (required for sentinels) - Changed all dictionary table loops to start from i=0 to include sentinels - Added attribute_table encoding (was completely missing) - Updated JSON encoding to match protobuf encoding - Fixed test to use correct event type (datadog.ObjectSample) All profcheck validation tests now pass with "conformance checks passed". 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
1 parent 747aa1d commit 8bba34a

File tree

3 files changed

+110
-16
lines changed

3 files changed

+110
-16
lines changed

‎dd-java-agent/agent-profiling/profiling-otel/src/main/java/com/datadog/profiling/otel/JfrToOtlpConverter.java‎

Lines changed: 102 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -659,7 +659,10 @@ private void encodeSample(ProtobufEncoder encoder, SampleData sample) {
659659
// Field 1: stack_index
660660
encoder.writeVarintField(OtlpProtoFields.Sample.STACK_INDEX, sample.stackIndex);
661661

662-
// Field 3: link_index (skip field 2 attribute_indices for now)
662+
// Field 2: attribute_indices - skip for now (no attribute data from JFR)
663+
// TODO: When JFR provides attributes, encode them here
664+
665+
// Field 3: link_index
663666
encoder.writeVarintField(OtlpProtoFields.Sample.LINK_INDEX, sample.linkIndex);
664667

665668
// Field 4: values (packed)
@@ -676,21 +679,24 @@ private void encodeDictionary(ProtobufEncoder encoder) {
676679
// ProfilesDictionary message
677680

678681
// Field 2: location_table
679-
for (int i = 1; i < locationTable.size(); i++) {
682+
// Note: Include index 0 (null/unset sentinel) required by OTLP spec
683+
for (int i = 0; i < locationTable.size(); i++) {
680684
final int idx = i;
681685
encoder.writeNestedMessage(
682686
OtlpProtoFields.ProfilesDictionary.LOCATION_TABLE, enc -> encodeLocation(enc, idx));
683687
}
684688

685689
// Field 3: function_table
686-
for (int i = 1; i < functionTable.size(); i++) {
690+
// Note: Include index 0 (null/unset sentinel) required by OTLP spec
691+
for (int i = 0; i < functionTable.size(); i++) {
687692
final int idx = i;
688693
encoder.writeNestedMessage(
689694
OtlpProtoFields.ProfilesDictionary.FUNCTION_TABLE, enc -> encodeFunction(enc, idx));
690695
}
691696

692697
// Field 4: link_table
693-
for (int i = 1; i < linkTable.size(); i++) {
698+
// Note: Include index 0 (null/unset sentinel) required by OTLP spec
699+
for (int i = 0; i < linkTable.size(); i++) {
694700
final int idx = i;
695701
encoder.writeNestedMessage(
696702
OtlpProtoFields.ProfilesDictionary.LINK_TABLE, enc -> encodeLink(enc, idx));
@@ -701,8 +707,17 @@ private void encodeDictionary(ProtobufEncoder encoder) {
701707
encoder.writeStringField(OtlpProtoFields.ProfilesDictionary.STRING_TABLE, s);
702708
}
703709

710+
// Field 6: attribute_table
711+
// Note: Must always include at least index 0 (null/unset sentinel) required by OTLP spec
712+
for (int i = 0; i < attributeTable.size(); i++) {
713+
final int idx = i;
714+
encoder.writeNestedMessage(
715+
OtlpProtoFields.ProfilesDictionary.ATTRIBUTE_TABLE, enc -> encodeAttribute(enc, idx));
716+
}
717+
704718
// Field 7: stack_table
705-
for (int i = 1; i < stackTable.size(); i++) {
719+
// Note: Include index 0 (null/unset sentinel) required by OTLP spec
720+
for (int i = 0; i < stackTable.size(); i++) {
706721
final int idx = i;
707722
encoder.writeNestedMessage(
708723
OtlpProtoFields.ProfilesDictionary.STACK_TABLE, enc -> encodeStack(enc, idx));
@@ -713,9 +728,11 @@ private void encodeLocation(ProtobufEncoder encoder, int index) {
713728
LocationTable.LocationEntry entry = locationTable.get(index);
714729

715730
// Field 1: mapping_index
731+
// Note: Always write, even for index 0 sentinel (value 0) to ensure non-empty message
716732
encoder.writeVarintField(OtlpProtoFields.Location.MAPPING_INDEX, entry.mappingIndex);
717733

718734
// Field 2: address
735+
// Note: For index 0 sentinel, this will be 0 but writeVarintField writes 0 values
719736
encoder.writeVarintField(OtlpProtoFields.Location.ADDRESS, entry.address);
720737

721738
// Field 3: lines (repeated)
@@ -749,9 +766,42 @@ private void encodeLink(ProtobufEncoder encoder, int index) {
749766
private void encodeStack(ProtobufEncoder encoder, int index) {
750767
StackTable.StackEntry entry = stackTable.get(index);
751768

769+
// For index 0 (null sentinel), location_indices is empty
770+
// writePackedVarintField handles empty arrays by writing nothing, but writeNestedMessage
771+
// now always writes the message envelope (tag + length=0) even if the content is empty
752772
encoder.writePackedVarintField(OtlpProtoFields.Stack.LOCATION_INDICES, entry.locationIndices);
753773
}
754774

775+
private void encodeAttribute(ProtobufEncoder encoder, int index) {
776+
AttributeTable.AttributeEntry entry = attributeTable.get(index);
777+
778+
// Field 1: key_strindex
779+
encoder.writeVarintField(OtlpProtoFields.KeyValueAndUnit.KEY_STRINDEX, entry.keyIndex);
780+
781+
// Field 2: value (AnyValue oneof)
782+
encoder.writeNestedMessage(OtlpProtoFields.KeyValueAndUnit.VALUE, enc -> {
783+
switch (entry.valueType) {
784+
case STRING:
785+
enc.writeStringField(OtlpProtoFields.AnyValue.STRING_VALUE, (String) entry.value);
786+
break;
787+
case BOOL:
788+
enc.writeBoolField(OtlpProtoFields.AnyValue.BOOL_VALUE, (Boolean) entry.value);
789+
break;
790+
case INT:
791+
enc.writeSignedVarintField(OtlpProtoFields.AnyValue.INT_VALUE, (Long) entry.value);
792+
break;
793+
case DOUBLE:
794+
// Note: protobuf doubles are fixed64, not varint
795+
long doubleBits = Double.doubleToRawLongBits((Double) entry.value);
796+
enc.writeFixed64Field(OtlpProtoFields.AnyValue.DOUBLE_VALUE, doubleBits);
797+
break;
798+
}
799+
});
800+
801+
// Field 3: unit_strindex
802+
encoder.writeVarintField(OtlpProtoFields.KeyValueAndUnit.UNIT_STRINDEX, entry.unitIndex);
803+
}
804+
755805
private byte[] generateProfileId() {
756806
UUID uuid = UUID.randomUUID();
757807
byte[] bytes = new byte[16];
@@ -973,22 +1023,25 @@ private void encodeDictionaryJson(JsonWriter json) {
9731023
json.beginObject();
9741024

9751025
// location_table array
1026+
// Note: Include index 0 (null/unset sentinel) required by OTLP spec
9761027
json.name("location_table").beginArray();
977-
for (int i = 1; i < locationTable.size(); i++) {
1028+
for (int i = 0; i < locationTable.size(); i++) {
9781029
encodeLocationJson(json, i);
9791030
}
9801031
json.endArray();
9811032

9821033
// function_table array
1034+
// Note: Include index 0 (null/unset sentinel) required by OTLP spec
9831035
json.name("function_table").beginArray();
984-
for (int i = 1; i < functionTable.size(); i++) {
1036+
for (int i = 0; i < functionTable.size(); i++) {
9851037
encodeFunctionJson(json, i);
9861038
}
9871039
json.endArray();
9881040

9891041
// link_table array
1042+
// Note: Include index 0 (null/unset sentinel) required by OTLP spec
9901043
json.name("link_table").beginArray();
991-
for (int i = 1; i < linkTable.size(); i++) {
1044+
for (int i = 0; i < linkTable.size(); i++) {
9921045
encodeLinkJson(json, i);
9931046
}
9941047
json.endArray();
@@ -1000,9 +1053,18 @@ private void encodeDictionaryJson(JsonWriter json) {
10001053
}
10011054
json.endArray();
10021055

1056+
// attribute_table array
1057+
// Note: Must always include at least index 0 (null/unset sentinel) required by OTLP spec
1058+
json.name("attribute_table").beginArray();
1059+
for (int i = 0; i < attributeTable.size(); i++) {
1060+
encodeAttributeJson(json, i);
1061+
}
1062+
json.endArray();
1063+
10031064
// stack_table array
1065+
// Note: Include index 0 (null/unset sentinel) required by OTLP spec
10041066
json.name("stack_table").beginArray();
1005-
for (int i = 1; i < stackTable.size(); i++) {
1067+
for (int i = 0; i < stackTable.size(); i++) {
10061068
encodeStackJson(json, i);
10071069
}
10081070
json.endArray();
@@ -1074,6 +1136,37 @@ private void encodeLinkJson(JsonWriter json, int index) {
10741136
json.endObject();
10751137
}
10761138

1139+
private void encodeAttributeJson(JsonWriter json, int index) {
1140+
AttributeTable.AttributeEntry entry = attributeTable.get(index);
1141+
json.beginObject();
1142+
1143+
// key_strindex
1144+
json.name("key_strindex").value(entry.keyIndex);
1145+
1146+
// value object (AnyValue)
1147+
json.name("value").beginObject();
1148+
switch (entry.valueType) {
1149+
case STRING:
1150+
json.name("string_value").value((String) entry.value);
1151+
break;
1152+
case BOOL:
1153+
json.name("bool_value").value((Boolean) entry.value);
1154+
break;
1155+
case INT:
1156+
json.name("int_value").value((Long) entry.value);
1157+
break;
1158+
case DOUBLE:
1159+
json.name("double_value").value((Double) entry.value);
1160+
break;
1161+
}
1162+
json.endObject();
1163+
1164+
// unit_strindex
1165+
json.name("unit_strindex").value(entry.unitIndex);
1166+
1167+
json.endObject();
1168+
}
1169+
10771170
private void encodeStackJson(JsonWriter json, int index) {
10781171
StackTable.StackEntry entry = stackTable.get(index);
10791172
json.beginObject();

‎dd-java-agent/agent-profiling/profiling-otel/src/main/java/com/datadog/profiling/otel/proto/ProtobufEncoder.java‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -135,9 +135,11 @@ public void writeNestedMessage(int fieldNumber, MessageWriter writer) {
135135
writer.write(nested);
136136
byte[] messageBytes = nested.toByteArray();
137137

138+
// ALWAYS write the message, even if empty (length 0)
139+
// This is REQUIRED for OTLP dictionary tables where index 0 must be present
140+
writeTag(fieldNumber, WIRETYPE_LENGTH_DELIMITED);
141+
writeVarint(messageBytes.length);
138142
if (messageBytes.length > 0) {
139-
writeTag(fieldNumber, WIRETYPE_LENGTH_DELIMITED);
140-
writeVarint(messageBytes.length);
141143
try {
142144
buffer.write(messageBytes);
143145
} catch (IOException e) {
@@ -280,7 +282,7 @@ public void writeBoolField(int fieldNumber, boolean value) {
280282
*/
281283
public void writePackedVarintField(int fieldNumber, int[] values) {
282284
if (values == null || values.length == 0) {
283-
return;
285+
return; // Empty packed arrays are omitted per protobuf3 spec
284286
}
285287

286288
// Calculate packed size

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -152,10 +152,9 @@ public void testAllocationProfile() throws Exception {
152152

153153
Type objectSampleType =
154154
recording.registerEventType(
155-
"jdk.ObjectAllocationSample",
155+
"datadog.ObjectSample",
156156
type -> {
157-
type.addField("objectClass", types.getType("java.lang.Class"));
158-
type.addField("weight", Types.Builtin.LONG);
157+
type.addField("allocationSize", Types.Builtin.LONG);
159158
type.addField("spanId", Types.Builtin.LONG);
160159
type.addField("localRootSpanId", Types.Builtin.LONG);
161160
});
@@ -177,7 +176,7 @@ public void testAllocationProfile() throws Exception {
177176
objectSampleType.asValue(
178177
valueBuilder -> {
179178
valueBuilder.putField("startTime", System.nanoTime() + index * 2000000L);
180-
valueBuilder.putField("weight", weight);
179+
valueBuilder.putField("allocationSize", weight);
181180
valueBuilder.putField("spanId", spanId);
182181
valueBuilder.putField("localRootSpanId", rootSpanId);
183182
valueBuilder.putField(

0 commit comments

Comments
 (0)