Skip to content

Commit

Permalink
Added a new test to help ensure that my setStatus method behaves corr…
Browse files Browse the repository at this point in the history
…ectly under the specified conditions, maintaining the integrity of the span's status.

Ref Fix Span#setStatus open-telemetry#6797
  • Loading branch information
Victorsesan committed Nov 8, 2024
1 parent d452c3e commit 8fc9659
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 8 deletions.
16 changes: 8 additions & 8 deletions sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkSpan.java
Original file line number Diff line number Diff line change
Expand Up @@ -433,31 +433,31 @@ public ReadWriteSpan setStatus(StatusCode statusCode, @Nullable String descripti
synchronized (lock) {
if (!isModifiableByCurrentThread()) {
logger.log(Level.FINE, "Calling setStatus() on an ended Span.");
return this; // Prevent modification if the span has ended
return this;
}

// Check the current status and enforce priority rules

StatusCode currentStatusCode = this.status.getStatusCode();

// Prevent setting a lower priority status.
if (currentStatusCode == StatusCode.OK) {
logger.log(Level.FINE, "Calling setStatus() on a Span that is already set to OK.");
return this; // Do not allow lower priority status to override OK
} else if (currentStatusCode == StatusCode.ERROR && statusCode == StatusCode.UNSET) {
logger.log(Level.WARNING, "Cannot set status to UNSET when current status is ERROR.");
} if (currentStatusCode == StatusCode.ERROR && statusCode == StatusCode.UNSET) {
logger.log(Level.FINE, "Cannot set status to UNSET when current status is ERROR.");
return this; // Do not allow UNSET to override ERROR
}

// Set the status, ignoring description if status is not ERROR.
if (statusCode == StatusCode.ERROR) {
this.status = StatusData.create(statusCode, description); // Allow description for ERROR
this.status = StatusData.create(statusCode, description);
} else {
if (currentStatusCode != statusCode) {
this.status =
StatusData.create(statusCode, null); // Ignore description for non-ERROR statuses
StatusData.create(statusCode, null);
}
}
}
return this; // Return the current span for method chaining
return this;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1446,6 +1446,29 @@ private void verifySpanData(
spanDataAttributes.forEach((key, value) -> assertThat(attributes.get(key)).isEqualTo(value));
}


@Test
void setStatusCannotOverrideStatusError() {
SdkSpan testSpan = createTestRootSpan();

// Set an initial status to ERROR
testSpan.setStatus(StatusCode.ERROR, "Initial error status");
assertThat(testSpan.toSpanData().getStatus().getStatusCode()).isEqualTo(StatusCode.ERROR);

// Attempt to set status to OK (should not change)
testSpan.setStatus(StatusCode.OK);
assertThat(testSpan.toSpanData().getStatus().getStatusCode()).isEqualTo(StatusCode.ERROR);

// Attempt to set status to UNSET (should not change)
testSpan.setStatus(StatusCode.UNSET);
assertThat(testSpan.toSpanData().getStatus().getStatusCode()).isEqualTo(StatusCode.ERROR);

// Attempt to set status to ERROR again (should remain ERROR)
testSpan.setStatus(StatusCode.ERROR, "Another error status");
assertThat(testSpan.toSpanData().getStatus().getStatusCode()).isEqualTo(StatusCode.ERROR);
}


@Test
void testAsSpanData() {
String name = "GreatSpan";
Expand Down

0 comments on commit 8fc9659

Please sign in to comment.