diff --git a/api/src/main/java/io/opentelemetry/internal/StringUtils.java b/api/src/main/java/io/opentelemetry/internal/StringUtils.java index aa58546f8f0..627ef8e9283 100644 --- a/api/src/main/java/io/opentelemetry/internal/StringUtils.java +++ b/api/src/main/java/io/opentelemetry/internal/StringUtils.java @@ -41,5 +41,9 @@ private static boolean isPrintableChar(char ch) { return ch >= ' ' && ch <= '~'; } + public static boolean isNullOrBlank(String value) { + return value == null || value.length() == 0; + } + private StringUtils() {} } diff --git a/api/src/main/java/io/opentelemetry/trace/AttributeValue.java b/api/src/main/java/io/opentelemetry/trace/AttributeValue.java index 298545c7644..7e8167e7055 100644 --- a/api/src/main/java/io/opentelemetry/trace/AttributeValue.java +++ b/api/src/main/java/io/opentelemetry/trace/AttributeValue.java @@ -17,7 +17,7 @@ package io.opentelemetry.trace; import com.google.auto.value.AutoValue; -import io.opentelemetry.internal.Utils; +import javax.annotation.Nullable; import javax.annotation.concurrent.Immutable; /** @@ -152,8 +152,7 @@ abstract static class AttributeValueString extends AttributeValue { AttributeValueString() {} static AttributeValue create(String stringValue) { - return new AutoValue_AttributeValue_AttributeValueString( - Utils.checkNotNull(stringValue, "stringValue")); + return new AutoValue_AttributeValue_AttributeValueString(stringValue); } @Override @@ -162,6 +161,7 @@ public final Type getType() { } @Override + @Nullable public abstract String getStringValue(); } diff --git a/api/src/main/java/io/opentelemetry/trace/DefaultSpan.java b/api/src/main/java/io/opentelemetry/trace/DefaultSpan.java index a8266d7135d..9f823c8d777 100644 --- a/api/src/main/java/io/opentelemetry/trace/DefaultSpan.java +++ b/api/src/main/java/io/opentelemetry/trace/DefaultSpan.java @@ -74,7 +74,6 @@ static DefaultSpan createRandom() { @Override public void setAttribute(String key, String value) { Utils.checkNotNull(key, "key"); - Utils.checkNotNull(value, "value"); } @Override diff --git a/api/src/main/java/io/opentelemetry/trace/DefaultTracer.java b/api/src/main/java/io/opentelemetry/trace/DefaultTracer.java index b94cdf2fe81..11bab3947e2 100644 --- a/api/src/main/java/io/opentelemetry/trace/DefaultTracer.java +++ b/api/src/main/java/io/opentelemetry/trace/DefaultTracer.java @@ -134,7 +134,6 @@ public NoopSpanBuilder addLink(Link link) { @Override public NoopSpanBuilder setAttribute(String key, String value) { Utils.checkNotNull(key, "key"); - Utils.checkNotNull(value, "value"); return this; } diff --git a/api/src/main/java/io/opentelemetry/trace/Span.java b/api/src/main/java/io/opentelemetry/trace/Span.java index 7dc79cbe9e5..5ee1377ba2e 100644 --- a/api/src/main/java/io/opentelemetry/trace/Span.java +++ b/api/src/main/java/io/opentelemetry/trace/Span.java @@ -17,6 +17,7 @@ package io.opentelemetry.trace; import java.util.Map; +import javax.annotation.Nullable; import javax.annotation.concurrent.ThreadSafe; /** @@ -30,6 +31,7 @@ */ @ThreadSafe public interface Span { + /** * Type of span. Can be used to specify additional relationships between spans in addition to a * parent/child relationship. @@ -81,11 +83,14 @@ enum Kind { * 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. * + *

If a null or empty String {@code value} is passed in, the attribute will be silently + * dropped. Note: this behavior could change in the future. + * * @param key the key for this attribute. * @param value the value for this attribute. * @since 0.1.0 */ - void setAttribute(String key, String value); + void setAttribute(String key, @Nullable String value); /** * Sets an attribute to the {@code Span}. If the {@code Span} previously contained a mapping for @@ -354,6 +359,7 @@ enum Kind { * @since 0.1.0 */ interface Builder { + /** * Sets the parent {@code Span} to use. If not set, the value of {@code Tracer.getCurrentSpan()} * at {@link #startSpan()} time will be used as parent. @@ -452,14 +458,16 @@ interface Builder { * Sets an attribute to the newly created {@code Span}. If {@code Span.Builder} previously * contained a mapping for the key, the old value is replaced by the specified value. * + *

If a null or empty String {@code value} is passed in, the attribute will be silently + * dropped. Note: this behavior could change in the future. + * * @param key the key for this attribute. * @param value the value for this attribute. * @return this. * @throws NullPointerException if {@code key} is {@code null}. - * @throws NullPointerException if {@code value} is {@code null}. * @since 0.3.0 */ - Builder setAttribute(String key, String value); + Builder setAttribute(String key, @Nullable String value); /** * Sets an attribute to the newly created {@code Span}. If {@code Span.Builder} previously diff --git a/api/src/test/java/io/opentelemetry/internal/StringUtilsTest.java b/api/src/test/java/io/opentelemetry/internal/StringUtilsTest.java index 8d9c12d519b..4702102a019 100644 --- a/api/src/test/java/io/opentelemetry/internal/StringUtilsTest.java +++ b/api/src/test/java/io/opentelemetry/internal/StringUtilsTest.java @@ -16,8 +16,7 @@ package io.opentelemetry.internal; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; +import static com.google.common.truth.Truth.assertThat; import org.junit.Test; import org.junit.runner.RunWith; @@ -29,7 +28,14 @@ public final class StringUtilsTest { @Test public void isPrintableString() { - assertTrue(StringUtils.isPrintableString("abcd")); - assertFalse(StringUtils.isPrintableString("\2ab\3cd")); + assertThat(StringUtils.isPrintableString("abcd")).isTrue(); + assertThat(StringUtils.isPrintableString("\2ab\3cd")).isFalse(); + } + + @Test + public void isNullOrEmpty() throws Exception { + assertThat(StringUtils.isNullOrBlank("")).isTrue(); + assertThat(StringUtils.isNullOrBlank(null)).isTrue(); + assertThat(StringUtils.isNullOrBlank("hello")).isFalse(); } } diff --git a/api/src/test/java/io/opentelemetry/trace/DefaultSpanTest.java b/api/src/test/java/io/opentelemetry/trace/DefaultSpanTest.java index 18bb48c036e..d49abdad7a4 100644 --- a/api/src/test/java/io/opentelemetry/trace/DefaultSpanTest.java +++ b/api/src/test/java/io/opentelemetry/trace/DefaultSpanTest.java @@ -53,6 +53,8 @@ public void doNotCrash() { "MyStringAttributeKey", AttributeValue.stringAttributeValue("MyStringAttributeValue")); span.setAttribute("MyBooleanAttributeKey", AttributeValue.booleanAttributeValue(true)); span.setAttribute("MyLongAttributeKey", AttributeValue.longAttributeValue(123)); + span.setAttribute("NullString", (String) null); + span.setAttribute("EmptyString", ""); span.addEvent("event"); span.addEvent("event", 0); span.addEvent( diff --git a/sdk/src/main/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpan.java b/sdk/src/main/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpan.java index 687792a46d0..fae3b265be1 100644 --- a/sdk/src/main/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpan.java +++ b/sdk/src/main/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpan.java @@ -18,11 +18,13 @@ import com.google.common.base.Preconditions; import com.google.common.collect.EvictingQueue; +import io.opentelemetry.internal.StringUtils; import io.opentelemetry.sdk.common.Clock; import io.opentelemetry.sdk.common.InstrumentationLibraryInfo; import io.opentelemetry.sdk.resources.Resource; import io.opentelemetry.sdk.trace.config.TraceConfig; import io.opentelemetry.trace.AttributeValue; +import io.opentelemetry.trace.AttributeValue.Type; import io.opentelemetry.trace.EndSpanOptions; import io.opentelemetry.trace.Event; import io.opentelemetry.trace.Link; @@ -325,6 +327,9 @@ public void setAttribute(String key, boolean value) { public void setAttribute(String key, AttributeValue value) { Preconditions.checkNotNull(key, "key"); Preconditions.checkNotNull(value, "value"); + if (value.getType() == Type.STRING && StringUtils.isNullOrBlank(value.getStringValue())) { + return; + } synchronized (lock) { if (hasEnded) { logger.log(Level.FINE, "Calling setAttribute() on an ended Span."); diff --git a/sdk/src/main/java/io/opentelemetry/sdk/trace/SpanBuilderSdk.java b/sdk/src/main/java/io/opentelemetry/sdk/trace/SpanBuilderSdk.java index d00af339861..080829b110e 100644 --- a/sdk/src/main/java/io/opentelemetry/sdk/trace/SpanBuilderSdk.java +++ b/sdk/src/main/java/io/opentelemetry/sdk/trace/SpanBuilderSdk.java @@ -16,6 +16,7 @@ package io.opentelemetry.sdk.trace; +import io.opentelemetry.internal.StringUtils; import io.opentelemetry.internal.Utils; import io.opentelemetry.sdk.common.Clock; import io.opentelemetry.sdk.common.InstrumentationLibraryInfo; @@ -24,6 +25,7 @@ import io.opentelemetry.sdk.trace.Sampler.Decision; import io.opentelemetry.sdk.trace.config.TraceConfig; import io.opentelemetry.trace.AttributeValue; +import io.opentelemetry.trace.AttributeValue.Type; import io.opentelemetry.trace.DefaultSpan; import io.opentelemetry.trace.Link; import io.opentelemetry.trace.Span; @@ -159,6 +161,9 @@ public Span.Builder setAttribute(String key, boolean value) { public Span.Builder setAttribute(String key, AttributeValue value) { Utils.checkNotNull(key, "key"); Utils.checkNotNull(value, "value"); + if (value.getType() == Type.STRING && StringUtils.isNullOrBlank(value.getStringValue())) { + return this; + } attributes.putAttribute(key, value); return this; } diff --git a/sdk/src/test/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpanTest.java b/sdk/src/test/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpanTest.java index 9ddf3f91472..0fa27a3a634 100644 --- a/sdk/src/test/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpanTest.java +++ b/sdk/src/test/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpanTest.java @@ -286,6 +286,10 @@ public void setAttribute() { RecordEventsReadableSpan span = createTestRootSpan(); try { span.setAttribute("StringKey", "StringVal"); + span.setAttribute("NullStringKey", (String) null); + span.setAttribute("EmptyStringKey", ""); + span.setAttribute("NullStringAttributeValue", AttributeValue.stringAttributeValue(null)); + span.setAttribute("EmptyStringAttributeValue", AttributeValue.stringAttributeValue("")); span.setAttribute("LongKey", 1000L); span.setAttribute("DoubleKey", 10.0); span.setAttribute("BooleanKey", false); diff --git a/sdk/src/test/java/io/opentelemetry/sdk/trace/SpanBuilderSdkTest.java b/sdk/src/test/java/io/opentelemetry/sdk/trace/SpanBuilderSdkTest.java index f6d456fe557..da0d7c05c7a 100644 --- a/sdk/src/test/java/io/opentelemetry/sdk/trace/SpanBuilderSdkTest.java +++ b/sdk/src/test/java/io/opentelemetry/sdk/trace/SpanBuilderSdkTest.java @@ -169,6 +169,17 @@ public void setAttribute() { } } + @Test + public void setAttribute_nullStringValue() throws Exception { + Span.Builder spanBuilder = tracerSdk.spanBuilder(SPAN_NAME); + spanBuilder.setAttribute("emptyString", ""); + spanBuilder.setAttribute("nullString", (String) null); + spanBuilder.setAttribute("nullStringAttributeValue", AttributeValue.stringAttributeValue(null)); + spanBuilder.setAttribute("emptyStringAttributeValue", AttributeValue.stringAttributeValue("")); + RecordEventsReadableSpan span = (RecordEventsReadableSpan) spanBuilder.startSpan(); + assertThat(span.toSpanData().getAttributes()).isEmpty(); + } + @Test public void droppingAttributes() { final int maxNumberOfAttrs = 8;