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

Commit

Permalink
revert some of the previous changes that added the key directly to th…
Browse files Browse the repository at this point in the history
…e SpanSetter since this wont be able to scale with the number of random baggage people can set
  • Loading branch information
black-adder committed Jul 28, 2017
1 parent 4d5d98b commit 495a656
Show file tree
Hide file tree
Showing 8 changed files with 25 additions and 33 deletions.
2 changes: 1 addition & 1 deletion jaeger-core/src/main/java/com/uber/jaeger/Span.java
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ public Span setBaggageItem(String key, String value) {
}
BaggageSetter setter = this.getTracer().getBaggageRestrictionManager().getBaggageSetter(key);
synchronized (this) {
this.context = setter.setBaggage(this, value);
this.context = setter.setBaggage(this, key, value);
return this;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
* given the baggage restrictions for that key.
*/
public interface BaggageRestrictionManager {
static final int DEFAULT_MAX_VALUE_LENGTH = 2048;
int DEFAULT_MAX_VALUE_LENGTH = 2048;

public abstract BaggageSetter getBaggageSetter(String key);
BaggageSetter getBaggageSetter(String key);
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
@Value(staticConstructor = "of")
public class BaggageSetter {

private final String key;
/**
* This flag represents whether the key is a valid baggage key. If valid
* the baggage key:value will be written to the span.
Expand All @@ -60,12 +59,12 @@ public class BaggageSetter {
* @param value the baggage value to set
* @return the SpanContext with the baggage set
*/
public SpanContext setBaggage(Span span, String value) {
public SpanContext setBaggage(Span span, String key, String value) {
boolean truncated = false;
String prevItem = span.getBaggageItem(key);
if (!valid) {
metrics.baggageUpdateFailure.inc(1);
logFields(span, value, prevItem, truncated);
logFields(span, key, value, prevItem, truncated);
return span.context();
}
if (value.length() > maxValueLength) {
Expand All @@ -74,12 +73,12 @@ public SpanContext setBaggage(Span span, String value) {
metrics.baggageTruncate.inc(1);
}

logFields(span, value, prevItem, truncated);
logFields(span, key, value, prevItem, truncated);
metrics.baggageUpdateSuccess.inc(1);
return span.context().withBaggageItem(key, value);
}

private void logFields(Span span, String value, String prevItem, boolean truncated) {
private void logFields(Span span, String key, String value, String prevItem, boolean truncated) {
if (!span.context().isSampled()) {
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,31 +24,23 @@

import com.uber.jaeger.metrics.Metrics;

import java.util.HashMap;
import java.util.Map;

/**
* DefaultBaggageRestrictionManager is a manager that returns a {@link BaggageSetter}
* that allows all baggage.
*/
public class DefaultBaggageRestrictionManager implements BaggageRestrictionManager {
private Map<String, BaggageSetter> baggageSetters;
private final int maxValueLength;
private final Metrics metrics;
private BaggageSetter setter;

public DefaultBaggageRestrictionManager(Metrics metrics) {
this(metrics, DEFAULT_MAX_VALUE_LENGTH);
}

public DefaultBaggageRestrictionManager(Metrics metrics, int maxValueLength) {
this.maxValueLength = maxValueLength;
this.metrics = metrics;
this.baggageSetters = new HashMap<String, BaggageSetter>();
DefaultBaggageRestrictionManager(Metrics metrics, int maxValueLength) {
this.setter = BaggageSetter.of(true, maxValueLength, metrics);
}

@Override
public BaggageSetter getBaggageSetter(String key) {
BaggageSetter setter = baggageSetters.get(key);
if (setter == null) {
setter = BaggageSetter.of(key, true, maxValueLength, metrics);
baggageSetters.put(key, setter);
}
return setter;
}
}
2 changes: 1 addition & 1 deletion jaeger-core/src/test/java/com/uber/jaeger/SpanTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ public void testSetAndGetBaggageItem() {

final String key = "key";
final String value = "value";
when(mgr.getBaggageSetter(key)).thenReturn(BaggageSetter.of(key, true, 10, metrics));
when(mgr.getBaggageSetter(key)).thenReturn(BaggageSetter.of(true, 10, metrics));
span.setBaggageItem(key, "value");
verify(mgr).getBaggageSetter(key);
assertEquals(value, span.getBaggageItem(key));
Expand Down
3 changes: 2 additions & 1 deletion jaeger-core/src/test/java/com/uber/jaeger/TracerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,8 @@ public void testWithBaggageRestrictionManager() {
.withMetrics(metrics)
.build();
Span span = (Span) tracer.buildSpan("some-operation").startManual();
tracer.getBaggageRestrictionManager().getBaggageSetter("key").setBaggage(span, "value");
final String key = "key";
tracer.getBaggageRestrictionManager().getBaggageSetter(key).setBaggage(span, key, "value");

assertEquals(
1L, metricsReporter.counters.get("jaeger.baggage-update.result=ok").longValue());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public void testGetBaggageSetter() {

final String key = "key";
BaggageSetter actual = undertest.getBaggageSetter(key);
BaggageSetter expected = BaggageSetter.of(key, true, 2048, nullMetrics);
BaggageSetter expected = BaggageSetter.of(true, 2048, nullMetrics);
assertEquals(expected, actual);

expected = actual;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,9 @@ public void setUp() throws Exception {

@Test
public void testInvalidBaggage() {
BaggageSetter setter = BaggageSetter.of(KEY, false, 0, metrics);
BaggageSetter setter = BaggageSetter.of(false, 0, metrics);
final String value = "value";
SpanContext ctx = setter.setBaggage(span, value);
SpanContext ctx = setter.setBaggage(span, KEY, value);

assertBaggageLogs(span, KEY, value, false, false, true);
assertNull(ctx.getBaggageItem(KEY));
Expand All @@ -79,10 +79,10 @@ public void testInvalidBaggage() {

@Test
public void testTruncatedBaggage() {
BaggageSetter setter = BaggageSetter.of(KEY, true, 5, metrics);
BaggageSetter setter = BaggageSetter.of(true, 5, metrics);
final String value = "0123456789";
final String expected = "01234";
SpanContext ctx = setter.setBaggage(span, value);
SpanContext ctx = setter.setBaggage(span, KEY, value);

assertBaggageLogs(span, KEY, expected, true, false, false);
assertEquals(expected, ctx.getBaggageItem(KEY));
Expand All @@ -95,11 +95,11 @@ public void testTruncatedBaggage() {

@Test
public void testOverrideBaggage() {
BaggageSetter setter = BaggageSetter.of(KEY, true, 5, metrics);
BaggageSetter setter = BaggageSetter.of(true, 5, metrics);
final String value = "value";
SpanContext ctx = setter.setBaggage(span, value);
SpanContext ctx = setter.setBaggage(span, KEY, value);
Span child = (Span) tracer.buildSpan("some-operation").asChildOf(ctx).startManual();
ctx = setter.setBaggage(child, value);
ctx = setter.setBaggage(child, KEY, value);

assertBaggageLogs(child, KEY, value, false, true, false);
assertEquals(value, ctx.getBaggageItem(KEY));
Expand Down

0 comments on commit 495a656

Please sign in to comment.