Skip to content
This repository was archived by the owner on Jul 1, 2022. It is now read-only.

Commit 3bb0b24

Browse files
amirhadadiyurishkuro
amirhadadi
and
yurishkuro
authored
Optimize TextMapCodec::contextFromString (#799)
* Optimize TextMapCodec::contextFromString Replace BigInteger for converting hex strings to longs with a dedicated method based on HexCodec::hexToUnsignedLong. Also change NumberFormatException thrown by BigInteger to MalformedTracerStateStringException on empty hex strings and illegal digits. Signed-off-by: amirhadadi <amirhadadi@hotmail.com> * Optimize TextMapCodec::contextAsString Avoid formatting the span ID as hex string, as JaegerSpanContext already contains the formatted span ID. Signed-off-by: amirhadadi <amirhadadi@hotmail.com> * Optimize TextMapCodec::contextAsString Checkstyle fix Signed-off-by: amirhadadi <amirhadadi@hotmail.com> * Optimize TextMapCodec::contextAsString Style changes Signed-off-by: amirhadadi <amirhadadi@hotmail.com> * Optimize TextMapCodec::contextAsString Cover different number of colons and empty elements between colons. Signed-off-by: amirhadadi <amirhadadi@hotmail.com> * Optimize TextMapCodec::contextAsString Cosmetics Signed-off-by: amirhadadi <amirhadadi@hotmail.com> * Optimize TextMapCodec::contextAsString Add a comment about replacing hexToUnsignedLong with Long.parseUnsignedLong in Java >= 9. Signed-off-by: amirhadadi <amirhadadi@hotmail.com> * Optimize TextMapCodec::contextAsString Avoid lower casing all header names, use case-insensitive comparison instead. Signed-off-by: amirhadadi <amirhadadi@hotmail.com> Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
1 parent c09fa4c commit 3bb0b24

File tree

2 files changed

+108
-49
lines changed

2 files changed

+108
-49
lines changed

‎jaeger-core/src/main/java/io/jaegertracing/internal/propagation/TextMapCodec.java‎

Lines changed: 63 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
import io.opentracing.propagation.TextMap;
2727

2828
import java.io.UnsupportedEncodingException;
29-
import java.math.BigInteger;
3029
import java.net.URLDecoder;
3130
import java.net.URLEncoder;
3231
import java.util.HashMap;
@@ -73,41 +72,69 @@ private TextMapCodec(Builder builder) {
7372

7473
static JaegerSpanContext contextFromString(String value)
7574
throws MalformedTracerStateStringException, EmptyTracerStateStringException {
76-
if (value == null || value.equals("")) {
75+
if (value == null || value.isEmpty()) {
7776
throw new EmptyTracerStateStringException();
7877
}
7978

80-
String[] parts = value.split(":");
81-
if (parts.length != 4) {
79+
final int traceIdEnd = value.indexOf(':');
80+
if (traceIdEnd == -1) {
8281
throw new MalformedTracerStateStringException(value);
8382
}
8483

85-
String traceId = parts[0];
86-
if (traceId.length() > 32 || traceId.length() < 1) {
87-
throw new TraceIdOutOfBoundException("Trace id [" + traceId + "] length is not withing 1 and 32");
84+
if (traceIdEnd > 32 || traceIdEnd == 0) {
85+
throw new TraceIdOutOfBoundException(
86+
"Trace id [" + value.substring(0, traceIdEnd) + "] length is not within 1 and 32");
8887
}
8988

90-
// TODO(isaachier): When we drop Java 1.6 support, use Long.parseUnsignedLong instead of using BigInteger.
89+
final int lowTraceIdStart = Math.max(0, traceIdEnd - 16);
90+
final long high = lowTraceIdStart == 0 ? 0 : hexToUnsignedLong("trace ID high part", value, 0, lowTraceIdStart);
91+
final long low = hexToUnsignedLong("trace ID low part", value, lowTraceIdStart, traceIdEnd);
92+
93+
final int spanIdEnd = value.indexOf(':', traceIdEnd + 1);
94+
if (spanIdEnd == -1) {
95+
throw new MalformedTracerStateStringException(value);
96+
}
97+
98+
final long spanId = hexToUnsignedLong("span ID", value, traceIdEnd + 1, spanIdEnd);
99+
100+
final int parentIdEnd = value.indexOf(':', spanIdEnd + 1);
101+
if (parentIdEnd == -1) {
102+
throw new MalformedTracerStateStringException(value);
103+
}
104+
105+
final long parentId = hexToUnsignedLong("parent ID", value, spanIdEnd + 1, parentIdEnd);
106+
107+
final byte flags = (byte)hexToUnsignedLong("flags", value, parentIdEnd + 1, value.length());
108+
91109
return new JaegerSpanContext(
92-
high(traceId),
93-
new BigInteger(traceId, 16).longValue(),
94-
new BigInteger(parts[1], 16).longValue(),
95-
new BigInteger(parts[2], 16).longValue(),
96-
new BigInteger(parts[3], 16).byteValue());
110+
high,
111+
low,
112+
spanId,
113+
parentId,
114+
flags);
97115
}
98116

99-
/**
100-
* Parses a full (low + high) traceId, trimming the lower 64 bits.
101-
* @param hexString a full traceId
102-
* @return the long value of the higher 64 bits for a 128 bit traceId or 0 for 64 bit traceIds
103-
*/
104-
private static long high(String hexString) {
105-
if (hexString.length() > 16) {
106-
int highLength = hexString.length() - 16;
107-
String highString = hexString.substring(0, highLength);
108-
return new BigInteger(highString, 16).longValue();
117+
// TODO(amirhadadi):
118+
// When supporting Java >= 9 use Long.parseUnsignedLong(CharSequence s, int beginIndex, int endIndex, int radix)
119+
// which allows avoiding creating a String.
120+
private static long hexToUnsignedLong(String label, String value, int beginIndex, int endIndex) {
121+
if (beginIndex >= endIndex) {
122+
throw new MalformedTracerStateStringException("Empty " + label + " in context string " + value);
109123
}
110-
return 0L;
124+
long result = 0;
125+
for (; beginIndex < endIndex; beginIndex++) {
126+
char c = value.charAt(beginIndex);
127+
result <<= 4;
128+
if (c >= '0' && c <= '9') {
129+
result |= c - '0';
130+
} else if (c >= 'a' && c <= 'f') {
131+
result |= c - 'a' + 10;
132+
} else {
133+
throw new MalformedTracerStateStringException("Failed to parse " + label + " in context string " + value
134+
+ ", '" + c + "' is not a legal hex character expecting only 0-9 and a-f");
135+
}
136+
}
137+
return result;
111138
}
112139

113140
/**
@@ -116,14 +143,11 @@ private static long high(String hexString) {
116143
* @return Encoded string representing span context.
117144
*/
118145
public static String contextAsString(JaegerSpanContext context) {
119-
int intFlag = context.getFlags() & 0xFF;
120-
return new StringBuilder()
121-
.append(context.getTraceId()).append(":")
122-
.append(Utils.to16HexString(context.getSpanId())).append(":")
123-
// parent=0 is special, no need to encode as full 16 characters, and more readable this way
124-
.append(context.getParentId() == 0 ? "0" : Utils.to16HexString(context.getParentId())).append(":")
125-
.append(Integer.toHexString(intFlag))
126-
.toString();
146+
return context.getTraceId() + ":"
147+
+ context.toSpanId() + ":"
148+
// parent=0 is special, no need to encode as full 16 characters, and more readable this way
149+
+ (context.getParentId() == 0 ? "0" : Utils.to16HexString(context.getParentId())) + ":"
150+
+ Integer.toHexString(context.getFlags() & 0xFF);
127151
}
128152

129153
@Override
@@ -140,18 +164,17 @@ public JaegerSpanContext extract(TextMap carrier) {
140164
Map<String, String> baggage = null;
141165
String debugId = null;
142166
for (Map.Entry<String, String> entry : carrier) {
143-
// TODO there should be no lower-case here
144-
String key = entry.getKey().toLowerCase(Locale.ROOT);
145-
if (key.equals(contextKey)) {
167+
String key = entry.getKey();
168+
if (key.equalsIgnoreCase(contextKey)) {
146169
context = contextFromString(decodedValue(entry.getValue()));
147-
} else if (key.equals(Constants.DEBUG_ID_HEADER_KEY)) {
170+
} else if (key.equalsIgnoreCase(Constants.DEBUG_ID_HEADER_KEY)) {
148171
debugId = decodedValue(entry.getValue());
149-
} else if (key.startsWith(baggagePrefix)) {
172+
} else if (key.regionMatches(true, 0, baggagePrefix, 0, baggagePrefix.length())) {
150173
if (baggage == null) {
151-
baggage = new HashMap<String, String>();
174+
baggage = new HashMap<>();
152175
}
153-
baggage.put(keys.unprefixedKey(key, baggagePrefix), decodedValue(entry.getValue()));
154-
} else if (key.equals(Constants.BAGGAGE_HEADER_KEY)) {
176+
baggage.put(keys.unprefixedKey(key.toLowerCase(Locale.ROOT), baggagePrefix), decodedValue(entry.getValue()));
177+
} else if (key.equalsIgnoreCase(Constants.BAGGAGE_HEADER_KEY)) {
155178
baggage = parseBaggageHeader(decodedValue(entry.getValue()), baggage);
156179
}
157180
}

‎jaeger-core/src/test/java/io/jaegertracing/internal/propagation/TextMapCodecTest.java‎

Lines changed: 45 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
import static org.junit.Assert.assertEquals;
1818
import static org.junit.Assert.assertNotNull;
19+
import static org.junit.Assert.assertNull;
1920
import static org.junit.Assert.assertTrue;
2021

2122
import io.jaegertracing.internal.JaegerSpanContext;
@@ -55,12 +56,47 @@ public void testWithoutBuilder() {
5556
}
5657

5758
@Test(expected = MalformedTracerStateStringException.class)
58-
public void testContextFromStringMalformedException() throws Exception {
59+
public void testContextFromStringNoColons() {
60+
TextMapCodec.contextFromString("ff");
61+
}
62+
63+
@Test(expected = MalformedTracerStateStringException.class)
64+
public void testContextFromStringOnly1Colon() {
65+
TextMapCodec.contextFromString("ff:ff");
66+
}
67+
68+
@Test(expected = MalformedTracerStateStringException.class)
69+
public void testContextFromStringOnly2Colons() {
5970
TextMapCodec.contextFromString("ff:ff:ff");
6071
}
6172

73+
@Test(expected = MalformedTracerStateStringException.class)
74+
public void testContextFromStringTooManyColons() {
75+
TextMapCodec.contextFromString("ff:ff:ff:ff:ff");
76+
}
77+
78+
@Test(expected = MalformedTracerStateStringException.class)
79+
public void testContextFromStringEmptySpanIdException() {
80+
TextMapCodec.contextFromString("ff::ff:ff");
81+
}
82+
83+
@Test(expected = MalformedTracerStateStringException.class)
84+
public void testContextFromStringEmptyParentIdException() {
85+
TextMapCodec.contextFromString("ff:ff::ff");
86+
}
87+
88+
@Test(expected = MalformedTracerStateStringException.class)
89+
public void testContextFromStringEmptyFlagsException() {
90+
TextMapCodec.contextFromString("ff:ff:ff:");
91+
}
92+
93+
@Test(expected = MalformedTracerStateStringException.class)
94+
public void testContextFromStringNonHexDigitException() {
95+
TextMapCodec.contextFromString("ff:ff:fg:ff");
96+
}
97+
6298
@Test(expected = EmptyTracerStateStringException.class)
63-
public void testContextFromStringEmptyException() throws Exception {
99+
public void testContextFromStringEmptyException() {
64100
TextMapCodec.contextFromString("");
65101
}
66102

@@ -75,7 +111,7 @@ public void testContextFromStringTraceIdOutOfBoundExceptionToSmall() {
75111
}
76112

77113
@Test
78-
public void testContextFromString() throws Exception {
114+
public void testContextFromString() {
79115
JaegerSpanContext context = TextMapCodec.contextFromString("ff:dd:cc:4");
80116
assertEquals(context.getTraceIdLow(), 255L);
81117
assertEquals(context.getTraceIdHigh(), 0L);
@@ -132,7 +168,7 @@ public void testContextAsStringFormatsPositiveFields() {
132168
@Test
133169
public void testAdhocBaggageWithTraceId() {
134170
TextMapCodec codec = new TextMapCodec(false);
135-
Map<String, String> headers = new HashMap<String, String>();
171+
Map<String, String> headers = new HashMap<>();
136172
long traceIdLow = 42;
137173
long spanId = 1;
138174
long parentId = 0;
@@ -151,19 +187,19 @@ public void testAdhocBaggageWithTraceId() {
151187
*/
152188
@Test
153189
public void testAdhocBaggageWithoutTraceId() {
154-
Map<String, String> headers = new HashMap<String, String>();
190+
Map<String, String> headers = new HashMap<>();
155191
headers.put("jaeger-baggage", "k1=v1, k2 = v2, k3=v3=d3");
156192
TextMapCodec codec = new TextMapCodec(false);
157193
JaegerSpanContext context = codec.extract(new TextMapAdapter(headers));
158194
assertEquals("v1", context.getBaggageItem("k1"));
159195
assertEquals("v2", context.getBaggageItem("k2"));
160-
assertEquals(null, context.getBaggageItem("k3"));
196+
assertNull(context.getBaggageItem("k3"));
161197
}
162198

163199
@Test
164200
public void testInjectDoNotEncodeSpanContext() {
165201
TextMapCodec codec = new TextMapCodec(true);
166-
Map<String, String> headers = new HashMap<String, String>();
202+
Map<String, String> headers = new HashMap<>();
167203
long traceIdLow = 42;
168204
long spanId = 1;
169205
long parentId = 0;
@@ -175,7 +211,7 @@ public void testInjectDoNotEncodeSpanContext() {
175211

176212
@Test
177213
public void testExtractSupportNonEncodedSpanContext() {
178-
Map<String, String> headers = new HashMap<String, String>();
214+
Map<String, String> headers = new HashMap<>();
179215
headers.put("uber-trace-id", "2a:1:0:1");
180216

181217
TextMapCodec codec = new TextMapCodec(true);
@@ -189,7 +225,7 @@ public void testExtractSupportNonEncodedSpanContext() {
189225

190226
@Test
191227
public void testExtractSupportEncodedSpanContext() {
192-
Map<String, String> headers = new HashMap<String, String>();
228+
Map<String, String> headers = new HashMap<>();
193229
headers.put("uber-trace-id", "2a%3A1%3A0%3A1");
194230

195231
TextMapCodec codec = new TextMapCodec(true);

0 commit comments

Comments
 (0)