Removes expectation a setter can clear a key (openzipkin#506)

I made a mistake documenting Propagation.Setter as being able to clear
a key. This is not guaranteed to work, for example you can't clear
headers made with URLConnection, only replace or add to them.

Instead, this documents that in reusable carriers, you must clear keys
on your own. If carriers are single-shot, there's no need to clear
anything first.

brave-tests/src/main/java/brave/propagation/PropagationSetterTest.java

@@ -49,13 +49,4 @@ public abstract class PropagationSetterTest<C, K> {
     assertThat(read(carrier(), key))
         .containsExactly("463ac35c9f6413ad");
   }
-
-  @Test public void overrideNull() throws Exception {
-    K key = keyFactory().create("X-B3-ParentSpanId");
-    setter().put(carrier(), key, "48485a3953bb6124");
-    setter().put(carrier(), key, null);
-
-    assertThat(read(carrier(), key))
-        .isEmpty();
-  }
 }

brave/src/main/java/brave/propagation/Propagation.java

@@ -54,13 +54,26 @@ interface Setter<C, K> {
     void put(C carrier, K key, String value);
   }
 
-  /** The propagation fields defined */
+  /**
+   * The propagation fields defined. If your carrier is reused, you should delete the fields here
+   * before calling {@link Setter#put(Object, Object, String)}.
+   *
+   * <p>For example, if the carrier is a single-use or immutable request object, you don't need to
+   * clear fields as they couldn't have been set before. If it is a mutable, retryable object,
+   * successive calls should clear these fields first.
+   */
   // The use cases of this are:
   // * allow pre-allocation of fields, especially in systems like gRPC Metadata
   // * allow a single-pass over an iterator (ex OpenTracing has no getter in TextMap)
   List<K> keys();
 
   /**
+   * Replaces a propagated field with the given value. Saved as a constant to avoid runtime
+   * allocations.
+   *
+   * For example, a setter for an {@link java.net.HttpURLConnection} would be the method reference
+   * {@link java.net.HttpURLConnection#addRequestProperty(String, String)}
+   *
    * @param setter invoked for each propagation key to add.
    */
   <C> TraceContext.Injector<C> injector(Setter<C, K> setter);

brave/src/main/java/brave/propagation/TraceContext.java

