Adds Span.timestamp, duration and address annotations

This updates Brave to the latest thrift specification, which includes
explicit settings for Span timestamp and duration, as well as binary
annotations that indicate the actual IP of a client or server.

This also updates to lowercase convention for Span.name and
Endpoint.serviceName, also noted in recent thrift documentation.

brave-core/src/main/java/com/github/kristofa/brave/AnnotationSubmitter.java

@@ -1,5 +1,6 @@
 package com.github.kristofa.brave;
 
+import com.github.kristofa.brave.internal.Nullable;
 import com.twitter.zipkin.gen.Annotation;
 import com.twitter.zipkin.gen.AnnotationType;
 import com.twitter.zipkin.gen.BinaryAnnotation;
@@ -28,7 +29,7 @@ public static AnnotationSubmitter create(SpanAndEndpoint spanAndEndpoint) {
     private static final Charset UTF_8 = Charset.forName("UTF-8");
 
     /**
-     * Submits custom annotation for current span. Use this method if your annotation has no duration assigned to it.
+     * Submits custom annotation for current span.
      *
      * @param annotationName Custom annotation for current span.
      */
@@ -43,6 +44,64 @@ public void submitAnnotation(String annotationName) {
         }
     }
 
+    /** This adds an annotation that corresponds with {@link Span#getTimestamp()} */
+    void submitStartAnnotation(String annotationName) {
+        Span span = spanAndEndpoint().span();
+        if (span != null) {
+            Annotation annotation = new Annotation();
+            annotation.setTimestamp(currentTimeMicroseconds());
+            annotation.setHost(spanAndEndpoint().endpoint());
+            annotation.setValue(annotationName);
+            synchronized (span) {
+                span.setTimestamp(annotation.getTimestamp());
+                span.addToAnnotations(annotation);
+            }
+        }
+    }
+
+    /**
+     * This adds an annotation that corresponds with {@link Span#getDuration()}, and sends the span
+     * for collection.
+     *
+     * @return true if a span was sent for collection.
+     */
+    boolean submitEndAnnotation(String annotationName, SpanCollector spanCollector) {
+        Span span = spanAndEndpoint().span();
+        if (span == null) {
+          return false;
+        }
+        Annotation annotation = new Annotation();
+        annotation.setTimestamp(currentTimeMicroseconds());
+        annotation.setHost(spanAndEndpoint().endpoint());
+        annotation.setValue(annotationName);
+        span.addToAnnotations(annotation);
+        span.setDuration(annotation.getTimestamp() - span.getTimestamp());
+        spanCollector.collect(span);
+        return true;
+    }
+
+    /**
+     * Internal api for submitting an address. Until a naming function is added, this coerces null
+     * {@code serviceName} to "unknown", as that's zipkin's convention.
+     *
+     * @param ipv4        ipv4 host address as int. Ex for the ip 1.2.3.4, it would be (1 << 24) | (2 << 16) | (3 << 8) | 4
+     * @param port        Port for service
+     * @param serviceName Name of service. Should be lowercase and not empty. {@code null} will coerce to "unknown", as that's zipkin's convention.
+     */
+    void submitAddress(String key, int ipv4, int port, @Nullable String serviceName) {
+        Span span = spanAndEndpoint().span();
+        if (span != null) {
+            serviceName = serviceName != null ? serviceName : "unknown";
+            Endpoint endpoint = new Endpoint(ipv4, (short) port, serviceName);
+            BinaryAnnotation ba = new BinaryAnnotation();
+            ba.setKey(key);
+            ba.setValue(new byte[]{1});
+            ba.setAnnotation_type(AnnotationType.BOOL);
+            ba.setHost(endpoint);
+            addBinaryAnnotation(span, ba);
+        }
+    }
+
     /**
      * Submits a binary (key/value) annotation with String value.
      *

brave-core/src/main/java/com/github/kristofa/brave/ClientSpanState.java

@@ -1,5 +1,6 @@
 package com.github.kristofa.brave;
 
+import com.github.kristofa.brave.internal.Nullable;
 import com.twitter.zipkin.gen.Endpoint;
 import com.twitter.zipkin.gen.Span;
 
@@ -37,5 +38,8 @@ public interface ClientSpanState extends CommonSpanState {
      */
     void setCurrentClientSpan(final Span span);
 
-    void setCurrentClientServiceName(String serviceName);
+    /**
+     * @param serviceName Name of the local service being traced. If specified, should be lowercase and not empty.
+     */
+    void setCurrentClientServiceName(@Nullable String serviceName);
 }

brave-core/src/main/java/com/github/kristofa/brave/ClientTracer.java

