Skip to content

Commit

Permalink
ensured span's status does not change after it has ended in the nothi…
Browse files Browse the repository at this point in the history 
…ngChangedAfterEnd test

Ref Fix Span#setStatus open-telemetry#6797
  • Loading branch information
@Victorsesan
Victorsesan committed Nov 3, 2024
1 parent 7991abf commit 448ece8
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 61 deletions.
73 changes: 38 additions & 35 deletions buildSrc/build.gradle.kts
View file
Original file line number Diff line number Diff line change
@@ -1,28 +1,29 @@
plugins {
`kotlin-dsl`
// When updating, update below in dependencies too
id("com.diffplug.spotless") version "6.25.0"
`kotlin-dsl`

// When updating, update below in dependencies too
id("com.diffplug.spotless") version "6.25.0"
}

if (!hasLauncherForJavaVersion(17)) {
throw GradleException(
"JDK 17 is required to build and gradle was unable to detect it on the system. " +
"Please install it and see https://docs.gradle.org/current/userguide/toolchains.html#sec:auto_detection " +
"for details on how gradle detects java toolchains."
)
throw GradleException(
"JDK 17 is required to build and gradle was unable to detect it on the system. " +
"Please install it and see https://docs.gradle.org/current/userguide/toolchains.html#sec:auto_detection " +
"for details on how gradle detects java toolchains."
)
}

fun hasLauncherForJavaVersion(version: Int): Boolean {
return try {
javaToolchains.launcherFor { languageVersion.set(JavaLanguageVersion.of(version)) }.get()
true
} catch (e: Exception) {
false
}
return try {
javaToolchains.launcherFor { languageVersion = JavaLanguageVersion.of(version) }.get()
true
} catch (e: Exception) {
false
}
}

