refactor: send payload to v2 endpoint

embrace-io · May 28, 2024 · 9b87a49 · 9b87a49
1 parent b5338a6
commit 9b87a49
Show file tree

Hide file tree

Showing 14 changed files with 22 additions and 76 deletions.
diff --git a/...ace-android-sdk/src/main/java/io/embrace/android/embracesdk/comms/api/ApiRequestMapper.kt b/...ace-android-sdk/src/main/java/io/embrace/android/embracesdk/comms/api/ApiRequestMapper.kt
@@ -68,9 +68,8 @@ internal class ApiRequestMapper(
         return requestBuilder(url)
     }
 
-    @Suppress("UNUSED_PARAMETER")
-    fun sessionRequest(v2Payload: Boolean): ApiRequest {
-        val url = Endpoint.SESSIONS // send to v1 endpoint for now.
+    fun sessionRequest(): ApiRequest {
+        val url = Endpoint.SESSIONS_V2
         return requestBuilder(url.asEmbraceUrl())
     }
 

diff --git a/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/comms/api/ApiService.kt b/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/comms/api/ApiService.kt
@@ -78,5 +78,5 @@ internal interface ApiService {
      * Sends a session to the API. This can be either a v1 or v2 session - the implementation
      * is responsible for routing the payload correctly.
      */
-    fun sendSession(isV2: Boolean, action: SerializationAction, onFinish: ((successful: Boolean) -> Unit)?): Future<*>?
+    fun sendSession(action: SerializationAction, onFinish: ((successful: Boolean) -> Unit)?): Future<*>?
 }
diff --git a/...ce-android-sdk/src/main/java/io/embrace/android/embracesdk/comms/api/EmbraceApiService.kt b/...ce-android-sdk/src/main/java/io/embrace/android/embracesdk/comms/api/EmbraceApiService.kt
@@ -156,8 +156,8 @@ internal class EmbraceApiService(
         return post(crash, mapper::eventMessageRequest) { cacheManager.deleteCrash() }
     }
 
-    override fun sendSession(isV2: Boolean, action: SerializationAction, onFinish: ((successful: Boolean) -> Unit)?): Future<*> {
-        return postOnWorker(action, mapper.sessionRequest(isV2), onFinish)
+    override fun sendSession(action: SerializationAction, onFinish: ((successful: Boolean) -> Unit)?): Future<*> {
+        return postOnWorker(action, mapper.sessionRequest(), onFinish)
     }
 
     private inline fun <reified T> post(

diff --git a/...e-android-sdk/src/main/java/io/embrace/android/embracesdk/comms/delivery/CachedSession.kt b/...e-android-sdk/src/main/java/io/embrace/android/embracesdk/comms/delivery/CachedSession.kt
@@ -18,7 +18,7 @@ internal class CachedSession private constructor(
         /**
          * Creates a [CachedSession] from a session id and timestamp.
          */
-        fun create(sessionId: String, timestampMs: Long, v2Payload: Boolean): CachedSession {
+        fun create(sessionId: String, timestampMs: Long, v2Payload: Boolean = true): CachedSession {
             val filename = when {
                 v2Payload -> getV2FileNameForSession(
                     sessionId,

diff --git a/...src/main/java/io/embrace/android/embracesdk/comms/delivery/EmbraceDeliveryCacheManager.kt b/...src/main/java/io/embrace/android/embracesdk/comms/delivery/EmbraceDeliveryCacheManager.kt
@@ -7,7 +7,6 @@ import io.embrace.android.embracesdk.internal.utils.Uuid
 import io.embrace.android.embracesdk.logging.EmbLogger
 import io.embrace.android.embracesdk.payload.EventMessage
 import io.embrace.android.embracesdk.payload.SessionMessage
-import io.embrace.android.embracesdk.payload.isV2Payload
 import io.embrace.android.embracesdk.session.orchestrator.SessionSnapshotType
 import io.embrace.android.embracesdk.worker.BackgroundWorker
 import io.embrace.android.embracesdk.worker.TaskPriority
@@ -54,8 +53,7 @@ internal class EmbraceDeliveryCacheManager(
                 sessionId,
                 sessionStartTimeMs,
                 writeSync,
-                snapshot,
-                sessionMessage.isV2Payload()
+                snapshot
             ) { filename: String ->
                 Systrace.traceSynchronous("serialize-session") {
                     cacheService.writeSession(filename, sessionMessage)
@@ -208,11 +206,10 @@ internal class EmbraceDeliveryCacheManager(
         sessionStartTimeMs: Long,
         writeSync: Boolean = false,
         snapshot: Boolean = false,
-        v2Payload: Boolean,
         saveAction: (filename: String) -> Unit
     ) {
         if (writeSync) {
-            saveSessionBytesImpl(sessionId, sessionStartTimeMs, v2Payload, saveAction)
+            saveSessionBytesImpl(sessionId, sessionStartTimeMs, saveAction)
         } else {
             // snapshots are low priority compared to state ends + loading/unloading other payload
             // types. State ends are critical as they contain the final information.
@@ -221,21 +218,20 @@ internal class EmbraceDeliveryCacheManager(
                 else -> TaskPriority.CRITICAL
             }
             backgroundWorker.submit(priority) {
-                saveSessionBytesImpl(sessionId, sessionStartTimeMs, v2Payload, saveAction)
+                saveSessionBytesImpl(sessionId, sessionStartTimeMs, saveAction)
             }
         }
     }
 
     private fun saveSessionBytesImpl(
         sessionId: String,
         sessionStartTimeMs: Long,
-        v2Payload: Boolean,
         saveAction: (filename: String) -> Unit
     ) {
         try {
             synchronized(cachedSessions) {
                 val cachedSession = cachedSessions.getOrElse(sessionId) {
-                    CachedSession.create(sessionId, sessionStartTimeMs, v2Payload)
+                    CachedSession.create(sessionId, sessionStartTimeMs)
                 }
                 saveAction(cachedSession.filename)
                 if (!cachedSessions.containsKey(cachedSession.sessionId)) {

diff --git a/...-sdk/src/main/java/io/embrace/android/embracesdk/comms/delivery/EmbraceDeliveryService.kt b/...-sdk/src/main/java/io/embrace/android/embracesdk/comms/delivery/EmbraceDeliveryService.kt
@@ -13,7 +13,6 @@ import io.embrace.android.embracesdk.payload.EventMessage
 import io.embrace.android.embracesdk.payload.NativeCrashData
 import io.embrace.android.embracesdk.payload.NetworkEvent
 import io.embrace.android.embracesdk.payload.SessionMessage
-import io.embrace.android.embracesdk.payload.isV2Payload
 import io.embrace.android.embracesdk.session.id.SessionIdTracker
 import io.embrace.android.embracesdk.session.orchestrator.SessionSnapshotType
 import io.embrace.android.embracesdk.worker.BackgroundWorker
@@ -52,7 +51,7 @@ internal class EmbraceDeliveryService(
                     serializer.toJson(sessionMessage, SessionMessage::class.java, it)
                 }
             }
-            val future = apiService.sendSession(sessionMessage.isV2Payload(), action) { successful ->
+            val future = apiService.sendSession(action) { successful ->
                 if (!successful) {
                     val message =
                         "Session deleted without request being sent: ID $sessionId, timestamp ${sessionMessage.session.startTime}"
@@ -157,9 +156,7 @@ internal class EmbraceDeliveryService(
                 val sessionId = cachedSession.sessionId
                 val action = cacheManager.loadSessionAsAction(sessionId)
                 if (action != null) {
-                    // temporarily assume all sessions are v1. Future changeset
-                    // will encode this information in the filename.
-                    apiService.sendSession(false, action) { successful ->
+                    apiService.sendSession(action) { successful ->
                         if (!successful) {
                             val message = "Cached session deleted without request being sent. File name: ${cachedSession.filename}"
                             logger.logWarning(message, SessionPurgeException(message))

diff --git a/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/payload/SessionMessage.kt b/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/payload/SessionMessage.kt
@@ -47,9 +47,3 @@ internal data class SessionMessage @JvmOverloads internal constructor(
     @Json(name = "data")
     val data: SessionPayload? = null
 )
-
-/**
- * Returns true if this message is a v2 payload. If so, it should be sent to a different
- * endpoint & handled differently.
- */
-internal fun SessionMessage.isV2Payload() = data != null
diff --git a/...android-sdk/src/test/java/io/embrace/android/embracesdk/comms/api/ApiRequestMapperTest.kt b/...android-sdk/src/test/java/io/embrace/android/embracesdk/comms/api/ApiRequestMapperTest.kt
@@ -85,15 +85,9 @@ internal class ApiRequestMapperTest {
     }
 
     @Test
-    fun testV1SessionRequest() {
-        val request = mapper.sessionRequest(false)
-        request.assertCoreFieldsPopulated("/v1/log/sessions")
-    }
-
-    @Test
-    fun testV2SessionRequest() {
-        val request = mapper.sessionRequest(true)
-        request.assertCoreFieldsPopulated("/v1/log/sessions")
+    fun testSessionRequest() {
+        val request = mapper.sessionRequest()
+        request.assertCoreFieldsPopulated("/v2/spans")
     }
 
     @Test

diff --git a/...ndroid-sdk/src/test/java/io/embrace/android/embracesdk/comms/api/EmbraceApiServiceTest.kt b/...ndroid-sdk/src/test/java/io/embrace/android/embracesdk/comms/api/EmbraceApiServiceTest.kt
@@ -242,27 +242,14 @@ internal class EmbraceApiServiceTest {
         )
     }
 
-    @Test
-    fun `send v1 session`() {
-        fakeApiClient.queueResponse(successfulPostResponse)
-        val payload = "{}".toByteArray(Charsets.UTF_8)
-        var finished = false
-        apiService.sendSession(false, { it.write(payload) }) { finished = true }
-        verifyOnlyRequest(
-            expectedUrl = "https://a-$fakeAppId.data.emb-api.com/v1/log/sessions",
-            expectedPayload = payload
-        )
-        assertTrue(finished)
-    }
-
     @Test
     fun `send v2 session`() {
         fakeApiClient.queueResponse(successfulPostResponse)
         val payload = "".toByteArray(Charsets.UTF_8)
         var finished = false
-        apiService.sendSession(true, { it.write(payload) }) { finished = true }
+        apiService.sendSession({ it.write(payload) }) { finished = true }
         verifyOnlyRequest(
-            expectedUrl = "https://a-$fakeAppId.data.emb-api.com/v1/log/sessions",
+            expectedUrl = "https://a-$fakeAppId.data.emb-api.com/v2/spans",
             expectedPayload = payload
         )
         assertTrue(finished)
@@ -349,7 +336,7 @@ internal class EmbraceApiServiceTest {
         testScheduledExecutor = mockk(relaxed = true)
         initApiService()
         val payload = "{}".toByteArray(Charsets.UTF_8)
-        apiService.sendSession(false, { it.write(payload) }) {}
+        apiService.sendSession({ it.write(payload) }) {}
         verify(exactly = 1) { testScheduledExecutor.submit(any<Runnable>()) }
     }
 

diff --git a/.../src/test/java/io/embrace/android/embracesdk/comms/delivery/EmbraceDeliveryServiceTest.kt b/.../src/test/java/io/embrace/android/embracesdk/comms/delivery/EmbraceDeliveryServiceTest.kt
@@ -277,7 +277,7 @@ internal class EmbraceDeliveryServiceTest {
         private val sessionFileName = CachedSession.create(
             sessionMessage.session.sessionId,
             sessionMessage.session.startTime,
-            false
+            true
         ).filename
         private val anotherMessage = sessionMessage.copy(session = fakeSession().copy(sessionId = "session2"))
         private val anotherMessageFileName = CachedSession.create(

diff --git a/...est/java/io/embrace/android/embracesdk/comms/delivery/EmbracePendingApiCallsSenderTest.kt b/...est/java/io/embrace/android/embracesdk/comms/delivery/EmbracePendingApiCallsSenderTest.kt
@@ -288,7 +288,7 @@ internal class EmbracePendingApiCallsSenderTest {
         assertEquals("il:message_id_6", pendingApiCalls.pollNextPendingApiCall()?.apiRequest?.logId)
 
         // now add some sessions for retry and verify they are returned first
-        val sessionRequest = mapper.sessionRequest(false).copy(logId = "is:session_id_0")
+        val sessionRequest = mapper.sessionRequest().copy(logId = "is:session_id_0")
         pendingApiCallsSender.savePendingApiCall(sessionRequest, {}, false)
         pendingApiCallsSender.scheduleRetry(ApiResponse.Incomplete(Throwable()))
         assertEquals(sessionRequest, pendingApiCalls.pollNextPendingApiCall()?.apiRequest)

diff --git a/embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/fakes/FakeApiService.kt b/embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/fakes/FakeApiService.kt
@@ -70,7 +70,7 @@ internal class FakeApiService : ApiService {
         blobRequests.add(blobMessage)
     }
 
-    override fun sendSession(isV2: Boolean, action: SerializationAction, onFinish: ((successful: Boolean) -> Unit)?): Future<*> {
+    override fun sendSession(action: SerializationAction, onFinish: ((successful: Boolean) -> Unit)?): Future<*> {
         if (throwExceptionSendSession) {
             error("FakeApiService.sendSession")
         }

diff --git a/...ace-android-sdk/src/test/java/io/embrace/android/embracesdk/payload/SessionMessageTest.kt b/...ace-android-sdk/src/test/java/io/embrace/android/embracesdk/payload/SessionMessageTest.kt
@@ -6,14 +6,11 @@ import io.embrace.android.embracesdk.deserializeEmptyJsonString
 import io.embrace.android.embracesdk.deserializeJsonFromResource
 import io.embrace.android.embracesdk.fakes.fakeSession
 import io.embrace.android.embracesdk.fixtures.testSpanSnapshot
-import io.embrace.android.embracesdk.internal.payload.SessionPayload
 import io.embrace.android.embracesdk.internal.payload.toOldPayload
 import io.embrace.android.embracesdk.internal.spans.EmbraceSpanData
 import io.opentelemetry.api.trace.StatusCode
 import org.junit.Assert.assertEquals
-import org.junit.Assert.assertFalse
 import org.junit.Assert.assertNotNull
-import org.junit.Assert.assertTrue
 import org.junit.Test
 
 internal class SessionMessageTest {
@@ -58,11 +55,4 @@ internal class SessionMessageTest {
     fun testEmptyObject() {
         deserializeEmptyJsonString<SessionMessage>()
     }
-
-    @Test
-    fun `is v2 payload`() {
-        assertFalse(info.isV2Payload())
-        val obj = SessionMessage(session = session, data = SessionPayload())
-        assertTrue(obj.isV2Payload())
-    }
 }
diff --git a/...sdk/src/test/java/io/embrace/android/embracesdk/session/message/PayloadFactoryImplTest.kt b/...sdk/src/test/java/io/embrace/android/embracesdk/session/message/PayloadFactoryImplTest.kt
@@ -22,10 +22,7 @@ import io.embrace.android.embracesdk.fakes.fakeOTelBehavior
 import io.embrace.android.embracesdk.fakes.injection.FakeInitModule
 import io.embrace.android.embracesdk.internal.serialization.EmbraceSerializer
 import io.embrace.android.embracesdk.payload.LegacyExceptionError
-import io.embrace.android.embracesdk.payload.isV2Payload
 import io.embrace.android.embracesdk.session.lifecycle.ProcessState.FOREGROUND
-import org.junit.Assert.assertFalse
-import org.junit.Assert.assertTrue
 import org.junit.Before
 import org.junit.Test
 
@@ -91,18 +88,10 @@ internal class PayloadFactoryImplTest {
         )
     }
 
-    @Test
-    fun `legacy payload generated`() {
-        val session = checkNotNull(factory.startPayloadWithState(FOREGROUND, 0, false))
-        val sessionMessage = checkNotNull(factory.endPayloadWithState(FOREGROUND, 0, session))
-        assertFalse(sessionMessage.isV2Payload())
-    }
-
     @Test
     fun `v2 payload generated`() {
         oTelConfig = OTelRemoteConfig(isDevEnabled = true)
         val session = checkNotNull(factory.startPayloadWithState(FOREGROUND, 0, false))
-        val sessionMessage = checkNotNull(factory.endPayloadWithState(FOREGROUND, 0, session))
-        assertTrue(sessionMessage.isV2Payload())
+        checkNotNull(factory.endPayloadWithState(FOREGROUND, 0, session))
     }
 }