Skip to content

Commit 2ee01ac

Browse files
committed
Backport some performance work for tracing
1 parent 56b16a5 commit 2ee01ac

File tree

3 files changed

+64
-24
lines changed

3 files changed

+64
-24
lines changed

main/api/src/main/java/org/elasticsoftware/elasticactors/tracing/TraceContext.java

Lines changed: 47 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -26,31 +26,69 @@ public TraceContext(
2626
@Nonnull String traceId,
2727
@Nullable String parentId,
2828
@Nullable Map<String, String> baggage)
29+
{
30+
this(spanId, traceId, parentId, baggage, false);
31+
}
32+
33+
/**
34+
* Constructor that allows the user to inform if the provided baggage map is immutable,
35+
* preventing unnecessary copies if so.
36+
*
37+
* This is provided for optimization purposes.
38+
* Passing a map that is not mutable here and changing it afterwards may
39+
* (and very likely will) corrupt the thread's MDC.
40+
*/
41+
public TraceContext(
42+
@Nonnull String spanId,
43+
@Nonnull String traceId,
44+
@Nullable String parentId,
45+
@Nullable Map<String, String> baggage,
46+
boolean immutableBaggageMap)
2947
{
3048
this.spanId = Objects.requireNonNull(spanId);
3149
this.traceId = Objects.requireNonNull(traceId);
3250
this.parentId = parentId;
33-
this.baggage = baggage;
51+
this.baggage = !immutableBaggageMap && baggage != null
52+
? Collections.unmodifiableMap(new HashMap<>(baggage))
53+
: baggage;
3454
}
3555

3656
public TraceContext(@Nullable TraceContext parent) {
37-
this(parent, parent == null ? null : parent.baggage);
57+
this(parent, parent == null ? null : parent.baggage, true);
3858
}
3959

4060
public TraceContext(@Nullable Map<String, String> baggage) {
41-
this(null, baggage);
61+
this(null, baggage, false);
62+
}
63+
64+
public TraceContext(@Nullable TraceContext parent, @Nullable Map<String, String> baggage) {
65+
this(parent, baggage, false);
4266
}
4367

44-
private TraceContext(@Nullable TraceContext parent, @Nullable Map<String, String> baggage) {
68+
/**
69+
* Constructor that allows the user to inform if the provided baggage map is immutable,
70+
* preventing unnecessary copies if so.
71+
*
72+
* This is provided for optimization purposes.
73+
* Passing a map that is not mutable here and changing it afterwards may
74+
* (and very likely will) corrupt the thread's MDC.
75+
*/
76+
public TraceContext(
77+
@Nullable TraceContext parent,
78+
@Nullable Map<String, String> baggage,
79+
boolean immutableBaggageMap)
80+
{
4581
Random prng = ThreadLocalRandom.current();
4682
this.spanId = toHexString(prng.nextLong());
4783
this.traceId = parent == null || parent.getTraceId().isEmpty()
48-
? nextTraceIdHigh(prng) + this.spanId
49-
: parent.getTraceId();
84+
? nextTraceIdHigh(prng) + this.spanId
85+
: parent.getTraceId();
5086
this.parentId = parent == null || parent.getSpanId().isEmpty()
51-
? null
52-
: parent.getSpanId();
53-
this.baggage = baggage == null ? null : Collections.unmodifiableMap(new HashMap<>(baggage));
87+
? null
88+
: parent.getSpanId();
89+
this.baggage = !immutableBaggageMap && baggage != null
90+
? Collections.unmodifiableMap(new HashMap<>(baggage))
91+
: baggage;
5492
}
5593

5694
/**

main/messaging/src/main/java/org/elasticsoftware/elasticactors/serialization/internal/tracing/TraceContextDeserializer.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@ public static TraceContext deserialize(Messaging.TraceContext traceContext) {
1818
traceContext.hasSpanId() ? traceContext.getSpanId() : "",
1919
traceContext.hasTraceId() ? traceContext.getTraceId() : "",
2020
traceContext.hasParentId() ? traceContext.getParentId() : null,
21-
baggage != null && !baggage.isEmpty() ? baggage : null
21+
baggage.isEmpty() ? null : baggage,
22+
true
2223
);
2324
return deserialized.isEmpty() ? null : deserialized;
2425
}

main/tracing/src/main/java/org/elasticsoftware/elasticactors/tracing/service/TracingMessagingContextManager.java

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -326,31 +326,32 @@ private static void updateBaggage(TraceContext current, TraceContext next) {
326326
try {
327327
Map<String, String> oldBaggage = current != null ? current.getBaggage() : null;
328328
Map<String, String> nextBaggage = next != null ? next.getBaggage() : null;
329-
if (oldBaggage != null) {
330-
for (Map.Entry<String, String> oldEntry : oldBaggage.entrySet()) {
331-
String key = oldEntry.getKey();
332-
String value = oldEntry.getValue();
333-
String nextValue = nextBaggage != null ? nextBaggage.get(key) : null;
329+
if (oldBaggage == nextBaggage) {
330+
return;
331+
}
332+
if (oldBaggage == null) {
333+
nextBaggage.forEach(TracingMessagingContextManager::putOnMDC);
334+
} else if (nextBaggage == null) {
335+
oldBaggage.keySet().forEach(key -> putOnMDC(key, null));
336+
} else {
337+
oldBaggage.forEach((key, value) -> {
338+
String nextValue = nextBaggage.get(key);
334339
// Will set the new value if present, and remove it if absent
335340
if (!Objects.equals(value, nextValue)) {
336341
putOnMDC(key, nextValue);
337342
}
338-
}
339-
}
340-
if (nextBaggage != null) {
341-
for (Map.Entry<String, String> nextEntry : nextBaggage.entrySet()) {
342-
String key = nextEntry.getKey();
343-
String value = nextEntry.getValue();
344-
String oldValue = oldBaggage != null ? oldBaggage.get(key) : null;
343+
});
344+
nextBaggage.forEach((key, value) -> {
345+
String oldValue = oldBaggage.get(key);
345346
// Would have been processed when processing oldBaggage
346347
if (oldValue != null) {
347-
continue;
348+
return;
348349
}
349350
// Only insert if the old value was null and the new one isn't
350351
if (value != null) {
351352
putOnMDC(key, value);
352353
}
353-
}
354+
});
354355
}
355356
} catch (Exception e) {
356357
logger.error(

0 commit comments

Comments
 (0)