Do not allow null description in the StatusData (#2896)

Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
This commit is contained in:
Bogdan Drutu 2021-02-22 15:31:45 -08:00 committed by GitHub
parent 013643c707
commit 9e1c76b324
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 19 additions and 22 deletions

View File

@ -278,7 +278,7 @@ public interface Span extends ImplicitContextKeyed {
* @return this.
*/
default Span setStatus(StatusCode statusCode) {
return setStatus(statusCode, null);
return setStatus(statusCode, "");
}
/**

View File

@ -96,7 +96,7 @@ final class Adapter {
.setVStr(span.getKind().name().toLowerCase(Locale.ROOT)));
}
if (span.getStatus().getDescription() != null) {
if (!span.getStatus().getDescription().isEmpty()) {
tags.add(
new Tag(KEY_SPAN_STATUS_MESSAGE, TagType.STRING)
.setVStr(span.getStatus().getDescription()));

View File

@ -96,7 +96,7 @@ final class Adapter {
.build());
}
if (span.getStatus().getDescription() != null) {
if (!span.getStatus().getDescription().isEmpty()) {
target.addTags(
Model.KeyValue.newBuilder()
.setKey(KEY_SPAN_STATUS_MESSAGE)

View File

@ -168,7 +168,7 @@ public final class SpanAdapter {
@SuppressWarnings("deprecation") // setDeprecatedCode is deprecated.
Status.Builder builder =
Status.newBuilder().setCode(protoStatusCode).setDeprecatedCode(deprecatedStatusCode);
if (status.getDescription() != null) {
if (!status.getDescription().isEmpty()) {
builder.setMessage(status.getDescription());
}
return builder.build();

View File

@ -278,12 +278,12 @@ class ZipkinSpanExporterTest {
}
@Test
void generateSpan_WithRpcErrorStatus_WithNullErrorDescription() {
void generateSpan_WithRpcErrorStatus_WithEmptyErrorDescription() {
Attributes attributeMap = Attributes.of(SemanticAttributes.RPC_SERVICE, "my service name");
SpanData data =
buildStandardSpan()
.setStatus(StatusData.create(StatusCode.ERROR, null))
.setStatus(StatusData.create(StatusCode.ERROR, ""))
.setAttributes(attributeMap)
.build();
@ -302,7 +302,7 @@ class ZipkinSpanExporterTest {
SpanData data =
buildStandardSpan()
.setStatus(StatusData.create(StatusCode.UNSET, null))
.setStatus(StatusData.create(StatusCode.UNSET, ""))
.setAttributes(attributeMap)
.build();

View File

@ -9,7 +9,6 @@ import com.google.auto.value.AutoValue;
import io.opentelemetry.api.trace.Span;
import io.opentelemetry.api.trace.StatusCode;
import java.util.EnumMap;
import javax.annotation.Nullable;
import javax.annotation.concurrent.Immutable;
/**
@ -25,13 +24,13 @@ abstract class ImmutableStatusData implements StatusData {
* The operation has been validated by an Application developers or Operator to have completed
* successfully.
*/
static final StatusData OK = createInternal(StatusCode.OK, null);
static final StatusData OK = createInternal(StatusCode.OK, "");
/** The default status. */
static final StatusData UNSET = createInternal(StatusCode.UNSET, null);
static final StatusData UNSET = createInternal(StatusCode.UNSET, "");
/** The operation contains an error. */
static final StatusData ERROR = createInternal(StatusCode.ERROR, null);
static final StatusData ERROR = createInternal(StatusCode.ERROR, "");
// Visible for test
static final EnumMap<StatusCode, StatusData> codeToStatus = new EnumMap<>(StatusCode.class);
@ -47,7 +46,7 @@ abstract class ImmutableStatusData implements StatusData {
for (StatusCode code : codes) {
StatusData status = codeToStatus.get(code);
if (status == null) {
codeToStatus.put(code, createInternal(code, null));
codeToStatus.put(code, createInternal(code, ""));
}
}
}
@ -58,14 +57,14 @@ abstract class ImmutableStatusData implements StatusData {
* @param description the new description of the {@code Status}.
* @return The newly created {@code Status} with the given description.
*/
static StatusData create(StatusCode statusCode, @Nullable String description) {
if (description == null) {
static StatusData create(StatusCode statusCode, String description) {
if (description == null || description.isEmpty()) {
return codeToStatus.get(statusCode);
}
return createInternal(statusCode, description);
}
private static StatusData createInternal(StatusCode statusCode, @Nullable String description) {
private static StatusData createInternal(StatusCode statusCode, String description) {
return new AutoValue_ImmutableStatusData(statusCode, description);
}
}

View File

@ -7,7 +7,6 @@ package io.opentelemetry.sdk.trace.data;
import io.opentelemetry.api.trace.Span;
import io.opentelemetry.api.trace.StatusCode;
import javax.annotation.Nullable;
import javax.annotation.concurrent.Immutable;
/**
@ -39,7 +38,7 @@ public interface StatusData {
* Returns a {@link StatusData} with the given {@code code} and {@code description}. If {@code
* description} is {@code null}, the returned {@link StatusData} does not have a description.
*/
static StatusData create(StatusCode code, @Nullable String description) {
static StatusData create(StatusCode code, String description) {
return ImmutableStatusData.create(code, description);
}
@ -51,6 +50,5 @@ public interface StatusData {
*
* @return the description of this {@code Status}.
*/
@Nullable
String getDescription();
}

View File

@ -17,11 +17,11 @@ class ImmutableStatusDataTest {
StatusCode[] codes = StatusCode.values();
assertThat(codes).hasSize(3);
assertThat(StatusData.unset().getStatusCode()).isEqualTo(StatusCode.UNSET);
assertThat(StatusData.unset().getDescription()).isNull();
assertThat(StatusData.unset().getDescription()).isEmpty();
assertThat(StatusData.ok().getStatusCode()).isEqualTo(StatusCode.OK);
assertThat(StatusData.ok().getDescription()).isNull();
assertThat(StatusData.ok().getDescription()).isEmpty();
assertThat(StatusData.error().getStatusCode()).isEqualTo(StatusCode.ERROR);
assertThat(StatusData.error().getDescription()).isNull();
assertThat(StatusData.error().getDescription()).isEmpty();
}
@Test
@ -31,7 +31,7 @@ class ImmutableStatusDataTest {
for (StatusCode code : codes) {
assertThat(ImmutableStatusData.codeToStatus.get(code)).isNotNull();
assertThat(ImmutableStatusData.codeToStatus.get(code).getStatusCode()).isEqualTo(code);
assertThat(ImmutableStatusData.codeToStatus.get(code).getDescription()).isNull();
assertThat(ImmutableStatusData.codeToStatus.get(code).getDescription()).isEmpty();
}
}
}