From cb831daf49b826ba5ae31638e8404a8e816bada6 Mon Sep 17 00:00:00 2001 From: Giovanni Liva Date: Wed, 29 Jan 2020 09:19:47 +0100 Subject: [PATCH 01/17] [API] - Add Arrays for span attributes --- .../opentelemetry/common/AttributeValue.java | 189 +++++++++++++++++- .../io/opentelemetry/trace/DefaultSpan.java | 20 ++ .../java/io/opentelemetry/trace/Span.java | 43 ++++ 3 files changed, 251 insertions(+), 1 deletion(-) diff --git a/api/src/main/java/io/opentelemetry/common/AttributeValue.java b/api/src/main/java/io/opentelemetry/common/AttributeValue.java index bf86a586cfb..eef5db118f1 100644 --- a/api/src/main/java/io/opentelemetry/common/AttributeValue.java +++ b/api/src/main/java/io/opentelemetry/common/AttributeValue.java @@ -17,6 +17,9 @@ package io.opentelemetry.common; import com.google.auto.value.AutoValue; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; import javax.annotation.Nullable; import javax.annotation.concurrent.Immutable; @@ -39,7 +42,11 @@ public enum Type { STRING, BOOLEAN, LONG, - DOUBLE + DOUBLE, + STRING_ARRAY, + BOOLEAN_ARRAY, + LONG_ARRAY, + DOUBLE_ARRAY } /** @@ -86,6 +93,50 @@ public static AttributeValue doubleAttributeValue(double doubleValue) { return AttributeValueDouble.create(doubleValue); } + /** + * Returns an {@code AttributeValue} with a String array value. + * + * @param stringValues The new values. + * @return an {@code AttributeValue} with a String array value. + * @since 0.1.0 + */ + public static AttributeValue arrayAttributeValue(String... stringValues) { + return AttributeValueStringArray.create(stringValues); + } + + /** + * Returns an {@code AttributeValue} with a boolean array value. + * + * @param booleanValues The new values. + * @return an {@code AttributeValue} with a boolean array value. + * @since 0.1.0 + */ + public static AttributeValue arrayAttributeValue(boolean... booleanValues) { + return AttributeValueBooleanArray.create(booleanValues); + } + + /** + * Returns an {@code AttributeValue} with a long array value. + * + * @param longValues The new values. + * @return an {@code AttributeValue} with a long array value. + * @since 0.1.0 + */ + public static AttributeValue arrayAttributeValue(long... longValues) { + return AttributeValueLongArray.create(longValues); + } + + /** + * Returns an {@code AttributeValue} with a double array value. + * + * @param doubleValues The new values. + * @return an {@code AttributeValue} with a double array value. + * @since 0.1.0 + */ + public static AttributeValue arrayAttributeValue(double... doubleValues) { + return AttributeValueDoubleArray.create(doubleValues); + } + AttributeValue() {} /** @@ -136,6 +187,54 @@ public double getDoubleValue() { String.format("This type can only return %s data", getType().name())); } + /** + * Returns the String array value of this {@code AttributeValue}. An UnsupportedOperationException + * will be thrown if getType() is not {@link Type#STRING_ARRAY}. + * + * @return the array values of this {@code AttributeValue}. + * @since 0.1.0 + */ + public List getStringArrayValue() { + throw new UnsupportedOperationException( + String.format("This type can only return %s data", getType().name())); + } + + /** + * Returns the boolean array value of this {@code AttributeValue}. An + * UnsupportedOperationException will be thrown if getType() is not {@link Type#BOOLEAN_ARRAY}. + * + * @return the array values of this {@code AttributeValue}. + * @since 0.1.0 + */ + public List getBooleanArrayValue() { + throw new UnsupportedOperationException( + String.format("This type can only return %s data", getType().name())); + } + + /** + * Returns the long array value of this {@code AttributeValue}. An UnsupportedOperationException + * will be thrown if getType() is not {@link Type#LONG_ARRAY}. + * + * @return the array values of this {@code AttributeValue}. + * @since 0.1.0 + */ + public List getLongArrayValue() { + throw new UnsupportedOperationException( + String.format("This type can only return %s data", getType().name())); + } + + /** + * Returns the double array value of this {@code AttributeValue}. An UnsupportedOperationException + * will be thrown if getType() is not {@link Type#DOUBLE_ARRAY}. + * + * @return the array values of this {@code AttributeValue}. + * @since 0.1.0 + */ + public List getDoubleArrayValue() { + throw new UnsupportedOperationException( + String.format("This type can only return %s data", getType().name())); + } + /** * Returns a {@code Type} corresponding to the underlying value of this {@code AttributeValue}. * @@ -220,4 +319,92 @@ public final Type getType() { @Override public abstract double getDoubleValue(); } + + @Immutable + @AutoValue + abstract static class AttributeValueStringArray extends AttributeValue { + + AttributeValueStringArray() {} + + static AttributeValue create(String... stringValues) { + return new AutoValue_AttributeValue_AttributeValueStringArray(Arrays.asList(stringValues)); + } + + @Override + public final Type getType() { + return Type.STRING_ARRAY; + } + + @Override + public abstract List getStringArrayValue(); + } + + @Immutable + @AutoValue + abstract static class AttributeValueBooleanArray extends AttributeValue { + + AttributeValueBooleanArray() {} + + static AttributeValue create(boolean... booleanValues) { + List values = new ArrayList<>(booleanValues.length); + for (boolean value : booleanValues) { + values.add(value); + } + return new AutoValue_AttributeValue_AttributeValueBooleanArray(values); + } + + @Override + public final Type getType() { + return Type.BOOLEAN_ARRAY; + } + + @Override + public abstract List getBooleanArrayValue(); + } + + @Immutable + @AutoValue + abstract static class AttributeValueLongArray extends AttributeValue { + + AttributeValueLongArray() {} + + static AttributeValue create(long... longValues) { + List values = new ArrayList<>(longValues.length); + for (long value : longValues) { + values.add(value); + } + return new AutoValue_AttributeValue_AttributeValueLongArray(values); + } + + @Override + public final Type getType() { + return Type.LONG_ARRAY; + } + + @Override + public abstract List getLongArrayValue(); + } + + @Immutable + @AutoValue + abstract static class AttributeValueDoubleArray extends AttributeValue { + + AttributeValueDoubleArray() {} + + static AttributeValue create(double... doubleValues) { + List values = new ArrayList<>(doubleValues.length); + for (double value : doubleValues) { + values.add(value); + } + return new AutoValue_AttributeValue_AttributeValueDoubleArray(values); + } + + @Override + public final Type getType() { + return Type.DOUBLE_ARRAY; + } + + @Override + public abstract List getDoubleArrayValue(); + } } diff --git a/api/src/main/java/io/opentelemetry/trace/DefaultSpan.java b/api/src/main/java/io/opentelemetry/trace/DefaultSpan.java index cfdd4dc720a..58fbb9fe8b7 100644 --- a/api/src/main/java/io/opentelemetry/trace/DefaultSpan.java +++ b/api/src/main/java/io/opentelemetry/trace/DefaultSpan.java @@ -92,6 +92,26 @@ public void setAttribute(String key, boolean value) { Utils.checkNotNull(key, "key"); } + @Override + public void setAttribute(String key, String... values) { + Utils.checkNotNull(key, "key"); + } + + @Override + public void setAttribute(String key, long... values) { + Utils.checkNotNull(key, "key"); + } + + @Override + public void setAttribute(String key, double... values) { + Utils.checkNotNull(key, "key"); + } + + @Override + public void setAttribute(String key, boolean... values) { + Utils.checkNotNull(key, "key"); + } + @Override public void setAttribute(String key, AttributeValue value) { Utils.checkNotNull(key, "key"); diff --git a/api/src/main/java/io/opentelemetry/trace/Span.java b/api/src/main/java/io/opentelemetry/trace/Span.java index b18949eb197..5b358d45b4a 100644 --- a/api/src/main/java/io/opentelemetry/trace/Span.java +++ b/api/src/main/java/io/opentelemetry/trace/Span.java @@ -123,6 +123,49 @@ enum Kind { */ void setAttribute(String key, boolean value); + /** + * 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 values the value for this attribute. + * @since 0.1.0 + */ + void setAttribute(String key, String... values); + + /** + * 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 values the value for this attribute. + * @since 0.1.0 + */ + void setAttribute(String key, long... values); + + /** + * 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 values the value for this attribute. + * @since 0.1.0 + */ + void setAttribute(String key, double... values); + + /** + * 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 values the value for this attribute. + * @since 0.1.0 + */ + void setAttribute(String key, boolean... values); + /** * 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. From 7e0c4e2803f7c5c04a12d74b18f34b4033122a9c Mon Sep 17 00:00:00 2001 From: Giovanni Liva Date: Wed, 29 Jan 2020 09:22:18 +0100 Subject: [PATCH 02/17] [SDK] - Add Arrays for span attributes --- .../sdk/trace/RecordEventsReadableSpan.java | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) 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 ef173620949..b7467f697aa 100644 --- a/sdk/src/main/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpan.java +++ b/sdk/src/main/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpan.java @@ -329,6 +329,26 @@ public void setAttribute(String key, boolean value) { setAttribute(key, AttributeValue.booleanAttributeValue(value)); } + @Override + public void setAttribute(String key, String... values) { + setAttribute(key, AttributeValue.arrayAttributeValue(values)); + } + + @Override + public void setAttribute(String key, long... values) { + setAttribute(key, AttributeValue.arrayAttributeValue(values)); + } + + @Override + public void setAttribute(String key, double... values) { + setAttribute(key, AttributeValue.arrayAttributeValue(values)); + } + + @Override + public void setAttribute(String key, boolean... values) { + setAttribute(key, AttributeValue.arrayAttributeValue(values)); + } + @Override public void setAttribute(String key, AttributeValue value) { Preconditions.checkNotNull(key, "key"); From 721f255372aa6f822113c599bf73d7afd91b8aa2 Mon Sep 17 00:00:00 2001 From: Giovanni Liva Date: Wed, 29 Jan 2020 10:17:12 +0100 Subject: [PATCH 03/17] [Exporters/Shim] - Add Arrays for span attributes --- .../exporters/jaeger/Adapter.java | 28 +++++++++++++++++-- .../opentracingshim/testbed/TestUtils.java | 8 ++++++ .../sdk/contrib/trace/testbed/TestUtils.java | 8 ++++++ 3 files changed, 42 insertions(+), 2 deletions(-) diff --git a/exporters/jaeger/src/main/java/io/opentelemetry/exporters/jaeger/Adapter.java b/exporters/jaeger/src/main/java/io/opentelemetry/exporters/jaeger/Adapter.java index 848e8406e82..213b5c2d246 100644 --- a/exporters/jaeger/src/main/java/io/opentelemetry/exporters/jaeger/Adapter.java +++ b/exporters/jaeger/src/main/java/io/opentelemetry/exporters/jaeger/Adapter.java @@ -155,7 +155,30 @@ static Model.Log toJaegerLog(TimedEvent timedEvent) { static Collection toKeyValues(Map attributes) { ArrayList tags = new ArrayList<>(attributes.size()); for (Entry entry : attributes.entrySet()) { - tags.add(toKeyValue(entry.getKey(), entry.getValue())); + switch (entry.getValue().getType()) { + case STRING_ARRAY: + for (String value : entry.getValue().getStringArrayValue()) { + tags.add(toKeyValue(entry.getKey(), AttributeValue.stringAttributeValue(value))); + } + break; + case BOOLEAN_ARRAY: + for (boolean value : entry.getValue().getBooleanArrayValue()) { + tags.add(toKeyValue(entry.getKey(), AttributeValue.booleanAttributeValue(value))); + } + break; + case LONG_ARRAY: + for (long value : entry.getValue().getLongArrayValue()) { + tags.add(toKeyValue(entry.getKey(), AttributeValue.doubleAttributeValue(value))); + } + break; + case DOUBLE_ARRAY: + for (double value : entry.getValue().getDoubleArrayValue()) { + tags.add(toKeyValue(entry.getKey(), AttributeValue.doubleAttributeValue(value))); + } + break; + default: + tags.add(toKeyValue(entry.getKey(), entry.getValue())); + } } return tags; } @@ -189,8 +212,9 @@ static Model.KeyValue toKeyValue(String key, AttributeValue value) { builder.setVFloat64(value.getDoubleValue()); builder.setVType(Model.ValueType.FLOAT64); break; + default: + break; } - return builder.build(); } diff --git a/opentracing_shim/src/test/java/io/opentelemetry/opentracingshim/testbed/TestUtils.java b/opentracing_shim/src/test/java/io/opentelemetry/opentracingshim/testbed/TestUtils.java index f7ca497b11c..2f70d8f9304 100644 --- a/opentracing_shim/src/test/java/io/opentelemetry/opentracingshim/testbed/TestUtils.java +++ b/opentracing_shim/src/test/java/io/opentelemetry/opentracingshim/testbed/TestUtils.java @@ -67,6 +67,14 @@ public boolean check(SpanData span) { return value.equals(attrValue.getBooleanValue()); case DOUBLE: return value.equals(attrValue.getDoubleValue()); + case STRING_ARRAY: + return value.equals(attrValue.getStringArrayValue()); + case LONG_ARRAY: + return value.equals(attrValue.getLongArrayValue()); + case BOOLEAN_ARRAY: + return value.equals(attrValue.getBooleanArrayValue()); + case DOUBLE_ARRAY: + return value.equals(attrValue.getDoubleArrayValue()); } return false; diff --git a/sdk_contrib/testbed/src/test/java/io/opentelemetry/sdk/contrib/trace/testbed/TestUtils.java b/sdk_contrib/testbed/src/test/java/io/opentelemetry/sdk/contrib/trace/testbed/TestUtils.java index 43d2de9be6b..c76f8b1ba76 100644 --- a/sdk_contrib/testbed/src/test/java/io/opentelemetry/sdk/contrib/trace/testbed/TestUtils.java +++ b/sdk_contrib/testbed/src/test/java/io/opentelemetry/sdk/contrib/trace/testbed/TestUtils.java @@ -66,6 +66,14 @@ public boolean check(SpanData span) { return value.equals(attrValue.getDoubleValue()); case LONG: return value.equals(attrValue.getLongValue()); + case STRING_ARRAY: + return value.equals(attrValue.getStringArrayValue()); + case LONG_ARRAY: + return value.equals(attrValue.getLongArrayValue()); + case BOOLEAN_ARRAY: + return value.equals(attrValue.getBooleanArrayValue()); + case DOUBLE_ARRAY: + return value.equals(attrValue.getDoubleArrayValue()); } return false; } From 95b2b8d1e2a51ac26200bd5a412c66bd3de685b4 Mon Sep 17 00:00:00 2001 From: Giovanni Liva Date: Thu, 30 Jan 2020 08:17:43 +0100 Subject: [PATCH 04/17] add tests --- .../common/AttributeValueTest.java | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/api/src/test/java/io/opentelemetry/common/AttributeValueTest.java b/api/src/test/java/io/opentelemetry/common/AttributeValueTest.java index c9755b9700d..230529dfb4e 100644 --- a/api/src/test/java/io/opentelemetry/common/AttributeValueTest.java +++ b/api/src/test/java/io/opentelemetry/common/AttributeValueTest.java @@ -43,6 +43,24 @@ public void attributeValue_EqualsAndHashCode() { tester.addEqualityGroup( AttributeValue.doubleAttributeValue(1.23456), AttributeValue.doubleAttributeValue(1.23456)); tester.addEqualityGroup(AttributeValue.doubleAttributeValue(1.234567)); + tester.addEqualityGroup( + AttributeValue.arrayAttributeValue( + "MyArrayStringAttributeValue1", "MyArrayStringAttributeValue2"), + AttributeValue.arrayAttributeValue( + "MyArrayStringAttributeValue1", "MyArrayStringAttributeValue2")); + tester.addEqualityGroup(AttributeValue.arrayAttributeValue("MyArrayStringAttributeDiffValue")); + tester.addEqualityGroup( + AttributeValue.arrayAttributeValue(true, false, true), + AttributeValue.arrayAttributeValue(true, false, true)); + tester.addEqualityGroup(AttributeValue.arrayAttributeValue(false)); + tester.addEqualityGroup( + AttributeValue.arrayAttributeValue(123456L, 7890L), + AttributeValue.arrayAttributeValue(123456L, 7890L)); + tester.addEqualityGroup(AttributeValue.arrayAttributeValue(1234567L)); + tester.addEqualityGroup( + AttributeValue.arrayAttributeValue(1.23456, 7.890), + AttributeValue.arrayAttributeValue(1.23456, 7.890)); + tester.addEqualityGroup(AttributeValue.arrayAttributeValue(1.234567)); tester.testEquals(); } @@ -56,5 +74,19 @@ public void attributeValue_ToString() { assertThat(attribute.toString()).contains("123456"); attribute = AttributeValue.doubleAttributeValue(1.23456); assertThat(attribute.toString()).contains("1.23456"); + attribute = + AttributeValue.arrayAttributeValue( + "MyArrayStringAttributeValue1", "MyArrayStringAttributeValue2"); + assertThat(attribute.toString()).contains("MyArrayStringAttributeValue1"); + assertThat(attribute.toString()).contains("MyArrayStringAttributeValue2"); + attribute = AttributeValue.arrayAttributeValue(true, false); + assertThat(attribute.toString()).contains("true"); + assertThat(attribute.toString()).contains("false"); + attribute = AttributeValue.arrayAttributeValue(12345L, 67890L); + assertThat(attribute.toString()).contains("12345"); + assertThat(attribute.toString()).contains("67890"); + attribute = AttributeValue.arrayAttributeValue(1.2345, 6.789); + assertThat(attribute.toString()).contains("1.2345"); + assertThat(attribute.toString()).contains("6.789"); } } From aa68a8f3d665d90ec51153e22dc592ff5b943d96 Mon Sep 17 00:00:00 2001 From: Giovanni Liva Date: Thu, 30 Jan 2020 10:47:42 +0100 Subject: [PATCH 05/17] Adjust jaeger exporter to specification. Add tests. --- .../exporters/jaeger/Adapter.java | 42 ++++++++----------- .../exporters/jaeger/AdapterTest.java | 20 +++++++++ 2 files changed, 37 insertions(+), 25 deletions(-) diff --git a/exporters/jaeger/src/main/java/io/opentelemetry/exporters/jaeger/Adapter.java b/exporters/jaeger/src/main/java/io/opentelemetry/exporters/jaeger/Adapter.java index 213b5c2d246..37f5cc10d60 100644 --- a/exporters/jaeger/src/main/java/io/opentelemetry/exporters/jaeger/Adapter.java +++ b/exporters/jaeger/src/main/java/io/opentelemetry/exporters/jaeger/Adapter.java @@ -17,6 +17,7 @@ package io.opentelemetry.exporters.jaeger; import com.google.common.annotations.VisibleForTesting; +import com.google.gson.Gson; import com.google.protobuf.Timestamp; import com.google.protobuf.util.Timestamps; import io.opentelemetry.common.AttributeValue; @@ -155,30 +156,7 @@ static Model.Log toJaegerLog(TimedEvent timedEvent) { static Collection toKeyValues(Map attributes) { ArrayList tags = new ArrayList<>(attributes.size()); for (Entry entry : attributes.entrySet()) { - switch (entry.getValue().getType()) { - case STRING_ARRAY: - for (String value : entry.getValue().getStringArrayValue()) { - tags.add(toKeyValue(entry.getKey(), AttributeValue.stringAttributeValue(value))); - } - break; - case BOOLEAN_ARRAY: - for (boolean value : entry.getValue().getBooleanArrayValue()) { - tags.add(toKeyValue(entry.getKey(), AttributeValue.booleanAttributeValue(value))); - } - break; - case LONG_ARRAY: - for (long value : entry.getValue().getLongArrayValue()) { - tags.add(toKeyValue(entry.getKey(), AttributeValue.doubleAttributeValue(value))); - } - break; - case DOUBLE_ARRAY: - for (double value : entry.getValue().getDoubleArrayValue()) { - tags.add(toKeyValue(entry.getKey(), AttributeValue.doubleAttributeValue(value))); - } - break; - default: - tags.add(toKeyValue(entry.getKey(), entry.getValue())); - } + tags.add(toKeyValue(entry.getKey(), entry.getValue())); } return tags; } @@ -212,7 +190,21 @@ static Model.KeyValue toKeyValue(String key, AttributeValue value) { builder.setVFloat64(value.getDoubleValue()); builder.setVType(Model.ValueType.FLOAT64); break; - default: + case STRING_ARRAY: + builder.setVStr(new Gson().toJson(value.getStringArrayValue())); + builder.setVType(Model.ValueType.STRING); + break; + case LONG_ARRAY: + builder.setVStr(new Gson().toJson(value.getLongArrayValue())); + builder.setVType(Model.ValueType.STRING); + break; + case BOOLEAN_ARRAY: + builder.setVStr(new Gson().toJson(value.getBooleanArrayValue())); + builder.setVType(Model.ValueType.STRING); + break; + case DOUBLE_ARRAY: + builder.setVStr(new Gson().toJson(value.getDoubleArrayValue())); + builder.setVType(Model.ValueType.STRING); break; } return builder.build(); diff --git a/exporters/jaeger/src/test/java/io/opentelemetry/exporters/jaeger/AdapterTest.java b/exporters/jaeger/src/test/java/io/opentelemetry/exporters/jaeger/AdapterTest.java index 424b948b572..9c3b5bacb62 100644 --- a/exporters/jaeger/src/test/java/io/opentelemetry/exporters/jaeger/AdapterTest.java +++ b/exporters/jaeger/src/test/java/io/opentelemetry/exporters/jaeger/AdapterTest.java @@ -167,12 +167,20 @@ public void testKeyValue() { AttributeValue valueD = AttributeValue.doubleAttributeValue(1.); AttributeValue valueI = AttributeValue.longAttributeValue(2); AttributeValue valueS = AttributeValue.stringAttributeValue("foobar"); + AttributeValue valueAB = AttributeValue.arrayAttributeValue(true, false); + AttributeValue valueAD = AttributeValue.arrayAttributeValue(1.2345, 6.789); + AttributeValue valueAI = AttributeValue.arrayAttributeValue(12345L, 67890L); + AttributeValue valueAS = AttributeValue.arrayAttributeValue("foobar", "barfoo"); // test Model.KeyValue kvB = Adapter.toKeyValue("valueB", valueB); Model.KeyValue kvD = Adapter.toKeyValue("valueD", valueD); Model.KeyValue kvI = Adapter.toKeyValue("valueI", valueI); Model.KeyValue kvS = Adapter.toKeyValue("valueS", valueS); + Model.KeyValue kvAB = Adapter.toKeyValue("valueAB", valueAB); + Model.KeyValue kvAD = Adapter.toKeyValue("valueAD", valueAD); + Model.KeyValue kvAI = Adapter.toKeyValue("valueAI", valueAI); + Model.KeyValue kvAS = Adapter.toKeyValue("valueAS", valueAS); // verify assertTrue(kvB.getVBool()); @@ -184,6 +192,18 @@ public void testKeyValue() { assertEquals("foobar", kvS.getVStr()); assertEquals("foobar", kvS.getVStrBytes().toStringUtf8()); assertEquals(Model.ValueType.STRING, kvS.getVType()); + assertEquals("[true,false]", kvAB.getVStr()); + assertEquals("[true,false]", kvAB.getVStrBytes().toStringUtf8()); + assertEquals(Model.ValueType.STRING, kvAB.getVType()); + assertEquals("[1.2345,6.789]", kvAD.getVStr()); + assertEquals("[1.2345,6.789]", kvAD.getVStrBytes().toStringUtf8()); + assertEquals(Model.ValueType.STRING, kvAD.getVType()); + assertEquals("[12345,67890]", kvAI.getVStr()); + assertEquals("[12345,67890]", kvAI.getVStrBytes().toStringUtf8()); + assertEquals(Model.ValueType.STRING, kvAI.getVType()); + assertEquals("[\"foobar\",\"barfoo\"]", kvAS.getVStr()); + assertEquals("[\"foobar\",\"barfoo\"]", kvAS.getVStrBytes().toStringUtf8()); + assertEquals(Model.ValueType.STRING, kvAS.getVType()); } @Test From fac7d6510584e69494b58019333711f294f41fd4 Mon Sep 17 00:00:00 2001 From: Giovanni Liva Date: Thu, 30 Jan 2020 10:54:27 +0100 Subject: [PATCH 06/17] Fix checkstyle naming issue --- .../exporters/jaeger/AdapterTest.java | 40 +++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/exporters/jaeger/src/test/java/io/opentelemetry/exporters/jaeger/AdapterTest.java b/exporters/jaeger/src/test/java/io/opentelemetry/exporters/jaeger/AdapterTest.java index 9c3b5bacb62..408c09bba56 100644 --- a/exporters/jaeger/src/test/java/io/opentelemetry/exporters/jaeger/AdapterTest.java +++ b/exporters/jaeger/src/test/java/io/opentelemetry/exporters/jaeger/AdapterTest.java @@ -167,20 +167,20 @@ public void testKeyValue() { AttributeValue valueD = AttributeValue.doubleAttributeValue(1.); AttributeValue valueI = AttributeValue.longAttributeValue(2); AttributeValue valueS = AttributeValue.stringAttributeValue("foobar"); - AttributeValue valueAB = AttributeValue.arrayAttributeValue(true, false); - AttributeValue valueAD = AttributeValue.arrayAttributeValue(1.2345, 6.789); - AttributeValue valueAI = AttributeValue.arrayAttributeValue(12345L, 67890L); - AttributeValue valueAS = AttributeValue.arrayAttributeValue("foobar", "barfoo"); + AttributeValue valueArrayB = AttributeValue.arrayAttributeValue(true, false); + AttributeValue valueArrayD = AttributeValue.arrayAttributeValue(1.2345, 6.789); + AttributeValue valueArrayI = AttributeValue.arrayAttributeValue(12345L, 67890L); + AttributeValue valueArrayS = AttributeValue.arrayAttributeValue("foobar", "barfoo"); // test Model.KeyValue kvB = Adapter.toKeyValue("valueB", valueB); Model.KeyValue kvD = Adapter.toKeyValue("valueD", valueD); Model.KeyValue kvI = Adapter.toKeyValue("valueI", valueI); Model.KeyValue kvS = Adapter.toKeyValue("valueS", valueS); - Model.KeyValue kvAB = Adapter.toKeyValue("valueAB", valueAB); - Model.KeyValue kvAD = Adapter.toKeyValue("valueAD", valueAD); - Model.KeyValue kvAI = Adapter.toKeyValue("valueAI", valueAI); - Model.KeyValue kvAS = Adapter.toKeyValue("valueAS", valueAS); + Model.KeyValue kvArrayB = Adapter.toKeyValue("valueArrayB", valueArrayB); + Model.KeyValue kvArrayD = Adapter.toKeyValue("valueArrayD", valueArrayD); + Model.KeyValue kvArrayI = Adapter.toKeyValue("valueArrayI", valueArrayI); + Model.KeyValue kvArrayS = Adapter.toKeyValue("valueArrayS", valueArrayS); // verify assertTrue(kvB.getVBool()); @@ -192,18 +192,18 @@ public void testKeyValue() { assertEquals("foobar", kvS.getVStr()); assertEquals("foobar", kvS.getVStrBytes().toStringUtf8()); assertEquals(Model.ValueType.STRING, kvS.getVType()); - assertEquals("[true,false]", kvAB.getVStr()); - assertEquals("[true,false]", kvAB.getVStrBytes().toStringUtf8()); - assertEquals(Model.ValueType.STRING, kvAB.getVType()); - assertEquals("[1.2345,6.789]", kvAD.getVStr()); - assertEquals("[1.2345,6.789]", kvAD.getVStrBytes().toStringUtf8()); - assertEquals(Model.ValueType.STRING, kvAD.getVType()); - assertEquals("[12345,67890]", kvAI.getVStr()); - assertEquals("[12345,67890]", kvAI.getVStrBytes().toStringUtf8()); - assertEquals(Model.ValueType.STRING, kvAI.getVType()); - assertEquals("[\"foobar\",\"barfoo\"]", kvAS.getVStr()); - assertEquals("[\"foobar\",\"barfoo\"]", kvAS.getVStrBytes().toStringUtf8()); - assertEquals(Model.ValueType.STRING, kvAS.getVType()); + assertEquals("[true,false]", kvArrayB.getVStr()); + assertEquals("[true,false]", kvArrayB.getVStrBytes().toStringUtf8()); + assertEquals(Model.ValueType.STRING, kvArrayB.getVType()); + assertEquals("[1.2345,6.789]", kvArrayD.getVStr()); + assertEquals("[1.2345,6.789]", kvArrayD.getVStrBytes().toStringUtf8()); + assertEquals(Model.ValueType.STRING, kvArrayD.getVType()); + assertEquals("[12345,67890]", kvArrayI.getVStr()); + assertEquals("[12345,67890]", kvArrayI.getVStrBytes().toStringUtf8()); + assertEquals(Model.ValueType.STRING, kvArrayI.getVType()); + assertEquals("[\"foobar\",\"barfoo\"]", kvArrayS.getVStr()); + assertEquals("[\"foobar\",\"barfoo\"]", kvArrayS.getVStrBytes().toStringUtf8()); + assertEquals(Model.ValueType.STRING, kvArrayS.getVType()); } @Test From b50ed2655f3bc023886f1854cf00640f28951be8 Mon Sep 17 00:00:00 2001 From: Giovanni Liva Date: Thu, 30 Jan 2020 14:48:46 +0100 Subject: [PATCH 07/17] Add further tests. --- .../io/opentelemetry/common/AttributeValue.java | 12 ++++++++++++ .../io/opentelemetry/trace/DefaultSpanTest.java | 4 ++++ .../sdk/trace/RecordEventsReadableSpanTest.java | 14 +++++++++++++- 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/api/src/main/java/io/opentelemetry/common/AttributeValue.java b/api/src/main/java/io/opentelemetry/common/AttributeValue.java index eef5db118f1..bf786210f95 100644 --- a/api/src/main/java/io/opentelemetry/common/AttributeValue.java +++ b/api/src/main/java/io/opentelemetry/common/AttributeValue.java @@ -19,6 +19,7 @@ import com.google.auto.value.AutoValue; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.List; import javax.annotation.Nullable; import javax.annotation.concurrent.Immutable; @@ -346,6 +347,10 @@ abstract static class AttributeValueBooleanArray extends AttributeValue { AttributeValueBooleanArray() {} static AttributeValue create(boolean... booleanValues) { + if (booleanValues == null) { + return new AutoValue_AttributeValue_AttributeValueBooleanArray( + Collections.emptyList()); + } List values = new ArrayList<>(booleanValues.length); for (boolean value : booleanValues) { values.add(value); @@ -369,6 +374,9 @@ abstract static class AttributeValueLongArray extends AttributeValue { AttributeValueLongArray() {} static AttributeValue create(long... longValues) { + if (longValues == null) { + return new AutoValue_AttributeValue_AttributeValueLongArray(Collections.emptyList()); + } List values = new ArrayList<>(longValues.length); for (long value : longValues) { values.add(value); @@ -392,6 +400,10 @@ abstract static class AttributeValueDoubleArray extends AttributeValue { AttributeValueDoubleArray() {} static AttributeValue create(double... doubleValues) { + if (doubleValues == null) { + return new AutoValue_AttributeValue_AttributeValueDoubleArray( + Collections.emptyList()); + } List values = new ArrayList<>(doubleValues.length); for (double value : doubleValues) { values.add(value); diff --git a/api/src/test/java/io/opentelemetry/trace/DefaultSpanTest.java b/api/src/test/java/io/opentelemetry/trace/DefaultSpanTest.java index 0a930e0a342..cbf7f86b80e 100644 --- a/api/src/test/java/io/opentelemetry/trace/DefaultSpanTest.java +++ b/api/src/test/java/io/opentelemetry/trace/DefaultSpanTest.java @@ -56,6 +56,10 @@ public void doNotCrash() { span.setAttribute("MyLongAttributeKey", AttributeValue.longAttributeValue(123)); span.setAttribute("NullString", (String) null); span.setAttribute("EmptyString", ""); + span.setAttribute("NullArrayString", (String[]) null); + span.setAttribute("NullArrayBoolean", (boolean[]) null); + span.setAttribute("NullArrayLong", (long[]) null); + span.setAttribute("NullArrayDouble", (double[]) null); span.addEvent("event"); span.addEvent("event", 0); span.addEvent( 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 88c06de0432..bdfb58d174f 100644 --- a/sdk/src/test/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpanTest.java +++ b/sdk/src/test/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpanTest.java @@ -294,11 +294,23 @@ public void setAttribute() { span.setAttribute("LongKey", 1000L); span.setAttribute("DoubleKey", 10.0); span.setAttribute("BooleanKey", false); + span.setAttribute("ArrayStringKey", "StringVal", null, "", "StringVal2"); + span.setAttribute("ArrayLongKey", 1L, 2L, 3L, 4L, 5L); + span.setAttribute("ArrayDoubleKey", 0.1, 2.3, 4.5, 6.7, 8.9); + span.setAttribute("ArrayBooleanKey", true, false, false, true); } finally { span.end(); } SpanData spanData = span.toSpanData(); - assertThat(spanData.getAttributes().size()).isEqualTo(4); + assertThat(spanData.getAttributes().size()).isEqualTo(8); + assertThat(spanData.getAttributes().get("ArrayStringKey").getStringArrayValue().size()) + .isEqualTo(4); + assertThat(spanData.getAttributes().get("ArrayLongKey").getLongArrayValue().size()) + .isEqualTo(5); + assertThat(spanData.getAttributes().get("ArrayDoubleKey").getDoubleArrayValue().size()) + .isEqualTo(5); + assertThat(spanData.getAttributes().get("ArrayBooleanKey").getBooleanArrayValue().size()) + .isEqualTo(4); } @Test From 23a1637620c7095641409a38b56fd8d9a8a08798 Mon Sep 17 00:00:00 2001 From: Giovanni Liva Date: Thu, 30 Jan 2020 15:22:06 +0100 Subject: [PATCH 08/17] Align null value behavior of attributes with spec --- .../java/io/opentelemetry/common/AttributeValue.java | 12 ------------ .../sdk/trace/RecordEventsReadableSpan.java | 12 ++++++++++++ .../sdk/trace/RecordEventsReadableSpanTest.java | 4 ++++ 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/api/src/main/java/io/opentelemetry/common/AttributeValue.java b/api/src/main/java/io/opentelemetry/common/AttributeValue.java index bf786210f95..eef5db118f1 100644 --- a/api/src/main/java/io/opentelemetry/common/AttributeValue.java +++ b/api/src/main/java/io/opentelemetry/common/AttributeValue.java @@ -19,7 +19,6 @@ import com.google.auto.value.AutoValue; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collections; import java.util.List; import javax.annotation.Nullable; import javax.annotation.concurrent.Immutable; @@ -347,10 +346,6 @@ abstract static class AttributeValueBooleanArray extends AttributeValue { AttributeValueBooleanArray() {} static AttributeValue create(boolean... booleanValues) { - if (booleanValues == null) { - return new AutoValue_AttributeValue_AttributeValueBooleanArray( - Collections.emptyList()); - } List values = new ArrayList<>(booleanValues.length); for (boolean value : booleanValues) { values.add(value); @@ -374,9 +369,6 @@ abstract static class AttributeValueLongArray extends AttributeValue { AttributeValueLongArray() {} static AttributeValue create(long... longValues) { - if (longValues == null) { - return new AutoValue_AttributeValue_AttributeValueLongArray(Collections.emptyList()); - } List values = new ArrayList<>(longValues.length); for (long value : longValues) { values.add(value); @@ -400,10 +392,6 @@ abstract static class AttributeValueDoubleArray extends AttributeValue { AttributeValueDoubleArray() {} static AttributeValue create(double... doubleValues) { - if (doubleValues == null) { - return new AutoValue_AttributeValue_AttributeValueDoubleArray( - Collections.emptyList()); - } List values = new ArrayList<>(doubleValues.length); for (double value : doubleValues) { values.add(value); 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 b7467f697aa..36a0dfa727a 100644 --- a/sdk/src/main/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpan.java +++ b/sdk/src/main/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpan.java @@ -331,21 +331,33 @@ public void setAttribute(String key, boolean value) { @Override public void setAttribute(String key, String... values) { + if (values == null) { + return; + } setAttribute(key, AttributeValue.arrayAttributeValue(values)); } @Override public void setAttribute(String key, long... values) { + if (values == null) { + return; + } setAttribute(key, AttributeValue.arrayAttributeValue(values)); } @Override public void setAttribute(String key, double... values) { + if (values == null) { + return; + } setAttribute(key, AttributeValue.arrayAttributeValue(values)); } @Override public void setAttribute(String key, boolean... values) { + if (values == null) { + return; + } setAttribute(key, AttributeValue.arrayAttributeValue(values)); } 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 bdfb58d174f..55f1a5fae53 100644 --- a/sdk/src/test/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpanTest.java +++ b/sdk/src/test/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpanTest.java @@ -298,6 +298,10 @@ public void setAttribute() { span.setAttribute("ArrayLongKey", 1L, 2L, 3L, 4L, 5L); span.setAttribute("ArrayDoubleKey", 0.1, 2.3, 4.5, 6.7, 8.9); span.setAttribute("ArrayBooleanKey", true, false, false, true); + span.setAttribute("NullArrayStringKey", (String[]) null); + span.setAttribute("NullArrayLongKey", (long[]) null); + span.setAttribute("NullArrayDoubleKey", (double[]) null); + span.setAttribute("NullArrayBooleanKey", (boolean[]) null); } finally { span.end(); } From ae03f6f8e20150e44c2c5d992c99b7606106c15d Mon Sep 17 00:00:00 2001 From: Giovanni Liva Date: Thu, 30 Jan 2020 16:45:58 +0100 Subject: [PATCH 09/17] fix javadoc @since --- .../io/opentelemetry/common/AttributeValue.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/api/src/main/java/io/opentelemetry/common/AttributeValue.java b/api/src/main/java/io/opentelemetry/common/AttributeValue.java index eef5db118f1..6d40a31ec3f 100644 --- a/api/src/main/java/io/opentelemetry/common/AttributeValue.java +++ b/api/src/main/java/io/opentelemetry/common/AttributeValue.java @@ -98,7 +98,7 @@ public static AttributeValue doubleAttributeValue(double doubleValue) { * * @param stringValues The new values. * @return an {@code AttributeValue} with a String array value. - * @since 0.1.0 + * @since 0.3.0 */ public static AttributeValue arrayAttributeValue(String... stringValues) { return AttributeValueStringArray.create(stringValues); @@ -109,7 +109,7 @@ public static AttributeValue arrayAttributeValue(String... stringValues) { * * @param booleanValues The new values. * @return an {@code AttributeValue} with a boolean array value. - * @since 0.1.0 + * @since 0.3.0 */ public static AttributeValue arrayAttributeValue(boolean... booleanValues) { return AttributeValueBooleanArray.create(booleanValues); @@ -120,7 +120,7 @@ public static AttributeValue arrayAttributeValue(boolean... booleanValues) { * * @param longValues The new values. * @return an {@code AttributeValue} with a long array value. - * @since 0.1.0 + * @since 0.3.0 */ public static AttributeValue arrayAttributeValue(long... longValues) { return AttributeValueLongArray.create(longValues); @@ -131,7 +131,7 @@ public static AttributeValue arrayAttributeValue(long... longValues) { * * @param doubleValues The new values. * @return an {@code AttributeValue} with a double array value. - * @since 0.1.0 + * @since 0.3.0 */ public static AttributeValue arrayAttributeValue(double... doubleValues) { return AttributeValueDoubleArray.create(doubleValues); @@ -192,7 +192,7 @@ public double getDoubleValue() { * will be thrown if getType() is not {@link Type#STRING_ARRAY}. * * @return the array values of this {@code AttributeValue}. - * @since 0.1.0 + * @since 0.3.0 */ public List getStringArrayValue() { throw new UnsupportedOperationException( @@ -204,7 +204,7 @@ public List getStringArrayValue() { * UnsupportedOperationException will be thrown if getType() is not {@link Type#BOOLEAN_ARRAY}. * * @return the array values of this {@code AttributeValue}. - * @since 0.1.0 + * @since 0.3.0 */ public List getBooleanArrayValue() { throw new UnsupportedOperationException( @@ -216,7 +216,7 @@ public List getBooleanArrayValue() { * will be thrown if getType() is not {@link Type#LONG_ARRAY}. * * @return the array values of this {@code AttributeValue}. - * @since 0.1.0 + * @since 0.3.0 */ public List getLongArrayValue() { throw new UnsupportedOperationException( @@ -228,7 +228,7 @@ public List getLongArrayValue() { * will be thrown if getType() is not {@link Type#DOUBLE_ARRAY}. * * @return the array values of this {@code AttributeValue}. - * @since 0.1.0 + * @since 0.3.0 */ public List getDoubleArrayValue() { throw new UnsupportedOperationException( From 9ffba0c75b6bb5f3c7a8f7061cbde586c40c6178 Mon Sep 17 00:00:00 2001 From: Giovanni Liva Date: Thu, 30 Jan 2020 17:02:54 +0100 Subject: [PATCH 10/17] API must not crash on misusage of AttributeValue --- .../io/opentelemetry/common/AttributeValue.java | 16 ++++++++++++++++ .../opentelemetry/common/AttributeValueTest.java | 9 +++++++++ 2 files changed, 25 insertions(+) diff --git a/api/src/main/java/io/opentelemetry/common/AttributeValue.java b/api/src/main/java/io/opentelemetry/common/AttributeValue.java index 6d40a31ec3f..59371ec2342 100644 --- a/api/src/main/java/io/opentelemetry/common/AttributeValue.java +++ b/api/src/main/java/io/opentelemetry/common/AttributeValue.java @@ -19,6 +19,7 @@ import com.google.auto.value.AutoValue; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.List; import javax.annotation.Nullable; import javax.annotation.concurrent.Immutable; @@ -327,6 +328,10 @@ abstract static class AttributeValueStringArray extends AttributeValue { AttributeValueStringArray() {} static AttributeValue create(String... stringValues) { + if (stringValues == null) { + return new AutoValue_AttributeValue_AttributeValueStringArray( + Collections.emptyList()); + } return new AutoValue_AttributeValue_AttributeValueStringArray(Arrays.asList(stringValues)); } @@ -346,6 +351,10 @@ abstract static class AttributeValueBooleanArray extends AttributeValue { AttributeValueBooleanArray() {} static AttributeValue create(boolean... booleanValues) { + if (booleanValues == null) { + return new AutoValue_AttributeValue_AttributeValueBooleanArray( + Collections.emptyList()); + } List values = new ArrayList<>(booleanValues.length); for (boolean value : booleanValues) { values.add(value); @@ -369,6 +378,9 @@ abstract static class AttributeValueLongArray extends AttributeValue { AttributeValueLongArray() {} static AttributeValue create(long... longValues) { + if (longValues == null) { + return new AutoValue_AttributeValue_AttributeValueLongArray(Collections.emptyList()); + } List values = new ArrayList<>(longValues.length); for (long value : longValues) { values.add(value); @@ -392,6 +404,10 @@ abstract static class AttributeValueDoubleArray extends AttributeValue { AttributeValueDoubleArray() {} static AttributeValue create(double... doubleValues) { + if (doubleValues == null) { + return new AutoValue_AttributeValue_AttributeValueDoubleArray( + Collections.emptyList()); + } List values = new ArrayList<>(doubleValues.length); for (double value : doubleValues) { values.add(value); diff --git a/api/src/test/java/io/opentelemetry/common/AttributeValueTest.java b/api/src/test/java/io/opentelemetry/common/AttributeValueTest.java index 230529dfb4e..58de445b30e 100644 --- a/api/src/test/java/io/opentelemetry/common/AttributeValueTest.java +++ b/api/src/test/java/io/opentelemetry/common/AttributeValueTest.java @@ -64,6 +64,15 @@ public void attributeValue_EqualsAndHashCode() { tester.testEquals(); } + @Test + public void doNotCrashOnNull(){ + AttributeValue.stringAttributeValue(null); + AttributeValue.arrayAttributeValue((String[])null); + AttributeValue.arrayAttributeValue((boolean[])null); + AttributeValue.arrayAttributeValue((long[])null); + AttributeValue.arrayAttributeValue((double[])null); + } + @Test public void attributeValue_ToString() { AttributeValue attribute = AttributeValue.stringAttributeValue("MyStringAttributeValue"); From 9b6cc070ca2de512e29e0b91d0c638452f207ac4 Mon Sep 17 00:00:00 2001 From: Giovanni Liva Date: Fri, 31 Jan 2020 08:35:49 +0100 Subject: [PATCH 11/17] API - Remove Attribute ArrayValues from Span surface --- .../io/opentelemetry/trace/DefaultSpan.java | 20 ------ .../java/io/opentelemetry/trace/Span.java | 43 ------------- .../common/AttributeValueTest.java | 10 +-- .../opentelemetry/trace/DefaultSpanTest.java | 8 +-- .../sdk/trace/RecordEventsReadableSpan.java | 62 +++++++++---------- .../trace/RecordEventsReadableSpanTest.java | 21 ++++--- 6 files changed, 50 insertions(+), 114 deletions(-) diff --git a/api/src/main/java/io/opentelemetry/trace/DefaultSpan.java b/api/src/main/java/io/opentelemetry/trace/DefaultSpan.java index 58fbb9fe8b7..cfdd4dc720a 100644 --- a/api/src/main/java/io/opentelemetry/trace/DefaultSpan.java +++ b/api/src/main/java/io/opentelemetry/trace/DefaultSpan.java @@ -92,26 +92,6 @@ public void setAttribute(String key, boolean value) { Utils.checkNotNull(key, "key"); } - @Override - public void setAttribute(String key, String... values) { - Utils.checkNotNull(key, "key"); - } - - @Override - public void setAttribute(String key, long... values) { - Utils.checkNotNull(key, "key"); - } - - @Override - public void setAttribute(String key, double... values) { - Utils.checkNotNull(key, "key"); - } - - @Override - public void setAttribute(String key, boolean... values) { - Utils.checkNotNull(key, "key"); - } - @Override public void setAttribute(String key, AttributeValue value) { Utils.checkNotNull(key, "key"); diff --git a/api/src/main/java/io/opentelemetry/trace/Span.java b/api/src/main/java/io/opentelemetry/trace/Span.java index 5b358d45b4a..b18949eb197 100644 --- a/api/src/main/java/io/opentelemetry/trace/Span.java +++ b/api/src/main/java/io/opentelemetry/trace/Span.java @@ -123,49 +123,6 @@ enum Kind { */ void setAttribute(String key, boolean value); - /** - * 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 values the value for this attribute. - * @since 0.1.0 - */ - void setAttribute(String key, String... values); - - /** - * 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 values the value for this attribute. - * @since 0.1.0 - */ - void setAttribute(String key, long... values); - - /** - * 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 values the value for this attribute. - * @since 0.1.0 - */ - void setAttribute(String key, double... values); - - /** - * 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 values the value for this attribute. - * @since 0.1.0 - */ - void setAttribute(String key, boolean... values); - /** * 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. diff --git a/api/src/test/java/io/opentelemetry/common/AttributeValueTest.java b/api/src/test/java/io/opentelemetry/common/AttributeValueTest.java index 58de445b30e..48b7d1532a1 100644 --- a/api/src/test/java/io/opentelemetry/common/AttributeValueTest.java +++ b/api/src/test/java/io/opentelemetry/common/AttributeValueTest.java @@ -65,12 +65,12 @@ public void attributeValue_EqualsAndHashCode() { } @Test - public void doNotCrashOnNull(){ + public void doNotCrashOnNull() { AttributeValue.stringAttributeValue(null); - AttributeValue.arrayAttributeValue((String[])null); - AttributeValue.arrayAttributeValue((boolean[])null); - AttributeValue.arrayAttributeValue((long[])null); - AttributeValue.arrayAttributeValue((double[])null); + AttributeValue.arrayAttributeValue((String[]) null); + AttributeValue.arrayAttributeValue((boolean[]) null); + AttributeValue.arrayAttributeValue((long[]) null); + AttributeValue.arrayAttributeValue((double[]) null); } @Test diff --git a/api/src/test/java/io/opentelemetry/trace/DefaultSpanTest.java b/api/src/test/java/io/opentelemetry/trace/DefaultSpanTest.java index cbf7f86b80e..851ee47c5cd 100644 --- a/api/src/test/java/io/opentelemetry/trace/DefaultSpanTest.java +++ b/api/src/test/java/io/opentelemetry/trace/DefaultSpanTest.java @@ -56,10 +56,10 @@ public void doNotCrash() { span.setAttribute("MyLongAttributeKey", AttributeValue.longAttributeValue(123)); span.setAttribute("NullString", (String) null); span.setAttribute("EmptyString", ""); - span.setAttribute("NullArrayString", (String[]) null); - span.setAttribute("NullArrayBoolean", (boolean[]) null); - span.setAttribute("NullArrayLong", (long[]) null); - span.setAttribute("NullArrayDouble", (double[]) null); + span.setAttribute("NullArrayString", AttributeValue.arrayAttributeValue((String[]) null)); + span.setAttribute("NullArrayBoolean", AttributeValue.arrayAttributeValue((boolean[]) null)); + span.setAttribute("NullArrayLong", AttributeValue.arrayAttributeValue((long[]) null)); + span.setAttribute("NullArrayDouble", AttributeValue.arrayAttributeValue((double[]) null)); 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 36a0dfa727a..41e37cfbcfd 100644 --- a/sdk/src/main/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpan.java +++ b/sdk/src/main/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpan.java @@ -329,44 +329,38 @@ public void setAttribute(String key, boolean value) { setAttribute(key, AttributeValue.booleanAttributeValue(value)); } - @Override - public void setAttribute(String key, String... values) { - if (values == null) { - return; - } - setAttribute(key, AttributeValue.arrayAttributeValue(values)); - } - - @Override - public void setAttribute(String key, long... values) { - if (values == null) { - return; - } - setAttribute(key, AttributeValue.arrayAttributeValue(values)); - } - - @Override - public void setAttribute(String key, double... values) { - if (values == null) { - return; - } - setAttribute(key, AttributeValue.arrayAttributeValue(values)); - } - - @Override - public void setAttribute(String key, boolean... values) { - if (values == null) { - return; - } - setAttribute(key, AttributeValue.arrayAttributeValue(values)); - } - @Override public void setAttribute(String key, AttributeValue value) { Preconditions.checkNotNull(key, "key"); Preconditions.checkNotNull(value, "value"); - if (value.getType() == Type.STRING && StringUtils.isNullOrEmpty(value.getStringValue())) { - return; + switch (value.getType()) { + case STRING: + if (StringUtils.isNullOrEmpty(value.getStringValue())) { + return; + } + break; + case STRING_ARRAY: + if (value.getStringArrayValue().size() == 0) { + return; + } + break; + case BOOLEAN_ARRAY: + if (value.getBooleanArrayValue().size() == 0) { + return; + } + break; + case LONG_ARRAY: + if (value.getLongArrayValue().size() == 0) { + return; + } + break; + case DOUBLE_ARRAY: + if (value.getDoubleArrayValue().size() == 0) { + return; + } + break; + default: + break; } synchronized (lock) { if (hasEnded) { 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 55f1a5fae53..8eccc7eda74 100644 --- a/sdk/src/test/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpanTest.java +++ b/sdk/src/test/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpanTest.java @@ -294,14 +294,19 @@ public void setAttribute() { span.setAttribute("LongKey", 1000L); span.setAttribute("DoubleKey", 10.0); span.setAttribute("BooleanKey", false); - span.setAttribute("ArrayStringKey", "StringVal", null, "", "StringVal2"); - span.setAttribute("ArrayLongKey", 1L, 2L, 3L, 4L, 5L); - span.setAttribute("ArrayDoubleKey", 0.1, 2.3, 4.5, 6.7, 8.9); - span.setAttribute("ArrayBooleanKey", true, false, false, true); - span.setAttribute("NullArrayStringKey", (String[]) null); - span.setAttribute("NullArrayLongKey", (long[]) null); - span.setAttribute("NullArrayDoubleKey", (double[]) null); - span.setAttribute("NullArrayBooleanKey", (boolean[]) null); + span.setAttribute( + "ArrayStringKey", + AttributeValue.arrayAttributeValue("StringVal", null, "", "StringVal2")); + span.setAttribute("ArrayLongKey", AttributeValue.arrayAttributeValue(1L, 2L, 3L, 4L, 5L)); + span.setAttribute( + "ArrayDoubleKey", AttributeValue.arrayAttributeValue(0.1, 2.3, 4.5, 6.7, 8.9)); + span.setAttribute( + "ArrayBooleanKey", AttributeValue.arrayAttributeValue(true, false, false, true)); + span.setAttribute("NullArrayStringKey", AttributeValue.arrayAttributeValue((String[]) null)); + span.setAttribute("NullArrayLongKey", AttributeValue.arrayAttributeValue((long[]) null)); + span.setAttribute("NullArrayDoubleKey", AttributeValue.arrayAttributeValue((double[]) null)); + span.setAttribute( + "NullArrayBooleanKey", AttributeValue.arrayAttributeValue((boolean[]) null)); } finally { span.end(); } From 1e394b29bf537e59253da00a9fb3e3357a167ea5 Mon Sep 17 00:00:00 2001 From: Giovanni Liva Date: Fri, 31 Jan 2020 10:50:46 +0100 Subject: [PATCH 12/17] Immutable String array values for AttributeValue --- api/src/main/java/io/opentelemetry/common/AttributeValue.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/api/src/main/java/io/opentelemetry/common/AttributeValue.java b/api/src/main/java/io/opentelemetry/common/AttributeValue.java index 59371ec2342..6ca1dd70749 100644 --- a/api/src/main/java/io/opentelemetry/common/AttributeValue.java +++ b/api/src/main/java/io/opentelemetry/common/AttributeValue.java @@ -332,7 +332,8 @@ static AttributeValue create(String... stringValues) { return new AutoValue_AttributeValue_AttributeValueStringArray( Collections.emptyList()); } - return new AutoValue_AttributeValue_AttributeValueStringArray(Arrays.asList(stringValues)); + return new AutoValue_AttributeValue_AttributeValueStringArray( + Collections.unmodifiableList(Arrays.asList(stringValues))); } @Override From e3d04a2fac18cb3bc7454f5f7cf13596425e5e8e Mon Sep 17 00:00:00 2001 From: Giovanni Liva Date: Fri, 31 Jan 2020 11:43:33 +0100 Subject: [PATCH 13/17] Immutable values for AttributeValue arrays --- .../main/java/io/opentelemetry/common/AttributeValue.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/api/src/main/java/io/opentelemetry/common/AttributeValue.java b/api/src/main/java/io/opentelemetry/common/AttributeValue.java index 6ca1dd70749..10a40d1222d 100644 --- a/api/src/main/java/io/opentelemetry/common/AttributeValue.java +++ b/api/src/main/java/io/opentelemetry/common/AttributeValue.java @@ -360,7 +360,7 @@ static AttributeValue create(boolean... booleanValues) { for (boolean value : booleanValues) { values.add(value); } - return new AutoValue_AttributeValue_AttributeValueBooleanArray(values); + return new AutoValue_AttributeValue_AttributeValueBooleanArray(Collections.unmodifiableList(values)); } @Override @@ -386,7 +386,7 @@ static AttributeValue create(long... longValues) { for (long value : longValues) { values.add(value); } - return new AutoValue_AttributeValue_AttributeValueLongArray(values); + return new AutoValue_AttributeValue_AttributeValueLongArray(Collections.unmodifiableList(values)); } @Override @@ -413,7 +413,7 @@ static AttributeValue create(double... doubleValues) { for (double value : doubleValues) { values.add(value); } - return new AutoValue_AttributeValue_AttributeValueDoubleArray(values); + return new AutoValue_AttributeValue_AttributeValueDoubleArray(Collections.unmodifiableList(values)); } @Override From 036487928fd4accbcac8ff023eb4de2d30285058 Mon Sep 17 00:00:00 2001 From: Giovanni Liva Date: Fri, 31 Jan 2020 11:48:34 +0100 Subject: [PATCH 14/17] ./gradlew goJF --- .../java/io/opentelemetry/common/AttributeValue.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/api/src/main/java/io/opentelemetry/common/AttributeValue.java b/api/src/main/java/io/opentelemetry/common/AttributeValue.java index 10a40d1222d..9525532692c 100644 --- a/api/src/main/java/io/opentelemetry/common/AttributeValue.java +++ b/api/src/main/java/io/opentelemetry/common/AttributeValue.java @@ -360,7 +360,8 @@ static AttributeValue create(boolean... booleanValues) { for (boolean value : booleanValues) { values.add(value); } - return new AutoValue_AttributeValue_AttributeValueBooleanArray(Collections.unmodifiableList(values)); + return new AutoValue_AttributeValue_AttributeValueBooleanArray( + Collections.unmodifiableList(values)); } @Override @@ -386,7 +387,8 @@ static AttributeValue create(long... longValues) { for (long value : longValues) { values.add(value); } - return new AutoValue_AttributeValue_AttributeValueLongArray(Collections.unmodifiableList(values)); + return new AutoValue_AttributeValue_AttributeValueLongArray( + Collections.unmodifiableList(values)); } @Override @@ -413,7 +415,8 @@ static AttributeValue create(double... doubleValues) { for (double value : doubleValues) { values.add(value); } - return new AutoValue_AttributeValue_AttributeValueDoubleArray(Collections.unmodifiableList(values)); + return new AutoValue_AttributeValue_AttributeValueDoubleArray( + Collections.unmodifiableList(values)); } @Override From 331e27584197ea4fb42b4eb485109a26d414b744 Mon Sep 17 00:00:00 2001 From: Giovanni Liva Date: Mon, 16 Mar 2020 14:13:47 +0100 Subject: [PATCH 15/17] Implement ArrayAttribute --- .../opentelemetry/common/AttributeValue.java | 18 +++++++-------- .../common/AttributeValueTest.java | 23 ++++++++++++++++--- .../opentelemetry/trace/DefaultSpanTest.java | 6 ++--- .../exporters/otlp/CommonAdapter.java | 5 ++++ .../sdk/trace/RecordEventsReadableSpan.java | 1 - .../trace/RecordEventsReadableSpanTest.java | 6 ++--- 6 files changed, 40 insertions(+), 19 deletions(-) diff --git a/api/src/main/java/io/opentelemetry/common/AttributeValue.java b/api/src/main/java/io/opentelemetry/common/AttributeValue.java index 9525532692c..8fc8d00fdf5 100644 --- a/api/src/main/java/io/opentelemetry/common/AttributeValue.java +++ b/api/src/main/java/io/opentelemetry/common/AttributeValue.java @@ -112,7 +112,7 @@ public static AttributeValue arrayAttributeValue(String... stringValues) { * @return an {@code AttributeValue} with a boolean array value. * @since 0.3.0 */ - public static AttributeValue arrayAttributeValue(boolean... booleanValues) { + public static AttributeValue arrayAttributeValue(Boolean... booleanValues) { return AttributeValueBooleanArray.create(booleanValues); } @@ -123,7 +123,7 @@ public static AttributeValue arrayAttributeValue(boolean... booleanValues) { * @return an {@code AttributeValue} with a long array value. * @since 0.3.0 */ - public static AttributeValue arrayAttributeValue(long... longValues) { + public static AttributeValue arrayAttributeValue(Long... longValues) { return AttributeValueLongArray.create(longValues); } @@ -134,7 +134,7 @@ public static AttributeValue arrayAttributeValue(long... longValues) { * @return an {@code AttributeValue} with a double array value. * @since 0.3.0 */ - public static AttributeValue arrayAttributeValue(double... doubleValues) { + public static AttributeValue arrayAttributeValue(Double... doubleValues) { return AttributeValueDoubleArray.create(doubleValues); } @@ -351,13 +351,13 @@ abstract static class AttributeValueBooleanArray extends AttributeValue { AttributeValueBooleanArray() {} - static AttributeValue create(boolean... booleanValues) { + static AttributeValue create(Boolean... booleanValues) { if (booleanValues == null) { return new AutoValue_AttributeValue_AttributeValueBooleanArray( Collections.emptyList()); } List values = new ArrayList<>(booleanValues.length); - for (boolean value : booleanValues) { + for (Boolean value : booleanValues) { values.add(value); } return new AutoValue_AttributeValue_AttributeValueBooleanArray( @@ -379,12 +379,12 @@ abstract static class AttributeValueLongArray extends AttributeValue { AttributeValueLongArray() {} - static AttributeValue create(long... longValues) { + static AttributeValue create(Long... longValues) { if (longValues == null) { return new AutoValue_AttributeValue_AttributeValueLongArray(Collections.emptyList()); } List values = new ArrayList<>(longValues.length); - for (long value : longValues) { + for (Long value : longValues) { values.add(value); } return new AutoValue_AttributeValue_AttributeValueLongArray( @@ -406,13 +406,13 @@ abstract static class AttributeValueDoubleArray extends AttributeValue { AttributeValueDoubleArray() {} - static AttributeValue create(double... doubleValues) { + static AttributeValue create(Double... doubleValues) { if (doubleValues == null) { return new AutoValue_AttributeValue_AttributeValueDoubleArray( Collections.emptyList()); } List values = new ArrayList<>(doubleValues.length); - for (double value : doubleValues) { + for (Double value : doubleValues) { values.add(value); } return new AutoValue_AttributeValue_AttributeValueDoubleArray( diff --git a/api/src/test/java/io/opentelemetry/common/AttributeValueTest.java b/api/src/test/java/io/opentelemetry/common/AttributeValueTest.java index 48b7d1532a1..713227bbca6 100644 --- a/api/src/test/java/io/opentelemetry/common/AttributeValueTest.java +++ b/api/src/test/java/io/opentelemetry/common/AttributeValueTest.java @@ -68,9 +68,9 @@ public void attributeValue_EqualsAndHashCode() { public void doNotCrashOnNull() { AttributeValue.stringAttributeValue(null); AttributeValue.arrayAttributeValue((String[]) null); - AttributeValue.arrayAttributeValue((boolean[]) null); - AttributeValue.arrayAttributeValue((long[]) null); - AttributeValue.arrayAttributeValue((double[]) null); + AttributeValue.arrayAttributeValue((Boolean[]) null); + AttributeValue.arrayAttributeValue((Long[]) null); + AttributeValue.arrayAttributeValue((Double[]) null); } @Test @@ -98,4 +98,21 @@ public void attributeValue_ToString() { assertThat(attribute.toString()).contains("1.2345"); assertThat(attribute.toString()).contains("6.789"); } + + @Test + public void arrayAttributeValue_nullValuesWithinArray() { + AttributeValue attribute; + + attribute = AttributeValue.arrayAttributeValue("string", null, "", "string"); + assertThat(attribute.getStringArrayValue().size()).isEqualTo(4); + + attribute = AttributeValue.arrayAttributeValue(10L, null, 20L); + assertThat(attribute.getLongArrayValue().size()).isEqualTo(3); + + attribute = AttributeValue.arrayAttributeValue(true, null, false); + assertThat(attribute.getBooleanArrayValue().size()).isEqualTo(3); + + attribute = AttributeValue.arrayAttributeValue(1.2, null, 3.4); + assertThat(attribute.getDoubleArrayValue().size()).isEqualTo(3); + } } diff --git a/api/src/test/java/io/opentelemetry/trace/DefaultSpanTest.java b/api/src/test/java/io/opentelemetry/trace/DefaultSpanTest.java index 851ee47c5cd..c27e606c68b 100644 --- a/api/src/test/java/io/opentelemetry/trace/DefaultSpanTest.java +++ b/api/src/test/java/io/opentelemetry/trace/DefaultSpanTest.java @@ -57,9 +57,9 @@ public void doNotCrash() { span.setAttribute("NullString", (String) null); span.setAttribute("EmptyString", ""); span.setAttribute("NullArrayString", AttributeValue.arrayAttributeValue((String[]) null)); - span.setAttribute("NullArrayBoolean", AttributeValue.arrayAttributeValue((boolean[]) null)); - span.setAttribute("NullArrayLong", AttributeValue.arrayAttributeValue((long[]) null)); - span.setAttribute("NullArrayDouble", AttributeValue.arrayAttributeValue((double[]) null)); + span.setAttribute("NullArrayBoolean", AttributeValue.arrayAttributeValue((Boolean[]) null)); + span.setAttribute("NullArrayLong", AttributeValue.arrayAttributeValue((Long[]) null)); + span.setAttribute("NullArrayDouble", AttributeValue.arrayAttributeValue((Double[]) null)); span.addEvent("event"); span.addEvent("event", 0); span.addEvent( diff --git a/exporters/otlp/src/main/java/io/opentelemetry/exporters/otlp/CommonAdapter.java b/exporters/otlp/src/main/java/io/opentelemetry/exporters/otlp/CommonAdapter.java index a66524faf2c..e6c1f03d4f7 100644 --- a/exporters/otlp/src/main/java/io/opentelemetry/exporters/otlp/CommonAdapter.java +++ b/exporters/otlp/src/main/java/io/opentelemetry/exporters/otlp/CommonAdapter.java @@ -43,6 +43,11 @@ static AttributeKeyValue toProtoAttribute(String key, AttributeValue attributeVa .setType(ValueType.DOUBLE) .setDoubleValue(attributeValue.getDoubleValue()) .build(); + case BOOLEAN_ARRAY: + case LONG_ARRAY: + case DOUBLE_ARRAY: + case STRING_ARRAY: + return builder.setType(ValueType.UNRECOGNIZED).build(); } return builder.setType(ValueType.UNRECOGNIZED).build(); } 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 41e37cfbcfd..d3e60268a19 100644 --- a/sdk/src/main/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpan.java +++ b/sdk/src/main/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpan.java @@ -19,7 +19,6 @@ import com.google.common.base.Preconditions; import com.google.common.collect.EvictingQueue; import io.opentelemetry.common.AttributeValue; -import io.opentelemetry.common.AttributeValue.Type; import io.opentelemetry.internal.StringUtils; import io.opentelemetry.sdk.common.Clock; import io.opentelemetry.sdk.common.InstrumentationLibraryInfo; 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 8eccc7eda74..a3ddab5c302 100644 --- a/sdk/src/test/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpanTest.java +++ b/sdk/src/test/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpanTest.java @@ -303,10 +303,10 @@ public void setAttribute() { span.setAttribute( "ArrayBooleanKey", AttributeValue.arrayAttributeValue(true, false, false, true)); span.setAttribute("NullArrayStringKey", AttributeValue.arrayAttributeValue((String[]) null)); - span.setAttribute("NullArrayLongKey", AttributeValue.arrayAttributeValue((long[]) null)); - span.setAttribute("NullArrayDoubleKey", AttributeValue.arrayAttributeValue((double[]) null)); + span.setAttribute("NullArrayLongKey", AttributeValue.arrayAttributeValue((Long[]) null)); + span.setAttribute("NullArrayDoubleKey", AttributeValue.arrayAttributeValue((Double[]) null)); span.setAttribute( - "NullArrayBooleanKey", AttributeValue.arrayAttributeValue((boolean[]) null)); + "NullArrayBooleanKey", AttributeValue.arrayAttributeValue((Boolean[]) null)); } finally { span.end(); } From 8ca0412fc822b0d73eb580ffb1515e7c6c32a897 Mon Sep 17 00:00:00 2001 From: Giovanni Liva Date: Fri, 20 Mar 2020 18:28:52 +0100 Subject: [PATCH 16/17] Rebase and add tests --- .../sdk/trace/SpanBuilderSdk.java | 18 ++----------- .../sdk/trace/SpanBuilderSdkTest.java | 26 ++++++++++++++++++- 2 files changed, 27 insertions(+), 17 deletions(-) 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 cf3d9fef1b4..0ce9df556c9 100644 --- a/sdk/src/main/java/io/opentelemetry/sdk/trace/SpanBuilderSdk.java +++ b/sdk/src/main/java/io/opentelemetry/sdk/trace/SpanBuilderSdk.java @@ -168,7 +168,8 @@ public Span.Builder setAttribute(String key, boolean value) { @Override public Span.Builder setAttribute(String key, AttributeValue value) { Utils.checkNotNull(key, "key"); - if (isRemovedValue(value)) { + if (value == null + || (value.getType().equals(AttributeValue.Type.STRING) && value.getStringValue() == null)) { attributes.remove(key); } else { attributes.putAttribute(key, value); @@ -176,21 +177,6 @@ public Span.Builder setAttribute(String key, AttributeValue value) { return this; } - private static boolean isRemovedValue(AttributeValue value) { - if (value == null) { - return true; - } - switch (value.getType()) { - case STRING: - return value.getStringValue() == null; - case BOOLEAN: - case LONG: - case DOUBLE: - return false; - } - return false; - } - @Override public Span.Builder setStartTimestamp(long startTimestamp) { Utils.checkArgument(startTimestamp >= 0, "Negative startTimestamp"); 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 fc52f9973df..3b3b1430bd1 100644 --- a/sdk/src/test/java/io/opentelemetry/sdk/trace/SpanBuilderSdkTest.java +++ b/sdk/src/test/java/io/opentelemetry/sdk/trace/SpanBuilderSdkTest.java @@ -172,6 +172,20 @@ public void setAttribute() { } } + @Test + public void setAttribute_emptyArrayAttributeValue() throws Exception { + Span.Builder spanBuilder = tracerSdk.spanBuilder(SPAN_NAME); + spanBuilder.setAttribute( + "stringArrayAttribute", AttributeValue.arrayAttributeValue(new String[0])); + spanBuilder.setAttribute( + "boolArrayAttribute", AttributeValue.arrayAttributeValue(new Boolean[0])); + spanBuilder.setAttribute("longArrayAttribute", AttributeValue.arrayAttributeValue(new Long[0])); + spanBuilder.setAttribute( + "doubleArrayAttribute", AttributeValue.arrayAttributeValue(new Double[0])); + RecordEventsReadableSpan span = (RecordEventsReadableSpan) spanBuilder.startSpan(); + assertThat(span.toSpanData().getAttributes().size()).isEqualTo(4); + } + @Test public void setAttribute_nullStringValue() throws Exception { Span.Builder spanBuilder = tracerSdk.spanBuilder(SPAN_NAME); @@ -196,13 +210,23 @@ public void setAttribute_nullAttributeValue() throws Exception { spanBuilder.setAttribute("longAttribute", 0L); spanBuilder.setAttribute("boolAttribute", false); spanBuilder.setAttribute("doubleAttribute", 0.12345f); + spanBuilder.setAttribute("stringArrayAttribute", AttributeValue.arrayAttributeValue("", null)); + spanBuilder.setAttribute("boolArrayAttribute", AttributeValue.arrayAttributeValue(true, null)); + spanBuilder.setAttribute( + "longArrayAttribute", AttributeValue.arrayAttributeValue(12345L, null)); + spanBuilder.setAttribute( + "doubleArrayAttribute", AttributeValue.arrayAttributeValue(1.2345, null)); RecordEventsReadableSpan span = (RecordEventsReadableSpan) spanBuilder.startSpan(); - assertThat(span.toSpanData().getAttributes().size()).isEqualTo(5); + assertThat(span.toSpanData().getAttributes().size()).isEqualTo(9); spanBuilder.setAttribute("emptyString", (AttributeValue) null); spanBuilder.setAttribute("emptyStringAttributeValue", (AttributeValue) null); spanBuilder.setAttribute("longAttribute", (AttributeValue) null); spanBuilder.setAttribute("boolAttribute", (AttributeValue) null); spanBuilder.setAttribute("doubleAttribute", (AttributeValue) null); + spanBuilder.setAttribute("stringArrayAttribute", (AttributeValue) null); + spanBuilder.setAttribute("boolArrayAttribute", (AttributeValue) null); + spanBuilder.setAttribute("longArrayAttribute", (AttributeValue) null); + spanBuilder.setAttribute("doubleArrayAttribute", (AttributeValue) null); assertThat(span.toSpanData().getAttributes()).isEmpty(); } From 84c0eb35c6a1f231301b11fdfa9780f041382ee5 Mon Sep 17 00:00:00 2001 From: Giovanni Liva Date: Fri, 20 Mar 2020 18:40:38 +0100 Subject: [PATCH 17/17] Adapt RecordEventsReadableSpan --- .../sdk/trace/RecordEventsReadableSpan.java | 37 +++---------- .../trace/RecordEventsReadableSpanTest.java | 52 ++++++++++++++++++- 2 files changed, 58 insertions(+), 31 deletions(-) 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 d3e60268a19..c4b075f4904 100644 --- a/sdk/src/main/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpan.java +++ b/sdk/src/main/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpan.java @@ -16,10 +16,11 @@ 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.internal.StringUtils; import io.opentelemetry.sdk.common.Clock; import io.opentelemetry.sdk.common.InstrumentationLibraryInfo; import io.opentelemetry.sdk.resources.Resource; @@ -331,35 +332,11 @@ public void setAttribute(String key, boolean value) { @Override public void setAttribute(String key, AttributeValue value) { Preconditions.checkNotNull(key, "key"); - Preconditions.checkNotNull(value, "value"); - switch (value.getType()) { - case STRING: - if (StringUtils.isNullOrEmpty(value.getStringValue())) { - return; - } - break; - case STRING_ARRAY: - if (value.getStringArrayValue().size() == 0) { - return; - } - break; - case BOOLEAN_ARRAY: - if (value.getBooleanArrayValue().size() == 0) { - return; - } - break; - case LONG_ARRAY: - if (value.getLongArrayValue().size() == 0) { - return; - } - break; - case DOUBLE_ARRAY: - if (value.getDoubleArrayValue().size() == 0) { - return; - } - break; - default: - break; + if (value == null || (value.getType().equals(STRING) && value.getStringValue() == null)) { + synchronized (lock) { + attributes.remove(key); + } + return; } synchronized (lock) { if (hasEnded) { 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 a3ddab5c302..30feab4f5e3 100644 --- a/sdk/src/test/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpanTest.java +++ b/sdk/src/test/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpanTest.java @@ -311,7 +311,7 @@ public void setAttribute() { span.end(); } SpanData spanData = span.toSpanData(); - assertThat(spanData.getAttributes().size()).isEqualTo(8); + assertThat(spanData.getAttributes().size()).isEqualTo(14); assertThat(spanData.getAttributes().get("ArrayStringKey").getStringArrayValue().size()) .isEqualTo(4); assertThat(spanData.getAttributes().get("ArrayLongKey").getLongArrayValue().size()) @@ -322,6 +322,56 @@ public void setAttribute() { .isEqualTo(4); } + @Test + public void setAttribute_emptyArrayAttributeValue() throws Exception { + RecordEventsReadableSpan span = createTestRootSpan(); + span.setAttribute("stringArrayAttribute", AttributeValue.arrayAttributeValue(new String[0])); + span.setAttribute("boolArrayAttribute", AttributeValue.arrayAttributeValue(new Boolean[0])); + span.setAttribute("longArrayAttribute", AttributeValue.arrayAttributeValue(new Long[0])); + span.setAttribute("doubleArrayAttribute", AttributeValue.arrayAttributeValue(new Double[0])); + assertThat(span.toSpanData().getAttributes().size()).isEqualTo(4); + } + + @Test + public void setAttribute_nullStringValue() throws Exception { + RecordEventsReadableSpan span = createTestRootSpan(); + span.setAttribute("emptyString", ""); + span.setAttribute("nullString", (String) null); + span.setAttribute("nullStringAttributeValue", AttributeValue.stringAttributeValue(null)); + span.setAttribute("emptyStringAttributeValue", AttributeValue.stringAttributeValue("")); + assertThat(span.toSpanData().getAttributes().size()).isEqualTo(2); + span.setAttribute("emptyString", (String) null); + span.setAttribute("emptyStringAttributeValue", (String) null); + assertThat(span.toSpanData().getAttributes()).isEmpty(); + } + + @Test + public void setAttribute_nullAttributeValue() throws Exception { + RecordEventsReadableSpan span = createTestRootSpan(); + span.setAttribute("emptyString", ""); + span.setAttribute("nullString", (AttributeValue) null); + span.setAttribute("nullStringAttributeValue", AttributeValue.stringAttributeValue(null)); + span.setAttribute("emptyStringAttributeValue", AttributeValue.stringAttributeValue("")); + span.setAttribute("longAttribute", 0L); + span.setAttribute("boolAttribute", false); + span.setAttribute("doubleAttribute", 0.12345f); + span.setAttribute("stringArrayAttribute", AttributeValue.arrayAttributeValue("", null)); + span.setAttribute("boolArrayAttribute", AttributeValue.arrayAttributeValue(true, null)); + span.setAttribute("longArrayAttribute", AttributeValue.arrayAttributeValue(12345L, null)); + span.setAttribute("doubleArrayAttribute", AttributeValue.arrayAttributeValue(1.2345, null)); + assertThat(span.toSpanData().getAttributes().size()).isEqualTo(9); + span.setAttribute("emptyString", (AttributeValue) null); + span.setAttribute("emptyStringAttributeValue", (AttributeValue) null); + span.setAttribute("longAttribute", (AttributeValue) null); + span.setAttribute("boolAttribute", (AttributeValue) null); + span.setAttribute("doubleAttribute", (AttributeValue) null); + span.setAttribute("stringArrayAttribute", (AttributeValue) null); + span.setAttribute("boolArrayAttribute", (AttributeValue) null); + span.setAttribute("longArrayAttribute", (AttributeValue) null); + span.setAttribute("doubleArrayAttribute", (AttributeValue) null); + assertThat(span.toSpanData().getAttributes()).isEmpty(); + } + @Test public void addEvent() { RecordEventsReadableSpan span = createTestRootSpan();