Skip to content

Commit cfc19fa

Browse files
randomandersonrandomanderson
committed
Change String ids to BigInteger ids
1 parent 466fdf2 commit cfc19fa

29 files changed

Lines changed: 269 additions & 343 deletions

‎dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/asserts/SpanAssert.groovy‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,16 +81,16 @@ class SpanAssert {
8181
}
8282

8383
def parent() {
84-
assert span.parentId == "0"
84+
assert span.parentId == BigInteger.ZERO
8585
checked.parentId = true
8686
}
8787

88-
def parentId(String parentId) {
88+
def parentId(BigInteger parentId) {
8989
assert span.parentId == parentId
9090
checked.parentId = true
9191
}
9292

93-
def traceId(String traceId) {
93+
def traceId(BigInteger traceId) {
9494
assert span.traceId == traceId
9595
checked.traceId = true
9696
}

‎dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/asserts/TagsAssert.groovy‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import groovy.transform.stc.SimpleType
99
import java.util.regex.Pattern
1010

1111
class TagsAssert {
12-
private final String spanParentId
12+
private final BigInteger spanParentId
1313
private final Map<String, Object> tags
1414
private final Set<String> assertedTags = new TreeSet<>()
1515

@@ -43,7 +43,7 @@ class TagsAssert {
4343

4444
// FIXME: DQH - Too much conditional logic? Maybe create specialized methods for client & server cases
4545

46-
boolean isRoot = ("0" == spanParentId)
46+
boolean isRoot = (BigInteger.ZERO == spanParentId)
4747
if (isRoot || distributedRootSpan) {
4848
assert tags[Config.RUNTIME_ID_TAG] == Config.get().runtimeId
4949
} else {

‎dd-java-agent/testing/src/test/groovy/TraceCorrelationTest.groovy‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@ class TraceCorrelationTest extends AgentTestRunner {
1212
DDSpan span = (DDSpan) scope.span()
1313

1414
then:
15-
CorrelationIdentifier.traceId == span.traceId
16-
CorrelationIdentifier.spanId == span.spanId
15+
CorrelationIdentifier.traceId == span.traceId.toString()
16+
CorrelationIdentifier.spanId == span.spanId.toString()
1717

1818
when:
1919
scope.close()

‎dd-trace-ot/src/main/java/datadog/opentracing/DDSpan.java‎

Lines changed: 4 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,12 @@
44

55
import com.fasterxml.jackson.annotation.JsonGetter;
66
import com.fasterxml.jackson.annotation.JsonIgnore;
7-
import com.fasterxml.jackson.core.JsonGenerator;
8-
import com.fasterxml.jackson.databind.SerializerProvider;
9-
import com.fasterxml.jackson.databind.annotation.JsonSerialize;
10-
import com.fasterxml.jackson.databind.ser.std.StdSerializer;
117
import datadog.trace.api.DDTags;
128
import datadog.trace.api.interceptor.MutableSpan;
139
import datadog.trace.api.sampling.PrioritySampling;
1410
import datadog.trace.common.util.Clock;
1511
import io.opentracing.Span;
1612
import io.opentracing.tag.Tag;
17-
import java.io.IOException;
1813
import java.io.PrintWriter;
1914
import java.io.StringWriter;
2015
import java.lang.ref.WeakReference;
@@ -126,7 +121,7 @@ public DDSpan setError(final boolean error) {
126121
*/
127122
@JsonIgnore
128123
public final boolean isRootSpan() {
129-
return "0".equals(context.getParentId());
124+
return BigInteger.ZERO.equals(context.getParentId());
130125
}
131126

132127
@Override
@@ -346,20 +341,17 @@ public String getServiceName() {
346341
}
347342

348343
@JsonGetter("trace_id")
349-
@JsonSerialize(using = UInt64IDStringSerializer.class)
350-
public String getTraceId() {
344+
public BigInteger getTraceId() {
351345
return context.getTraceId();
352346
}
353347

354348
@JsonGetter("span_id")
355-
@JsonSerialize(using = UInt64IDStringSerializer.class)
356-
public String getSpanId() {
349+
public BigInteger getSpanId() {
357350
return context.getSpanId();
358351
}
359352

360353
@JsonGetter("parent_id")
361-
@JsonSerialize(using = UInt64IDStringSerializer.class)
362-
public String getParentId() {
354+
public BigInteger getParentId() {
363355
return context.getParentId();
364356
}
365357

@@ -422,31 +414,4 @@ public String toString() {
422414
.append(durationNano)
423415
.toString();
424416
}
425-
426-
protected static class UInt64IDStringSerializer extends StdSerializer<String> {
427-
private static final int LONG_PARSE_LIMIT = String.valueOf(Long.MAX_VALUE).length();
428-
429-
public UInt64IDStringSerializer() {
430-
this(null);
431-
}
432-
433-
public UInt64IDStringSerializer(final Class<String> stringClass) {
434-
super(stringClass);
435-
}
436-
437-
@Override
438-
public void serialize(
439-
final String value, final JsonGenerator gen, final SerializerProvider provider)
440-
throws IOException {
441-
final int length = value.length();
442-
// BigInteger's are expensive, so lets try to avoid using them if possible.
443-
// This is a rough approximation for optimization.
444-
// There are some values that would pass this test that could be parsed with Long.parseLong.
445-
if (length > LONG_PARSE_LIMIT || (length == LONG_PARSE_LIMIT && value.startsWith("9"))) {
446-
gen.writeNumber(new BigInteger(value));
447-
} else {
448-
gen.writeNumber(Long.parseLong(value));
449-
}
450-
}
451-
}
452417
}

‎dd-trace-ot/src/main/java/datadog/opentracing/DDSpanContext.java‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import datadog.opentracing.decorators.AbstractDecorator;
55
import datadog.trace.api.DDTags;
66
import datadog.trace.api.sampling.PrioritySampling;
7+
import java.math.BigInteger;
78
import java.util.Collections;
89
import java.util.List;
910
import java.util.Map;
@@ -39,9 +40,9 @@ public class DDSpanContext implements io.opentracing.SpanContext {
3940
private final Map<String, String> baggageItems;
4041

4142
// Not Shared with other span contexts
42-
private final String traceId;
43-
private final String spanId;
44-
private final String parentId;
43+
private final BigInteger traceId;
44+
private final BigInteger spanId;
45+
private final BigInteger parentId;
4546

4647
/** Tags are associated to the current span, they will not propagate to the children span */
4748
private final Map<String, Object> tags = new ConcurrentHashMap<>();
@@ -73,9 +74,9 @@ public class DDSpanContext implements io.opentracing.SpanContext {
7374
private final long threadId = Thread.currentThread().getId();
7475

7576
public DDSpanContext(
76-
final String traceId,
77-
final String spanId,
78-
final String parentId,
77+
final BigInteger traceId,
78+
final BigInteger spanId,
79+
final BigInteger parentId,
7980
final String serviceName,
8081
final String operationName,
8182
final String resourceName,
@@ -128,26 +129,26 @@ public DDSpanContext(
128129
this.tags.put(DDTags.THREAD_ID, threadId);
129130
}
130131

131-
public String getTraceId() {
132+
public BigInteger getTraceId() {
132133
return traceId;
133134
}
134135

135136
@Override
136137
public String toTraceId() {
137-
return traceId;
138+
return traceId.toString();
138139
}
139140

140-
public String getParentId() {
141+
public BigInteger getParentId() {
141142
return parentId;
142143
}
143144

144-
public String getSpanId() {
145+
public BigInteger getSpanId() {
145146
return spanId;
146147
}
147148

148149
@Override
149150
public String toSpanId() {
150-
return spanId;
151+
return spanId.toString();
151152
}
152153

153154
public String getServiceName() {

‎dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java‎

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import io.opentracing.tag.Tag;
2929
import java.io.Closeable;
3030
import java.lang.ref.WeakReference;
31+
import java.math.BigInteger;
3132
import java.util.ArrayList;
3233
import java.util.Collection;
3334
import java.util.Collections;
@@ -48,6 +49,10 @@
4849
/** DDTracer makes it easy to send traces and span to DD using the OpenTracing API. */
4950
@Slf4j
5051
public class DDTracer implements io.opentracing.Tracer, Closeable, datadog.trace.api.Tracer {
52+
// UINT64 max value
53+
public static final BigInteger TRACE_ID_MAX =
54+
BigInteger.valueOf(2).pow(64).subtract(BigInteger.ONE);
55+
public static final BigInteger TRACE_ID_MIN = BigInteger.ZERO;
5156

5257
/** Default service name if none provided on the trace or span */
5358
final String serviceName;
@@ -400,7 +405,7 @@ void incrementTraceCount() {
400405
public String getTraceId() {
401406
final Span activeSpan = activeSpan();
402407
if (activeSpan instanceof DDSpan) {
403-
return ((DDSpan) activeSpan).getTraceId();
408+
return ((DDSpan) activeSpan).getTraceId().toString();
404409
}
405410
return "0";
406411
}
@@ -409,7 +414,7 @@ public String getTraceId() {
409414
public String getSpanId() {
410415
final Span activeSpan = activeSpan();
411416
if (activeSpan instanceof DDSpan) {
412-
return ((DDSpan) activeSpan).getSpanId();
417+
return ((DDSpan) activeSpan).getSpanId().toString();
413418
}
414419
return "0";
415420
}
@@ -604,10 +609,15 @@ private DDSpanBuilder withTag(final String tag, final Object value) {
604609
return this;
605610
}
606611

607-
private String generateNewId() {
608-
// TODO: expand the range of numbers generated to be from 1 to uint 64 MAX
609-
// Ensure the generated ID is in a valid range:
610-
return String.valueOf(ThreadLocalRandom.current().nextLong(1, Long.MAX_VALUE));
612+
private BigInteger generateNewId() {
613+
// It is **extremely** unlikely to generate the value "0" but we still need to handle that
614+
// case
615+
BigInteger value;
616+
do {
617+
value = new BigInteger(64, ThreadLocalRandom.current());
618+
} while (value.signum() == 0);
619+
620+
return value;
611621
}
612622

613623
/**
@@ -617,9 +627,9 @@ private String generateNewId() {
617627
* @return the context
618628
*/
619629
private DDSpanContext buildSpanContext() {
620-
final String traceId;
621-
final String spanId = generateNewId();
622-
final String parentSpanId;
630+
final BigInteger traceId;
631+
final BigInteger spanId = generateNewId();
632+
final BigInteger parentSpanId;
623633
final Map<String, String> baggage;
624634
final PendingTrace parentTrace;
625635
final int samplingPriority;
@@ -661,7 +671,7 @@ private DDSpanContext buildSpanContext() {
661671
} else {
662672
// Start a new trace
663673
traceId = generateNewId();
664-
parentSpanId = "0";
674+
parentSpanId = BigInteger.ZERO;
665675
samplingPriority = PrioritySampling.UNSET;
666676
baggage = null;
667677
}

‎dd-trace-ot/src/main/java/datadog/opentracing/PendingTrace.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import java.lang.ref.Reference;
77
import java.lang.ref.ReferenceQueue;
88
import java.lang.ref.WeakReference;
9+
import java.math.BigInteger;
910
import java.util.ArrayList;
1011
import java.util.Collections;
1112
import java.util.Iterator;
@@ -28,7 +29,7 @@ public class PendingTrace extends ConcurrentLinkedDeque<DDSpan> {
2829
private static final AtomicReference<SpanCleaner> SPAN_CLEANER = new AtomicReference<>();
2930

3031
private final DDTracer tracer;
31-
private final String traceId;
32+
private final BigInteger traceId;
3233
private final Map<String, String> serviceNameMappings;
3334

3435
// TODO: consider moving these time fields into DDTracer to ensure that traces have precise
@@ -62,7 +63,9 @@ public class PendingTrace extends ConcurrentLinkedDeque<DDSpan> {
6263
private final AtomicBoolean isWritten = new AtomicBoolean(false);
6364

6465
PendingTrace(
65-
final DDTracer tracer, final String traceId, final Map<String, String> serviceNameMappings) {
66+
final DDTracer tracer,
67+
final BigInteger traceId,
68+
final Map<String, String> serviceNameMappings) {
6669
this.tracer = tracer;
6770
this.traceId = traceId;
6871
this.serviceNameMappings = serviceNameMappings;

‎dd-trace-ot/src/main/java/datadog/opentracing/propagation/B3HttpCodec.java‎

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
package datadog.opentracing.propagation;
22

3-
import static datadog.opentracing.propagation.HttpCodec.ZERO;
43
import static datadog.opentracing.propagation.HttpCodec.validateUInt64BitsID;
54

65
import datadog.opentracing.DDSpanContext;
@@ -40,12 +39,8 @@ public static class Injector implements HttpCodec.Injector {
4039
@Override
4140
public void inject(final DDSpanContext context, final TextMapInject carrier) {
4241
try {
43-
// TODO: should we better store ids as BigInteger in context to avoid parsing it twice.
44-
final BigInteger traceId = new BigInteger(context.getTraceId());
45-
final BigInteger spanId = new BigInteger(context.getSpanId());
46-
47-
carrier.put(TRACE_ID_KEY, traceId.toString(HEX_RADIX).toLowerCase());
48-
carrier.put(SPAN_ID_KEY, spanId.toString(HEX_RADIX).toLowerCase());
42+
carrier.put(TRACE_ID_KEY, context.getTraceId().toString(HEX_RADIX).toLowerCase());
43+
carrier.put(SPAN_ID_KEY, context.getSpanId().toString(HEX_RADIX).toLowerCase());
4944

5045
if (context.lockSamplingPriority()) {
5146
carrier.put(
@@ -78,8 +73,8 @@ public Extractor(final Map<String, String> taggedHeaders) {
7873
public SpanContext extract(final TextMapExtract carrier) {
7974
try {
8075
Map<String, String> tags = Collections.emptyMap();
81-
String traceId = ZERO;
82-
String spanId = ZERO;
76+
BigInteger traceId = BigInteger.ZERO;
77+
BigInteger spanId = BigInteger.ZERO;
8378
int samplingPriority = PrioritySampling.UNSET;
8479

8580
for (final Map.Entry<String, String> entry : carrier) {
@@ -95,7 +90,7 @@ public SpanContext extract(final TextMapExtract carrier) {
9590
final int length = value.length();
9691
if (length > 32) {
9792
log.debug("Header {} exceeded max length of 32: {}", TRACE_ID_KEY, value);
98-
traceId = "0";
93+
traceId = BigInteger.ZERO;
9994
continue;
10095
} else if (length > 16) {
10196
trimmedValue = value.substring(length - 16);
@@ -117,7 +112,7 @@ public SpanContext extract(final TextMapExtract carrier) {
117112
}
118113
}
119114

120-
if (!ZERO.equals(traceId)) {
115+
if (!BigInteger.ZERO.equals(traceId)) {
121116
final ExtractedContext context =
122117
new ExtractedContext(
123118
traceId,

0 commit comments

Comments
 (0)