Skip to content

Commit 9804100

Browse files
vandonrvandonr
committed
change hash computation for protobuf to be closer to impacting changes
1 parent 0df72c4 commit 9804100

File tree

5 files changed

+48
-19
lines changed

5 files changed

+48
-19
lines changed

‎dd-java-agent/instrumentation/protobuf/src/main/java/datadog/trace/instrumentation/protobuf_java/SchemaExtractor.java‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import datadog.trace.bootstrap.instrumentation.api.Schema;
1111
import datadog.trace.bootstrap.instrumentation.api.SchemaBuilder;
1212
import datadog.trace.bootstrap.instrumentation.api.SchemaIterator;
13+
import java.util.Comparator;
1314
import java.util.List;
1415
import java.util.stream.Collectors;
1516

@@ -57,7 +58,8 @@ public static boolean extractProperty(
5758
if (field.isRepeated()) {
5859
array = true;
5960
}
60-
switch (field.getType().toProto().getNumber()) {
61+
int typeCode = field.getType().toProto().getNumber();
62+
switch (typeCode) {
6163
case TYPE_DOUBLE:
6264
type = "number";
6365
format = "double";
@@ -107,6 +109,7 @@ public static boolean extractProperty(
107109
if (!extractSchema(field.getMessageType(), builder, depth)) {
108110
return false;
109111
}
112+
builder.addToHash(field.getMessageType().getFullName());
110113
break;
111114
case TYPE_BYTES:
112115
type = "string";
@@ -123,6 +126,7 @@ public static boolean extractProperty(
123126
enumValues =
124127
field.getEnumType().getValues().stream()
125128
.map(Descriptors.EnumValueDescriptor::getName)
129+
.peek(builder::addToHash)
126130
.collect(Collectors.toList());
127131
break;
128132
case TYPE_SFIXED32:
@@ -140,6 +144,9 @@ public static boolean extractProperty(
140144
description = "Unknown type";
141145
break;
142146
}
147+
builder.addToHash(field.getNumber());
148+
builder.addToHash(typeCode);
149+
builder.addToHash(depth);
143150
return builder.addProperty(
144151
schemaName, fieldName, array, type, description, ref, format, enumValues);
145152
}
@@ -150,7 +157,11 @@ public static boolean extractSchema(Descriptor descriptor, SchemaBuilder builder
150157
if (!builder.shouldExtractSchema(schemaName, depth)) {
151158
return false;
152159
}
153-
for (FieldDescriptor field : descriptor.getFields()) {
160+
// iterate fields in number order to ensure hash stability
161+
for (FieldDescriptor field :
162+
descriptor.getFields().stream()
163+
.sorted(Comparator.comparingInt(FieldDescriptor::getNumber))
164+
.collect(Collectors.toList())) {
154165
if (!extractProperty(field, schemaName, field.getName(), builder, depth)) {
155166
return false;
156167
}

‎dd-java-agent/instrumentation/protobuf/src/test/groovy/com/datadog/instrumentation/protobuf/AbstractMessageInstrumentationTest.groovy‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,23 @@
11
package com.datadog.instrumentation.protobuf
22

3+
import com.datadog.instrumentation.protobuf.generated.Message.MyMessage
4+
import com.datadog.instrumentation.protobuf.generated.Message.OtherMessage
35
import com.google.protobuf.InvalidProtocolBufferException
46
import datadog.trace.agent.test.AgentTestRunner
57
import datadog.trace.api.DDTags
68
import datadog.trace.bootstrap.instrumentation.api.AgentSpan
79

810
import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace
911
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activeSpan
10-
import com.datadog.instrumentation.protobuf.generated.Message.MyMessage
11-
import com.datadog.instrumentation.protobuf.generated.Message.OtherMessage
1212

1313
class AbstractMessageInstrumentationTest extends AgentTestRunner {
1414
@Override
1515
protected boolean isDataStreamsEnabled() {
1616
return true
1717
}
1818

19-
String schema = "{\"components\":{\"schemas\":{\"com.datadog.instrumentation.protobuf.generated.MyMessage\":{\"properties\":{\"id\":{\"type\":\"string\"},\"value\":{\"type\":\"string\"},\"other_message\":{\"items\":{\"\$ref\":\"#/components/schemas/com.datadog.instrumentation.protobuf.generated.OtherMessage\"},\"type\":\"array\"}},\"type\":\"object\"},\"com.datadog.instrumentation.protobuf.generated.OtherMessage\":{\"properties\":{\"name\":{\"type\":\"string\"},\"age\":{\"format\":\"int32\",\"type\":\"integer\"}},\"type\":\"object\"}}},\"openapi\":\"3.0.0\"}"
20-
String schemaID = "9054678588020233022"
19+
String expectedSchema = "{\"components\":{\"schemas\":{\"com.datadog.instrumentation.protobuf.generated.MyMessage\":{\"properties\":{\"id\":{\"type\":\"string\"},\"value\":{\"type\":\"string\"},\"other_message\":{\"items\":{\"\$ref\":\"#/components/schemas/com.datadog.instrumentation.protobuf.generated.OtherMessage\"},\"type\":\"array\"}},\"type\":\"object\"},\"com.datadog.instrumentation.protobuf.generated.OtherMessage\":{\"properties\":{\"name\":{\"type\":\"string\"},\"age\":{\"format\":\"int32\",\"type\":\"integer\"}},\"type\":\"object\"}}},\"openapi\":\"3.0.0\"}"
20+
String expectedSchemaID = "4690647329509494987"
2121

2222

2323
void 'test extract protobuf schema on serialize & deserialize'() {
@@ -50,12 +50,12 @@ class AbstractMessageInstrumentationTest extends AgentTestRunner {
5050
errored false
5151
measured false
5252
tags {
53-
"$DDTags.SCHEMA_DEFINITION" schema
53+
"$DDTags.SCHEMA_DEFINITION" expectedSchema
5454
"$DDTags.SCHEMA_WEIGHT" 1
5555
"$DDTags.SCHEMA_TYPE" "protobuf"
5656
"$DDTags.SCHEMA_NAME" "com.datadog.instrumentation.protobuf.generated.MyMessage"
5757
"$DDTags.SCHEMA_OPERATION" "serialization"
58-
"$DDTags.SCHEMA_ID" schemaID
58+
"$DDTags.SCHEMA_ID" expectedSchemaID
5959
defaultTags(false)
6060
}
6161
}
@@ -68,12 +68,12 @@ class AbstractMessageInstrumentationTest extends AgentTestRunner {
6868
errored false
6969
measured false
7070
tags {
71-
"$DDTags.SCHEMA_DEFINITION" schema
71+
"$DDTags.SCHEMA_DEFINITION" expectedSchema
7272
"$DDTags.SCHEMA_WEIGHT" 1
7373
"$DDTags.SCHEMA_TYPE" "protobuf"
7474
"$DDTags.SCHEMA_NAME" "com.datadog.instrumentation.protobuf.generated.MyMessage"
7575
"$DDTags.SCHEMA_OPERATION" "deserialization"
76-
"$DDTags.SCHEMA_ID" schemaID
76+
"$DDTags.SCHEMA_ID" expectedSchemaID
7777
defaultTags(false)
7878
}
7979
}

‎dd-java-agent/instrumentation/protobuf/src/test/groovy/com/datadog/instrumentation/protobuf/DynamicMessageInstrumentationTest.groovy‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
package com.datadog.instrumentation.protobuf
22

3+
import com.datadog.instrumentation.protobuf.generated.Message.MyMessage
34
import com.google.protobuf.DynamicMessage
45
import datadog.trace.agent.test.AgentTestRunner
56
import datadog.trace.api.DDTags
67
import datadog.trace.bootstrap.instrumentation.api.AgentSpan
7-
import com.datadog.instrumentation.protobuf.generated.Message.MyMessage
88

99
import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace
1010
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activeSpan
@@ -22,8 +22,8 @@ class DynamicMessageInstrumentationTest extends AgentTestRunner {
2222
.setValue("Hello from Protobuf!")
2323
.build()
2424
when:
25-
String schema = "{\"components\":{\"schemas\":{\"com.datadog.instrumentation.protobuf.generated.MyMessage\":{\"properties\":{\"id\":{\"type\":\"string\"},\"value\":{\"type\":\"string\"},\"other_message\":{\"items\":{\"\$ref\":\"#/components/schemas/com.datadog.instrumentation.protobuf.generated.OtherMessage\"},\"type\":\"array\"}},\"type\":\"object\"},\"com.datadog.instrumentation.protobuf.generated.OtherMessage\":{\"properties\":{\"name\":{\"type\":\"string\"},\"age\":{\"format\":\"int32\",\"type\":\"integer\"}},\"type\":\"object\"}}},\"openapi\":\"3.0.0\"}"
26-
String schemaID = "9054678588020233022"
25+
String expectedSchema = "{\"components\":{\"schemas\":{\"com.datadog.instrumentation.protobuf.generated.MyMessage\":{\"properties\":{\"id\":{\"type\":\"string\"},\"value\":{\"type\":\"string\"},\"other_message\":{\"items\":{\"\$ref\":\"#/components/schemas/com.datadog.instrumentation.protobuf.generated.OtherMessage\"},\"type\":\"array\"}},\"type\":\"object\"},\"com.datadog.instrumentation.protobuf.generated.OtherMessage\":{\"properties\":{\"name\":{\"type\":\"string\"},\"age\":{\"format\":\"int32\",\"type\":\"integer\"}},\"type\":\"object\"}}},\"openapi\":\"3.0.0\"}"
26+
String expectedSchemaID = "4690647329509494987"
2727
var bytes
2828
runUnderTrace("parent_serialize") {
2929
AgentSpan span = activeSpan()
@@ -46,12 +46,12 @@ class DynamicMessageInstrumentationTest extends AgentTestRunner {
4646
errored false
4747
measured false
4848
tags {
49-
"$DDTags.SCHEMA_DEFINITION" schema
49+
"$DDTags.SCHEMA_DEFINITION" expectedSchema
5050
"$DDTags.SCHEMA_WEIGHT" 1
5151
"$DDTags.SCHEMA_TYPE" "protobuf"
5252
"$DDTags.SCHEMA_NAME" "com.datadog.instrumentation.protobuf.generated.MyMessage"
5353
"$DDTags.SCHEMA_OPERATION" "serialization"
54-
"$DDTags.SCHEMA_ID" schemaID
54+
"$DDTags.SCHEMA_ID" expectedSchemaID
5555
defaultTags(false)
5656
}
5757
}
@@ -64,12 +64,12 @@ class DynamicMessageInstrumentationTest extends AgentTestRunner {
6464
errored false
6565
measured false
6666
tags {
67-
"$DDTags.SCHEMA_DEFINITION" schema
67+
"$DDTags.SCHEMA_DEFINITION" expectedSchema
6868
"$DDTags.SCHEMA_WEIGHT" 1
6969
"$DDTags.SCHEMA_TYPE" "protobuf"
7070
"$DDTags.SCHEMA_NAME" "com.datadog.instrumentation.protobuf.generated.MyMessage"
7171
"$DDTags.SCHEMA_OPERATION" "deserialization"
72-
"$DDTags.SCHEMA_ID" schemaID
72+
"$DDTags.SCHEMA_ID" expectedSchemaID
7373
defaultTags(false)
7474
}
7575
}

‎dd-trace-core/src/main/java/datadog/trace/core/datastreams/SchemaBuilder.java‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ public class SchemaBuilder implements datadog.trace.bootstrap.instrumentation.ap
1717
private static final DDCache<String, Schema> CACHE = DDCaches.newFixedSizeCache(32);
1818
private static final int maxDepth = 10;
1919
private static final int maxProperties = 1000;
20+
private static final long HASH_INIT = FNV64Hash.generateHash(new byte[0], FNV64Hash.Version.v1A);
21+
private long currentHash = HASH_INIT;
2022
private int properties;
2123
private final SchemaIterator iterator;
2224

@@ -47,13 +49,25 @@ public boolean addProperty(
4749
return true;
4850
}
4951

52+
public void addToHash(int value) {
53+
addToHash(Integer.toString(value));
54+
}
55+
56+
public void addToHash(String value) {
57+
currentHash = FNV64Hash.continueHash(currentHash, value, FNV64Hash.Version.v1A);
58+
}
59+
5060
public Schema build() {
5161
this.iterator.iterateOverSchema(this);
5262
Moshi moshi = new Moshi.Builder().build();
5363
JsonAdapter<OpenApiSchema> jsonAdapter = moshi.adapter(OpenApiSchema.class);
5464
String definition = jsonAdapter.toJson(this.schema);
55-
String id = Long.toUnsignedString(FNV64Hash.generateHash(definition, FNV64Hash.Version.v1A));
56-
return new Schema(definition, id);
65+
if (currentHash == HASH_INIT) {
66+
// if hash was not computed along the way,
67+
// we fall back to computing it from the json representation of the schema
68+
currentHash = FNV64Hash.generateHash(definition, FNV64Hash.Version.v1A);
69+
}
70+
return new Schema(definition, Long.toUnsignedString(currentHash));
5771
}
5872

5973
@Override

‎internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/SchemaBuilder.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,5 +13,9 @@ boolean addProperty(
1313
String format,
1414
List<String> enumValues);
1515

16+
void addToHash(int value);
17+
18+
void addToHash(String value);
19+
1620
boolean shouldExtractSchema(String schemaName, int depth);
1721
}

0 commit comments

Comments
 (0)