spotless {
kotlinGradle {
kotlinGradle {
ktlint().editorConfigOverride(mapOf(
"indent_size" to "2",
"continuation_indent_size" to "2",
Expand All @@ -43,28 +44,30 @@ kotlinGradle {
}

repositories {
mavenCentral()
gradlePluginPortal()
mavenLocal()
mavenCentral()
gradlePluginPortal()
mavenLocal()
}

dependencies {
implementation(enforcedPlatform("com.squareup.wire:wire-bom:5.1.0"))
implementation("com.google.auto.value:auto-value-annotations:1.11.0")
implementation("com.diffplug.spotless:spotless-plugin-gradle:6.25.0")
implementation("com.google.guava:guava:33.3.1-jre")
implementation("com.squareup:javapoet:1.13.0")
implementation("com.squareup.wire:wire-compiler")
implementation("com.squareup.wire:wire-gradle-plugin")
implementation("gradle.plugin.com.google.protobuf:protobuf-gradle-plugin:0.8.18")
implementation("gradle.plugin.io.morethan.jmhreport:gradle-jmh-report:0.9.6")
implementation("me.champeau.gradle:japicmp-gradle-plugin:0.4.3")
implementation("me.champeau.jmh:jmh-gradle-plugin:0.7.2")
implementation("net.ltgt.gradle:gradle-errorprone-plugin:4.0.1")
implementation("net.ltgt.gradle:gradle-nullaway-plugin:2.0.0")
implementation("org.jetbrains.kotlin:kotlin-gradle-plugin:2.0.21")
implementation("org.owasp:dependency-check-gradle:10.0.4")
implementation("ru.vyarus:gradle-animalsniffer-plugin:1.7.1")
implementation(enforcedPlatform("com.squareup.wire:wire-bom:5.1.0"))
implementation("com.google.auto.value:auto-value-annotations:1.11.0")
// When updating, update above in plugins too
implementation("com.diffplug.spotless:spotless-plugin-gradle:6.25.0")
// Needed for japicmp but not automatically brought in for some reason.
implementation("com.google.guava:guava:33.3.1-jre")
implementation("com.squareup:javapoet:1.13.0")
implementation("com.squareup.wire:wire-compiler")
implementation("com.squareup.wire:wire-gradle-plugin")
implementation("gradle.plugin.com.google.protobuf:protobuf-gradle-plugin:0.8.18")
implementation("gradle.plugin.io.morethan.jmhreport:gradle-jmh-report:0.9.6")
implementation("me.champeau.gradle:japicmp-gradle-plugin:0.4.4")
implementation("me.champeau.jmh:jmh-gradle-plugin:0.7.2")
implementation("net.ltgt.gradle:gradle-errorprone-plugin:4.1.0")
implementation("net.ltgt.gradle:gradle-nullaway-plugin:2.1.0")
implementation("org.jetbrains.kotlin:kotlin-gradle-plugin:2.0.21")
implementation("org.owasp:dependency-check-gradle:11.1.0")
implementation("ru.vyarus:gradle-animalsniffer-plugin:1.7.1")
}

// We can't apply conventions to this build so include important ones such as the Java compilation
Expand All @@ -73,4 +76,4 @@ java {
toolchain {
languageVersion.set(JavaLanguageVersion.of(17))
}
}
}
54 changes: 28 additions & 26 deletions sdk/trace/src/test/java/io/opentelemetry/sdk/trace/SdkSpanTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,11 @@
import io.opentelemetry.sdk.resources.Resource;
import io.opentelemetry.sdk.testing.time.TestClock;
import io.opentelemetry.sdk.trace.data.EventData;
import io.opentelemetry.sdk.trace.data.ExceptionEventData;
import io.opentelemetry.sdk.trace.data.LinkData;
import io.opentelemetry.sdk.trace.data.SpanData;
import io.opentelemetry.sdk.trace.data.StatusData;
import io.opentelemetry.sdk.trace.internal.ExtendedSpanProcessor;
import io.opentelemetry.sdk.trace.internal.data.ExceptionEventData;
import java.io.PrintWriter;
import java.io.StringWriter;
import java.time.Duration;
Expand Down Expand Up @@ -119,11 +119,25 @@ void setUp() {
@Test
void nothingChangedAfterEnd() {
SdkSpan span = createTestSpan(SpanKind.INTERNAL);

// Set an initial status before ending the span
span.setStatus(StatusCode.OK, "Initial status");
assertThat(span.toSpanData().getStatus().getStatusCode()).isEqualTo(StatusCode.OK);

// End the span
span.end();
// Check that adding trace events or update fields after Span#end() does not throw any thrown

// Store the current status for verification
StatusData currentStatus = span.toSpanData().getStatus();

// Check that adding trace events or update fields after Span#end() does not throw any exception
// and are ignored.
spanDoWork(span, StatusCode.ERROR, "CANCELLED");

// Verify that the status has not changed after the span has ended
SpanData spanData = span.toSpanData();
assertThat(spanData.getStatus()).isEqualTo(currentStatus);

verifySpanData(
spanData,
Attributes.empty(),
Expand Down Expand Up @@ -1166,20 +1180,16 @@ void recordException() {
EventData event = events.get(0);
assertThat(event.getName()).isEqualTo("exception");
assertThat(event.getEpochNanos()).isEqualTo(timestamp);
assertThat(event.getAttributes())
.isEqualTo(
Attributes.builder()
.put("exception.type", "java.lang.IllegalStateException")
.put("exception.message", "there was an exception")
.put("exception.stacktrace", stacktrace)
.build());

assertThat(event.getAttributes().get(stringKey("exception.message")))
.isEqualTo("there was an exception");
assertThat(event.getAttributes().get(stringKey("exception.type")))
.isEqualTo(exception.getClass().getName());
assertThat(event.getAttributes().get(stringKey("exception.stacktrace"))).isEqualTo(stacktrace);
assertThat(event)
.isInstanceOfSatisfying(
ExceptionEventData.class,
exceptionEvent -> {
assertThat(exceptionEvent.getException()).isSameAs(exception);
assertThat(exceptionEvent.getAdditionalAttributes()).isEqualTo(Attributes.empty());
});
}

Expand Down Expand Up @@ -1237,27 +1247,19 @@ void recordException_additionalAttributes() {
EventData event = events.get(0);
assertThat(event.getName()).isEqualTo("exception");
assertThat(event.getEpochNanos()).isEqualTo(timestamp);
assertThat(event.getAttributes())
.isEqualTo(
Attributes.builder()
.put("key1", "this is an additional attribute")
.put("exception.type", "java.lang.IllegalStateException")
.put("exception.message", "this is a precedence attribute")
.put("exception.stacktrace", stacktrace)
.build());
assertThat(event.getAttributes().get(stringKey("exception.message")))
.isEqualTo("this is a precedence attribute");
assertThat(event.getAttributes().get(stringKey("key1")))
.isEqualTo("this is an additional attribute");
assertThat(event.getAttributes().get(stringKey("exception.type")))
.isEqualTo("java.lang.IllegalStateException");
assertThat(event.getAttributes().get(stringKey("exception.stacktrace"))).isEqualTo(stacktrace);

assertThat(event)
.isInstanceOfSatisfying(
ExceptionEventData.class,
exceptionEvent -> {
assertThat(exceptionEvent.getException()).isSameAs(exception);
assertThat(exceptionEvent.getAdditionalAttributes())
.isEqualTo(
Attributes.of(
stringKey("key1"),
"this is an additional attribute",
stringKey("exception.message"),
"this is a precedence attribute"));
});
}

Expand Down

0 comments on commit 448ece8

Please sign in to comment.