Skip to content

Commit

Permalink
Consolidate APIs to return the current SDK time
Browse files Browse the repository at this point in the history
  • Loading branch information
bidetofevil committed Jan 17, 2025
1 parent 58bbf3e commit 75ef997
Show file tree
Hide file tree
Showing 21 changed files with 36 additions and 81 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -105,15 +105,15 @@ internal class OpenTelemetryModuleImpl(

override val embraceTracer: EmbraceTracer by singleton {
EmbraceTracer(
clock = initModule.clock,
spanService = spanService,
)
}

override val internalTracer: InternalTracer by lazy {
InternalTracer(
spanRepository = spanRepository,
embraceTracer = embraceTracer
embraceTracer = embraceTracer,
clock = initModule.clock,
)
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package io.embrace.android.embracesdk.internal.spans

import io.embrace.android.embracesdk.internal.clock.Clock
import io.embrace.android.embracesdk.internal.clock.normalizeTimestampAsMillis
import io.embrace.android.embracesdk.spans.AutoTerminationMode
import io.embrace.android.embracesdk.spans.EmbraceSpan
Expand All @@ -9,7 +8,6 @@ import io.embrace.android.embracesdk.spans.ErrorCode
import io.embrace.android.embracesdk.spans.TracingApi

class EmbraceTracer(
private val clock: Clock,
private val spanService: SpanService,
) : TracingApi {

Expand Down Expand Up @@ -80,10 +78,4 @@ class EmbraceTracer(
)

override fun getSpan(spanId: String): EmbraceSpan? = spanService.getSpan(spanId = spanId)

/**
* Return the current time in millis for the clock instance used by the Embrace SDK. This should be used to obtain the time
* in used for [recordCompletedSpan] so the timestamps will be in sync with those used by the SDK when a time is implicitly recorded.
*/
fun getSdkCurrentTimeMs(): Long = clock.now()
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package io.embrace.android.embracesdk.internal.spans

import io.embrace.android.embracesdk.internal.InternalTracingApi
import io.embrace.android.embracesdk.internal.clock.Clock
import io.embrace.android.embracesdk.internal.clock.nanosToMillis
import io.embrace.android.embracesdk.internal.clock.normalizeTimestampAsMillis
import io.embrace.android.embracesdk.spans.EmbraceSpan
Expand All @@ -10,6 +11,7 @@ import io.embrace.android.embracesdk.spans.ErrorCode
class InternalTracer(
private val spanRepository: SpanRepository,
private val embraceTracer: EmbraceTracer,
private val clock: Clock,
) : InternalTracingApi {

override fun startSpan(name: String, parentSpanId: String?, startTimeMs: Long?): String? {
Expand Down Expand Up @@ -111,7 +113,7 @@ class InternalTracer(
// else if timestampNanos isn't specified, use the current time in millis
// Otherwise, it means we have an invalid type of timestampNanos so we don't create the event
val validatedTimeMs = timestampMs ?: timestampNanos ?: if (map["timestampNanos"] == null) {
embraceTracer.getSdkCurrentTimeMs()
clock.now()
} else {
return null
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -277,11 +277,6 @@ internal class EmbraceTracerTest {
assertSame(spanFromTracer, embraceSpan)
}

@Test
fun `getSdkClockTimeMs is the same as the internal clock time`() {
assertEquals(clock.now(), embraceTracer.getSdkCurrentTimeMs())
}

@Test
fun `event timestamp will be converted to millis if an inappropriate value detected`() {
spanSink.flushSpans()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ internal class InternalTracerTest {
internalTracer = InternalTracer(
initModule.openTelemetryModule.spanRepository,
initModule.openTelemetryModule.embraceTracer,
initModule.clock
)
spanSink.flushSpans()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class EmbraceOkHttp3ApplicationInterceptor(

@Throws(IOException::class)
override fun intercept(chain: Interceptor.Chain): Response {
val startTime = embraceInternalApi.internalInterface.getSdkCurrentTime()
val startTime = embrace.getSdkCurrentTimeMs()
val request: Request = chain.request()
return try {
// we are not interested in response, just proceed
Expand All @@ -43,7 +43,7 @@ class EmbraceOkHttp3ApplicationInterceptor(
urlString,
HttpMethod.fromString(request.method),
startTime,
embraceInternalApi.internalInterface.getSdkCurrentTime(),
embrace.getSdkCurrentTimeMs(),
causeName(e, UNKNOWN_EXCEPTION),
causeMessage(e, UNKNOWN_MESSAGE),
request.header(CUSTOM_TRACE_ID_HEADER_NAME),
Expand All @@ -70,7 +70,7 @@ class EmbraceOkHttp3ApplicationInterceptor(
urlString,
HttpMethod.fromString(request.method),
startTime,
embraceInternalApi.internalInterface.getSdkCurrentTime(),
embrace.getSdkCurrentTimeMs(),
errorType ?: UNKNOWN_EXCEPTION,
errorMessage ?: UNKNOWN_MESSAGE,
request.header(CUSTOM_TRACE_ID_HEADER_NAME),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -254,9 +254,9 @@ class EmbraceOkHttp3NetworkInterceptor @JvmOverloads constructor(
// Any difference that is greater than 1 ms is likely the result of a change to the system clock during this process, or some
// scheduling quirk that makes the result not trustworthy. In that case, we simply don't return an offset.

val sdkTime1 = embraceInternalApi.internalInterface.getSdkCurrentTime()
val sdkTime1 = embrace.getSdkCurrentTimeMs()
val systemTime1 = systemClock.now()
val sdkTime2 = embraceInternalApi.internalInterface.getSdkCurrentTime()
val sdkTime2 = embrace.getSdkCurrentTimeMs()
val systemTime2 = systemClock.now()

val diff1 = sdkTime1 - systemTime1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ internal class EmbraceOkHttp3InterceptorsTest {
every { mockInternalInterface.shouldCaptureNetworkBody(any(), "POST") } answers { true }
every { mockInternalInterface.shouldCaptureNetworkBody(any(), "GET") } answers { false }
every { mockInternalInterface.isNetworkSpanForwardingEnabled() } answers { isNetworkSpanForwardingEnabled }
every { mockInternalInterface.getSdkCurrentTime() } answers { FAKE_SDK_TIME }
every { mockEmbrace.getSdkCurrentTimeMs() } answers { FAKE_SDK_TIME }
applicationInterceptor = EmbraceOkHttp3ApplicationInterceptor(mockEmbrace, mockEmbraceInternalApi)
preNetworkInterceptorTestInterceptor = TestInspectionInterceptor(
beforeRequestSent = { request -> preNetworkInterceptorBeforeRequestSupplier.invoke(request) },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import io.embrace.android.embracesdk.spans.ErrorCode
import io.embrace.android.embracesdk.testframework.IntegrationTestRule
import io.embrace.android.embracesdk.testframework.assertions.assertMatches
import io.opentelemetry.semconv.HttpAttributes
import java.net.SocketException
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertNotNull
Expand All @@ -28,6 +27,7 @@ import org.junit.Assert.assertTrue
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
import java.net.SocketException

/**
* Validation of the internal API
Expand Down Expand Up @@ -99,7 +99,6 @@ internal class EmbraceInternalInterfaceTest {
logComposeTap(Pair(0.0f, 0.0f), "")
assertFalse(shouldCaptureNetworkBody("", ""))
assertFalse(isNetworkSpanForwardingEnabled())
getSdkCurrentTime()
}
assertFalse(embrace.isStarted)
}
Expand Down Expand Up @@ -236,17 +235,6 @@ internal class EmbraceInternalInterfaceTest {
)
}

@Test
fun `test sdk time`() {
testRule.runTest(
testCaseAction = {
assertEquals(clock.now(), EmbraceInternalApi.getInstance().internalInterface.getSdkCurrentTime())
clock.tick()
assertEquals(clock.now(), EmbraceInternalApi.getInstance().internalInterface.getSdkCurrentTime())
}
)
}

@Test
fun `internal tracing APIs work as expected`() {
testRule.runTest(
Expand All @@ -267,7 +255,7 @@ internal class EmbraceInternalInterfaceTest {
recordCompletedSpan(
name = "tz-old-span",
startTimeMs = clock.now() - 1L,
endTimeMs = EmbraceInternalApi.getInstance().internalInterface.getSdkCurrentTime(),
endTimeMs = embrace.getSdkCurrentTimeMs(),
)
stopSpan(spanId = childSpanId, errorCode = ErrorCode.USER_ABANDON)
stopSpan(parentSpanId)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,4 +113,15 @@ internal class PublicApiTest {
}
)
}

@Test
fun `test sdk time`() {
testRule.runTest(
testCaseAction = {
assertEquals(clock.now(), embrace.getSdkCurrentTimeMs())
clock.tick()
assertEquals(clock.now(), embrace.getSdkCurrentTimeMs())
}
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import io.embrace.android.embracesdk.fakes.config.FakeEnabledFeatureConfig
import io.embrace.android.embracesdk.fakes.config.FakeInstrumentedConfig
import io.embrace.android.embracesdk.fixtures.TOO_LONG_ATTRIBUTE_KEY
import io.embrace.android.embracesdk.fixtures.TOO_LONG_ATTRIBUTE_VALUE
import io.embrace.android.embracesdk.internal.EmbraceInternalApi
import io.embrace.android.embracesdk.internal.clock.millisToNanos
import io.embrace.android.embracesdk.internal.payload.ApplicationState
import io.embrace.android.embracesdk.internal.payload.Attribute
Expand All @@ -23,8 +22,6 @@ import io.embrace.android.embracesdk.testframework.IntegrationTestRule
import io.embrace.android.embracesdk.testframework.actions.EmbracePayloadAssertionInterface
import io.opentelemetry.api.trace.SpanId
import io.opentelemetry.context.Context
import java.util.concurrent.CountDownLatch
import java.util.concurrent.TimeUnit
import org.junit.Assert.assertEquals
import org.junit.Assert.assertNotEquals
import org.junit.Assert.assertNotNull
Expand All @@ -35,6 +32,8 @@ import org.junit.Before
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
import java.util.concurrent.CountDownLatch
import java.util.concurrent.TimeUnit

@RunWith(AndroidJUnit4::class)
internal class TracingApiTest {
Expand Down Expand Up @@ -85,10 +84,10 @@ internal class TracingApiTest {
)
true
})
val failedOpStartTimeMs = EmbraceInternalApi.getInstance().internalInterface.getSdkCurrentTime()
val failedOpStartTimeMs = embrace.getSdkCurrentTimeMs()
clock.tick(200L)
parentSpan.addEvent(name = "delayed event", timestampMs = clock.now() - 50L, null)
val failedOpEndTimeMs = EmbraceInternalApi.getInstance().internalInterface.getSdkCurrentTime()
val failedOpEndTimeMs = embrace.getSdkCurrentTimeMs()

assertTrue(parentSpan.stop())

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,6 @@ internal class EmbraceInternalInterfaceImpl(
override fun isNetworkSpanForwardingEnabled(): Boolean =
configService.networkSpanForwardingBehavior.isNetworkSpanForwardingEnabled()

override fun getSdkCurrentTime(): Long = initModule.clock.now()

override fun isAnrCaptureEnabled(): Boolean = configService.anrBehavior.isAnrCaptureEnabled()

override fun isNdkEnabled(): Boolean = configService.autoDataCaptureBehavior.isNativeCrashCaptureEnabled()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ public EmbraceUrlConnectionDelegate(@NonNull T connection, boolean enableWrapIoS
this.connection = connection;
this.enableWrapIoStreams = enableWrapIoStreams;
this.internalNetworkApi = internalNetworkApi;
this.createdTime = internalNetworkApi.getSdkCurrentTime();
this.createdTime = internalNetworkApi.getSdkCurrentTimeMs();
this.isSDKStarted = internalNetworkApi.isStarted();
}

Expand Down Expand Up @@ -533,7 +533,7 @@ synchronized void internalLogNetworkCall() {
// We are proactive with setting this flag so that we don't get nested calls to log the network call by virtue of
// extracting the data we need to log the network call.
this.didLogNetworkCall = true; // TODO: Wouldn't this mean that the network call might not be logged
long endTime = internalNetworkApi.getSdkCurrentTime();
long endTime = internalNetworkApi.getSdkCurrentTimeMs();

String url = EmbraceHttpPathOverride.getURLString(new EmbraceHttpUrlConnectionOverride(this.connection));

Expand Down Expand Up @@ -754,7 +754,7 @@ public Principal getPeerPrincipal() throws SSLPeerUnverifiedException {
@Nullable
private InputStream getWrappedInputStream(InputStream connectionInputStream) {
identifyTraceId();
setStartTime(internalNetworkApi.getSdkCurrentTime());
setStartTime(internalNetworkApi.getSdkCurrentTimeMs());
InputStream in = null;
if (shouldUncompressGzip()) {
try {
Expand Down Expand Up @@ -798,7 +798,7 @@ private void cacheNetworkCallData(@Nullable byte[] responseBody) {
return;
}

setStartTime(internalNetworkApi.getSdkCurrentTime());
setStartTime(internalNetworkApi.getSdkCurrentTimeMs());

if (headerFields.get() == null) {
synchronized (headerFields) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ internal interface InternalNetworkApi {
fun recordNetworkRequest(embraceNetworkRequest: EmbraceNetworkRequest)
fun shouldCaptureNetworkBody(url: String, method: String): Boolean
fun logInternalError(error: Throwable)
fun getSdkCurrentTime(): Long
fun getSdkCurrentTimeMs(): Long
fun isStarted(): Boolean
fun generateW3cTraceparent(): String?
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ internal class InternalNetworkApiImpl : InternalNetworkApi {
private fun getInternalInterface(): EmbraceInternalInterface =
checkNotNull(EmbraceInternalApi.getInstance().internalInterface)

override fun getSdkCurrentTime(): Long = getInternalInterface().getSdkCurrentTime()
override fun getSdkCurrentTimeMs(): Long = embrace.getSdkCurrentTimeMs()

override fun isStarted(): Boolean = embrace.isStarted

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ internal class FakeInternalNetworkApi(
var started: Boolean = true,
var w3cTraceparent: String? = "00-3c72a77a7b51af6fb3778c06d4c165ce-4c1d710fffc88e35-01",
) : InternalNetworkApi {
override fun getSdkCurrentTime(): Long = time
override fun getSdkCurrentTimeMs(): Long = time
override fun isStarted(): Boolean = started
override fun generateW3cTraceparent(): String? = w3cTraceparent
override fun isNetworkSpanForwardingEnabled(): Boolean = internalInterface.isNetworkSpanForwardingEnabled()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,14 +165,6 @@ internal class EmbraceInternalInterfaceImplTest {
assertEquals(url, captor.captured.url)
}

@Test
fun `check usage of SDK time`() {
assertEquals(beforeObjectInitTime, internalImpl.getSdkCurrentTime())
assertTrue(internalImpl.getSdkCurrentTime() < System.currentTimeMillis())
fakeClock.tick(10L)
assertEquals(fakeClock.now(), internalImpl.getSdkCurrentTime())
}

@Test
fun `check isNetworkSpanForwardingEnabled`() {
assertFalse(internalImpl.isNetworkSpanForwardingEnabled())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,12 +127,6 @@ interface EmbraceInternalInterface : InternalTracingApi {
*/
fun isNetworkSpanForwardingEnabled(): Boolean

/**
* Return internal time the SDK is using in milliseconds. It is equivalent to [System.currentTimeMillis] assuming the system clock did
* not change after the SDK has started.
*/
fun getSdkCurrentTime(): Long

/**
* Whether the ANR capture service is enabled
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,6 @@ internal class NoopEmbraceInternalInterface(

override fun isNetworkSpanForwardingEnabled(): Boolean = false

override fun getSdkCurrentTime(): Long = System.currentTimeMillis()

override fun isAnrCaptureEnabled(): Boolean = false

override fun isNdkEnabled(): Boolean = false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import io.embrace.android.embracesdk.network.EmbraceNetworkRequest
import io.embrace.android.embracesdk.network.http.HttpMethod
import io.mockk.mockk
import org.junit.Assert.assertFalse
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Test
import java.net.SocketException
Expand Down Expand Up @@ -66,12 +65,6 @@ internal class NoopEmbraceInternalInterfaceTest {
impl.stopSdk()
}

@Test
fun `check default SDK time implementation`() {
assertTrue(beforeObjectInitTime < impl.getSdkCurrentTime())
assertTrue(impl.getSdkCurrentTime() <= System.currentTimeMillis())
}

@Test
fun `check isNetworkSpanForwardingEnabled before SDK starts`() {
assertFalse(impl.isNetworkSpanForwardingEnabled())
Expand All @@ -86,8 +79,4 @@ internal class NoopEmbraceInternalInterfaceTest {
fun `check isNdkEnabled before SDK starts`() {
assertFalse(impl.isNdkEnabled())
}

companion object {
private val beforeObjectInitTime = System.currentTimeMillis() - 1
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,6 @@ class FakeEmbraceInternalInterface(

override fun isNetworkSpanForwardingEnabled(): Boolean = networkSpanForwardingEnabled

override fun getSdkCurrentTime(): Long {
return 0
}

override fun isAnrCaptureEnabled(): Boolean {
return true
}
Expand Down

0 comments on commit 75ef997

Please sign in to comment.