Add new api that is easier to use for user to add only one Attribute. (#571)

* Add new api that is easier to use for user to add only one Attribute.

* Rename addAttribute[s] to putAttribute[s] and deprecate the old API.

* Update tests to use putAttributes.

9 files changed, 131 insertions, 33 deletions

diff --git a/api/src/main/java/io/opencensus/trace/BlankSpan.java b/api/src/main/java/io/opencensus/trace/BlankSpan.java
index 05087983..b7cfb08f 100644
--- a/api/src/main/java/io/opencensus/trace/BlankSpan.java
+++ b/api/src/main/java/io/opencensus/trace/BlankSpan.java
@@ -34,8 +34,13 @@ public final class BlankSpan extends Span {
     super(SpanContext.INVALID, null);
   }
 
+  /** No-op implementation of the {@link Span#putAttribute(String, AttributeValue)} method. */
+  @Override
+  public void putAttribute(String key, AttributeValue value) {}
+
   /** No-op implementation of the {@link Span#addAttributes(Map)} method. */
   @Override
+  @SuppressWarnings("deprecation")
   public void addAttributes(Map<String, AttributeValue> attributes) {}
 
   /** No-op implementation of the {@link Span#addAnnotation(String, Map)} method. */
diff --git a/api/src/main/java/io/opencensus/trace/Span.java b/api/src/main/java/io/opencensus/trace/Span.java
index c609c13f..20a859e1 100644
--- a/api/src/main/java/io/opencensus/trace/Span.java
+++ b/api/src/main/java/io/opencensus/trace/Span.java
@@ -77,10 +77,38 @@ public abstract class Span {
   }
 
   /**
-   * Adds a set of attributes to the {@code Span}.
+   * Sets an attribute to the {@code Span}. If the {@code Span} previously contained a mapping
+   * for the key, the old value is replaced by the specified value.
    *
+   * @param key the key for this attribute.
+   * @param value the value for this attribute.
+   */
+  public void putAttribute(String key, AttributeValue value) {
+    // Not final because for performance reasons we want to override this in the implementation.
+    // Also a default implementation is needed to not break the compatibility (users may extend this
+    // for testing).
+    addAttributes(Collections.singletonMap(key, value));
+  }
+
+  /**
+   * Sets a set of attributes to the {@code Span}. The effect of this call is equivalent to that of
+   * calling {@link #putAttribute(String, AttributeValue)} once for each element in the specified
+   * map.
+   *
+   * @param attributes the attributes that will be added and associated with the {@code Span}.
+   */
+  public void putAttributes(Map<String, AttributeValue> attributes) {
+    // Not final because we want to start overriding this method from the beginning, this will
+    // allow us to remove the addAttributes faster.
+    addAttributes(attributes);
+  }
+
+
+  /**
+   * @deprecated Use {@link #putAttributes(Map)}
    * @param attributes the attributes that will be added and associated with the {@code Span}.
    */
+  @Deprecated
   public abstract void addAttributes(Map<String, AttributeValue> attributes);
 
   /**
@@ -97,7 +125,7 @@ public abstract class Span {
    *
    * @param description the description of the annotation time event.
    * @param attributes the attributes that will be added; these are associated with this annotation,
-   *     not the {@code Span} as for {@link #addAttributes}.
+   *     not the {@code Span} as for {@link #addAttributes(Map)}.
    */
   public abstract void addAnnotation(String description, Map<String, AttributeValue> attributes);
 
diff --git a/api/src/test/java/io/opencensus/trace/BlankSpanTest.java b/api/src/test/java/io/opencensus/trace/BlankSpanTest.java
index 8f7993ef..4e8342e2 100644
--- a/api/src/test/java/io/opencensus/trace/BlankSpanTest.java
+++ b/api/src/test/java/io/opencensus/trace/BlankSpanTest.java
@@ -44,6 +44,8 @@ public class BlankSpanTest {
     multipleAttributes.put("MyBooleanAttributeKey", AttributeValue.booleanAttributeValue(true));
     multipleAttributes.put("MyLongAttributeKey", AttributeValue.longAttributeValue(123));
     // Tests only that all the methods are not crashing/throwing errors.
+    BlankSpan.INSTANCE.putAttribute(
+        "MyStringAttributeKey2", AttributeValue.stringAttributeValue("MyStringAttributeValue2"));
     BlankSpan.INSTANCE.addAttributes(attributes);
     BlankSpan.INSTANCE.addAttributes(multipleAttributes);
     BlankSpan.INSTANCE.addAnnotation("MyAnnotation");
diff --git a/api/src/test/java/io/opencensus/trace/SpanTest.java b/api/src/test/java/io/opencensus/trace/SpanTest.java
index 1a25d6b7..5d36bb14 100644
--- a/api/src/test/java/io/opencensus/trace/SpanTest.java
+++ b/api/src/test/java/io/opencensus/trace/SpanTest.java
@@ -17,9 +17,11 @@
 package io.opencensus.trace;
 
 import static com.google.common.truth.Truth.assertThat;
+import static org.mockito.Matchers.eq;
 import static org.mockito.Matchers.same;
 import static org.mockito.Mockito.verify;
 
+import java.util.Collections;
 import java.util.EnumSet;
 import java.util.Map;
 import java.util.Random;
@@ -82,6 +84,16 @@ public class SpanTest {
   }
 
   @Test
+  public void addAttributeCallsAddAttributesByDefault() {
+    Span span = Mockito.spy(new NoopSpan(spanContext, spanOptions));
+    span.putAttribute("MyKey", AttributeValue.booleanAttributeValue(true));
+    span.end();
+    verify(span)
+        .addAttributes(
+            eq(Collections.singletonMap("MyKey", AttributeValue.booleanAttributeValue(true))));
+  }
+
+  @Test
   public void endCallsEndWithDefaultOptions() {
     Span span = Mockito.spy(new NoopSpan(spanContext, spanOptions));
     span.end();
diff --git a/benchmarks/src/jmh/java/io/opencensus/trace/RecordTraceEventsNonSampledSpanBenchmark.java b/benchmarks/src/jmh/java/io/opencensus/trace/RecordTraceEventsNonSampledSpanBenchmark.java
index 1ac66261..3819124d 100644
--- a/benchmarks/src/jmh/java/io/opencensus/trace/RecordTraceEventsNonSampledSpanBenchmark.java
+++ b/benchmarks/src/jmh/java/io/opencensus/trace/RecordTraceEventsNonSampledSpanBenchmark.java
@@ -18,7 +18,6 @@ package io.opencensus.trace;
 
 import io.opencensus.implcore.trace.SpanImpl;
 import io.opencensus.trace.samplers.Samplers;
-import java.util.HashMap;
 import java.util.concurrent.TimeUnit;
 import org.openjdk.jmh.annotations.Benchmark;
 import org.openjdk.jmh.annotations.BenchmarkMode;
@@ -58,10 +57,8 @@ public class RecordTraceEventsNonSampledSpanBenchmark {
   @Benchmark
   @BenchmarkMode(Mode.SampleTime)
   @OutputTimeUnit(TimeUnit.NANOSECONDS)
-  public Span addAttributes() {
-    HashMap<String, AttributeValue> attributes = new HashMap<String, AttributeValue>();
-    attributes.put(ATTRIBUTE_KEY, AttributeValue.stringAttributeValue(ATTRIBUTE_VALUE));
-    span.addAttributes(attributes);
+  public Span putAttribute() {
+    span.putAttribute(ATTRIBUTE_KEY, AttributeValue.stringAttributeValue(ATTRIBUTE_VALUE));
     return span;
   }
 
diff --git a/benchmarks/src/jmh/java/io/opencensus/trace/RecordTraceEventsSampledSpanBenchmark.java b/benchmarks/src/jmh/java/io/opencensus/trace/RecordTraceEventsSampledSpanBenchmark.java
index 64ef9604..05ab2e43 100644
--- a/benchmarks/src/jmh/java/io/opencensus/trace/RecordTraceEventsSampledSpanBenchmark.java
+++ b/benchmarks/src/jmh/java/io/opencensus/trace/RecordTraceEventsSampledSpanBenchmark.java
@@ -18,7 +18,6 @@ package io.opencensus.trace;
 
 import io.opencensus.implcore.trace.SpanImpl;
 import io.opencensus.trace.samplers.Samplers;
-import java.util.HashMap;
 import java.util.concurrent.TimeUnit;
 import org.openjdk.jmh.annotations.Benchmark;
 import org.openjdk.jmh.annotations.BenchmarkMode;
@@ -58,10 +57,8 @@ public class RecordTraceEventsSampledSpanBenchmark {
   @Benchmark
   @BenchmarkMode(Mode.SampleTime)
   @OutputTimeUnit(TimeUnit.NANOSECONDS)
-  public Span addAttributes() {
-    HashMap<String, AttributeValue> attributes = new HashMap<String, AttributeValue>();
-    attributes.put(ATTRIBUTE_KEY, AttributeValue.stringAttributeValue(ATTRIBUTE_VALUE));
-    span.addAttributes(attributes);
+  public Span putAttribute() {
+    span.putAttribute(ATTRIBUTE_KEY, AttributeValue.stringAttributeValue(ATTRIBUTE_VALUE));
     return span;
   }
 
diff --git a/contrib/zpages/src/main/java/io/opencensus/contrib/zpages/ZPageHttpHandler.java b/contrib/zpages/src/main/java/io/opencensus/contrib/zpages/ZPageHttpHandler.java
index d5e76bbf..9c05446b 100644
--- a/contrib/zpages/src/main/java/io/opencensus/contrib/zpages/ZPageHttpHandler.java
+++ b/contrib/zpages/src/main/java/io/opencensus/contrib/zpages/ZPageHttpHandler.java
@@ -17,7 +17,6 @@
 package io.opencensus.contrib.zpages;
 
 import com.google.common.annotations.VisibleForTesting;
-import com.google.common.collect.ImmutableMap;
 import com.sun.net.httpserver.HttpExchange;
 import com.sun.net.httpserver.HttpHandler;
 import io.opencensus.common.Scope;
@@ -56,12 +55,9 @@ final class ZPageHttpHandler implements HttpHandler {
             .startScopedSpan()) {
       tracer
           .getCurrentSpan()
-          .addAttributes(
-              ImmutableMap.<String, AttributeValue>builder()
-                  .put(
-                      "RequestMethod",
-                      AttributeValue.stringAttributeValue(httpExchange.getRequestMethod()))
-                  .build());
+          .putAttribute(
+              "/http/method ",
+              AttributeValue.stringAttributeValue(httpExchange.getRequestMethod()));
       httpExchange.sendResponseHeaders(200, 0);
       zpageHandler.emitHtml(
           uriQueryToMap(httpExchange.getRequestURI()), httpExchange.getResponseBody());
diff --git a/impl_core/src/main/java/io/opencensus/implcore/trace/SpanImpl.java b/impl_core/src/main/java/io/opencensus/implcore/trace/SpanImpl.java
index 7715a89f..0a22f726 100644
--- a/impl_core/src/main/java/io/opencensus/implcore/trace/SpanImpl.java
+++ b/impl_core/src/main/java/io/opencensus/implcore/trace/SpanImpl.java
@@ -241,16 +241,36 @@ public final class SpanImpl extends Span implements Element<SpanImpl> {
   }
 
   @Override
+  public void putAttribute(String key, AttributeValue value) {
+    if (!getOptions().contains(Options.RECORD_EVENTS)) {
+      return;
+    }
+    synchronized (this) {
+      if (hasBeenEnded) {
+        logger.log(Level.FINE, "Calling putAttributes() on an ended Span.");
+        return;
+      }
+      getInitializedAttributes().putAttribute(key, value);
+    }
+  }
+
+  @Override
+  @SuppressWarnings("deprecation")
   public void addAttributes(Map<String, AttributeValue> attributes) {
+    putAttributes(attributes);
+  }
+
+  @Override
+  public void putAttributes(Map<String, AttributeValue> attributes) {
     if (!getOptions().contains(Options.RECORD_EVENTS)) {
       return;
     }
     synchronized (this) {
       if (hasBeenEnded) {
-        logger.log(Level.FINE, "Calling addAttributes() on an ended Span.");
+        logger.log(Level.FINE, "Calling putAttributes() on an ended Span.");
         return;
       }
-      getInitializedAttributes().addAttributes(attributes);
+      getInitializedAttributes().putAttributes(attributes);
     }
   }
 
@@ -437,9 +457,16 @@ public final class SpanImpl extends Span implements Element<SpanImpl> {
       this.capacity = capacity;
     }
 
-    // Users must call this method instead of put or putAll to keep count of the total number of
-    // entries inserted.
-    private void addAttributes(Map<String, AttributeValue> attributes) {
+    // Users must call this method instead of put to keep count of the total number of entries
+    // inserted.
+    private void putAttribute(String key, AttributeValue value) {
+      totalRecordedAttributes += 1;
+      put(key, value);
+    }
+
+    // Users must call this method instead of putAll to keep count of the total number of entries
+    // inserted.
+    private void putAttributes(Map<String, AttributeValue> attributes) {
       totalRecordedAttributes += attributes.size();
       putAll(attributes);
     }
diff --git a/impl_core/src/test/java/io/opencensus/implcore/trace/SpanImplTest.java b/impl_core/src/test/java/io/opencensus/implcore/trace/SpanImplTest.java
index 6356393c..fd3dbefd 100644
--- a/impl_core/src/test/java/io/opencensus/implcore/trace/SpanImplTest.java
+++ b/impl_core/src/test/java/io/opencensus/implcore/trace/SpanImplTest.java
@@ -66,6 +66,8 @@ public class SpanImplTest {
   private final EnumSet<Options> noRecordSpanOptions = EnumSet.noneOf(Options.class);
   private final EnumSet<Options> recordSpanOptions = EnumSet.of(Options.RECORD_EVENTS);
   private final Map<String, AttributeValue> attributes = new HashMap<String, AttributeValue>();
+  private final Map<String, AttributeValue> expectedAttributes =
+      new HashMap<String, AttributeValue>();
   @Mock private StartEndHandler startEndHandler;
   @Rule public final ExpectedException exception = ExpectedException.none();
 
@@ -76,6 +78,10 @@ public class SpanImplTest {
         "MyStringAttributeKey", AttributeValue.stringAttributeValue("MyStringAttributeValue"));
     attributes.put("MyLongAttributeKey", AttributeValue.longAttributeValue(123L));
     attributes.put("MyBooleanAttributeKey", AttributeValue.booleanAttributeValue(false));
+    expectedAttributes.putAll(attributes);
+    expectedAttributes.put(
+        "MySingleStringAttributeKey",
+        AttributeValue.stringAttributeValue("MySingleStringAttributeValue"));
   }
 
   @Test
@@ -92,7 +98,7 @@ public class SpanImplTest {
             timestampConverter,
             testClock);
     // Check that adding trace events after Span#end() does not throw any exception.
-    span.addAttributes(attributes);
+    span.putAttributes(attributes);
     span.addAnnotation(Annotation.fromDescription(ANNOTATION_DESCRIPTION));
     span.addAnnotation(ANNOTATION_DESCRIPTION, attributes);
     span.addNetworkEvent(
@@ -119,7 +125,10 @@ public class SpanImplTest {
     span.end();
     // Check that adding trace events after Span#end() does not throw any exception and are not
     // recorded.
-    span.addAttributes(attributes);
+    span.putAttributes(attributes);
+    span.putAttribute(
+        "MySingleStringAttributeKey",
+        AttributeValue.stringAttributeValue("MySingleStringAttributeValue"));
     span.addAnnotation(Annotation.fromDescription(ANNOTATION_DESCRIPTION));
     span.addAnnotation(ANNOTATION_DESCRIPTION, attributes);
     span.addNetworkEvent(
@@ -136,6 +145,25 @@ public class SpanImplTest {
   }
 
   @Test
+  public void deprecatedAddAttributesStillWorks() {
+    SpanImpl span =
+        SpanImpl.startSpan(
+            spanContext,
+            recordSpanOptions,
+            SPAN_NAME,
+            parentSpanId,
+            false,
+            TraceParams.DEFAULT,
+            startEndHandler,
+            timestampConverter,
+            testClock);
+    span.addAttributes(attributes);
+    span.end();
+    SpanData spanData = span.toSpanData();
+    assertThat(spanData.getAttributes().getAttributeMap()).isEqualTo(attributes);
+  }
+
+  @Test
   public void toSpanData_ActiveSpan() {
     SpanImpl span =
         SpanImpl.startSpan(
@@ -149,7 +177,10 @@ public class SpanImplTest {
             timestampConverter,
             testClock);
     Mockito.verify(startEndHandler, Mockito.times(1)).onStart(span);
-    span.addAttributes(attributes);
+    span.putAttribute(
+        "MySingleStringAttributeKey",
+        AttributeValue.stringAttributeValue("MySingleStringAttributeValue"));
+    span.putAttributes(attributes);
     testClock.advanceTime(Duration.create(0, 100));
     span.addAnnotation(Annotation.fromDescription(ANNOTATION_DESCRIPTION));
     testClock.advanceTime(Duration.create(0, 100));
@@ -167,7 +198,7 @@ public class SpanImplTest {
     assertThat(spanData.getParentSpanId()).isEqualTo(parentSpanId);
     assertThat(spanData.getHasRemoteParent()).isTrue();
     assertThat(spanData.getAttributes().getDroppedAttributesCount()).isEqualTo(0);
-    assertThat(spanData.getAttributes().getAttributeMap()).isEqualTo(attributes);
+    assertThat(spanData.getAttributes().getAttributeMap()).isEqualTo(expectedAttributes);
     assertThat(spanData.getAnnotations().getDroppedEventsCount()).isEqualTo(0);
     assertThat(spanData.getAnnotations().getEvents().size()).isEqualTo(2);
     assertThat(spanData.getAnnotations().getEvents().get(0).getTimestamp())
@@ -205,7 +236,10 @@ public class SpanImplTest {
             timestampConverter,
             testClock);
     Mockito.verify(startEndHandler, Mockito.times(1)).onStart(span);
-    span.addAttributes(attributes);
+    span.putAttribute(
+        "MySingleStringAttributeKey",
+        AttributeValue.stringAttributeValue("MySingleStringAttributeValue"));
+    span.putAttributes(attributes);
     testClock.advanceTime(Duration.create(0, 100));
     span.addAnnotation(Annotation.fromDescription(ANNOTATION_DESCRIPTION));
     testClock.advanceTime(Duration.create(0, 100));
@@ -225,7 +259,7 @@ public class SpanImplTest {
     assertThat(spanData.getParentSpanId()).isEqualTo(parentSpanId);
     assertThat(spanData.getHasRemoteParent()).isFalse();
     assertThat(spanData.getAttributes().getDroppedAttributesCount()).isEqualTo(0);
-    assertThat(spanData.getAttributes().getAttributeMap()).isEqualTo(attributes);
+    assertThat(spanData.getAttributes().getAttributeMap()).isEqualTo(expectedAttributes);
     assertThat(spanData.getAnnotations().getDroppedEventsCount()).isEqualTo(0);
     assertThat(spanData.getAnnotations().getEvents().size()).isEqualTo(2);
     assertThat(spanData.getAnnotations().getEvents().get(0).getTimestamp())
@@ -268,7 +302,7 @@ public class SpanImplTest {
     for (int i = 0; i < 2 * maxNumberOfAttributes; i++) {
       Map<String, AttributeValue> attributes = new HashMap<String, AttributeValue>();
       attributes.put("MyStringAttributeKey" + i, AttributeValue.longAttributeValue(i));
-      span.addAttributes(attributes);
+      span.putAttributes(attributes);
     }
     SpanData spanData = span.toSpanData();
     assertThat(spanData.getAttributes().getDroppedAttributesCount())
@@ -316,7 +350,7 @@ public class SpanImplTest {
     for (int i = 0; i < 2 * maxNumberOfAttributes; i++) {
       Map<String, AttributeValue> attributes = new HashMap<String, AttributeValue>();
       attributes.put("MyStringAttributeKey" + i, AttributeValue.longAttributeValue(i));
-      span.addAttributes(attributes);
+      span.putAttributes(attributes);
     }
     SpanData spanData = span.toSpanData();
     assertThat(spanData.getAttributes().getDroppedAttributesCount())
@@ -333,7 +367,7 @@ public class SpanImplTest {
     for (int i = 0; i < maxNumberOfAttributes / 2; i++) {
       Map<String, AttributeValue> attributes = new HashMap<String, AttributeValue>();
       attributes.put("MyStringAttributeKey" + i, AttributeValue.longAttributeValue(i));
-      span.addAttributes(attributes);
+      span.putAttributes(attributes);
     }
     spanData = span.toSpanData();
     assertThat(spanData.getAttributes().getDroppedAttributesCount())