new API

jaeger-core/src/main/java/com/uber/jaeger/Span.java

@@ -120,9 +120,8 @@ public Span setBaggageItem(String key, String value) {
     if (key == null || value == null) {
       return this;
     }
-    BaggageSetter setter = this.getTracer().getBaggageRestrictionManager().getBaggageSetter(key);
     synchronized (this) {
-      this.context = setter.setBaggage(this, key, value);
+      this.context = getTracer().setBaggage(this, key, value);
       return this;
     }
   }

jaeger-core/src/main/java/com/uber/jaeger/Tracer.java

@@ -23,6 +23,7 @@
 package com.uber.jaeger;
 
 import com.uber.jaeger.baggage.BaggageRestrictionManager;
+import com.uber.jaeger.baggage.BaggageSetter;
 import com.uber.jaeger.baggage.DefaultBaggageRestrictionManager;
 import com.uber.jaeger.exceptions.UnsupportedFormatException;
 import com.uber.jaeger.metrics.Metrics;
@@ -75,7 +76,7 @@ public class Tracer implements io.opentracing.Tracer {
   private final Map<String, ?> tags;
   private final boolean zipkinSharedRpcSpan;
   private final ActiveSpanSource activeSpanSource;
-  private final BaggageRestrictionManager baggageRestrictionManager;
+  private final BaggageSetter baggageSetter;
 
   private Tracer(
       String serviceName,
@@ -96,7 +97,7 @@ private Tracer(
     this.metrics = metrics;
     this.zipkinSharedRpcSpan = zipkinSharedRpcSpan;
     this.activeSpanSource = activeSpanSource;
-    this.baggageRestrictionManager = baggageRestrictionManager;
+    this.baggageSetter = new BaggageSetter(baggageRestrictionManager, metrics);
 
     this.version = loadVersion();
 
@@ -144,10 +145,6 @@ Reporter getReporter() {
     return reporter;
   }
 
-  BaggageRestrictionManager getBaggageRestrictionManager() {
-    return baggageRestrictionManager;
-  }
-
   void reportSpan(Span span) {
     reporter.report(span);
     metrics.spansFinished.inc(1);
@@ -456,7 +453,7 @@ public static final class Builder {
     private Map<String, Object> tags = new HashMap<String, Object>();
     private boolean zipkinSharedRpcSpan;
     private ActiveSpanSource activeSpanSource = new ThreadLocalActiveSpanSource();
-    private BaggageRestrictionManager baggageRestrictionManager;
+    private BaggageRestrictionManager baggageRestrictionManager = new DefaultBaggageRestrictionManager();
 
     public Builder(String serviceName, Reporter reporter, Sampler sampler) {
       if (serviceName == null || serviceName.trim().length() == 0) {
@@ -538,9 +535,6 @@ public Builder withBaggageRestrictionManager(BaggageRestrictionManager baggageRe
     }
 
     public Tracer build() {
-      if (baggageRestrictionManager == null) {
-        baggageRestrictionManager = new DefaultBaggageRestrictionManager(this.metrics);
-      }
       return new Tracer(this.serviceName, reporter, sampler, registry, clock, metrics, tags,
           zipkinSharedRpcSpan, activeSpanSource, baggageRestrictionManager);
     }
@@ -598,4 +592,7 @@ String getHostName() {
     }
   }
 
+  SpanContext setBaggage(Span span, String key, String value) {
+    return baggageSetter.setBaggage(span, key, value);
+  }
 }

jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageRestrictionManager.java

@@ -24,12 +24,12 @@
 
 /**
  * BaggageRestrictionManager is an interface for a class that manages baggage
- * restrictions for baggage keys. The manager will return a {@link BaggageSetter}
- * for a specific baggage key which will set the baggage on the {@link com.uber.jaeger.Span}
- * given the baggage restrictions for that key.
+ * restrictions for baggage keys. The manager will return a {@link Restriction}
+ * for a specific baggage key which will determine whether the baggage key is
+ * allowed and any other applicable restrictions on the baggage value.
  */
 public interface BaggageRestrictionManager {
   int DEFAULT_MAX_VALUE_LENGTH = 2048;
 
-  BaggageSetter getBaggageSetter(String key);
+  Restriction getRestriction(String key);
 }

jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageSetter.java

@@ -24,28 +24,26 @@
 
 import com.uber.jaeger.Span;
 import com.uber.jaeger.SpanContext;
+import com.uber.jaeger.metrics.Metric;
 import com.uber.jaeger.metrics.Metrics;
 
 import java.util.HashMap;
 import java.util.Map;
 
-import lombok.Value;
-
 /**
  * BaggageSetter is a class that sets baggage and the logs associated
  * with the baggage on a given {@link Span}.
  */
-@Value(staticConstructor = "of")
 public class BaggageSetter {
 
-  /**
-   * This flag represents whether the key is a valid baggage key. If valid
-   * the baggage key:value will be written to the span.
-   */
-  private final boolean valid;
-  private final int maxValueLength;
+  private final BaggageRestrictionManager restrictionManager;
   private final Metrics metrics;
 
+  public BaggageSetter(BaggageRestrictionManager restrictionManager, Metrics metrics) {
+    this.restrictionManager = restrictionManager;
+    this.metrics = metrics;
+  }
+
   /**
    * Sets the baggage key:value on the {@link Span} and the corresponding
    * logs. Whether the baggage is set on the span depends on if the key
@@ -60,25 +58,26 @@ public class BaggageSetter {
    * @return       the SpanContext with the baggage set
    */
   public SpanContext setBaggage(Span span, String key, String value) {
+    Restriction restriction = restrictionManager.getRestriction(key);
     boolean truncated = false;
-    String prevItem = span.getBaggageItem(key);
-    if (!valid) {
+    String prevItem = null;
+    if (!restriction.isKeyAllowed()) {
       metrics.baggageUpdateFailure.inc(1);
-      logFields(span, key, value, prevItem, truncated);
+      logFields(span, key, value, prevItem, truncated, restriction.isKeyAllowed());
       return span.context();
     }
-    if (value.length() > maxValueLength) {
+    if (value.length() > restriction.getMaxValueLength()) {
       truncated = true;
-      value = value.substring(0, maxValueLength);
+      value = value.substring(0, restriction.getMaxValueLength());
       metrics.baggageTruncate.inc(1);
     }
-
-    logFields(span, key, value, prevItem, truncated);
+    prevItem = span.getBaggageItem(key);
+    logFields(span, key, value, prevItem, truncated, restriction.isKeyAllowed());
     metrics.baggageUpdateSuccess.inc(1);
     return span.context().withBaggageItem(key, value);
   }
 
-  private void logFields(Span span, String key, String value, String prevItem, boolean truncated) {
+  private void logFields(Span span, String key, String value, String prevItem, boolean truncated, boolean valid) {
     if (!span.context().isSampled()) {
       return;
     }

jaeger-core/src/main/java/com/uber/jaeger/baggage/DefaultBaggageRestrictionManager.java

@@ -22,25 +22,23 @@
 
 package com.uber.jaeger.baggage;
 
-import com.uber.jaeger.metrics.Metrics;
-
 /**
- * DefaultBaggageRestrictionManager is a manager that returns a {@link BaggageSetter}
+ * DefaultBaggageRestrictionManager is a manager that returns a {@link Restriction}
  * that allows all baggage.
  */
 public class DefaultBaggageRestrictionManager implements BaggageRestrictionManager {
-  private BaggageSetter setter;
+  private final Restriction restriction;
 
-  public DefaultBaggageRestrictionManager(Metrics metrics) {
-    this(metrics, DEFAULT_MAX_VALUE_LENGTH);
+  public DefaultBaggageRestrictionManager() {
+    this(DEFAULT_MAX_VALUE_LENGTH);
   }
 
-  DefaultBaggageRestrictionManager(Metrics metrics, int maxValueLength) {
-    this.setter = BaggageSetter.of(true, maxValueLength, metrics);
+  DefaultBaggageRestrictionManager(int maxValueLength) {
+    this.restriction = Restriction.of(true, maxValueLength);
   }
 
   @Override
-  public BaggageSetter getBaggageSetter(String key) {
-    return setter;
+  public Restriction getRestriction(String key) {
+    return restriction;
   }
 }

jaeger-core/src/main/java/com/uber/jaeger/baggage/Restriction.java

@@ -0,0 +1,35 @@
+/*
+ * Copyright (c) 2017, Uber Technologies, Inc
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+package com.uber.jaeger.baggage;
+
+import lombok.Value;
+
+/**
+ * Restriction determines whether a baggage key is allowed and contains any
+ * restrictions on the baggage value.
+ */
+@Value(staticConstructor = "of")
+public class Restriction {
+  boolean keyAllowed;
+  int maxValueLength;
+}

jaeger-core/src/test/java/com/uber/jaeger/SpanTest.java

@@ -32,6 +32,7 @@
 import com.uber.jaeger.baggage.BaggageRestrictionManager;
 import com.uber.jaeger.baggage.BaggageSetter;
 import com.uber.jaeger.baggage.DefaultBaggageRestrictionManager;
+import com.uber.jaeger.baggage.Restriction;
 import com.uber.jaeger.metrics.InMemoryStatsReporter;
 import com.uber.jaeger.metrics.Metrics;
 import com.uber.jaeger.metrics.StatsFactoryImpl;
@@ -68,7 +69,7 @@ public void setUp() throws Exception {
         new Tracer.Builder("SamplerTest", reporter, new ConstSampler(true))
             .withStatsReporter(metricsReporter)
             .withClock(clock)
-            .withBaggageRestrictionManager(new DefaultBaggageRestrictionManager(metrics))
+            .withBaggageRestrictionManager(new DefaultBaggageRestrictionManager())
             .build();
     span = (Span) tracer.buildSpan("some-operation").startManual();
   }
@@ -95,9 +96,9 @@ public void testSetAndGetBaggageItem() {
 
     final String key = "key";
     final String value = "value";
-    when(mgr.getBaggageSetter(key)).thenReturn(BaggageSetter.of(true, 10, metrics));
+    when(mgr.getRestriction(key)).thenReturn(Restriction.of(true, 10));
     span.setBaggageItem(key, "value");
-    verify(mgr).getBaggageSetter(key);
+    verify(mgr).getRestriction(key);
     assertEquals(value, span.getBaggageItem(key));
   }
 

jaeger-core/src/test/java/com/uber/jaeger/TracerTest.java

@@ -131,7 +131,7 @@ public void testWithBaggageRestrictionManager() {
             .build();
     Span span = (Span) tracer.buildSpan("some-operation").startManual();
     final String key = "key";
-    tracer.getBaggageRestrictionManager().getBaggageSetter(key).setBaggage(span, key, "value");
+    tracer.setBaggage(span, key, "value");
 
     assertEquals(
         1L, metricsReporter.counters.get("jaeger.baggage-update.result=ok").longValue());

jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageSetterTest.java

@@ -24,6 +24,8 @@
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 import com.uber.jaeger.LogData;
 import com.uber.jaeger.Span;
@@ -48,6 +50,8 @@ public class BaggageSetterTest {
   private Span span;
   private InMemoryStatsReporter metricsReporter;
   private Metrics metrics;
+  private BaggageRestrictionManager mgr;
+  private BaggageSetter setter;
 
   private static final String KEY = "key";
 
@@ -56,17 +60,19 @@ public void setUp() throws Exception {
     metricsReporter = new InMemoryStatsReporter();
     reporter = new InMemoryReporter();
     metrics = new Metrics(new StatsFactoryImpl(metricsReporter));
+    mgr = mock(DefaultBaggageRestrictionManager.class);
+    setter = new BaggageSetter(mgr, metrics);
     tracer =
         new Tracer.Builder("SamplerTest", reporter, new ConstSampler(true))
             .withStatsReporter(metricsReporter)
-            .withBaggageRestrictionManager(new DefaultBaggageRestrictionManager(metrics, 15))
             .build();
     span = (Span) tracer.buildSpan("some-operation").startManual();
   }
 
   @Test
   public void testInvalidBaggage() {
-    BaggageSetter setter = BaggageSetter.of(false, 0, metrics);
+    when(mgr.getRestriction(KEY)).thenReturn(Restriction.of(false, 0));
+
     final String value = "value";
     SpanContext ctx = setter.setBaggage(span, KEY, value);
 
@@ -79,7 +85,7 @@ public void testInvalidBaggage() {
 
   @Test
   public void testTruncatedBaggage() {
-    BaggageSetter setter = BaggageSetter.of(true, 5, metrics);
+    when(mgr.getRestriction(KEY)).thenReturn(Restriction.of(true, 5));
     final String value = "0123456789";
     final String expected = "01234";
     SpanContext ctx = setter.setBaggage(span, KEY, value);
@@ -95,7 +101,7 @@ public void testTruncatedBaggage() {
 
   @Test
   public void testOverrideBaggage() {
-    BaggageSetter setter = BaggageSetter.of(true, 5, metrics);
+    when(mgr.getRestriction(KEY)).thenReturn(Restriction.of(true, 5));
     final String value = "value";
     SpanContext ctx = setter.setBaggage(span, KEY, value);
     Span child = (Span) tracer.buildSpan("some-operation").asChildOf(ctx).startManual();
@@ -108,6 +114,23 @@ public void testOverrideBaggage() {
         2L, metricsReporter.counters.get("jaeger.baggage-update.result=ok").longValue());
   }
 
+  @Test
+  public void testUnsampledSpan() {
+    tracer =
+        new Tracer.Builder("SamplerTest", reporter, new ConstSampler(false))
+            .withStatsReporter(metricsReporter)
+            .build();
+    span = (Span) tracer.buildSpan("some-operation").startManual();
+
+    when(mgr.getRestriction(KEY)).thenReturn(Restriction.of(true, 5));
+    final String value = "value";
+    SpanContext ctx = setter.setBaggage(span, KEY, value);
+
+    assertEquals(value, ctx.getBaggageItem(KEY));
+    // No logs should be written if the span is not sampled
+    assertNull(span.getLogs());
+  }
+
   private void assertBaggageLogs(
       Span span,
       String key,

jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageRestrictionManagerTest.java → jaeger-core/src/test/java/com/uber/jaeger/baggage/DefaultBaggageRestrictionManagerTest.java

@@ -30,20 +30,19 @@
 import com.uber.jaeger.metrics.StatsFactoryImpl;
 import org.junit.Test;
 
-public class BaggageRestrictionManagerTest {
+public class DefaultBaggageRestrictionManagerTest {
 
   @Test
-  public void testGetBaggageSetter() {
-    final Metrics nullMetrics = new Metrics(new StatsFactoryImpl(new NullStatsReporter()));
-    final DefaultBaggageRestrictionManager undertest = new DefaultBaggageRestrictionManager(nullMetrics);
+  public void testGetRestriction() {
+    final DefaultBaggageRestrictionManager undertest = new DefaultBaggageRestrictionManager();
 
     final String key = "key";
-    BaggageSetter actual = undertest.getBaggageSetter(key);
-    BaggageSetter expected = BaggageSetter.of(true, 2048, nullMetrics);
+    Restriction actual = undertest.getRestriction(key);
+    Restriction expected = Restriction.of(true, 2048);
     assertEquals(expected, actual);
 
     expected = actual;
-    actual = undertest.getBaggageSetter(key);
+    actual = undertest.getRestriction(key);
     assertSame(actual, expected);
   }
 }