@@ -3,6 +3,8 @@
 import com.google.auto.value.AutoValue;
 
 import com.github.kristofa.brave.SpanAndEndpoint.ClientSpanAndEndpoint;
+import com.github.kristofa.brave.internal.Nullable;
+import com.twitter.zipkin.gen.Endpoint;
 import com.twitter.zipkin.gen.Span;
 import com.twitter.zipkin.gen.zipkinCoreConstants;
 
@@ -66,30 +68,38 @@ public Builder state(ServerAndClientSpanState state) {
      * Sets 'client sent' event for current thread.
      */
     public void setClientSent() {
-        submitAnnotation(zipkinCoreConstants.CLIENT_SEND);
+        submitStartAnnotation(zipkinCoreConstants.CLIENT_SEND);
+    }
+
+    /**
+     * Like {@link #setClientSent()}, except you can log the network context of the destination.
+     *
+     * @param ipv4        ipv4 of the server as an int. Ex for 1.2.3.4, it would be (1 << 24) | (2 << 16) | (3 << 8) | 4
+     * @param port        listen port the client is connecting to, or 0 if unknown
+     * @param serviceName lowercase {@link Endpoint#service_name name} of the service being called
+     *                    or null if unknown
+     */
+    public void setClientSent(int ipv4, int port, @Nullable String serviceName) {
+        submitAddress(zipkinCoreConstants.SERVER_ADDR, ipv4, port, serviceName);
+        submitStartAnnotation(zipkinCoreConstants.CLIENT_SEND);
     }
 
     /**
      * Sets the 'client received' event for current thread. This will also submit span because setting a client received
      * event means this span is finished.
      */
     public void setClientReceived() {
-
-        Span currentSpan = spanAndEndpoint().span();
-        if (currentSpan != null) {
-            submitAnnotation(zipkinCoreConstants.CLIENT_RECV);
-            spanCollector().collect(currentSpan);
+        if (submitEndAnnotation(zipkinCoreConstants.CLIENT_RECV, spanCollector())) {
             spanAndEndpoint().state().setCurrentClientSpan(null);
             spanAndEndpoint().state().setCurrentClientServiceName(null);
         }
-
     }
 
     /**
      * Start a new span for a new client request that will be bound to current thread. The ClientTracer can decide to return
      * <code>null</code> in case this request should not be traced (eg sampling).
      *
-     * @param requestName Request name. Should not be <code>null</code> or empty.
+     * @param requestName Request name. Should be lowercase and not <code>null</code> or empty.
      * @return Span id for new request or <code>null</code> in case we should not trace this new client request.
      */
     public SpanId startNewSpan(String requestName) {
@@ -130,7 +140,7 @@ public SpanId startNewSpan(String requestName) {
      * This should be set before submitting any annotations. So after invoking {@link ClientTracer#startNewSpan(String)} and
      * before {@link ClientTracer#setClientSent()}.
      *
-     * @param serviceName should be the same as the name of the service the client is calling.
+     * @param serviceName Name of the local service being traced. Should be lowercase and not <code>null</code> or empty.
      */
     public void setCurrentClientServiceName(String serviceName) {
         spanAndEndpoint().state().setCurrentClientServiceName(serviceName);

brave-core/src/main/java/com/github/kristofa/brave/ServerSpan.java

@@ -1,7 +1,5 @@
 package com.github.kristofa.brave;
 
-import java.util.concurrent.atomic.AtomicLong;
-
 import com.github.kristofa.brave.internal.Nullable;
 import com.google.auto.value.AutoValue;
 
@@ -46,7 +44,7 @@ static ServerSpan create(Span span, Boolean sample) {
      * @param traceId Trace id.
      * @param spanId Span id.
      * @param parentSpanId Parent span id, can be <code>null</code>.
-     * @param name Span name.
+     * @param name Span name. Should be lowercase and not <code>null</code> or empty.
      */
      static ServerSpan create(long traceId, long spanId, Long parentSpanId, String name) {
         Span span = new Span();

brave-core/src/main/java/com/github/kristofa/brave/ServerTracer.java

@@ -1,15 +1,15 @@
 package com.github.kristofa.brave;
 
-import java.util.List;
-import java.util.Random;
+import com.google.auto.value.AutoValue;
 
 import com.github.kristofa.brave.SpanAndEndpoint.ServerSpanAndEndpoint;
 import com.github.kristofa.brave.internal.Nullable;
-import com.google.auto.value.AutoValue;
-
-import com.twitter.zipkin.gen.Span;
+import com.twitter.zipkin.gen.Endpoint;
 import com.twitter.zipkin.gen.zipkinCoreConstants;
 
+import java.util.List;
+import java.util.Random;
+
 import static com.github.kristofa.brave.internal.Util.checkNotBlank;
 
 /**
@@ -125,17 +125,28 @@ public void setStateUnknown(String spanName) {
      * {@link ServerTracer#setStateUnknown(String)}.
      */
     public void setServerReceived() {
-        submitAnnotation(zipkinCoreConstants.SERVER_RECV);
+        submitStartAnnotation(zipkinCoreConstants.SERVER_RECV);
+    }
+
+    /**
+     * Like {@link #setServerReceived()}, except you can log the network context of the caller, for
+     * example an IP address from the {@code X-Forwarded-For} header.
+     *
+     * @param ipv4          ipv4 of the client as an int. Ex for 1.2.3.4, it would be (1 << 24) | (2 << 16) | (3 << 8) | 4
+     * @param port          port for client-side of the socket, or 0 if unknown
+     * @param clientService lowercase {@link Endpoint#service_name name} of the callee service or
+     *                      null if unknown
+     */
+    public void setServerReceived(int ipv4, int port, @Nullable String clientService) {
+        submitAddress(zipkinCoreConstants.CLIENT_ADDR, ipv4, port, clientService);
+        submitStartAnnotation(zipkinCoreConstants.SERVER_RECV);
     }
 
     /**
      * Sets the server sent event for current thread.
      */
     public void setServerSend() {
-        Span currentSpan = spanAndEndpoint().state().getCurrentServerSpan().getSpan();
-        if (currentSpan != null) {
-            submitAnnotation(zipkinCoreConstants.SERVER_SEND);
-            spanCollector().collect(currentSpan);
+        if (submitEndAnnotation(zipkinCoreConstants.SERVER_SEND, spanCollector())) {
             spanAndEndpoint().state().setCurrentServerSpan(null);
         }
     }

brave-core/src/main/java/com/github/kristofa/brave/SimpleServerAndClientSpanState.java

@@ -22,7 +22,7 @@ final class SimpleServerAndClientSpanState implements ServerAndClientSpanState {
      *
      * @param ip InetAddress of current host. If you don't have access to InetAddress you can use InetAddressUtilities#getLocalHostLANAddress()
      * @param port port on which current process is listening.
-     * @param serviceName Service name. Only relevant if we do server side tracing.
+     * @param serviceName Name of the local service being traced. Should be lowercase and not <code>null</code> or empty.
      */
     public SimpleServerAndClientSpanState(InetAddress ip, int port, String serviceName) {
         Util.checkNotNull(ip, "ip address must be specified.");
@@ -82,6 +82,9 @@ public void setCurrentClientSpan(final Span span) {
         currentClientSpan = span;
     }
 
+    /**
+     * {@inheritDoc}
+     */
     @Override
     public void setCurrentClientServiceName(String serviceName) {
         currentClientServiceName = serviceName;

brave-core/src/main/java/com/github/kristofa/brave/ThreadLocalServerAndClientSpanState.java

@@ -5,7 +5,6 @@
 import com.twitter.zipkin.gen.Span;
 
 import java.net.InetAddress;
-import java.net.UnknownHostException;
 
 /**
  * {@link ServerAndClientSpanState} implementation that keeps trace state using a ThreadLocal variable.
@@ -32,9 +31,9 @@ protected ServerSpan initialValue() {
      *
      * @param ip InetAddress of current host. If you don't have access to InetAddress you can use InetAddressUtilities#getLocalHostLANAddress()
      * @param port port on which current process is listening.
-     * @param serviceName Service name. Only relevant if we do server side tracing.
+     * @param serviceName Name of the local service being traced. Should be lowercase and not <code>null</code> or empty.
      * @deprecated Please switch to constructor that takes 'int' for ip. This only does a conversion from the InetAddress to integer anyway
-     * and using InetAddress can result in ns lookup and nasty side effects.
+     *             and using InetAddress can result in ns lookup and nasty side effects.
      */
     @Deprecated
     public ThreadLocalServerAndClientSpanState(InetAddress ip, int port, String serviceName) {
@@ -48,7 +47,7 @@ public ThreadLocalServerAndClientSpanState(InetAddress ip, int port, String serv
      *
      * @param ip Int representation of ipv4 address.
      * @param port port on which current process is listening.
-     * @param serviceName Service name. Only relevant if we do server side tracing.
+     * @param serviceName Name of the local service being traced. Should be lowercase and not <code>null</code> or empty.
      */
     public ThreadLocalServerAndClientSpanState(int ip, int port, String serviceName) {
         Util.checkNotNull(ip, "ip address must be specified.");