Skip to content

Update Attributes API to comply the spec #1437

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Aug 18, 2020
Merged

Update Attributes API to comply the spec #1437

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
d349e45
Forbit null values
thisthat Jul 18, 2020
0209edd
Simplify code and better naming
thisthat Jul 21, 2020
a648e44
Fix error prone error
thisthat Jul 21, 2020
a97baaf
Uniform null handling
thisthat Aug 4, 2020
83fe7c4
Improve tests for SpanAttributes
thisthat Aug 4, 2020
34cdcca
Improve tests for Resource Attributes
thisthat Aug 4, 2020
c3785f6
Change interface of AttributeValue
thisthat Aug 5, 2020
22e1948
isEmpty overrided only when necessary
thisthat Aug 6, 2020
b90449d
Update Attributes API
thisthat Aug 17, 2020
2bdb1cf
Remove unused code
thisthat Aug 17, 2020
ed949be
Rename method
thisthat Aug 17, 2020
075ce06
Address PR feedback
thisthat Aug 18, 2020
0387979
fix javadoc
thisthat Aug 18, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 48 additions & 7 deletions api/src/main/java/io/opentelemetry/common/AttributeValue.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,17 @@ public List<Double> getDoubleArrayValue() {
*/
public abstract Type getType();

/**
* Returns {@code true} if the {@code AttributeValue} contains a {@code null} value or it is an
* empty array.
*
* @return {@code true} if the {@code AttributeValue} is empty.
* @since 0.8.0
*/
public boolean isEmpty() {
return false;
}

@Immutable
@AutoValue
abstract static class AttributeValueString extends AttributeValue {
Expand All @@ -259,6 +270,11 @@ public final Type getType() {
return Type.STRING;
}

@Override
public boolean isEmpty() {
return getStringValue() == null;
}

@Override
@Nullable
public abstract String getStringValue();
Expand Down Expand Up @@ -325,12 +341,13 @@ public final Type getType() {
@AutoValue
abstract static class AttributeValueStringArray extends AttributeValue {

private static final List<String> EMPTY = Collections.<String>emptyList();

AttributeValueStringArray() {}

static AttributeValue create(String... stringValues) {
if (stringValues == null) {
return new AutoValue_AttributeValue_AttributeValueStringArray(
Collections.<String>emptyList());
return new AutoValue_AttributeValue_AttributeValueStringArray(EMPTY);
}
return new AutoValue_AttributeValue_AttributeValueStringArray(
Collections.unmodifiableList(Arrays.asList(stringValues)));
Expand All @@ -341,6 +358,11 @@ public final Type getType() {
return Type.STRING_ARRAY;
}

@Override
public boolean isEmpty() {
return getStringArrayValue() == EMPTY;
}

@Override
public abstract List<String> getStringArrayValue();
}
Expand All @@ -349,12 +371,13 @@ public final Type getType() {
@AutoValue
abstract static class AttributeValueBooleanArray extends AttributeValue {

private static final List<Boolean> EMPTY = Collections.<Boolean>emptyList();

AttributeValueBooleanArray() {}

static AttributeValue create(Boolean... booleanValues) {
if (booleanValues == null) {
return new AutoValue_AttributeValue_AttributeValueBooleanArray(
Collections.<Boolean>emptyList());
return new AutoValue_AttributeValue_AttributeValueBooleanArray(EMPTY);
}
List<Boolean> values = new ArrayList<>(booleanValues.length);
values.addAll(Arrays.asList(booleanValues));
Expand All @@ -367,6 +390,11 @@ public final Type getType() {
return Type.BOOLEAN_ARRAY;
}

@Override
public boolean isEmpty() {
return getBooleanArrayValue() == EMPTY;
}

@Override
public abstract List<Boolean> getBooleanArrayValue();
}
Expand All @@ -375,11 +403,13 @@ public final Type getType() {
@AutoValue
abstract static class AttributeValueLongArray extends AttributeValue {

private static final List<Long> EMPTY = Collections.<Long>emptyList();

AttributeValueLongArray() {}

static AttributeValue create(Long... longValues) {
if (longValues == null) {
return new AutoValue_AttributeValue_AttributeValueLongArray(Collections.<Long>emptyList());
return new AutoValue_AttributeValue_AttributeValueLongArray(EMPTY);
}
List<Long> values = new ArrayList<>(longValues.length);
values.addAll(Arrays.asList(longValues));
Expand All @@ -392,6 +422,11 @@ public final Type getType() {
return Type.LONG_ARRAY;
}

@Override
public boolean isEmpty() {
return getLongArrayValue() == EMPTY;
}

@Override
public abstract List<Long> getLongArrayValue();
}
Expand All @@ -400,12 +435,13 @@ public final Type getType() {
@AutoValue
abstract static class AttributeValueDoubleArray extends AttributeValue {

private static final List<Double> EMPTY = Collections.<Double>emptyList();

AttributeValueDoubleArray() {}

static AttributeValue create(Double... doubleValues) {
if (doubleValues == null) {
return new AutoValue_AttributeValue_AttributeValueDoubleArray(
Collections.<Double>emptyList());
return new AutoValue_AttributeValue_AttributeValueDoubleArray(EMPTY);
}
List<Double> values = new ArrayList<>(doubleValues.length);
values.addAll(Arrays.asList(doubleValues));
Expand All @@ -418,6 +454,11 @@ public final Type getType() {
return Type.DOUBLE_ARRAY;
}

@Override
public boolean isEmpty() {
return getDoubleArrayValue() == EMPTY;
}

@Override
public abstract List<Double> getDoubleArrayValue();
}
Expand Down
68 changes: 60 additions & 8 deletions api/src/main/java/io/opentelemetry/common/Attributes.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,12 +155,32 @@ public Attributes build() {
return sortAndFilterToAttributes(data.toArray());
}

private boolean doNotAdd(String key, AttributeValue value) {
if (key == null || key.length() == 0) {
return true;
}
if (value == null || value.isEmpty()) {
int index = data.indexOf(key);
if (index == -1) {
return true;
}
// Remove key/value pair
data.remove(index);
data.remove(index);
return true;
}
return false;
}

/**
* Sets a bare {@link AttributeValue} into this.
*
* @return this Builder
*/
public Builder setAttribute(String key, AttributeValue value) {
if (doNotAdd(key, value)) {
return this;
}
data.add(key);
data.add(value);
return this;
Expand All @@ -172,8 +192,12 @@ public Builder setAttribute(String key, AttributeValue value) {
* @return this Builder
*/
public Builder setAttribute(String key, String value) {
AttributeValue v = stringAttributeValue(value);
if (doNotAdd(key, v)) {
return this;
}
data.add(key);
data.add(stringAttributeValue(value));
data.add(v);
return this;
}

Expand All @@ -183,8 +207,12 @@ public Builder setAttribute(String key, String value) {
* @return this Builder
*/
public Builder setAttribute(String key, long value) {
AttributeValue v = longAttributeValue(value);
if (doNotAdd(key, v)) {
return this;
}
data.add(key);
data.add(longAttributeValue(value));
data.add(v);
return this;
}

Expand All @@ -194,8 +222,12 @@ public Builder setAttribute(String key, long value) {
* @return this Builder
*/
public Builder setAttribute(String key, double value) {
AttributeValue v = doubleAttributeValue(value);
if (doNotAdd(key, v)) {
return this;
}
data.add(key);
data.add(doubleAttributeValue(value));
data.add(v);
return this;
}

Expand All @@ -205,8 +237,12 @@ public Builder setAttribute(String key, double value) {
* @return this Builder
*/
public Builder setAttribute(String key, boolean value) {
AttributeValue v = booleanAttributeValue(value);
if (doNotAdd(key, v)) {
return this;
}
data.add(key);
data.add(booleanAttributeValue(value));
data.add(v);
return this;
}

Expand All @@ -216,8 +252,12 @@ public Builder setAttribute(String key, boolean value) {
* @return this Builder
*/
public Builder setAttribute(String key, String... value) {
AttributeValue v = arrayAttributeValue(value);
if (doNotAdd(key, v)) {
return this;
}
data.add(key);
data.add(arrayAttributeValue(value));
data.add(v);
return this;
}

Expand All @@ -227,8 +267,12 @@ public Builder setAttribute(String key, String... value) {
* @return this Builder
*/
public Builder setAttribute(String key, Long... value) {
AttributeValue v = arrayAttributeValue(value);
if (doNotAdd(key, v)) {
return this;
}
data.add(key);
data.add(arrayAttributeValue(value));
data.add(v);
return this;
}

Expand All @@ -238,8 +282,12 @@ public Builder setAttribute(String key, Long... value) {
* @return this Builder
*/
public Builder setAttribute(String key, Double... value) {
AttributeValue v = arrayAttributeValue(value);
if (doNotAdd(key, v)) {
return this;
}
data.add(key);
data.add(arrayAttributeValue(value));
data.add(v);
return this;
}

Expand All @@ -249,8 +297,12 @@ public Builder setAttribute(String key, Double... value) {
* @return this Builder
*/
public Builder setAttribute(String key, Boolean... value) {
AttributeValue v = arrayAttributeValue(value);
if (doNotAdd(key, v)) {
return this;
}
data.add(key);
data.add(arrayAttributeValue(value));
data.add(v);
return this;
}
}
Expand Down
22 changes: 22 additions & 0 deletions api/src/test/java/io/opentelemetry/common/AttributesTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,4 +219,26 @@ void toBuilder() {
// Original not mutated.
assertThat(partial).isEqualTo(Attributes.newBuilder().setAttribute("cat", "meow").build());
}

@Test
void deleteByNull() {
Attributes.Builder attributes = Attributes.newBuilder();
attributes.setAttribute("attrValue", AttributeValue.stringAttributeValue("attrValue"));
attributes.setAttribute("string", "string");
attributes.setAttribute("long", 10);
attributes.setAttribute("double", 1.0);
attributes.setAttribute("bool", true);
attributes.setAttribute("arrayString", new String[] {"string"});
attributes.setAttribute("arrayLong", new Long[] {10L});
attributes.setAttribute("arrayDouble", new Double[] {1.0});
attributes.setAttribute("arrayBool", new Boolean[] {true});
assertThat(attributes.build().size()).isEqualTo(9);
attributes.setAttribute("attrValue", (AttributeValue) null);
attributes.setAttribute("string", (String) null);
attributes.setAttribute("arrayString", (String[]) null);
attributes.setAttribute("arrayLong", (Long[]) null);
attributes.setAttribute("arrayDouble", (Double[]) null);
attributes.setAttribute("arrayBool", (Boolean[]) null);
assertThat(attributes.build().size()).isEqualTo(3);
}
}
9 changes: 3 additions & 6 deletions sdk/src/main/java/io/opentelemetry/sdk/trace/RecordEventsReadableSpan.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@

package io.opentelemetry.sdk.trace;

import static io.opentelemetry.common.AttributeValue.Type.STRING;

import com.google.common.collect.EvictingQueue;
import io.opentelemetry.common.AttributeValue;
import io.opentelemetry.common.Attributes;
Expand Down Expand Up @@ -314,11 +312,10 @@ public void setAttribute(String key, AttributeValue value) {
logger.log(Level.FINE, "Calling setAttribute() on an ended Span.");
return;
}
if (value == null || (value.getType().equals(STRING) && value.getStringValue() == null)) {
if (attributes == null) {
return;
if (value == null || value.isEmpty()) {
if (attributes != null) {
attributes.remove(key);
}
attributes.remove(key);
return;
}
if (attributes == null) {
Expand Down
Loading