Remove null checks in Span API. (#1433)

* Remove null checks in Span API.

* Add tests and loosen events as well.
This commit is contained in:
Anuraag Agrawal 2020-07-21 11:00:13 +09:00 committed by GitHub
parent 1b0f20d647
commit f5452f4f2b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 70 additions and 58 deletions

View File

@ -18,7 +18,6 @@ package io.opentelemetry.trace;
import io.opentelemetry.common.AttributeValue;
import io.opentelemetry.common.Attributes;
import io.opentelemetry.internal.Utils;
import java.util.Random;
import javax.annotation.concurrent.Immutable;
@ -73,81 +72,49 @@ public final class DefaultSpan implements Span {
}
@Override
public void setAttribute(String key, String value) {
Utils.checkNotNull(key, "key");
}
public void setAttribute(String key, String value) {}
@Override
public void setAttribute(String key, long value) {
Utils.checkNotNull(key, "key");
}
public void setAttribute(String key, long value) {}
@Override
public void setAttribute(String key, double value) {
Utils.checkNotNull(key, "key");
}
public void setAttribute(String key, double value) {}
@Override
public void setAttribute(String key, boolean value) {
Utils.checkNotNull(key, "key");
}
public void setAttribute(String key, boolean value) {}
@Override
public void setAttribute(String key, AttributeValue value) {
Utils.checkNotNull(key, "key");
Utils.checkNotNull(value, "value");
}
public void setAttribute(String key, AttributeValue value) {}
@Override
public void addEvent(String name) {}
@Override
public void addEvent(String name, long timestamp) {
Utils.checkNotNull(name, "name");
Utils.checkArgument(timestamp >= 0, "Negative timestamp");
}
public void addEvent(String name, long timestamp) {}
@Override
public void addEvent(String name, Attributes attributes) {
Utils.checkNotNull(name, "name");
Utils.checkNotNull(attributes, "attributes");
}
public void addEvent(String name, Attributes attributes) {}
@Override
public void addEvent(String name, Attributes attributes, long timestamp) {
Utils.checkNotNull(name, "name");
Utils.checkNotNull(attributes, "attributes");
Utils.checkArgument(timestamp >= 0, "Negative timestamp");
}
public void addEvent(String name, Attributes attributes, long timestamp) {}
@Override
public void addEvent(Event event) {
Utils.checkNotNull(event, "event");
}
public void addEvent(Event event) {}
@Override
public void addEvent(Event event, long timestamp) {
Utils.checkNotNull(event, "event");
Utils.checkArgument(timestamp >= 0, "Negative timestamp");
}
public void addEvent(Event event, long timestamp) {}
@Override
public void setStatus(Status status) {
Utils.checkNotNull(status, "status");
}
public void setStatus(Status status) {}
@Override
public void updateName(String name) {
Utils.checkNotNull(name, "name");
}
public void updateName(String name) {}
@Override
public void end() {}
@Override
public void end(EndSpanOptions endOptions) {
Utils.checkNotNull(endOptions, "endOptions");
}
public void end(EndSpanOptions endOptions) {}
@Override
public SpanContext getContext() {

View File

@ -59,6 +59,7 @@ public class DefaultSpanTest {
span.setAttribute("NullArrayBoolean", AttributeValue.arrayAttributeValue((Boolean[]) null));
span.setAttribute("NullArrayLong", AttributeValue.arrayAttributeValue((Long[]) null));
span.setAttribute("NullArrayDouble", AttributeValue.arrayAttributeValue((Double[]) null));
span.setAttribute(null, (String) null);
span.addEvent("event");
span.addEvent("event", 0);
span.addEvent(
@ -70,9 +71,11 @@ public class DefaultSpanTest {
0);
span.addEvent(new TestEvent());
span.addEvent(new TestEvent(), 0);
span.addEvent((Event) null);
span.setStatus(Status.OK);
span.end();
span.end(EndSpanOptions.getDefault());
span.end(null);
}
@Test
@ -81,13 +84,6 @@ public class DefaultSpanTest {
assertThat(span.toString()).isEqualTo("DefaultSpan");
}
@Test
public void defaultSpan_NullEndSpanOptions() {
DefaultSpan span = DefaultSpan.getInvalid();
thrown.expect(NullPointerException.class);
span.end(null);
}
static final class TestEvent implements Event {
@Override
public String getName() {

View File

@ -18,7 +18,6 @@ package io.opentelemetry.sdk.trace;
import static io.opentelemetry.common.AttributeValue.Type.STRING;
import com.google.common.base.Preconditions;
import com.google.common.collect.EvictingQueue;
import io.opentelemetry.common.AttributeValue;
import io.opentelemetry.common.Attributes;
@ -300,7 +299,9 @@ final class RecordEventsReadableSpan implements ReadableSpan, Span {
@Override
public void setAttribute(String key, AttributeValue value) {
Preconditions.checkNotNull(key, "key");
if (key == null) {
return;
}
synchronized (lock) {
if (hasEnded) {
logger.log(Level.FINE, "Calling setAttribute() on an ended Span.");
@ -322,16 +323,25 @@ final class RecordEventsReadableSpan implements ReadableSpan, Span {
@Override
public void addEvent(String name) {
if (name == null) {
return;
}
addTimedEvent(TimedEvent.create(clock.now(), name, Attributes.empty(), 0));
}
@Override
public void addEvent(String name, long timestamp) {
if (name == null) {
return;
}
addTimedEvent(TimedEvent.create(timestamp, name, Attributes.empty(), 0));
}
@Override
public void addEvent(String name, Attributes attributes) {
if (name == null) {
return;
}
int totalAttributeCount = attributes.size();
addTimedEvent(
TimedEvent.create(
@ -343,6 +353,9 @@ final class RecordEventsReadableSpan implements ReadableSpan, Span {
@Override
public void addEvent(String name, Attributes attributes, long timestamp) {
if (name == null) {
return;
}
int totalAttributeCount = attributes.size();
addTimedEvent(
TimedEvent.create(
@ -354,11 +367,17 @@ final class RecordEventsReadableSpan implements ReadableSpan, Span {
@Override
public void addEvent(io.opentelemetry.trace.Event event) {
if (event == null) {
return;
}
addTimedEvent(TimedEvent.create(clock.now(), event));
}
@Override
public void addEvent(io.opentelemetry.trace.Event event, long timestamp) {
if (event == null) {
return;
}
addTimedEvent(TimedEvent.create(timestamp, event));
}
@ -385,7 +404,9 @@ final class RecordEventsReadableSpan implements ReadableSpan, Span {
@Override
public void setStatus(Status status) {
Preconditions.checkNotNull(status, "status");
if (status == null) {
return;
}
synchronized (lock) {
if (hasEnded) {
logger.log(Level.FINE, "Calling setStatus() on an ended Span.");
@ -397,7 +418,9 @@ final class RecordEventsReadableSpan implements ReadableSpan, Span {
@Override
public void updateName(String name) {
Preconditions.checkNotNull(name, "name");
if (name == null) {
return;
}
synchronized (lock) {
if (hasEnded) {
logger.log(Level.FINE, "Calling updateName() on an ended Span.");
@ -414,7 +437,10 @@ final class RecordEventsReadableSpan implements ReadableSpan, Span {
@Override
public void end(EndSpanOptions endOptions) {
Preconditions.checkNotNull(endOptions, "endOptions");
if (endOptions == null) {
end();
return;
}
endInternal(endOptions.getEndTimestamp() == 0 ? clock.now() : endOptions.getEndTimestamp());
}

View File

@ -562,6 +562,29 @@ public class RecordEventsReadableSpanTest {
}
}
@Test
public void badArgsIgnored() {
RecordEventsReadableSpan span = createTestRootSpan();
// Should be no exceptions
span.setAttribute(null, 0);
span.setStatus(null);
span.updateName(null);
span.addEvent((Event) null);
span.addEvent((String) null);
span.addEvent((Event) null, 0);
span.addEvent((String) null, 0);
span.addEvent(null, null);
span.addEvent(null, null, 0);
span.end(null);
// Ignored the bad calls
SpanData data = span.toSpanData();
assertThat(data.getAttributes().isEmpty()).isTrue();
assertThat(data.getStatus()).isEqualTo(Status.OK);
assertThat(data.getName()).isEqualTo(SPAN_NAME);
}
private RecordEventsReadableSpan createTestSpanWithAttributes(
Map<String, AttributeValue> attributes) {
AttributesMap attributesMap =