diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e20083b79..4c1c719453 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ ### Fixes +- Apply OkHttp listener auto finish timestamp to all running spans ([#3167](https://github.com/getsentry/sentry-java/pull/3167)) - Fix not eligible for auto proxying warnings ([#3154](https://github.com/getsentry/sentry-java/pull/3154)) - Set default fingerprint for ANRv2 events to correctly group background and foreground ANRs ([#3164](https://github.com/getsentry/sentry-java/pull/3164)) - This will improve grouping of ANRs that have similar stacktraces but differ in background vs foreground state. Only affects newly-ingested ANR events with `mechanism:AppExitInfo` diff --git a/sentry-okhttp/src/main/java/io/sentry/okhttp/SentryOkHttpEvent.kt b/sentry-okhttp/src/main/java/io/sentry/okhttp/SentryOkHttpEvent.kt index d98505dcb4..ff98b61801 100644 --- a/sentry-okhttp/src/main/java/io/sentry/okhttp/SentryOkHttpEvent.kt +++ b/sentry-okhttp/src/main/java/io/sentry/okhttp/SentryOkHttpEvent.kt @@ -7,7 +7,6 @@ import io.sentry.ISpan import io.sentry.SentryDate import io.sentry.SentryLevel import io.sentry.SpanDataConvention -import io.sentry.SpanStatus import io.sentry.TypeCheckHint import io.sentry.okhttp.SentryOkHttpEventListener.Companion.CONNECTION_EVENT import io.sentry.okhttp.SentryOkHttpEventListener.Companion.CONNECT_EVENT @@ -158,10 +157,12 @@ internal class SentryOkHttpEvent(private val hub: IHub, private val request: Req // We forcefully finish all spans, even if they should already have been finished through finishSpan() eventSpans.values.filter { !it.isFinished }.forEach { - // If a status was set on the span, we use that, otherwise we set its status as error. - it.status = it.status ?: SpanStatus.INTERNAL_ERROR moveThrowableToRootSpan(it) - it.finish() + if (finishDate != null) { + it.finish(it.status, finishDate) + } else { + it.finish() + } } beforeFinish?.invoke(callRootSpan) // We report the client error here, after all sub-spans finished, so that it will be bound diff --git a/sentry-okhttp/src/test/java/io/sentry/okhttp/SentryOkHttpEventTest.kt b/sentry-okhttp/src/test/java/io/sentry/okhttp/SentryOkHttpEventTest.kt index 308a7bbf26..79c266c14e 100644 --- a/sentry-okhttp/src/test/java/io/sentry/okhttp/SentryOkHttpEventTest.kt +++ b/sentry-okhttp/src/test/java/io/sentry/okhttp/SentryOkHttpEventTest.kt @@ -23,6 +23,7 @@ import io.sentry.okhttp.SentryOkHttpEventListener.Companion.REQUEST_HEADERS_EVEN import io.sentry.okhttp.SentryOkHttpEventListener.Companion.RESPONSE_BODY_EVENT import io.sentry.okhttp.SentryOkHttpEventListener.Companion.RESPONSE_HEADERS_EVENT import io.sentry.okhttp.SentryOkHttpEventListener.Companion.SECURE_CONNECT_EVENT +import io.sentry.test.ImmediateExecutorService import io.sentry.test.getProperty import okhttp3.Protocol import okhttp3.Request @@ -31,7 +32,6 @@ import okhttp3.mockwebserver.MockWebServer import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.argThat -import org.mockito.kotlin.argumentCaptor import org.mockito.kotlin.check import org.mockito.kotlin.eq import org.mockito.kotlin.mock @@ -39,9 +39,7 @@ import org.mockito.kotlin.never import org.mockito.kotlin.spy import org.mockito.kotlin.verify import org.mockito.kotlin.whenever -import java.util.concurrent.Future import java.util.concurrent.RejectedExecutionException -import kotlin.RuntimeException import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse @@ -254,16 +252,6 @@ class SentryOkHttpEventTest { ) } - @Test - fun `when finishEvent, all running spans are finished with internal_error status`() { - val sut = fixture.getSut() - sut.startSpan("span") - val spans = sut.getEventSpans() - assertNull(spans["span"]!!.status) - sut.finishEvent() - assertEquals(SpanStatus.INTERNAL_ERROR, spans["span"]!!.status) - } - @Test fun `when finishEvent, does not override running spans status if set`() { val sut = fixture.getSut() @@ -273,6 +261,7 @@ class SentryOkHttpEventTest { spans["span"]!!.status = SpanStatus.OK assertEquals(SpanStatus.OK, spans["span"]!!.status) sut.finishEvent() + assertTrue(spans["span"]!!.isFinished) assertEquals(SpanStatus.OK, spans["span"]!!.status) } @@ -533,18 +522,15 @@ class SentryOkHttpEventTest { } @Test - fun `scheduleFinish schedules finishEvent`() { - val mockExecutor = mock() - val captor = argumentCaptor() - whenever(mockExecutor.schedule(captor.capture(), any())).then { - captor.lastValue.run() - mock>() - } - fixture.hub.options.executorService = mockExecutor + fun `scheduleFinish schedules finishEvent and finish running spans to specific timestamp`() { + fixture.hub.options.executorService = ImmediateExecutorService() val sut = spy(fixture.getSut()) val timestamp = mock() + sut.startSpan(CONNECTION_EVENT) sut.scheduleFinish(timestamp) verify(sut).finishEvent(eq(timestamp), anyOrNull()) + val spans = sut.getEventSpans() + assertEquals(timestamp, spans[CONNECTION_EVENT]?.finishDate) } @Test