Skip to content

Fix peer connection previous-connect analytics scope #1763

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
rahul-lohra wants to merge 1 commit into
base: develop
Choose a base branch
from
Draft

Fix peer connection previous-connect analytics scope #1763

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions ...kotlin/io/getstream/video/android/core/analytics/call/observer/PeerConnectionAnalytics.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,17 @@ internal class PeerConnectionAnalytics(
iceState: VideoAnalyticsIceState,
peerConnectionState: PeerConnection.PeerConnectionState?,
) {
when (peerConnectionState) {
PeerConnection.PeerConnectionState.CONNECTED -> {
if (role == PeerConnectionRole.PUBLISH) {
stateHolder.updatePublisherEverConnected(true)
} else {
stateHolder.updateSubscriberEverConnected(true)
}
}
else -> {}
}

reporter.onPeerConnectionStateChanged(
peerConnectionHashCode = peerConnectionHashCode,
callId = callId,
Expand All @@ -172,6 +183,7 @@ internal class PeerConnectionAnalytics(
joinStageAttemptId = joinAnalyticsStateHolder.state.value.joinStageAttemptId ?: "unknown",
joinReason = joinAnalyticsStateHolder.state.value.joinReason ?: JoinReason.Unknown,
sfuId = sfuAnalyticsStateHolder.sfuId.value,
wasPrevConnected = stateHolder.isPcEverConnected(role),
callSessionId = joinAnalyticsStateHolder.state.value.callSessionId,
)
}
Expand Down
19 changes: 19 additions & 0 deletions ...etstream/video/android/core/analytics/call/observer/PeerConnectionAnalyticsStateHolder.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package io.getstream.video.android.core.analytics.call.observer

import io.getstream.video.android.core.analytics.call.observer.model.Stage
import io.getstream.video.android.core.analytics.reporting.model.PeerConnectionRole
import kotlinx.coroutines.Job
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.StateFlow
Expand Down Expand Up @@ -47,6 +48,22 @@ internal class PeerConnectionAnalyticsStateHolder {
_state.update { it.copy(subscriberStage = stage) }
}

fun updatePublisherEverConnected(wasPublisherEverConnected: Boolean) {
_state.update { it.copy(wasPublisherEverConnected = wasPublisherEverConnected) }
}

fun updateSubscriberEverConnected(wasSubscriberEverConnected: Boolean) {
_state.update { it.copy(wasSubscriberEverConnected = wasSubscriberEverConnected) }
}

fun isPcEverConnected(role: PeerConnectionRole): Boolean {
return if (role == PeerConnectionRole.PUBLISH) {
_state.value.wasPublisherEverConnected
} else {
_state.value.wasSubscriberEverConnected
}
}

fun update(
peerConnectionObserverJob: Job? = state.value.peerConnectionObserverJob,
publisherJob: Job? = state.value.publisherJob,
Expand Down Expand Up @@ -76,4 +93,6 @@ internal data class PeerConnectionAnalyticsState(
val subscriberJob: Job? = null,
val publisherStage: Stage = Stage.NOT_STARTED,
val subscriberStage: Stage = Stage.NOT_STARTED,
val wasPublisherEverConnected: Boolean = false,
val wasSubscriberEverConnected: Boolean = false,
)
7 changes: 3 additions & 4 deletions ...rc/main/kotlin/io/getstream/video/android/core/analytics/reporting/ClientEventReporter.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@
ClientEventFactory(sdkVersion, userAgent, coordinatorAnalyticsStateHolder)

private val postCallFlightSessions = ConcurrentHashMap<StageId, InFlightSession>()
private val pcEverConnected = ConcurrentHashMap<PeerConnectionRole, PcConnected>()
private val pcEventReporterStateHolder = PeerConnectionEventReporterStateHolder()

// --- Coordinator WS ---
Expand Down Expand Up @@ -275,7 +274,7 @@

// --- PeerConnectionConnect (ICE state machine) ---

internal fun onPeerConnectionStateChanged(

Check warning on line 277 in stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/analytics/reporting/ClientEventReporter.kt

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

This function has 11 parameters, which is greater than the 7 authorized. 


See more on https://sonarcloud.io/project/issues?id=GetStream_stream-video-android&issues=AZ_XBrJvApb-8PCsGGJR&open=AZ_XBrJvApb-8PCsGGJR&pullRequest=1763
peerConnectionHashCode: Int,
callId: String,
callType: String,
Expand All @@ -285,6 +284,7 @@
joinReason: JoinReason,
role: PeerConnectionRole,
iceState: VideoAnalyticsIceState,
wasPrevConnected: Boolean,
peerConnectionState: PeerConnection.PeerConnectionState?,
) {
when (peerConnectionState) {
Expand All @@ -299,6 +299,7 @@
joinReason,
role,
iceState,
wasPrevConnected,
peerConnectionState,
)
}
Expand Down Expand Up @@ -337,7 +338,7 @@
}
}

