Changes builders to use CorrelationScopeConfig and BaggagePropagation…

…Config (#1143)

This fixes a configuration cul-de-sac where you cannot later add
something that supports dynamic fields. It does so by adding a base
configuration type for each of correlation and baggage propagation.

There's currently only one implementation and the types are sealed.
The current implementations support single field config without
preventing dynamic fields later.

brave-tests/src/main/java/brave/test/ITRemote.java

@@ -16,6 +16,7 @@
 import brave.Tracing;
 import brave.baggage.BaggageField;
 import brave.baggage.BaggagePropagation;
+import brave.baggage.BaggagePropagationConfig.SingleBaggageField;
 import brave.propagation.B3Propagation;
 import brave.propagation.CurrentTraceContext;
 import brave.propagation.Propagation;
@@ -96,7 +97,7 @@ protected ITRemote() {
       checkForLeakedScopes = strictScopeDecorator;
     }
     propagationFactory = BaggagePropagation.newFactoryBuilder(B3Propagation.FACTORY)
-      .addRemoteField(BAGGAGE_FIELD).build();
+      .add(SingleBaggageField.remote(BAGGAGE_FIELD)).build();
     tracing = tracingBuilder(Sampler.ALWAYS_SAMPLE).build();
   }
 

brave-tests/src/main/java/brave/test/propagation/CurrentTraceContextTest.java

@@ -15,7 +15,8 @@
 
 import brave.baggage.BaggageField;
 import brave.baggage.BaggagePropagation;
-import brave.baggage.CorrelationField;
+import brave.baggage.BaggagePropagationConfig.SingleBaggageField;
+import brave.baggage.CorrelationScopeConfig.SingleCorrelationField;
 import brave.internal.Nullable;
 import brave.propagation.B3Propagation;
 import brave.propagation.CurrentTraceContext;
