Added null checking to Span implementations (#1150)

This PR adds null checking to classes that implement Span. Specifically,

- BlankSpan
- NoopSpan
- Span
- SpanImpl

For the latter, I had to use Preconditions.checkNotNull because io.opencensus.internal shouldn't be imported into ImplCore.

4 files changed, 56 insertions, 16 deletions

diff --git a/api/src/main/java/io/opencensus/trace/BlankSpan.java b/api/src/main/java/io/opencensus/trace/BlankSpan.java
index 641bad7e..3109f013 100644
--- a/api/src/main/java/io/opencensus/trace/BlankSpan.java
+++ b/api/src/main/java/io/opencensus/trace/BlankSpan.java
@@ -16,6 +16,7 @@
 
 package io.opencensus.trace;
 
+import io.opencensus.internal.Utils;
 import java.util.Map;
 import javax.annotation.concurrent.Immutable;
 
@@ -42,19 +43,29 @@ public final class BlankSpan extends Span {
 
   /** No-op implementation of the {@link Span#putAttribute(String, AttributeValue)} method. */
   @Override
-  public void putAttribute(String key, AttributeValue value) {}
+  public void putAttribute(String key, AttributeValue value) {
+    Utils.checkNotNull(key, "key");
+    Utils.checkNotNull(value, "value");
+  }
 
   /** No-op implementation of the {@link Span#putAttributes(Map)} method. */
   @Override
-  public void putAttributes(Map<String, AttributeValue> attributes) {}
+  public void putAttributes(Map<String, AttributeValue> attributes) {
+    Utils.checkNotNull(attributes, "attributes");
+  }
 
   /** No-op implementation of the {@link Span#addAnnotation(String, Map)} method. */
   @Override
-  public void addAnnotation(String description, Map<String, AttributeValue> attributes) {}
+  public void addAnnotation(String description, Map<String, AttributeValue> attributes) {
+    Utils.checkNotNull(description, "description");
+    Utils.checkNotNull(attributes, "attributes");
+  }
 
   /** No-op implementation of the {@link Span#addAnnotation(Annotation)} method. */
   @Override
-  public void addAnnotation(Annotation annotation) {}
+  public void addAnnotation(Annotation annotation) {
+    Utils.checkNotNull(annotation, "annotation");
+  }
 
   /** No-op implementation of the {@link Span#addNetworkEvent(NetworkEvent)} method. */
   @Override
@@ -63,14 +74,20 @@ public final class BlankSpan extends Span {
 
   /** No-op implementation of the {@link Span#addMessageEvent(MessageEvent)} method. */
   @Override
-  public void addMessageEvent(MessageEvent messageEvent) {}
+  public void addMessageEvent(MessageEvent messageEvent) {
+    Utils.checkNotNull(messageEvent, "messageEvent");
+  }
 
   /** No-op implementation of the {@link Span#addLink(Link)} method. */
   @Override
-  public void addLink(Link link) {}
+  public void addLink(Link link) {
+    Utils.checkNotNull(link, "link");
+  }
 
   @Override
-  public void setStatus(Status status) {}
+  public void setStatus(Status status) {
+    Utils.checkNotNull(status, "status");
+  }
 
   /** No-op implementation of the {@link Span#end(EndSpanOptions)} method. */
   @Override
diff --git a/api/src/main/java/io/opencensus/trace/Span.java b/api/src/main/java/io/opencensus/trace/Span.java
index 4a8c6f04..cb82038d 100644
--- a/api/src/main/java/io/opencensus/trace/Span.java
+++ b/api/src/main/java/io/opencensus/trace/Span.java
@@ -96,6 +96,8 @@ public abstract class Span {
     // 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).
+    Utils.checkNotNull(key, "key");
+    Utils.checkNotNull(value, "value");
     putAttributes(Collections.singletonMap(key, value));
   }
 
@@ -110,6 +112,7 @@ public abstract class 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. All implementations MUST override this method.
+    Utils.checkNotNull(attributes, "attributes");
     addAttributes(attributes);
   }
 
@@ -130,6 +133,7 @@ public abstract class Span {
    * @since 0.5
    */
   public final void addAnnotation(String description) {
+    Utils.checkNotNull(description, "description");
     addAnnotation(description, EMPTY_ATTRIBUTES);
   }
 
@@ -180,6 +184,7 @@ public abstract class Span {
   public void addMessageEvent(MessageEvent messageEvent) {
     // Default implementation by invoking addNetworkEvent() so that any existing derived classes,
     // including implementation and the mocked ones, do not need to override this method explicitly.
+    Utils.checkNotNull(messageEvent, "messageEvent");
     addNetworkEvent(BaseMessageEventUtils.asNetworkEvent(messageEvent));
   }
 
@@ -209,6 +214,7 @@ public abstract class Span {
   public void setStatus(Status status) {
     // Implemented as no-op for backwards compatibility (for example gRPC extends Span in tests).
     // Implementation must override this method.
+    Utils.checkNotNull(status, "status");
   }
 
   /**
diff --git a/api/src/test/java/io/opencensus/trace/NoopSpan.java b/api/src/test/java/io/opencensus/trace/NoopSpan.java
index 7c8d9b7a..e90f68cd 100644
--- a/api/src/test/java/io/opencensus/trace/NoopSpan.java
+++ b/api/src/test/java/io/opencensus/trace/NoopSpan.java
@@ -16,6 +16,7 @@
 
 package io.opencensus.trace;
 
+import io.opencensus.internal.Utils;
 import java.util.EnumSet;
 import java.util.Map;
 import javax.annotation.Nullable;
@@ -33,22 +34,33 @@ public class NoopSpan extends Span {
   }
 
   @Override
-  public void putAttributes(Map<String, AttributeValue> attributes) {}
+  public void putAttributes(Map<String, AttributeValue> attributes) {
+    Utils.checkNotNull(attributes, "attributes");
+  }
 
   @Override
-  public void addAnnotation(String description, Map<String, AttributeValue> attributes) {}
+  public void addAnnotation(String description, Map<String, AttributeValue> attributes) {
+    Utils.checkNotNull(description, "description");
+    Utils.checkNotNull(attributes, "attributes");
+  }
 
   @Override
-  public void addAnnotation(Annotation annotation) {}
+  public void addAnnotation(Annotation annotation) {
+    Utils.checkNotNull(annotation, "annotation");
+  }
 
   @Override
   public void addNetworkEvent(NetworkEvent networkEvent) {}
 
   @Override
-  public void addMessageEvent(MessageEvent messageEvent) {}
+  public void addMessageEvent(MessageEvent messageEvent) {
+    Utils.checkNotNull(messageEvent, "messageEvent");
+  }
 
   @Override
-  public void addLink(Link link) {}
+  public void addLink(Link link) {
+    Utils.checkNotNull(link, "link");
+  }
 
   @Override
   public void end(EndSpanOptions options) {}
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 f9a7f859..81b27e32 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
@@ -20,6 +20,7 @@ import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Preconditions.checkState;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Preconditions;
 import com.google.common.collect.EvictingQueue;
 import io.opencensus.common.Clock;
 import io.opencensus.implcore.internal.CheckerFrameworkUtils;
@@ -281,6 +282,7 @@ public final class SpanImpl extends Span implements Element<SpanImpl> {
 
   @Override
   public void putAttributes(Map<String, AttributeValue> attributes) {
+    Preconditions.checkNotNull(attributes, "attributes");
     if (!getOptions().contains(Options.RECORD_EVENTS)) {
       return;
     }
@@ -295,6 +297,8 @@ public final class SpanImpl extends Span implements Element<SpanImpl> {
 
   @Override
   public void addAnnotation(String description, Map<String, AttributeValue> attributes) {
+    Preconditions.checkNotNull(description, "description");
+    Preconditions.checkNotNull(attributes, "attribute");
     if (!getOptions().contains(Options.RECORD_EVENTS)) {
       return;
     }
@@ -313,6 +317,7 @@ public final class SpanImpl extends Span implements Element<SpanImpl> {
 
   @Override
   public void addAnnotation(Annotation annotation) {
+    Preconditions.checkNotNull(annotation, "annotation");
     if (!getOptions().contains(Options.RECORD_EVENTS)) {
       return;
     }
@@ -322,9 +327,7 @@ public final class SpanImpl extends Span implements Element<SpanImpl> {
         return;
       }
       getInitializedAnnotations()
-          .addEvent(
-              new EventWithNanoTime<Annotation>(
-                  clock.nowNanos(), checkNotNull(annotation, "annotation")));
+          .addEvent(new EventWithNanoTime<Annotation>(clock.nowNanos(), annotation));
     }
   }
 
@@ -348,6 +351,7 @@ public final class SpanImpl extends Span implements Element<SpanImpl> {
 
   @Override
   public void addLink(Link link) {
+    Preconditions.checkNotNull(link, "link");
     if (!getOptions().contains(Options.RECORD_EVENTS)) {
       return;
     }
@@ -356,12 +360,13 @@ public final class SpanImpl extends Span implements Element<SpanImpl> {
         logger.log(Level.FINE, "Calling addLink() on an ended Span.");
         return;
       }
-      getInitializedLinks().addEvent(checkNotNull(link, "link"));
+      getInitializedLinks().addEvent(link);
     }
   }
 
   @Override
   public void setStatus(Status status) {
+    Preconditions.checkNotNull(status, "status");
     if (!getOptions().contains(Options.RECORD_EVENTS)) {
       return;
     }