fun handleOnPeerConnectionConnectingState(

Check warning on line 341 in stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/analytics/reporting/ClientEventReporter.kt

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

This function has 11 parameters, which is greater than the 7 authorized. 


See more on https://sonarcloud.io/project/issues?id=GetStream_stream-video-android&issues=AZ_XBrJvApb-8PCsGGJS&open=AZ_XBrJvApb-8PCsGGJS&pullRequest=1763
peerConnectionHashCode: Int,
callId: String,
callType: String,
Expand All @@ -347,9 +348,9 @@
joinReason: JoinReason,
role: PeerConnectionRole,
iceState: VideoAnalyticsIceState,
wasPrevConnected: Boolean,
peerConnectionState: PeerConnection.PeerConnectionState,
) {
val wasPrevConnected = pcEverConnected[role] != null
val stageId = UUID.randomUUID().toString()
val now = System.currentTimeMillis()
postCallFlightSessions[stageId] = PostCallFlightSession(
Expand Down Expand Up @@ -387,7 +388,7 @@
)
}

fun handleOnPeerConnectionConnectedState(

Check warning on line 391 in stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/analytics/reporting/ClientEventReporter.kt

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

This function has 10 parameters, which is greater than the 7 authorized. 


See more on https://sonarcloud.io/project/issues?id=GetStream_stream-video-android&issues=AZ_XBrJvApb-8PCsGGJT&open=AZ_XBrJvApb-8PCsGGJT&pullRequest=1763
peerConnectionHashCode: Int,
callId: String,
callType: String,
Expand All @@ -400,7 +401,6 @@
peerConnectionState:
PeerConnection.PeerConnectionState,
) {
pcEverConnected[role] = PcConnected(System.currentTimeMillis())
val pcState = pcEventReporterStateHolder.map.remove(peerConnectionHashCode) ?: return
val stageId = pcState.stageId

Expand All @@ -418,7 +418,7 @@
)
}

fun handleOnPeerConnectionFailedState(

Check warning on line 421 in stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/analytics/reporting/ClientEventReporter.kt

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

This function has 10 parameters, which is greater than the 7 authorized. 


See more on https://sonarcloud.io/project/issues?id=GetStream_stream-video-android&issues=AZ_XBrJvApb-8PCsGGJU&open=AZ_XBrJvApb-8PCsGGJU&pullRequest=1763
peerConnectionHashCode: Int,
callId: String,
callType: String,
Expand Down Expand Up @@ -448,7 +448,7 @@
)
}

private fun completePeerConnectionSession(

Check warning on line 451 in stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/analytics/reporting/ClientEventReporter.kt

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

This function has 12 parameters, which is greater than the 7 authorized. 


See more on https://sonarcloud.io/project/issues?id=GetStream_stream-video-android&issues=AZ_XBrJvApb-8PCsGGJV&open=AZ_XBrJvApb-8PCsGGJV&pullRequest=1763
callId: String,
callType: String,
stageId: String,
Expand Down Expand Up @@ -630,4 +630,3 @@
var stageId: String,
val peerConnectionRole: PeerConnectionRole,
)
internal class PcConnected(val lastConnectedTime: Long)
65 changes: 65 additions & 0 deletions ...in/io/getstream/video/android/core/analytics/call/observer/PeerConnectionAnalyticsTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ class PeerConnectionAnalyticsTest {
joinReason = JoinReason.ReJoin,
role = PeerConnectionRole.SUBSCRIBE,
iceState = VideoAnalyticsIceState.NOT_CONNECTED,
wasPrevConnected = false,
peerConnectionState = PeerConnection.PeerConnectionState.CONNECTING,
)
}
Expand Down Expand Up @@ -131,6 +132,7 @@ class PeerConnectionAnalyticsTest {
joinReason = any(),
role = PeerConnectionRole.PUBLISH,
iceState = VideoAnalyticsIceState.CONNECTED,
wasPrevConnected = false,
peerConnectionState = PeerConnection.PeerConnectionState.CONNECTING,
)
}
Expand Down Expand Up @@ -161,6 +163,7 @@ class PeerConnectionAnalyticsTest {
joinReason = any(),
role = PeerConnectionRole.PUBLISH,
iceState = VideoAnalyticsIceState.NOT_CONNECTED,
wasPrevConnected = true,
peerConnectionState = PeerConnection.PeerConnectionState.CONNECTED,
)
}
Expand Down Expand Up @@ -191,6 +194,7 @@ class PeerConnectionAnalyticsTest {
joinReason = any(),
role = PeerConnectionRole.PUBLISH,
iceState = VideoAnalyticsIceState.NOT_CONNECTED,
wasPrevConnected = false,
peerConnectionState = PeerConnection.PeerConnectionState.CONNECTING,
)
}
Expand Down Expand Up @@ -220,6 +224,7 @@ class PeerConnectionAnalyticsTest {
joinReason = any(),
role = PeerConnectionRole.PUBLISH,
iceState = VideoAnalyticsIceState.FAILED,
wasPrevConnected = false,
peerConnectionState = PeerConnection.PeerConnectionState.FAILED,
)
}
Expand Down Expand Up @@ -285,12 +290,72 @@ class PeerConnectionAnalyticsTest {
joinReason = any(),
role = PeerConnectionRole.PUBLISH,
iceState = VideoAnalyticsIceState.CONNECTED,
wasPrevConnected = true,
peerConnectionState = PeerConnection.PeerConnectionState.CONNECTED,
)
}
scope.cancel()
}