@@ -41,11 +42,11 @@
 import static org.assertj.core.api.Assertions.failBecauseExceptionWasNotThrown;
 
 public abstract class CurrentTraceContextTest {
-  protected static final CorrelationField CORRELATION_FIELD =
-    CorrelationField.create(BaggageField.create("user-id"));
+  protected static final SingleCorrelationField CORRELATION_FIELD =
+    SingleCorrelationField.create(BaggageField.create("user-id"));
 
   Propagation.Factory baggageFactory = BaggagePropagation.newFactoryBuilder(B3Propagation.FACTORY)
-    .addRemoteField(CORRELATION_FIELD.baggageField()).build();
+    .add(SingleBaggageField.remote(CORRELATION_FIELD.baggageField())).build();
 
   protected final CurrentTraceContext currentTraceContext;
   protected final TraceContext context = baggageFactory.decorate(

brave/README.md

@@ -328,13 +328,15 @@ context.
 For example, if you have a need to know the a specific request's country code, you can
 propagate it through the trace as an HTTP header with the same name:
 ```java
+import brave.baggage.BaggagePropagationConfig.SingleBaggageField;
+
 // Configure your baggage field
 COUNTRY_CODE = BaggageField.create("country-code");
 
 // When you initialize the builder, add the baggage you want to propagate
 tracingBuilder.propagationFactory(
   BaggagePropagation.newFactoryBuilder(B3Propagation.FACTORY)
-                    .addRemoteField(COUNTRY_CODE)
+                    .add(SingleBaggageField.remote(COUNTRY_CODE))
                     .build()
 );
 
@@ -371,29 +373,37 @@ For example, the following will propagate the field "x-vcap-request-id" as-is, b
 fields "countryCode" and "userId" on the wire as "baggage-country-code" and "baggage-user-id"
 respectively.
 ```java
+import brave.baggage.BaggagePropagationConfig.SingleBaggageField;
+
 REQUEST_ID = BaggageField.create("x-vcap-request-id");
 COUNTRY_CODE = BaggageField.create("countryCode");
 USER_ID = BaggageField.create("userId");
 
 tracingBuilder.propagationFactory(
     BaggagePropagation.newFactoryBuilder(B3Propagation.FACTORY)
-                      .addRemoteField(REQUEST_ID)
-                      .addRemoteField(COUNTRY_CODE, "baggage-country-code")
-                      .addRemoteField(USER_ID, "baggage-user-id").build())
+                      .add(SingleBaggageField.remote(REQUEST_ID))
+                      .add(SingleBaggageField.newBuilder(COUNTRY_CODE)
+                                             .addKeyName("baggage-country-code").build())
+                      .add(SingleBaggageField.newBuilder(USER_ID)
+                                             .addKeyName("baggage-user-id").build())
+                      .build()
 );
 ```
 ### Correlation
 
 You can also integrate baggage with other correlated contexts such as logging:
 ```java
+import brave.baggage.BaggagePropagationConfig.SingleBaggageField;
+import brave.baggage.CorrelationScopeConfig.SingleCorrelationField;
+
 AMZN_TRACE_ID = BaggageField.create("x-amzn-trace-id");
 
 // Allow logging patterns like %X{traceId} %X{x-amzn-trace-id}
 decorator = MDCScopeDecorator.newBuilder()
-                             .addField(AMZN_TRACE_ID).build();
+                             .add(SingleCorrelationField.create(AMZN_TRACE_ID)).build()
 
 tracingBuilder.propagationFactory(BaggagePropagation.newFactoryBuilder(B3Propagation.FACTORY)
-                                                    .addRemoteField(AMZN_TRACE_ID)
+                                                    .add(SingleBaggageField.remote(AMZN_TRACE_ID))
                                                     .build())
               .currentTraceContext(ThreadLocalCurrentTraceContext.newBuilder()
                                                                  .addScopeDecorator(decorator)
@@ -406,9 +416,11 @@ override them in the builder as needed.
 
 Ex. If your log property is %X{trace-id}, you can do this:
 ```java
-builder.clear(); // traceId is a default field!
-builder.addField(CorrelationField.newBuilder(BaggageFields.TRACE_ID)
-                                 .name("trace-id").build());
+import brave.baggage.CorrelationScopeConfig.SingleCorrelationField;
+
+scopeBuilder.clear() // TRACE_ID is a default field!
+            .add(SingleCorrelationField.newBuilder(BaggageFields.TRACE_ID)
+                                       .name("trace-id").build())
 ```
 
 ### Appropriate usage
@@ -428,11 +440,13 @@ Amazon Web Services environment, but not reporting data to X-Ray. To ensure X-Ra
 correctly, pass-through its tracing header like so.
 
 ```java
+import brave.baggage.BaggagePropagationConfig.SingleBaggageField;
+
 OTHER_TRACE_ID = BaggageField.create("x-amzn-trace-id");
 
 tracingBuilder.propagationFactory(
   BaggagePropagation.newFactoryBuilder(B3Propagation.FACTORY)
-                    .addRemoteField(OTHER_TRACE_ID)
+                    .add(SingleBaggageField.remote(OTHER_TRACE_ID))
                     .build()
 );
 ```

brave/src/main/java/brave/baggage/BaggageField.java

@@ -50,30 +50,32 @@
  * <p>Ex. once added to `BaggagePropagation`, you can call below to affect the country code
  * of the current trace context:
  * <pre>{@code
- *  COUNTRY_CODE.updateValue("FO");
- *  String countryCode = COUNTRY_CODE.get();
+ * COUNTRY_CODE.updateValue("FO");
+ * String countryCode = COUNTRY_CODE.get();
  * }</pre>
  *
  * <p>Or, if you have a reference to a trace context, it is more efficient to use it explicitly:
  * <pre>{@code
- *   COUNTRY_CODE.updateValue(span.context(), "FO");
- *  String countryCode = COUNTRY_CODE.get(span.context());
- *  Tags.BAGGAGE_FIELD.tag(COUNTRY_CODE, span);
+ * COUNTRY_CODE.updateValue(span.context(), "FO");
+ * String countryCode = COUNTRY_CODE.get(span.context());
+ * Tags.BAGGAGE_FIELD.tag(COUNTRY_CODE, span);
  * }</pre>
  *
  * <p>Correlation</p>
  *
- *
  * <p>You can also integrate baggage with other correlated contexts such as logging:
  * <pre>{@code
+ * import brave.baggage.BaggagePropagationConfig.SingleBaggageField;
+ * import brave.baggage.CorrelationScopeConfig.SingleCorrelationField;
+ *
  * AMZN_TRACE_ID = BaggageField.create("x-amzn-trace-id");
  *
  * // Allow logging patterns like %X{traceId} %X{x-amzn-trace-id}
  * decorator = MDCScopeDecorator.newBuilder()
- *                              .addField(AMZN_TRACE_ID).build();
+ *                              .add(SingleCorrelationField.create(AMZN_TRACE_ID)).build()
  *
  * tracingBuilder.propagationFactory(BaggagePropagation.newFactoryBuilder(B3Propagation.FACTORY)
- *                                                     .addRemoteField(AMZN_TRACE_ID)
+ *                                                     .add(SingleBaggageField.remote(AMZN_TRACE_ID))
  *                                                     .build())
  *               .currentTraceContext(ThreadLocalCurrentTraceContext.newBuilder()
  *                                                                  .addScopeDecorator(decorator)
@@ -101,7 +103,7 @@
  * propagate "arbitrary stuff" with a request.
  *
  * @see BaggagePropagation
- * @see CorrelationField
+ * @see CorrelationScopeConfig
  * @since 5.11
  */
 public final class BaggageField {
@@ -197,7 +199,7 @@ public static List<BaggageField> getAll(TraceContextOrSamplingFlags extracted) {
    * made current.
    *
    * @see #getByName(TraceContext, String)
-   * @see CorrelationField#name()
+   * @see CorrelationScopeConfig.SingleCorrelationField#name()
    * @since 5.11
    */
   public final String name() {

brave/src/main/java/brave/baggage/BaggagePropagation.java

@@ -13,6 +13,7 @@
  */
 package brave.baggage;
 
+import brave.baggage.BaggagePropagationConfig.SingleBaggageField;
 import brave.internal.baggage.BaggageHandler;
 import brave.internal.baggage.BaggageHandlers;
 import brave.internal.baggage.ExtraBaggageFields;
@@ -21,7 +22,6 @@
 import brave.propagation.TraceContext.Extractor;
 import brave.propagation.TraceContext.Injector;
 import brave.propagation.TraceContextOrSamplingFlags;
-import java.util.Arrays;
 import java.util.Collections;
 import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
@@ -38,46 +38,54 @@
  * <p>For example, if you have a need to know the a specific request's country code, you can
  * propagate it through the trace as HTTP headers.
  * <pre>{@code
+ * import brave.baggage.BaggagePropagationConfig.SingleBaggageField;
+ *
  * // Configure your baggage field
  * COUNTRY_CODE = BaggageField.create("country-code");
  *
  * // When you initialize the builder, add the baggage you want to propagate
  * tracingBuilder.propagationFactory(
  *   BaggagePropagation.newFactoryBuilder(B3Propagation.FACTORY)
- *                     .addRemoteField(COUNTRY_CODE)
+ *                     .add(SingleBaggageField.remote(COUNTRY_CODE))
  *                     .build()
  * );
  *
  * // later, you can tag that country code
  * Tags.BAGGAGE_FIELD.tag(COUNTRY_CODE, span);
  * }</pre>
  *
- * <h3>Customizing propagtion keys</h3>
- * By default, the name used as a propagation key (header) by {@link
- * FactoryBuilder#addRemoteField(BaggageField, String...)} is the same as the lowercase variant of
- * the field name. You can override this by supplying different key names. Note: they will be
- * lower-cased.
+ * <p>See {@link BaggageField} for baggage usage examples.
+ *
+ * <h3>Customizing propagation keys</h3>
+ * {@link SingleBaggageField#remote(BaggageField)} sets the name used as a propagation key (header)
+ * to the lowercase variant of the field name. You can override this by supplying different key
+ * names. Note: they will be lower-cased.
  *
  * <p>For example, the following will propagate the field "x-vcap-request-id" as-is, but send the
  * fields "countryCode" and "userId" on the wire as "baggage-country-code" and "baggage-user-id"
  * respectively.
  *
  * <pre>{@code
+ * import brave.baggage.BaggagePropagationConfig.SingleBaggageField;
+ *
  * REQUEST_ID = BaggageField.create("x-vcap-request-id");
  * COUNTRY_CODE = BaggageField.create("countryCode");
  * USER_ID = BaggageField.create("userId");
  *
  * tracingBuilder.propagationFactory(
  *     BaggagePropagation.newFactoryBuilder(B3Propagation.FACTORY)
- *                       .addRemoteField(REQUEST_ID)
- *                       .addRemoteField(COUNTRY_CODE, "baggage-country-code")
- *                       .addRemoteField(USER_ID, "baggage-user-id").build())
+ *                       .add(SingleBaggageField.remote(REQUEST_ID))
+ *                       .add(SingleBaggageField.newBuilder(COUNTRY_CODE)
+ *                                              .addKeyName("baggage-country-code").build())
+ *                       .add(SingleBaggageField.newBuilder(USER_ID)
+ *                                              .addKeyName("baggage-user-id").build())
+ *                       .build()
  * );
  * }</pre>
  *
- * <p>See {@link BaggageField} for usage examples
- *
  * @see BaggageField
+ * @see BaggagePropagationConfig
+ * @see BaggagePropagationCustomizer
  * @see CorrelationScopeDecorator
  * @since 5.11
  */
@@ -91,82 +99,51 @@ public static class FactoryBuilder { // not final to backport ExtraFieldPropagat
     final Propagation.Factory delegate;
     final Set<String> allKeyNames = new LinkedHashSet<>();
     final Map<BaggageField, Set<String>> fieldToKeyNames = new LinkedHashMap<>();
+    final Set<SingleBaggageField> configs = new LinkedHashSet<>();
 
     FactoryBuilder(Propagation.Factory delegate) {
       if (delegate == null) throw new NullPointerException("delegate == null");
       this.delegate = delegate;
     }
 
     /**
-     * Returns an immutable copy of the currently configured fields mapped to names for use in
-     * remote propagation. This allows those who can't create the builder to reconfigure this
-     * builder.
+     * Returns an immutable copy of the current {@linkplain #add(BaggagePropagationConfig)
+     * configuration}. This allows those who can't create the builder to reconfigure this builder.
      *
+     * @see #clear()
      * @since 5.11
      */
-    public Map<BaggageField, Set<String>> fieldToKeyNames() {
-      return Collections.unmodifiableMap(new LinkedHashMap<>(fieldToKeyNames));
+    public Set<BaggagePropagationConfig> configs() {
+      return Collections.unmodifiableSet(new LinkedHashSet<>(configs));
     }
 
     /**
      * Clears all state. This allows those who can't create the builder to reconfigure fields.
      *
-     * @see #fieldToKeyNames()
+     * @see #configs()
      * @see BaggagePropagationCustomizer
      * @since 5.11
      */
     public FactoryBuilder clear() {
       allKeyNames.clear();
       fieldToKeyNames.clear();
+      configs.clear();
       return this;
     }
 
-    /**
-     * Adds a {@linkplain BaggageField baggage field}, but does not configure remote propagation.
-     *
-     * @throws IllegalArgumentException if the field was already added
-     * @since 5.11
-     */
-    public FactoryBuilder addField(BaggageField field) {
-      if (field == null) throw new NullPointerException("field == null");
-      if (fieldToKeyNames.containsKey(field)) {
-        throw new IllegalArgumentException(field.name + " already added");
+    /** @since 5.11 */
+    public FactoryBuilder add(BaggagePropagationConfig config) {
+      if (config == null) throw new NullPointerException("config == null");
+      if (!(config instanceof SingleBaggageField)) {
+        throw new UnsupportedOperationException("dynamic fields not yet supported");
       }
-      fieldToKeyNames.put(field, Collections.emptySet());
-      return this;
-    }
-
-    /**
-     * Adds a {@linkplain BaggageField baggage field} for remote propagation.
-     *
-     * <p>When {@code keyNames} are not supplied the field is referenced the same in-process as it
-     * is on the wire. For example, the {@linkplain BaggageField#name() name} "x-vcap-request-id"
-     * would be set as-is including the prefix.
-     *
-     * @param keyNames possibly empty lower-case {@link Propagation#keys() propagation key names}.
-     * @throws IllegalArgumentException if the field was already added or a key name is already in
-     * use.
-     * @since 5.11
-     */
-    public FactoryBuilder addRemoteField(BaggageField field, String... keyNames) {
-      if (field == null) throw new NullPointerException("field == null");
-      if (keyNames == null) throw new NullPointerException("keyNames == null");
-      return addRemoteField(field, Arrays.asList(keyNames));
-    }
-
-    /**
-     * Same as {@link #addRemoteField(BaggageField, String...)}.
-     *
-     * @since 5.11
-     */
-    public FactoryBuilder addRemoteField(BaggageField field, Iterable<String> keyNames) {
-      if (field == null) throw new NullPointerException("field == null");
-      if (keyNames == null) throw new NullPointerException("keyNames == null");
-      if (fieldToKeyNames.containsKey(field)) {
-        throw new IllegalArgumentException(field.name + " already added");
+      SingleBaggageField field = (SingleBaggageField) config;
+      if (fieldToKeyNames.containsKey(field.field)) {
+        throw new IllegalArgumentException(field.field.name + " already added");
       }
+      configs.add(field);
       Set<String> lcKeyNames = new LinkedHashSet<>();
-      for (String keyName : keyNames) {
+      for (String keyName : field.keyNames) {
         String lcName = validateName(keyName).toLowerCase(Locale.ROOT);
         if (allKeyNames.contains(lcName)) {
           throw new IllegalArgumentException("Propagation key already in use: " + lcName);
@@ -175,12 +152,7 @@ public FactoryBuilder addRemoteField(BaggageField field, Iterable<String> keyNam
         lcKeyNames.add(lcName);
       }
 
-      if (lcKeyNames.isEmpty()) { // add the default name
-        allKeyNames.add(field.lcName);
-        lcKeyNames.add(field.lcName);
-      }
-
-      fieldToKeyNames.put(field, Collections.unmodifiableSet(lcKeyNames));
+      fieldToKeyNames.put(field.field, Collections.unmodifiableSet(lcKeyNames));
       return this;
     }