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

This closes #256 by better handling baggage null values #308

Merged
merged 3 commits into from
Feb 1, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion jaeger-core/src/main/java/com/uber/jaeger/Span.java
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,8 @@ public List<LogData> getLogs() {

@Override
public Span setBaggageItem(String key, String value) {
if (key == null || value == null) {
if (key == null || (value == null && context.getBaggageItem(key) == null)) {
//Ignore attempts to add new baggage items with null values, they're not accessible anyway
return this;
}
synchronized (this) {
Expand Down
6 changes: 5 additions & 1 deletion jaeger-core/src/main/java/com/uber/jaeger/SpanContext.java
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,11 @@ public static SpanContext contextFromString(String value)

public SpanContext withBaggageItem(String key, String val) {
Map<String, String> newBaggage = new HashMap<String, String>(this.baggage);
newBaggage.put(key, val);
if (val == null) {
newBaggage.remove(key);
} else {
newBaggage.put(key, val);
}
return new SpanContext(traceId, spanId, parentId, flags, newBaggage, debugId);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public SpanContext setBaggage(Span span, String key, String value) {
logFields(span, key, value, prevItem, truncated, restriction.isKeyAllowed());
return span.context();
}
if (value.length() > restriction.getMaxValueLength()) {
if (value != null && value.length() > restriction.getMaxValueLength()) {
truncated = true;
value = value.substring(0, restriction.getMaxValueLength());
metrics.baggageTruncate.inc(1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,32 @@ public void testUnsampledSpan() {
assertNull(span.getLogs());
}

@Test
public void testBaggageNullValueTolerated() {
when(mgr.getRestriction(SERVICE, KEY)).thenReturn(Restriction.of(true, 5));
final String value = null;
SpanContext ctx = setter.setBaggage(span, KEY, value);

assertBaggageLogs(span, KEY, null, false, false, false);
assertNull(ctx.getBaggageItem(KEY));
}

@Test
public void testBaggageNullRemoveValue() {
when(mgr.getRestriction(SERVICE, KEY)).thenReturn(Restriction.of(true, 5));
final String value = "value";
Span originalSpan = span.setBaggageItem(KEY, value);
assertEquals(value, originalSpan.getBaggageItem(KEY));
Span child = (Span) tracer.buildSpan("some-operation").asChildOf(originalSpan).startManual();
child = child.setBaggageItem(KEY, null);

assertBaggageLogs(child, KEY, null, false, true, false);
assertNull(child.getBaggageItem(KEY));

assertEquals(
2L, metricsReporter.counters.get("jaeger.baggage-update.result=ok").longValue());
}

private void assertBaggageLogs(
Span span,
String key,
Expand Down