Some cleanup in span land. (#2062)

CHANGELOG:  Deprecated the `getCanonicalCode` method on `SpanBuilder` ; replaced with `getStatusCode`

* Some cleanup in span land.
* Clean up the SpanBuilder javadoc to match the current APIs
* Scrub mentions of canonical status code
* Deprecate the getCanonicalCode method on SpanBuilder and replace with getStatusCode

* rename an internal method to make more sense

* Update api/src/main/java/io/opentelemetry/api/trace/SpanBuilder.java

Co-authored-by: Armin Ruech <armin.ruech@dynatrace.com>

* Update api/src/main/java/io/opentelemetry/api/trace/SpanBuilder.java

Co-authored-by: Armin Ruech <armin.ruech@dynatrace.com>

* Update api/src/main/java/io/opentelemetry/api/trace/SpanBuilder.java

Co-authored-by: Armin Ruech <armin.ruech@dynatrace.com>

* add tests for the deprecated method

Co-authored-by: Armin Ruech <armin.ruech@dynatrace.com>
This commit is contained in:
John Watson 2020-11-11 18:34:07 -08:00 committed by GitHub
parent 9d17c6619a
commit 1fe334ff12
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 72 additions and 57 deletions

View File

@ -87,12 +87,12 @@ final class PropagatedSpan implements Span {
}
@Override
public Span setStatus(StatusCode canonicalCode) {
public Span setStatus(StatusCode statusCode) {
return this;
}
@Override
public Span setStatus(StatusCode canonicalCode, String description) {
public Span setStatus(StatusCode statusCode, String description) {
return this;
}

View File

@ -295,10 +295,10 @@ public interface Span extends ImplicitContextKeyed {
* <p>Only the value of the last call will be recorded, and implementations are free to ignore
* previous calls.
*
* @param canonicalCode the {@link StatusCode} to set.
* @param statusCode the {@link StatusCode} to set.
* @return this.
*/
Span setStatus(StatusCode canonicalCode);
Span setStatus(StatusCode statusCode);
/**
* Sets the status to the {@code Span}.
@ -309,11 +309,11 @@ public interface Span extends ImplicitContextKeyed {
* <p>Only the value of the last call will be recorded, and implementations are free to ignore
* previous calls.
*
* @param canonicalCode the {@link StatusCode} to set.
* @param statusCode the {@link StatusCode} to set.
* @param description the description of the {@code Status}.
* @return this.
*/
Span setStatus(StatusCode canonicalCode, String description);
Span setStatus(StatusCode statusCode, String description);
/**
* Records information about the {@link Throwable} to the {@link Span}.

View File

@ -24,12 +24,13 @@ import javax.annotation.Nonnull;
*
* <pre>{@code
* class MyClass {
* private static final Tracer tracer = OpenTelemetry.getTracer();
* private static final Tracer tracer = OpenTelemetry.get().getTracer("com.anyco.rpc");
*
* void doWork {
* // Create a Span as a child of the current Span.
* Span span = tracer.spanBuilder("MyChildSpan").startSpan();
* try (Scope ss = TracingContextUtils.currentContextWith(span)) {
* TracingContextUtils.getCurrentSpan().addEvent("my event");
* try (Scope ss = span.makeCurrent()) {
* span.addEvent("my event");
* doSomeWork(); // Here the new span is in the current Context, so it can be used
* // implicitly anywhere down the stack.
* } finally {
@ -44,7 +45,7 @@ import javax.annotation.Nonnull;
*
* <pre>{@code
* class MyRpcServerInterceptorListener implements RpcServerInterceptor.Listener {
* private static final Tracer tracer = OpenTelemetry.getTracer();
* private static final Tracer tracer = OpenTelemetry.get().getTracer("com.example.rpc");
* private Span mySpan;
*
* public MyRpcInterceptor() {}
@ -52,12 +53,12 @@ import javax.annotation.Nonnull;
* public void onRequest(String rpcName, Metadata metadata) {
* // Create a Span as a child of the remote Span.
* mySpan = tracer.spanBuilder(rpcName)
* .setParent(getTraceContextFromMetadata(metadata)).startSpan();
* .setParent(extractContextFromMetadata(metadata)).startSpan();
* }
*
* public void onExecuteHandler(ServerCallHandler serverCallHandler) {
* try (Scope ws = TracingContextUtils.currentContextWith(mySpan)) {
* TracingContextUtils.getCurrentSpan().addEvent("Start rpc execution.");
* try (Scope ws = mySpan.makeCurrent()) {
* Span.current().addEvent("Start rpc execution.");
* serverCallHandler.run(); // Here the new span is in the current Context, so it can be
* // used implicitly anywhere down the stack.
* }
@ -66,7 +67,7 @@ import javax.annotation.Nonnull;
* // Called when the RPC is canceled and guaranteed onComplete will not be called.
* public void onCancel() {
* // IMPORTANT: DO NOT forget to ended the Span here as the work is done.
* mySpan.setStatus(Status.CANCELLED);
* mySpan.setStatus(StatusCode.ERROR);
* mySpan.end();
* }
*
@ -84,10 +85,12 @@ import javax.annotation.Nonnull;
*
* <pre>{@code
* class MyClass {
* private static final Tracer tracer = OpenTelemetry.getTracer();
* void DoWork(Span parent) {
* private static final Tracer tracer = OpenTelemetry.get().getTracer("com.example.rpc");
*
* void doWork(Span parent) {
* Span childSpan = tracer.spanBuilder("MyChildSpan")
* .setParent(parent).startSpan();
* .setParent(Context.current().with(parent))
* .startSpan();
* childSpan.addEvent("my event");
* try {
* doSomeWork(childSpan); // Manually propagate the new span down the stack.
@ -99,8 +102,7 @@ import javax.annotation.Nonnull;
* }
* }</pre>
*
* <p>If your Java version is less than Java SE 7, see {@link SpanBuilder#startSpan} for usage
* examples.
* <p>see {@link SpanBuilder#startSpan} for usage examples.
*/
public interface SpanBuilder {
@ -290,10 +292,11 @@ public interface SpanBuilder {
*
* <pre>{@code
* class MyClass {
* private static final Tracer tracer = OpenTelemetry.getTracer();
* void DoWork(Span parent) {
* private static final Tracer tracer = OpenTelemetry.get().getTracer("com.example.rpc");
*
* void doWork(Span parent) {
* Span childSpan = tracer.spanBuilder("MyChildSpan")
* .setParent(parent)
* .setParent(Context.current().with(parent))
* .startSpan();
* childSpan.addEvent("my event");
* try {

View File

@ -106,7 +106,7 @@ final class Adapter {
tags.add(
new Tag(KEY_SPAN_STATUS_CODE, TagType.LONG)
.setVLong(span.getStatus().getCanonicalCode().value()));
.setVLong(span.getStatus().getStatusCode().value()));
tags.add(
new Tag(KEY_INSTRUMENTATION_LIBRARY_NAME, TagType.STRING)

View File

@ -103,7 +103,7 @@ final class Adapter {
target.addTags(
Model.KeyValue.newBuilder()
.setKey(KEY_SPAN_STATUS_CODE)
.setVInt64(span.getStatus().getCanonicalCode().value())
.setVInt64(span.getStatus().getStatusCode().value())
.setVType(Model.ValueType.INT64)
.build());

View File

@ -173,9 +173,9 @@ final class SpanAdapter {
static Status toStatusProto(SpanData.Status status) {
Status.StatusCode protoStatusCode = Status.StatusCode.STATUS_CODE_UNSET;
Status.DeprecatedStatusCode deprecatedStatusCode = DEPRECATED_STATUS_CODE_OK;
if (status.getCanonicalCode() == StatusCode.OK) {
if (status.getStatusCode() == StatusCode.OK) {
protoStatusCode = Status.StatusCode.STATUS_CODE_OK;
} else if (status.getCanonicalCode() == StatusCode.ERROR) {
} else if (status.getStatusCode() == StatusCode.ERROR) {
protoStatusCode = Status.StatusCode.STATUS_CODE_ERROR;
deprecatedStatusCode = DEPRECATED_STATUS_CODE_UNKNOWN_ERROR;
}

View File

@ -141,14 +141,14 @@ public final class ZipkinSpanExporter implements SpanExporter {
SpanData.Status status = spanData.getStatus();
// include status code & description.
if (!status.isUnset()) {
spanBuilder.putTag(OTEL_STATUS_CODE, status.getCanonicalCode().toString());
spanBuilder.putTag(OTEL_STATUS_CODE, status.getStatusCode().toString());
if (status.getDescription() != null) {
spanBuilder.putTag(OTEL_STATUS_DESCRIPTION, status.getDescription());
}
}
// add the error tag, if it isn't already in the source span.
if (!status.isOk() && spanAttributes.get(STATUS_ERROR) == null) {
spanBuilder.putTag(STATUS_ERROR.getKey(), status.getCanonicalCode().toString());
spanBuilder.putTag(STATUS_ERROR.getKey(), status.getStatusCode().toString());
}
InstrumentationLibraryInfo instrumentationLibraryInfo =

View File

@ -53,7 +53,7 @@ public final class ErrorReportingTest {
List<SpanData> spans = otelTesting.getSpans();
assertThat(spans).hasSize(1);
assertThat(StatusCode.ERROR).isEqualTo(spans.get(0).getStatus().getCanonicalCode());
assertThat(StatusCode.ERROR).isEqualTo(spans.get(0).getStatus().getStatusCode());
}
/* Error handling in a callback capturing/activating the Span */
@ -75,7 +75,7 @@ public final class ErrorReportingTest {
List<SpanData> spans = otelTesting.getSpans();
assertThat(spans).hasSize(1);
assertThat(StatusCode.ERROR).isEqualTo(spans.get(0).getStatus().getCanonicalCode());
assertThat(StatusCode.ERROR).isEqualTo(spans.get(0).getStatus().getStatusCode());
}
/* Error handling for a max-retries task (such as url fetching).
@ -106,7 +106,7 @@ public final class ErrorReportingTest {
List<SpanData> spans = otelTesting.getSpans();
assertThat(spans).hasSize(1);
assertThat(StatusCode.ERROR).isEqualTo(spans.get(0).getStatus().getCanonicalCode());
assertThat(StatusCode.ERROR).isEqualTo(spans.get(0).getStatus().getStatusCode());
List<Event> events = spans.get(0).getEvents();
assertThat(events).hasSize(maxRetries);
@ -141,7 +141,7 @@ public final class ErrorReportingTest {
List<SpanData> spans = otelTesting.getSpans();
assertThat(spans).hasSize(1);
assertThat(StatusCode.ERROR).isEqualTo(spans.get(0).getStatus().getCanonicalCode());
assertThat(StatusCode.ERROR).isEqualTo(spans.get(0).getStatus().getStatusCode());
}
static class ScopedRunnable implements Runnable {

View File

@ -40,7 +40,7 @@ final class TracezSpanBuckets {
latencyBuckets.get(LatencyBoundary.getBoundary(span.getLatencyNanos())).add(span);
return;
}
errorBuckets.get(status.getCanonicalCode()).add(span);
errorBuckets.get(status.getStatusCode()).add(span);
}
Map<LatencyBoundary, Integer> getLatencyBoundaryToCountMap() {

View File

@ -197,7 +197,7 @@ final class RecordEventsReadableSpan implements ReadWriteSpan {
getImmutableAttributes(),
(attributes == null) ? 0 : attributes.getTotalAddedValues(),
totalRecordedEvents,
getStatusWithDefault(),
getSpanDataStatus(),
name,
endEpochNanos,
hasEnded);
@ -379,14 +379,14 @@ final class RecordEventsReadableSpan implements ReadWriteSpan {
}
@Override
public ReadWriteSpan setStatus(StatusCode canonicalCode) {
setStatus(canonicalCode, null);
public ReadWriteSpan setStatus(StatusCode statusCode) {
setStatus(statusCode, null);
return this;
}
@Override
public ReadWriteSpan setStatus(StatusCode canonicalCode, @Nullable String description) {
if (canonicalCode == null) {
public ReadWriteSpan setStatus(StatusCode statusCode, @Nullable String description) {
if (statusCode == null) {
return this;
}
synchronized (lock) {
@ -394,7 +394,7 @@ final class RecordEventsReadableSpan implements ReadWriteSpan {
logger.log(Level.FINE, "Calling setStatus() on an ended Span.");
return this;
}
this.status = SpanData.Status.create(canonicalCode, description);
this.status = SpanData.Status.create(statusCode, description);
}
return this;
}
@ -481,7 +481,7 @@ final class RecordEventsReadableSpan implements ReadWriteSpan {
}
@GuardedBy("lock")
private SpanData.Status getStatusWithDefault() {
private SpanData.Status getSpanDataStatus() {
synchronized (lock) {
return status;
}

View File

@ -59,15 +59,15 @@ abstract class ImmutableStatus implements SpanData.Status {
* @param description the new description of the {@code Status}.
* @return The newly created {@code Status} with the given description.
*/
public static SpanData.Status create(StatusCode canonicalCode, @Nullable String description) {
public static SpanData.Status create(StatusCode statusCode, @Nullable String description) {
if (description == null) {
return codeToStatus.get(canonicalCode);
return codeToStatus.get(statusCode);
}
return createInternal(canonicalCode, description);
return createInternal(statusCode, description);
}
private static SpanData.Status createInternal(
StatusCode canonicalCode, @Nullable String description) {
return new AutoValue_ImmutableStatus(canonicalCode, description);
StatusCode statusCode, @Nullable String description) {
return new AutoValue_ImmutableStatus(statusCode, description);
}
}

View File

@ -340,12 +340,19 @@ public interface SpanData {
return ImmutableStatus.create(code, description);
}
/** Returns the status code. */
StatusCode getStatusCode();
/**
* Returns the canonical status code.
* Returns the status code.
*
* @return the canonical status code.
* @deprecated Please use {@link #getStatusCode()}. This method will be removed in the next
* release.
*/
StatusCode getCanonicalCode();
@Deprecated
default StatusCode getCanonicalCode() {
return getStatusCode();
}
/**
* Returns the description of this {@code Status} for human consumption.
@ -362,7 +369,7 @@ public interface SpanData {
*/
// TODO: Consider to remove this in a future PR. Avoid too many changes in the initial PR.
default boolean isUnset() {
return StatusCode.UNSET == getCanonicalCode();
return StatusCode.UNSET == getStatusCode();
}
/**
@ -373,7 +380,7 @@ public interface SpanData {
*/
// TODO: Consider to remove this in a future PR. Avoid too many changes in the initial PR.
default boolean isOk() {
return isUnset() || StatusCode.OK == getCanonicalCode();
return isUnset() || StatusCode.OK == getStatusCode();
}
}
}

View File

@ -849,6 +849,7 @@ class RecordEventsReadableSpanTest {
}
}
@SuppressWarnings("deprecation") // will remove deprecated usages at the next release
private void verifySpanData(
SpanData spanData,
final ReadableAttributes attributes,
@ -871,7 +872,8 @@ class RecordEventsReadableSpanTest {
assertThat(spanData.getLinks()).isEqualTo(links);
assertThat(spanData.getStartEpochNanos()).isEqualTo(startEpochNanos);
assertThat(spanData.getEndEpochNanos()).isEqualTo(endEpochNanos);
assertThat(spanData.getStatus().getCanonicalCode()).isEqualTo(status.getCanonicalCode());
assertThat(spanData.getStatus().getStatusCode()).isEqualTo(status.getStatusCode());
assertThat(spanData.getStatus().getCanonicalCode()).isEqualTo(status.getStatusCode());
assertThat(spanData.hasEnded()).isEqualTo(hasEnded);
// verify equality manually, since the implementations don't all equals with each other.

View File

@ -12,15 +12,17 @@ import io.opentelemetry.sdk.trace.data.SpanData.Status;
import org.junit.jupiter.api.Test;
/** Unit tests for {@link ImmutableStatus}. */
@SuppressWarnings("deprecation")
class ImmutableStatusTest {
@Test
void defaultConstants() {
StatusCode[] codes = StatusCode.values();
assertThat(codes).hasSize(3);
assertThat(Status.unset().getCanonicalCode()).isEqualTo(StatusCode.UNSET);
assertThat(Status.unset().getStatusCode()).isEqualTo(StatusCode.UNSET);
assertThat(Status.unset().getDescription()).isNull();
assertThat(Status.ok().getCanonicalCode()).isEqualTo(StatusCode.OK);
assertThat(Status.ok().getStatusCode()).isEqualTo(StatusCode.OK);
assertThat(Status.ok().getDescription()).isNull();
assertThat(Status.error().getStatusCode()).isEqualTo(StatusCode.ERROR);
assertThat(Status.error().getCanonicalCode()).isEqualTo(StatusCode.ERROR);
assertThat(Status.error().getDescription()).isNull();
}
@ -31,6 +33,7 @@ class ImmutableStatusTest {
assertThat(ImmutableStatus.codeToStatus).hasSize(codes.length);
for (StatusCode code : codes) {
assertThat(ImmutableStatus.codeToStatus.get(code)).isNotNull();
assertThat(ImmutableStatus.codeToStatus.get(code).getStatusCode()).isEqualTo(code);
assertThat(ImmutableStatus.codeToStatus.get(code).getCanonicalCode()).isEqualTo(code);
assertThat(ImmutableStatus.codeToStatus.get(code).getDescription()).isNull();
}

View File

@ -50,7 +50,7 @@ public final class ErrorReportingTest {
List<SpanData> spans = otelTesting.getSpans();
assertThat(spans).hasSize(1);
assertThat(spans.get(0).getStatus().getCanonicalCode()).isEqualTo(StatusCode.ERROR);
assertThat(spans.get(0).getStatus().getStatusCode()).isEqualTo(StatusCode.ERROR);
}
/* Error handling in a callback capturing/activating the Span */
@ -72,7 +72,7 @@ public final class ErrorReportingTest {
List<SpanData> spans = otelTesting.getSpans();
assertThat(spans).hasSize(1);
assertThat(spans.get(0).getStatus().getCanonicalCode()).isEqualTo(StatusCode.ERROR);
assertThat(spans.get(0).getStatus().getStatusCode()).isEqualTo(StatusCode.ERROR);
}
/* Error handling for a max-retries task (such as url fetching).
@ -99,7 +99,7 @@ public final class ErrorReportingTest {
List<SpanData> spans = otelTesting.getSpans();
assertThat(spans).hasSize(1);
assertThat(spans.get(0).getStatus().getCanonicalCode()).isEqualTo(StatusCode.ERROR);
assertThat(spans.get(0).getStatus().getStatusCode()).isEqualTo(StatusCode.ERROR);
List<Event> events = spans.get(0).getEvents();
assertThat(events).hasSize(maxRetries);
@ -131,7 +131,7 @@ public final class ErrorReportingTest {
List<SpanData> spans = otelTesting.getSpans();
assertThat(spans).hasSize(1);
assertThat(StatusCode.ERROR).isEqualTo(spans.get(0).getStatus().getCanonicalCode());
assertThat(StatusCode.ERROR).isEqualTo(spans.get(0).getStatus().getStatusCode());
}
private static class ScopedRunnable implements Runnable {