diff --git a/sentry-opentelemetry/sentry-opentelemetry-agent/build.gradle.kts b/sentry-opentelemetry/sentry-opentelemetry-agent/build.gradle.kts index 80b68430db..4c00cb8d81 100644 --- a/sentry-opentelemetry/sentry-opentelemetry-agent/build.gradle.kts +++ b/sentry-opentelemetry/sentry-opentelemetry-agent/build.gradle.kts @@ -53,6 +53,7 @@ val upstreamAgent = configurations.create("upstreamAgent") { dependencies { bootstrapLibs(projects.sentry) + bootstrapLibs(projects.sentryOpentelemetry.sentryOpentelemetryBootstrap) javaagentLibs(projects.sentryOpentelemetry.sentryOpentelemetryAgentcustomization) upstreamAgent(Config.Libs.OpenTelemetry.otelJavaAgent) } diff --git a/sentry-opentelemetry/sentry-opentelemetry-agentcustomization/build.gradle.kts b/sentry-opentelemetry/sentry-opentelemetry-agentcustomization/build.gradle.kts index 79e3599cc8..475796d246 100644 --- a/sentry-opentelemetry/sentry-opentelemetry-agentcustomization/build.gradle.kts +++ b/sentry-opentelemetry/sentry-opentelemetry-agentcustomization/build.gradle.kts @@ -24,6 +24,7 @@ dependencies { exclude(group = "io.opentelemetry") exclude(group = "io.opentelemetry.javaagent") } + implementation(projects.sentryOpentelemetry.sentryOpentelemetryBootstrap) compileOnly(Config.Libs.OpenTelemetry.otelSdk) compileOnly(Config.Libs.OpenTelemetry.otelExtensionAutoconfigureSpi) diff --git a/sentry-opentelemetry/sentry-opentelemetry-bootstrap/api/sentry-opentelemetry-bootstrap.api b/sentry-opentelemetry/sentry-opentelemetry-bootstrap/api/sentry-opentelemetry-bootstrap.api new file mode 100644 index 0000000000..1edc8e6f3e --- /dev/null +++ b/sentry-opentelemetry/sentry-opentelemetry-bootstrap/api/sentry-opentelemetry-bootstrap.api @@ -0,0 +1,37 @@ +public final class io/sentry/opentelemetry/OtelContextScopeStorage : io/sentry/ScopeStorage { + public fun ()V + public fun close ()V + public fun get ()Lio/sentry/IHub; + public fun set (Lio/sentry/IHub;)Lio/sentry/SentryStorageToken; +} + +public final class io/sentry/opentelemetry/SentryContextStorage : io/opentelemetry/context/ContextStorage { + public static final field HUB_KEY Lio/opentelemetry/context/ContextKey; + public fun (Lio/opentelemetry/context/ContextStorage;)V + public fun attach (Lio/opentelemetry/context/Context;)Lio/opentelemetry/context/Scope; + public fun current ()Lio/opentelemetry/context/Context; +} + +public final class io/sentry/opentelemetry/SentryContextWrapper : io/opentelemetry/context/Context { + public fun get (Lio/opentelemetry/context/ContextKey;)Ljava/lang/Object; + public fun toString ()Ljava/lang/String; + public fun with (Lio/opentelemetry/context/ContextKey;Ljava/lang/Object;)Lio/opentelemetry/context/Context; + public static fun wrap (Lio/opentelemetry/context/Context;)Lio/sentry/opentelemetry/SentryContextWrapper; +} + +public final class io/sentry/opentelemetry/SentryOtelKeys { + public static final field SENTRY_BAGGAGE_KEY Lio/opentelemetry/context/ContextKey; + public static final field SENTRY_HUB_KEY Lio/opentelemetry/context/ContextKey; + public static final field SENTRY_SCOPES_KEY Lio/opentelemetry/context/ContextKey; + public static final field SENTRY_TRACE_KEY Lio/opentelemetry/context/ContextKey; + public fun ()V +} + +public final class io/sentry/opentelemetry/SentryWeakSpanStorage { + public fun getHub (Lio/opentelemetry/api/trace/SpanContext;)Lio/sentry/IHub; + public static fun getInstance ()Lio/sentry/opentelemetry/SentryWeakSpanStorage; + public fun getScopes (Lio/opentelemetry/api/trace/SpanContext;)Lio/sentry/Scopes; + public fun storeHub (Lio/opentelemetry/api/trace/SpanContext;Lio/sentry/IHub;)V + public fun storeScopes (Lio/opentelemetry/api/trace/SpanContext;Lio/sentry/Scopes;)V +} + diff --git a/sentry-opentelemetry/sentry-opentelemetry-bootstrap/build.gradle.kts b/sentry-opentelemetry/sentry-opentelemetry-bootstrap/build.gradle.kts new file mode 100644 index 0000000000..f5aeed0b44 --- /dev/null +++ b/sentry-opentelemetry/sentry-opentelemetry-bootstrap/build.gradle.kts @@ -0,0 +1,77 @@ +import net.ltgt.gradle.errorprone.errorprone +import org.jetbrains.kotlin.gradle.tasks.KotlinCompile + +plugins { + `java-library` + kotlin("jvm") + jacoco + id(Config.QualityPlugins.errorProne) + id(Config.QualityPlugins.gradleVersions) +} + +configure { + sourceCompatibility = JavaVersion.VERSION_1_8 + targetCompatibility = JavaVersion.VERSION_1_8 +} + +tasks.withType().configureEach { + kotlinOptions.jvmTarget = JavaVersion.VERSION_1_8.toString() +} + +dependencies { + compileOnly(projects.sentry) + + compileOnly(Config.Libs.OpenTelemetry.otelSdk) + + compileOnly(Config.CompileOnly.nopen) + errorprone(Config.CompileOnly.nopenChecker) + errorprone(Config.CompileOnly.errorprone) + compileOnly(Config.CompileOnly.jetbrainsAnnotations) + errorprone(Config.CompileOnly.errorProneNullAway) + + // tests + testImplementation(projects.sentryTestSupport) + testImplementation(kotlin(Config.kotlinStdLib)) + testImplementation(Config.TestLibs.kotlinTestJunit) + testImplementation(Config.TestLibs.mockitoKotlin) + testImplementation(Config.TestLibs.awaitility) + + testImplementation(Config.Libs.OpenTelemetry.otelSdk) + testImplementation(Config.Libs.OpenTelemetry.otelSemconv) +} + +configure { + test { + java.srcDir("src/test/java") + } +} + +jacoco { + toolVersion = Config.QualityPlugins.Jacoco.version +} + +tasks.jacocoTestReport { + reports { + xml.required.set(true) + html.required.set(false) + } +} + +tasks { + jacocoTestCoverageVerification { + violationRules { + rule { limit { minimum = Config.QualityPlugins.Jacoco.minimumCoverage } } + } + } + check { + dependsOn(jacocoTestCoverageVerification) + dependsOn(jacocoTestReport) + } +} + +tasks.withType().configureEach { + options.errorprone { + check("NullAway", net.ltgt.gradle.errorprone.CheckSeverity.ERROR) + option("NullAway:AnnotatedPackages", "io.sentry") + } +} diff --git a/sentry-opentelemetry/sentry-opentelemetry-bootstrap/src/main/java/io/sentry/opentelemetry/OtelContextScopeStorage.java b/sentry-opentelemetry/sentry-opentelemetry-bootstrap/src/main/java/io/sentry/opentelemetry/OtelContextScopeStorage.java new file mode 100644 index 0000000000..966a308e10 --- /dev/null +++ b/sentry-opentelemetry/sentry-opentelemetry-bootstrap/src/main/java/io/sentry/opentelemetry/OtelContextScopeStorage.java @@ -0,0 +1,46 @@ +package io.sentry.opentelemetry; + +import static io.sentry.opentelemetry.SentryOtelKeys.SENTRY_HUB_KEY; + +import io.opentelemetry.context.Context; +import io.opentelemetry.context.Scope; +import io.sentry.IHub; +import io.sentry.ScopeStorage; +import io.sentry.SentryStorageToken; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +@SuppressWarnings("MustBeClosedChecker") +public final class OtelContextScopeStorage implements ScopeStorage { + + @Override + public SentryStorageToken set(@Nullable IHub hub) { + // TODO use scopes key + Scope otelScope = Context.current().with(SENTRY_HUB_KEY, hub).makeCurrent(); + return new OtelContextScopeStorageToken(otelScope); + } + + @Override + public @Nullable IHub get() { + return Context.current().get(SENTRY_HUB_KEY); + } + + @Override + public void close() { + // TODO can we do something here? + } + + static final class OtelContextScopeStorageToken implements SentryStorageToken { + + private final @NotNull Scope otelScope; + + OtelContextScopeStorageToken(final @NotNull Scope otelScope) { + this.otelScope = otelScope; + } + + @Override + public void close() { + otelScope.close(); + } + } +} diff --git a/sentry/src/main/java/io/sentry/opentelemetry/SentryContextStorage.java b/sentry-opentelemetry/sentry-opentelemetry-bootstrap/src/main/java/io/sentry/opentelemetry/SentryContextStorage.java similarity index 100% rename from sentry/src/main/java/io/sentry/opentelemetry/SentryContextStorage.java rename to sentry-opentelemetry/sentry-opentelemetry-bootstrap/src/main/java/io/sentry/opentelemetry/SentryContextStorage.java diff --git a/sentry/src/main/java/io/sentry/opentelemetry/SentryContextWrapper.java b/sentry-opentelemetry/sentry-opentelemetry-bootstrap/src/main/java/io/sentry/opentelemetry/SentryContextWrapper.java similarity index 100% rename from sentry/src/main/java/io/sentry/opentelemetry/SentryContextWrapper.java rename to sentry-opentelemetry/sentry-opentelemetry-bootstrap/src/main/java/io/sentry/opentelemetry/SentryContextWrapper.java diff --git a/sentry/src/main/java/io/sentry/opentelemetry/SentryOtelKeys.java b/sentry-opentelemetry/sentry-opentelemetry-bootstrap/src/main/java/io/sentry/opentelemetry/SentryOtelKeys.java similarity index 84% rename from sentry/src/main/java/io/sentry/opentelemetry/SentryOtelKeys.java rename to sentry-opentelemetry/sentry-opentelemetry-bootstrap/src/main/java/io/sentry/opentelemetry/SentryOtelKeys.java index f8b8e5b9bf..95d8738084 100644 --- a/sentry/src/main/java/io/sentry/opentelemetry/SentryOtelKeys.java +++ b/sentry-opentelemetry/sentry-opentelemetry-bootstrap/src/main/java/io/sentry/opentelemetry/SentryOtelKeys.java @@ -2,6 +2,7 @@ import io.opentelemetry.context.ContextKey; import io.sentry.Baggage; +import io.sentry.IHub; import io.sentry.Scopes; import io.sentry.SentryTraceHeader; import org.jetbrains.annotations.ApiStatus; @@ -16,4 +17,5 @@ public final class SentryOtelKeys { ContextKey.named("sentry.baggage"); public static final @NotNull ContextKey SENTRY_SCOPES_KEY = ContextKey.named("sentry.scopes"); + public static final @NotNull ContextKey SENTRY_HUB_KEY = ContextKey.named("sentry.hub"); } diff --git a/sentry/src/main/java/io/sentry/opentelemetry/SentryWeakSpanStorage.java b/sentry-opentelemetry/sentry-opentelemetry-bootstrap/src/main/java/io/sentry/opentelemetry/SentryWeakSpanStorage.java similarity index 100% rename from sentry/src/main/java/io/sentry/opentelemetry/SentryWeakSpanStorage.java rename to sentry-opentelemetry/sentry-opentelemetry-bootstrap/src/main/java/io/sentry/opentelemetry/SentryWeakSpanStorage.java diff --git a/sentry-opentelemetry/sentry-opentelemetry-bootstrap/src/test/kotlin/SentrySpanProcessorTest.kt b/sentry-opentelemetry/sentry-opentelemetry-bootstrap/src/test/kotlin/SentrySpanProcessorTest.kt new file mode 100644 index 0000000000..5ed757ba16 --- /dev/null +++ b/sentry-opentelemetry/sentry-opentelemetry-bootstrap/src/test/kotlin/SentrySpanProcessorTest.kt @@ -0,0 +1,498 @@ +package io.sentry.opentelemetry + +import io.opentelemetry.api.OpenTelemetry +import io.opentelemetry.api.trace.Span +import io.opentelemetry.api.trace.SpanBuilder +import io.opentelemetry.api.trace.SpanContext +import io.opentelemetry.api.trace.SpanId +import io.opentelemetry.api.trace.SpanKind +import io.opentelemetry.api.trace.StatusCode +import io.opentelemetry.api.trace.TraceId +import io.opentelemetry.api.trace.Tracer +import io.opentelemetry.context.Context +import io.opentelemetry.context.propagation.ContextPropagators +import io.opentelemetry.context.propagation.TextMapGetter +import io.opentelemetry.context.propagation.TextMapSetter +import io.opentelemetry.sdk.OpenTelemetrySdk +import io.opentelemetry.sdk.trace.ReadWriteSpan +import io.opentelemetry.sdk.trace.ReadableSpan +import io.opentelemetry.sdk.trace.SdkTracerProvider +import io.opentelemetry.semconv.SemanticAttributes +import io.sentry.Baggage +import io.sentry.BaggageHeader +import io.sentry.Hint +import io.sentry.IHub +import io.sentry.ISpan +import io.sentry.ITransaction +import io.sentry.Instrumenter +import io.sentry.SentryDate +import io.sentry.SentryEvent +import io.sentry.SentryOptions +import io.sentry.SentryTraceHeader +import io.sentry.SpanStatus +import io.sentry.TransactionContext +import io.sentry.TransactionOptions +import io.sentry.protocol.TransactionNameSource +import org.mockito.kotlin.any +import org.mockito.kotlin.anyOrNull +import org.mockito.kotlin.check +import org.mockito.kotlin.eq +import org.mockito.kotlin.mock +import org.mockito.kotlin.never +import org.mockito.kotlin.verify +import org.mockito.kotlin.verifyNoInteractions +import org.mockito.kotlin.verifyNoMoreInteractions +import org.mockito.kotlin.whenever +import java.net.http.HttpHeaders +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertNotEquals +import kotlin.test.assertNotNull +import kotlin.test.assertNull +import kotlin.test.assertTrue + +class SentrySpanProcessorTest { + + companion object { + val SENTRY_TRACE_HEADER_STRING = "2722d9f6ec019ade60c776169d9a8904-cedf5b7571cb4972-1" + val BAGGAGE_HEADER_STRING = "sentry-public_key=502f25099c204a2fbf4cb16edc5975d1,sentry-sample_rate=1,sentry-trace_id=2722d9f6ec019ade60c776169d9a8904,sentry-transaction=HTTP%20GET" + } + + private class Fixture { + + val options = SentryOptions().also { + it.dsn = "https://key@sentry.io/proj" + it.instrumenter = Instrumenter.OTEL + } + val hub = mock() + val transaction = mock() + val span = mock() + val spanContext = mock() + lateinit var openTelemetry: OpenTelemetry + lateinit var tracer: Tracer + val sentryTrace = SentryTraceHeader(SENTRY_TRACE_HEADER_STRING) + val baggage = Baggage.fromHeader(BAGGAGE_HEADER_STRING) + + fun setup() { + whenever(hub.isEnabled).thenReturn(true) + whenever(hub.options).thenReturn(options) + whenever(hub.startTransaction(any(), any())).thenReturn(transaction) + + whenever(spanContext.operation).thenReturn("spanContextOp") + whenever(spanContext.parentSpanId).thenReturn(io.sentry.SpanId("cedf5b7571cb4972")) + + whenever(transaction.spanContext).thenReturn(spanContext) + whenever(span.spanContext).thenReturn(spanContext) + whenever(span.toSentryTrace()).thenReturn(sentryTrace) + whenever(transaction.toSentryTrace()).thenReturn(sentryTrace) + + val baggageHeader = BaggageHeader.fromBaggageAndOutgoingHeader(baggage, null) + whenever(span.toBaggageHeader(any())).thenReturn(baggageHeader) + whenever(transaction.toBaggageHeader(any())).thenReturn(baggageHeader) + + whenever(transaction.startChild(any(), anyOrNull(), anyOrNull(), eq(Instrumenter.OTEL))).thenReturn(span) + + val sdkTracerProvider = SdkTracerProvider.builder() + .addSpanProcessor(SentrySpanProcessor(hub)) + .build() + + openTelemetry = OpenTelemetrySdk.builder() + .setTracerProvider(sdkTracerProvider) + .setPropagators(ContextPropagators.create(SentryPropagator())) + .build() + + tracer = openTelemetry.getTracer("sentry-test") + } + } + + private val fixture = Fixture() + + @Test + fun `requires start`() { + val processor = SentrySpanProcessor() + assertTrue(processor.isStartRequired) + } + + @Test + fun `requires end`() { + val processor = SentrySpanProcessor() + assertTrue(processor.isEndRequired) + } + + @Test + fun `ignores sentry client request`() { + fixture.setup() + givenSpanBuilder(SpanKind.CLIENT) + .setAttribute(SemanticAttributes.HTTP_URL, "https://key@sentry.io/proj/some-api") + .startSpan() + + thenNoTransactionIsStarted() + } + + @Test + fun `ignores sentry internal request`() { + fixture.setup() + givenSpanBuilder(SpanKind.CLIENT) + .setAttribute(SemanticAttributes.HTTP_URL, "https://key@sentry.io/proj/some-api") + .startSpan() + + thenNoTransactionIsStarted() + } + + @Test + fun `does nothing on start if Sentry has not been initialized`() { + fixture.setup() + val context = mock() + val span = mock() + + whenever(fixture.hub.isEnabled).thenReturn(false) + + SentrySpanProcessor(fixture.hub).onStart(context, span) + + verify(fixture.hub).isEnabled + verify(fixture.hub).options + verifyNoMoreInteractions(fixture.hub) + verifyNoInteractions(context, span) + } + + @Test + fun `does nothing on end if Sentry has not been initialized`() { + fixture.setup() + val span = mock() + + whenever(fixture.hub.isEnabled).thenReturn(false) + + SentrySpanProcessor(fixture.hub).onEnd(span) + + verify(fixture.hub).isEnabled + verify(fixture.hub).options + verifyNoMoreInteractions(fixture.hub) + verifyNoInteractions(span) + } + + @Test + fun `does not start transaction for invalid SpanId`() { + fixture.setup() + val mockSpan = mock() + val mockSpanContext = mock() + whenever(mockSpanContext.spanId).thenReturn(SpanId.getInvalid()) + whenever(mockSpan.spanContext).thenReturn(mockSpanContext) + SentrySpanProcessor(fixture.hub).onStart(Context.current(), mockSpan) + thenNoTransactionIsStarted() + } + + @Test + fun `does not start transaction for invalid TraceId`() { + fixture.setup() + val mockSpan = mock() + val mockSpanContext = mock() + whenever(mockSpanContext.spanId).thenReturn(SpanId.fromBytes("seed".toByteArray())) + whenever(mockSpanContext.traceId).thenReturn(TraceId.getInvalid()) + whenever(mockSpan.spanContext).thenReturn(mockSpanContext) + SentrySpanProcessor(fixture.hub).onStart(Context.current(), mockSpan) + thenNoTransactionIsStarted() + } + + @Test + fun `creates transaction for first otel span and span for second`() { + fixture.setup() + val otelSpan = givenSpanBuilder().startSpan() + thenTransactionIsStarted(otelSpan, isContinued = false) + + val otelChildSpan = givenSpanBuilder(SpanKind.CLIENT, parentSpan = otelSpan) + .startSpan() + thenChildSpanIsStarted() + + otelChildSpan.end() + thenChildSpanIsFinished() + + otelSpan.end() + thenTransactionIsFinished() + } + + private fun whenExtractingHeaders(sentryTrace: Boolean = true, baggage: Boolean = true): Context { + val headers = givenHeaders(sentryTrace, baggage) + return fixture.openTelemetry.propagators.textMapPropagator.extract(Context.current(), headers, HeaderGetter()) + } + + @Test + fun `propagator can extract and result is used for transaction and attached on inject`() { + fixture.setup() + val extractedContext = whenExtractingHeaders() + + extractedContext.makeCurrent().use { _ -> + val otelSpan = givenSpanBuilder().startSpan() + thenTraceIdIsUsed(otelSpan) + thenTransactionIsStarted(otelSpan, isContinued = true) + + val otelChildSpan = givenSpanBuilder(SpanKind.CLIENT, parentSpan = otelSpan) + .startSpan() + thenChildSpanIsStarted() + + val map = mutableMapOf() + fixture.openTelemetry.propagators.textMapPropagator.inject(Context.current().with(otelSpan), map, TestSetter()) + + assertTrue(map.isNotEmpty()) + assertEquals(SENTRY_TRACE_HEADER_STRING, map["sentry-trace"]) + assertEquals(BAGGAGE_HEADER_STRING, map["baggage"]) + + otelChildSpan.end() + thenChildSpanIsFinished() + + otelSpan.end() + thenTransactionIsFinished() + } + } + + @Test + fun `incoming baggage without sentry-trace is ignored`() { + fixture.setup() + val extractedContext = whenExtractingHeaders(sentryTrace = false, baggage = true) + + extractedContext.makeCurrent().use { _ -> + val otelSpan = givenSpanBuilder() + .startSpan() + thenTraceIdIsNotUsed(otelSpan) + thenTransactionIsStarted(otelSpan, isContinued = false) + + val otelChildSpan = givenSpanBuilder(SpanKind.CLIENT, parentSpan = otelSpan) + .startSpan() + thenChildSpanIsStarted() + + otelChildSpan.end() + thenChildSpanIsFinished() + + otelSpan.end() + thenTransactionIsFinished() + } + } + + @Test + fun `sentry-trace without baggage continues trace`() { + fixture.setup() + val extractedContext = whenExtractingHeaders(sentryTrace = true, baggage = false) + + extractedContext.makeCurrent().use { _ -> + val otelSpan = givenSpanBuilder() + .startSpan() + + thenTraceIdIsUsed(otelSpan) + thenTransactionIsStarted(otelSpan, isContinued = true, continuesWithFilledBaggage = false) + + val otelChildSpan = givenSpanBuilder(SpanKind.CLIENT, parentSpan = otelSpan) + .startSpan() + thenChildSpanIsStarted() + + otelChildSpan.end() + thenChildSpanIsFinished() + + otelSpan.end() + thenTransactionIsFinished() + } + } + + @Test + fun `sets status for errored span`() { + fixture.setup() + val otelSpan = givenSpanBuilder().startSpan() + thenTransactionIsStarted(otelSpan, isContinued = false) + + val otelChildSpan = givenSpanBuilder(SpanKind.CLIENT, parentSpan = otelSpan) + .startSpan() + thenChildSpanIsStarted() + + otelChildSpan.setStatus(StatusCode.ERROR) + otelChildSpan.setAttribute(SemanticAttributes.HTTP_URL, "http://github.com/getsentry/sentry-java") + otelChildSpan.setAttribute(SemanticAttributes.HTTP_STATUS_CODE, 404L) + + otelChildSpan.end() + thenChildSpanIsFinished(SpanStatus.NOT_FOUND) + + otelSpan.end() + thenTransactionIsFinished() + } + + @Test + fun `sets status for errored span if not http`() { + fixture.setup() + val otelSpan = givenSpanBuilder().startSpan() + thenTransactionIsStarted(otelSpan, isContinued = false) + + val otelChildSpan = givenSpanBuilder(SpanKind.CLIENT, parentSpan = otelSpan) + .startSpan() + thenChildSpanIsStarted() + + otelChildSpan.setStatus(StatusCode.ERROR) + + otelChildSpan.end() + thenChildSpanIsFinished(SpanStatus.UNKNOWN_ERROR) + + otelSpan.end() + thenTransactionIsFinished() + } + + @Test + fun `links error to OTEL transaction`() { + fixture.setup() + val extractedContext = whenExtractingHeaders() + + extractedContext.makeCurrent().use { _ -> + val otelSpan = givenSpanBuilder().startSpan() + thenTransactionIsStarted(otelSpan, isContinued = true) + + otelSpan.makeCurrent().use { _ -> + val processedEvent = OpenTelemetryLinkErrorEventProcessor(fixture.hub).process(SentryEvent(), Hint()) + val traceContext = processedEvent!!.contexts.trace!! + + assertEquals("2722d9f6ec019ade60c776169d9a8904", traceContext.traceId.toString()) + assertEquals(otelSpan.spanContext.spanId, traceContext.spanId.toString()) + assertEquals("cedf5b7571cb4972", traceContext.parentSpanId.toString()) + assertEquals("spanContextOp", traceContext.operation) + } + + otelSpan.end() + thenTransactionIsFinished() + } + } + + @Test + fun `does not link error to OTEL transaction if instrumenter does not match`() { + fixture.options.instrumenter = Instrumenter.SENTRY + fixture.setup() + + val processedEvent = OpenTelemetryLinkErrorEventProcessor(fixture.hub).process(SentryEvent(), Hint()) + + thenNoTraceContextHasBeenAddedToEvent(processedEvent) + } + + private fun givenSpanBuilder(spanKind: SpanKind = SpanKind.SERVER, parentSpan: Span? = null): SpanBuilder { + val spanName = if (parentSpan == null) "testspan" else "childspan" + val spanBuilder = fixture.tracer + .spanBuilder(spanName) + .setAttribute("some-attribute", "some-value") + .setSpanKind(spanKind) + + parentSpan?.let { spanBuilder.setParent(Context.current().with(parentSpan)) } + + return spanBuilder + } + + private fun givenHeaders(sentryTrace: Boolean = true, baggage: Boolean = true): HttpHeaders? { + val headerMap = mutableMapOf>().also { + if (sentryTrace) { + it.put("sentry-trace", listOf(SENTRY_TRACE_HEADER_STRING)) + } + if (baggage) { + it.put("baggage", listOf(BAGGAGE_HEADER_STRING)) + } + } + + return HttpHeaders.of(headerMap) { _, _ -> true } + } + + private fun thenTransactionIsStarted(otelSpan: Span, isContinued: Boolean = false, continuesWithFilledBaggage: Boolean = true) { + if (isContinued) { + verify(fixture.hub).startTransaction( + check { + assertEquals("testspan", it.name) + assertEquals(TransactionNameSource.CUSTOM, it.transactionNameSource) + assertEquals("testspan", it.operation) + assertEquals(otelSpan.spanContext.spanId, it.spanId.toString()) + assertEquals("2722d9f6ec019ade60c776169d9a8904", it.traceId.toString()) + assertEquals("cedf5b7571cb4972", it.parentSpanId?.toString()) + assertTrue(it.parentSamplingDecision!!.sampled) + if (continuesWithFilledBaggage) { + assertEquals("2722d9f6ec019ade60c776169d9a8904", it.baggage?.traceId) + assertEquals("1", it.baggage?.sampleRate) + assertEquals("HTTP GET", it.baggage?.transaction) + assertEquals("502f25099c204a2fbf4cb16edc5975d1", it.baggage?.publicKey) + } else { + assertNotNull(it.baggage) + assertNull(it.baggage?.traceId) + assertNull(it.baggage?.sampleRate) + assertNull(it.baggage?.transaction) + assertNull(it.baggage?.publicKey) + assertFalse(it.baggage!!.isMutable) + } + assertFalse(it.baggage!!.isMutable) + }, + check { + assertNotNull(it.startTimestamp) + assertFalse(it.isBindToScope) + } + ) + } else { + verify(fixture.hub).startTransaction( + check { + assertEquals("testspan", it.name) + assertEquals(TransactionNameSource.CUSTOM, it.transactionNameSource) + assertEquals("testspan", it.operation) + assertEquals(otelSpan.spanContext.spanId, it.spanId.toString()) + assertEquals(otelSpan.spanContext.traceId, it.traceId.toString()) + assertNull(it.parentSpanId) + assertNull(it.parentSamplingDecision) + assertNull(it.baggage) + }, + check { + assertNotNull(it.startTimestamp) + assertFalse(it.isBindToScope) + } + ) + } + } + + private fun thenTraceIdIsUsed(otelSpan: Span) { + assertEquals("2722d9f6ec019ade60c776169d9a8904", otelSpan.spanContext.traceId) + } + + private fun thenTraceIdIsNotUsed(otelSpan: Span) { + assertNotEquals("2722d9f6ec019ade60c776169d9a8904", otelSpan.spanContext.traceId) + } + + private fun thenNoTransactionIsStarted() { + verify(fixture.hub, never()).startTransaction( + any(), + any() + ) + } + + private fun thenChildSpanIsStarted() { + verify(fixture.transaction).startChild( + eq("childspan"), + eq("childspan"), + any(), + eq(Instrumenter.OTEL) + ) + } + + private fun thenChildSpanIsFinished(status: SpanStatus = SpanStatus.OK) { + verify(fixture.span).finish(eq(status), any()) + } + + private fun thenTransactionIsFinished() { + verify(fixture.transaction).setContext(eq("otel"), any()) + verify(fixture.transaction).finish(eq(SpanStatus.OK), any()) + } + + private fun thenNoTraceContextHasBeenAddedToEvent(event: SentryEvent?) { + assertNotNull(event) + assertNull(event.contexts.trace) + } +} + +class HeaderGetter : TextMapGetter { + override fun keys(headers: HttpHeaders): MutableIterable { + return headers.map().map { it.key }.toMutableList() + } + + override fun get(headers: HttpHeaders?, key: String): String? { + return headers?.firstValue(key)?.orElse(null) + } +} + +class TestSetter : TextMapSetter> { + override fun set(values: MutableMap?, key: String, value: String) { + values?.put(key, value) + } +} diff --git a/sentry-opentelemetry/sentry-opentelemetry-core/build.gradle.kts b/sentry-opentelemetry/sentry-opentelemetry-core/build.gradle.kts index 1dad433555..a3011c6560 100644 --- a/sentry-opentelemetry/sentry-opentelemetry-core/build.gradle.kts +++ b/sentry-opentelemetry/sentry-opentelemetry-core/build.gradle.kts @@ -20,6 +20,8 @@ tasks.withType().configureEach { dependencies { compileOnly(projects.sentry) + // TODO implementation? + compileOnly(projects.sentryOpentelemetry.sentryOpentelemetryBootstrap) implementation(Config.Libs.OpenTelemetry.otelSdk) compileOnly(Config.Libs.OpenTelemetry.otelSemconv) diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SentryTaskDecorator.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SentryTaskDecorator.java index 7e23935c9b..1e12334fc2 100644 --- a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SentryTaskDecorator.java +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SentryTaskDecorator.java @@ -2,6 +2,7 @@ import io.sentry.IHub; import io.sentry.Sentry; +import io.sentry.SentryStorageToken; import java.util.concurrent.Callable; import org.jetbrains.annotations.NotNull; import org.springframework.core.task.TaskDecorator; @@ -14,15 +15,12 @@ */ public final class SentryTaskDecorator implements TaskDecorator { @Override + @SuppressWarnings("try") public @NotNull Runnable decorate(final @NotNull Runnable runnable) { - final IHub oldState = Sentry.getCurrentHub(); final IHub newHub = Sentry.getCurrentHub().clone(); return () -> { - Sentry.setCurrentHub(newHub); - try { + try (final @NotNull SentryStorageToken sentryStorageToken = Sentry.setCurrentHub(newHub)) { runnable.run(); - } finally { - Sentry.setCurrentHub(oldState); } }; } diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 799a8780e6..1eed2bab5b 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -259,6 +259,13 @@ public final class io/sentry/DeduplicateMultithreadedEventProcessor : io/sentry/ public fun process (Lio/sentry/SentryEvent;Lio/sentry/Hint;)Lio/sentry/SentryEvent; } +public final class io/sentry/DefaultScopeStorage : io/sentry/ScopeStorage { + public fun ()V + public fun close ()V + public fun get ()Lio/sentry/IHub; + public fun set (Lio/sentry/IHub;)Lio/sentry/SentryStorageToken; +} + public final class io/sentry/DefaultTransactionPerformanceCollector : io/sentry/TransactionPerformanceCollector { public fun (Lio/sentry/SentryOptions;)V public fun close ()V @@ -1629,6 +1636,17 @@ public abstract class io/sentry/ScopeObserverAdapter : io/sentry/IScopeObserver public fun setUser (Lio/sentry/protocol/User;)V } +public abstract interface class io/sentry/ScopeStorage { + public abstract fun close ()V + public abstract fun get ()Lio/sentry/IHub; + public abstract fun set (Lio/sentry/IHub;)Lio/sentry/SentryStorageToken; +} + +public final class io/sentry/ScopeStorageFactory { + public fun ()V + public static fun create ()Lio/sentry/ScopeStorage; +} + public final class io/sentry/Scopes { public static final field ROOT_ISOLATION_SCOPE Lio/sentry/IScope; public static final field ROOT_SCOPE Lio/sentry/IScope; @@ -1722,7 +1740,7 @@ public final class io/sentry/Sentry { public static fun removeTag (Ljava/lang/String;)V public static fun reportFullDisplayed ()V public static fun reportFullyDisplayed ()V - public static fun setCurrentHub (Lio/sentry/IHub;)V + public static fun setCurrentHub (Lio/sentry/IHub;)Lio/sentry/SentryStorageToken; public static fun setExtra (Ljava/lang/String;Ljava/lang/String;)V public static fun setFingerprint (Ljava/util/List;)V public static fun setLevel (Lio/sentry/SentryLevel;)V @@ -2423,6 +2441,10 @@ public final class io/sentry/SentryStackTraceFactory { public fun isInApp (Ljava/lang/String;)Ljava/lang/Boolean; } +public abstract interface class io/sentry/SentryStorageToken : java/lang/AutoCloseable { + public abstract fun close ()V +} + public final class io/sentry/SentryThreadFactory { public fun (Lio/sentry/SentryStackTraceFactory;Lio/sentry/SentryOptions;)V } @@ -3360,35 +3382,6 @@ public abstract interface class io/sentry/internal/viewhierarchy/ViewHierarchyEx public abstract fun export (Lio/sentry/protocol/ViewHierarchyNode;Ljava/lang/Object;)Z } -public final class io/sentry/opentelemetry/SentryContextStorage : io/opentelemetry/context/ContextStorage { - public static final field HUB_KEY Lio/opentelemetry/context/ContextKey; - public fun (Lio/opentelemetry/context/ContextStorage;)V - public fun attach (Lio/opentelemetry/context/Context;)Lio/opentelemetry/context/Scope; - public fun current ()Lio/opentelemetry/context/Context; -} - -public final class io/sentry/opentelemetry/SentryContextWrapper : io/opentelemetry/context/Context { - public fun get (Lio/opentelemetry/context/ContextKey;)Ljava/lang/Object; - public fun toString ()Ljava/lang/String; - public fun with (Lio/opentelemetry/context/ContextKey;Ljava/lang/Object;)Lio/opentelemetry/context/Context; - public static fun wrap (Lio/opentelemetry/context/Context;)Lio/sentry/opentelemetry/SentryContextWrapper; -} - -public final class io/sentry/opentelemetry/SentryOtelKeys { - public static final field SENTRY_BAGGAGE_KEY Lio/opentelemetry/context/ContextKey; - public static final field SENTRY_SCOPES_KEY Lio/opentelemetry/context/ContextKey; - public static final field SENTRY_TRACE_KEY Lio/opentelemetry/context/ContextKey; - public fun ()V -} - -public final class io/sentry/opentelemetry/SentryWeakSpanStorage { - public fun getHub (Lio/opentelemetry/api/trace/SpanContext;)Lio/sentry/IHub; - public static fun getInstance ()Lio/sentry/opentelemetry/SentryWeakSpanStorage; - public fun getScopes (Lio/opentelemetry/api/trace/SpanContext;)Lio/sentry/Scopes; - public fun storeHub (Lio/opentelemetry/api/trace/SpanContext;Lio/sentry/IHub;)V - public fun storeScopes (Lio/opentelemetry/api/trace/SpanContext;Lio/sentry/Scopes;)V -} - public final class io/sentry/profilemeasurements/ProfileMeasurement : io/sentry/JsonSerializable, io/sentry/JsonUnknown { public static final field ID_CPU_USAGE Ljava/lang/String; public static final field ID_FROZEN_FRAME_RENDERS Ljava/lang/String; @@ -4800,6 +4793,13 @@ public abstract interface class io/sentry/util/LazyEvaluator$Evaluator { public abstract fun evaluate ()Ljava/lang/Object; } +public final class io/sentry/util/LoadClass { + public fun ()V + public fun isClassAvailable (Ljava/lang/String;Lio/sentry/ILogger;)Z + public fun isClassAvailable (Ljava/lang/String;Lio/sentry/SentryOptions;)Z + public fun loadClass (Ljava/lang/String;Lio/sentry/ILogger;)Ljava/lang/Class; +} + public final class io/sentry/util/LogUtils { public fun ()V public static fun logNotInstanceOf (Ljava/lang/Class;Ljava/lang/Object;Lio/sentry/ILogger;)V diff --git a/sentry/src/main/java/io/sentry/DefaultScopeStorage.java b/sentry/src/main/java/io/sentry/DefaultScopeStorage.java new file mode 100644 index 0000000000..1959c61691 --- /dev/null +++ b/sentry/src/main/java/io/sentry/DefaultScopeStorage.java @@ -0,0 +1,44 @@ +package io.sentry; + +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +public final class DefaultScopeStorage implements ScopeStorage { + + // TODO singleton? + + private static final @NotNull ThreadLocal currentHub = new ThreadLocal<>(); + + @Override + public SentryStorageToken set(@Nullable IHub hub) { + final @Nullable IHub oldHub = get(); + currentHub.set(hub); + return new DefaultScopeStorageToken(oldHub); + } + + @Override + public @Nullable IHub get() { + return currentHub.get(); + } + + @Override + public void close() { + // TODO prevent further storing? would this cause problems if singleton, closed and + // re-initialized? + currentHub.remove(); + } + + static final class DefaultScopeStorageToken implements SentryStorageToken { + + private final @Nullable IHub oldValue; + + DefaultScopeStorageToken(final @Nullable IHub hub) { + this.oldValue = hub; + } + + @Override + public void close() { + currentHub.set(oldValue); + } + } +} diff --git a/sentry/src/main/java/io/sentry/ScopeStorage.java b/sentry/src/main/java/io/sentry/ScopeStorage.java new file mode 100644 index 0000000000..b0f52474ac --- /dev/null +++ b/sentry/src/main/java/io/sentry/ScopeStorage.java @@ -0,0 +1,16 @@ +package io.sentry; + +import org.jetbrains.annotations.Nullable; + +public interface ScopeStorage { + + // TODO use Scopes instead + SentryStorageToken set(final @Nullable IHub hub); + + // TODO use scopes + @Nullable + IHub get(); + + // TODO do we need this? + void close(); +} diff --git a/sentry/src/main/java/io/sentry/ScopeStorageFactory.java b/sentry/src/main/java/io/sentry/ScopeStorageFactory.java new file mode 100644 index 0000000000..8df65c0ad9 --- /dev/null +++ b/sentry/src/main/java/io/sentry/ScopeStorageFactory.java @@ -0,0 +1,41 @@ +package io.sentry; + +import io.sentry.util.LoadClass; +import java.lang.reflect.InvocationTargetException; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +public final class ScopeStorageFactory { + + private static final String OTEL_SCOPE_STORAGE = + "io.sentry.opentelemetry.OtelContextScopeStorage"; + + public static @NotNull ScopeStorage create() { + // TODO testable + LoadClass loadClass = new LoadClass(); + // TODO logger? + NoOpLogger logger = NoOpLogger.getInstance(); + if (loadClass.isClassAvailable(OTEL_SCOPE_STORAGE, logger)) { + Class otelScopeStorageClazz = loadClass.loadClass(OTEL_SCOPE_STORAGE, logger); + if (otelScopeStorageClazz != null) { + try { + final @Nullable Object otelScopeStorage = + otelScopeStorageClazz.getDeclaredConstructor().newInstance(); + if (otelScopeStorage != null && otelScopeStorage instanceof ScopeStorage) { + return (ScopeStorage) otelScopeStorage; + } + } catch (InstantiationException e) { + // TODO log + } catch (IllegalAccessException e) { + // TODO log + } catch (InvocationTargetException e) { + // TODO log + } catch (NoSuchMethodException e) { + // TODO log + } + } + } + + return new DefaultScopeStorage(); + } +} diff --git a/sentry/src/main/java/io/sentry/Sentry.java b/sentry/src/main/java/io/sentry/Sentry.java index e1f386e829..3a0ae5f104 100644 --- a/sentry/src/main/java/io/sentry/Sentry.java +++ b/sentry/src/main/java/io/sentry/Sentry.java @@ -42,12 +42,12 @@ public final class Sentry { private Sentry() {} - /** Holds Hubs per thread or only mainHub if globalHubMode is enabled. */ - private static final @NotNull ThreadLocal currentHub = new ThreadLocal<>(); + private static @NotNull ScopeStorage scopeStorage = ScopeStorageFactory.create(); /** The Main Hub or NoOp if Sentry is disabled. */ private static volatile @NotNull IHub mainHub = NoOpHub.getInstance(); + private static volatile @Nullable SentryStorageToken mainScopeStorageToken = null; /** Default value for globalHubMode is false */ private static final boolean GLOBAL_HUB_DEFAULT_MODE = false; @@ -74,10 +74,12 @@ private Sentry() {} if (globalHubMode) { return mainHub; } - IHub hub = currentHub.get(); + IHub hub = scopeStorage.get(); if (hub == null || hub instanceof NoOpHub) { hub = mainHub.clone(); - currentHub.set(hub); + // TODO how do we manage lifecycle of storage here? + // can this even happen with scopes already being there before Sentry.init? + scopeStorage.set(hub); } return hub; } @@ -97,8 +99,9 @@ private Sentry() {} } @ApiStatus.Internal // exposed for the coroutines integration in SentryContext - public static void setCurrentHub(final @NotNull IHub hub) { - currentHub.set(hub); + public static SentryStorageToken setCurrentHub(final @NotNull IHub hub) { + // TODO returned token is new and has to be closed + return scopeStorage.set(hub); } /** @@ -236,7 +239,11 @@ private static synchronized void init( final IHub hub = getCurrentHub(); mainHub = new Hub(options); - currentHub.set(mainHub); + final @Nullable SentryStorageToken previousStorageToken = mainScopeStorageToken; + if (previousStorageToken != null) { + previousStorageToken.close(); + } + mainScopeStorageToken = scopeStorage.set(mainHub); hub.close(true); @@ -486,7 +493,10 @@ public static synchronized void close() { final IHub hub = getCurrentHub(); mainHub = NoOpHub.getInstance(); // remove thread local to avoid memory leak - currentHub.remove(); + if (mainScopeStorageToken != null) { + mainScopeStorageToken.close(); + } + scopeStorage.close(); hub.close(false); } diff --git a/sentry/src/main/java/io/sentry/SentryStorageToken.java b/sentry/src/main/java/io/sentry/SentryStorageToken.java new file mode 100644 index 0000000000..ceb7deb15c --- /dev/null +++ b/sentry/src/main/java/io/sentry/SentryStorageToken.java @@ -0,0 +1,7 @@ +package io.sentry; + +public interface SentryStorageToken extends AutoCloseable { + // overriden to not have a checked exception on the method. + @Override + void close(); +} diff --git a/sentry/src/main/java/io/sentry/util/LoadClass.java b/sentry/src/main/java/io/sentry/util/LoadClass.java new file mode 100644 index 0000000000..d0eaea8ae4 --- /dev/null +++ b/sentry/src/main/java/io/sentry/util/LoadClass.java @@ -0,0 +1,46 @@ +package io.sentry.util; + +import io.sentry.ILogger; +import io.sentry.SentryLevel; +import io.sentry.SentryOptions; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +/** An Adapter for making Class.forName testable */ +public final class LoadClass { + + /** + * Try to load a class via reflection + * + * @param clazz the full class name + * @param logger an instance of ILogger + * @return a Class if it's available, or null + */ + public @Nullable Class loadClass(final @NotNull String clazz, final @Nullable ILogger logger) { + try { + return Class.forName(clazz); + } catch (ClassNotFoundException e) { + if (logger != null) { + logger.log(SentryLevel.DEBUG, "Class not available:" + clazz, e); + } + } catch (UnsatisfiedLinkError e) { + if (logger != null) { + logger.log(SentryLevel.ERROR, "Failed to load (UnsatisfiedLinkError) " + clazz, e); + } + } catch (Throwable e) { + if (logger != null) { + logger.log(SentryLevel.ERROR, "Failed to initialize " + clazz, e); + } + } + return null; + } + + public boolean isClassAvailable(final @NotNull String clazz, final @Nullable ILogger logger) { + return loadClass(clazz, logger) != null; + } + + public boolean isClassAvailable( + final @NotNull String clazz, final @Nullable SentryOptions options) { + return isClassAvailable(clazz, options != null ? options.getLogger() : null); + } +} diff --git a/settings.gradle.kts b/settings.gradle.kts index 028037372d..4d678bdd5c 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -42,6 +42,7 @@ include( "sentry-openfeign", "sentry-graphql", "sentry-jdbc", + "sentry-opentelemetry:sentry-opentelemetry-bootstrap", "sentry-opentelemetry:sentry-opentelemetry-core", "sentry-opentelemetry:sentry-opentelemetry-agentcustomization", "sentry-opentelemetry:sentry-opentelemetry-agent",