@Test
fun `a publisher reconnect is flagged as previously connected without affecting subscriber`() {
val scope = CoroutineScope(Dispatchers.Unconfined)
val peerConnectionAnalytics = analytics(scope)

peerConnectionAnalytics.onPeerConnectionStateChanged(
peerConnectionHashCode = 42,
role = PeerConnectionRole.PUBLISH,
iceState = VideoAnalyticsIceState.CONNECTED,
peerConnectionState = PeerConnection.PeerConnectionState.CONNECTED,
)

peerConnectionAnalytics.onPeerConnectionStateChanged(
peerConnectionHashCode = 43,
role = PeerConnectionRole.SUBSCRIBE,
iceState = VideoAnalyticsIceState.NOT_CONNECTED,
peerConnectionState = PeerConnection.PeerConnectionState.CONNECTING,
)

peerConnectionAnalytics.onPeerConnectionStateChanged(
peerConnectionHashCode = 44,
role = PeerConnectionRole.PUBLISH,
iceState = VideoAnalyticsIceState.NOT_CONNECTED,
peerConnectionState = PeerConnection.PeerConnectionState.CONNECTING,
)

verify {
reporter.onPeerConnectionStateChanged(
peerConnectionHashCode = 43,
callId = "call-1",
callType = "default",
joinStageAttemptId = any(),
callSessionId = any(),
sfuId = any(),
joinReason = any(),
role = PeerConnectionRole.SUBSCRIBE,
iceState = VideoAnalyticsIceState.NOT_CONNECTED,
wasPrevConnected = false,
peerConnectionState = PeerConnection.PeerConnectionState.CONNECTING,
)
}
verify {
reporter.onPeerConnectionStateChanged(
peerConnectionHashCode = 44,
callId = "call-1",
callType = "default",
joinStageAttemptId = any(),
callSessionId = any(),
sfuId = any(),
joinReason = any(),
role = PeerConnectionRole.PUBLISH,
iceState = VideoAnalyticsIceState.NOT_CONNECTED,
wasPrevConnected = true,
peerConnectionState = PeerConnection.PeerConnectionState.CONNECTING,
)
}
scope.cancel()
}

@Test
fun `a connected publisher resets the stage back to completed`() = runTest {
val session = mockSession(
Expand Down
3 changes: 3 additions & 0 deletions ...est/kotlin/io/getstream/video/android/core/analytics/reporting/ClientEventReporterTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ class ClientEventReporterTest {
iceState: VideoAnalyticsIceState,
role: PeerConnectionRole = PeerConnectionRole.PUBLISH,
pcHashCode: Int = 100,
wasPrevConnected: Boolean = false,
) = reporter.onPeerConnectionStateChanged(
peerConnectionHashCode = pcHashCode,
callId = "call-1",
Expand All @@ -97,6 +98,7 @@ class ClientEventReporterTest {
joinReason = JoinReason.FirstAttempt,
role = role,
iceState = iceState,
wasPrevConnected = wasPrevConnected,
peerConnectionState = pcState,
)

Expand Down Expand Up @@ -329,6 +331,7 @@ class ClientEventReporterTest {
pcState = PeerConnection.PeerConnectionState.CONNECTING,
iceState = VideoAnalyticsIceState.NOT_CONNECTED,
pcHashCode = 2,
wasPrevConnected = true,
)

val reconnectInitiated = dispatcher.sent.last()
Expand Down
Loading