@@ -21,6 +21,16 @@ public abstract class TraceContext extends SamplingFlags {
 
   /**
    * Used to send the trace context downstream. For example, as http headers.
+   *
+   * <p>For example, to put the context on an {@link java.net.HttpURLConnection}, you can do this:
+   * <pre>{@code
+   * // in your constructor
+   * injector = tracing.propagation().injector(URLConnection::setRequestProperty);
+   *
+   * // later in your code, reuse the function you created above to add trace headers
+   * HttpURLConnection connection = (HttpURLConnection) new URL("http://myserver").openConnection();
+   * injector.inject(span.context(), connection);
+   * }</pre>
    */
   public interface Injector<C> {
     /**

instrumentation/grpc/src/main/java/brave/grpc/TracingClientInterceptor.java

@@ -20,7 +20,11 @@ final class TracingClientInterceptor implements ClientInterceptor {
       new Setter<Metadata, Metadata.Key<String>>() { // retrolambda no like
         @Override public void put(Metadata metadata, Metadata.Key<String> key, String value) {
           metadata.removeAll(key);
-          if (value != null) metadata.put(key, value);
+          metadata.put(key, value);
+        }
+
+        @Override public String toString() {
+          return "Metadata::put";
         }
       };
 

instrumentation/httpasyncclient/src/main/java/brave/httpasyncclient/TracingHttpAsyncClientBuilder.java

@@ -36,10 +36,7 @@
  * added last}.
  */
 public final class TracingHttpAsyncClientBuilder extends HttpAsyncClientBuilder {
-  static final Propagation.Setter<HttpMessage, String> SETTER = (message, key, value) ->{
-    message.removeHeaders(key);
-    if (value != null) message.setHeader(key, value);
-  };
+  static final Propagation.Setter<HttpMessage, String> SETTER = HttpMessage::setHeader;
 
   public static HttpAsyncClientBuilder create(Tracing tracing) {
     return new TracingHttpAsyncClientBuilder(HttpTracing.create(tracing));

instrumentation/httpclient/src/main/java/brave/httpclient/TracingHttpClientBuilder.java

@@ -17,13 +17,12 @@
 import org.apache.http.client.methods.HttpRequestWrapper;
 import org.apache.http.impl.client.HttpClientBuilder;
 import org.apache.http.impl.execchain.ClientExecChain;
+import org.apache.http.message.AbstractHttpMessage;
 import zipkin2.Endpoint;
 
 public final class TracingHttpClientBuilder extends HttpClientBuilder {
-  static final Propagation.Setter<HttpRequestWrapper, String> SETTER = (wrapper, key, value) -> {
-    wrapper.removeHeaders(key);
-    if (value != null) wrapper.setHeader(key, value);
-  };
+  static final Propagation.Setter<HttpRequestWrapper, String> SETTER =
+      AbstractHttpMessage::setHeader;
 
   public static HttpClientBuilder create(Tracing tracing) {
     return new TracingHttpClientBuilder(HttpTracing.create(tracing));

instrumentation/kafka-clients/src/main/java/brave/kafka/clients/KafkaPropagation.java

@@ -1,26 +1,26 @@
-package brave.kafka.clients;
-
-import brave.propagation.Propagation.Getter;
-import brave.propagation.Propagation.Setter;
-import java.nio.charset.Charset;
-import org.apache.kafka.common.header.Header;
-import org.apache.kafka.common.header.Headers;
-
-final class KafkaPropagation {
-
-  static final Charset UTF_8 = Charset.forName("UTF-8");
-
-  static final Setter<Headers, String> HEADER_SETTER = (carrier, key, value) -> {
-    carrier.remove(key);
-    if (value != null) carrier.add(key, value.getBytes(UTF_8));
-  };
-
-  static final Getter<Headers, String> HEADER_GETTER = (carrier, key) -> {
-    Header header = carrier.lastHeader(key);
-    if (header == null) return null;
-    return new String(header.value(), UTF_8);
-  };
-
-  KafkaPropagation() {
-  }
-}
+package brave.kafka.clients;
+
+import brave.propagation.Propagation.Getter;
+import brave.propagation.Propagation.Setter;
+import java.nio.charset.Charset;
+import org.apache.kafka.common.header.Header;
+import org.apache.kafka.common.header.Headers;
+
+final class KafkaPropagation {
+
+  static final Charset UTF_8 = Charset.forName("UTF-8");
+
+  static final Setter<Headers, String> HEADER_SETTER = (carrier, key, value) -> {
+    carrier.remove(key);
+    carrier.add(key, value.getBytes(UTF_8));
+  };
+
+  static final Getter<Headers, String> HEADER_GETTER = (carrier, key) -> {
+    Header header = carrier.lastHeader(key);
+    if (header == null) return null;
+    return new String(header.value(), UTF_8);
+  };
+
+  KafkaPropagation() {
+  }
+}

instrumentation/kafka-clients/src/main/java/brave/kafka/clients/TracingConsumer.java

@@ -54,7 +54,6 @@ final class TracingConsumer<K, V> implements Consumer<K, V> {
   void handleConsumed(ConsumerRecord record) {
     Span span = startAndFinishConsumerSpan(record);
     // remove propagation headers from the record
-    // TODO: consider making remove a normal part of propagation
     tracing.propagation().keys().forEach(key -> record.headers().remove(key));
 
     // Inject new propagation headers

instrumentation/okhttp3/src/main/java/brave/okhttp3/TracingInterceptor.java

@@ -10,7 +10,6 @@
 import java.io.IOException;
 import java.net.InetSocketAddress;
 import okhttp3.Connection;
-import okhttp3.Headers;
 import okhttp3.Interceptor;
 import okhttp3.Request;
 import okhttp3.Response;
@@ -22,13 +21,7 @@
  * In cases like that, use {@link TracingCallFactory}.
  */
 public final class TracingInterceptor implements Interceptor {
-  static final Propagation.Setter<Request.Builder, String> SETTER = (builder, key, value) -> {
-    if (value == null) {
-      builder.headers(Headers.of());
-    } else {
-      builder.header(key, value);
-    }
-  };
+  static final Propagation.Setter<Request.Builder, String> SETTER = Request.Builder::header;
 
   public static Interceptor create(Tracing tracing) {
     return create(HttpTracing.create(tracing));

instrumentation/spring-web/src/main/java/brave/spring/web/TracingClientHttpRequestInterceptor.java

@@ -16,13 +16,7 @@
 import org.springframework.http.client.ClientHttpResponse;
 
 public final class TracingClientHttpRequestInterceptor implements ClientHttpRequestInterceptor {
-  static final Propagation.Setter<HttpHeaders, String> SETTER = (headers, key, value) -> {
-    if (value == null) {
-      headers.remove(key);
-    } else {
-      headers.set(key, value);
-    }
-  };
+  static final Propagation.Setter<HttpHeaders, String> SETTER = HttpHeaders::set;
 
   public static ClientHttpRequestInterceptor create(Tracing tracing) {
     return create(HttpTracing.create(tracing));