Skip to content

Add register api to FirebaseSessions so products can integrate #4989

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.

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

Merged
merged 15 commits into from
May 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 1 addition & 2 deletions firebase-sessions/api.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ package com.google.firebase.sessions {
public final class FirebaseSessions {
method @NonNull public static com.google.firebase.sessions.FirebaseSessions getInstance();
method @NonNull public static com.google.firebase.sessions.FirebaseSessions getInstance(@NonNull com.google.firebase.FirebaseApp app);
method @Discouraged(message="This will be replaced with a real API.") @NonNull public String greeting();
method public void register(@NonNull com.google.firebase.sessions.api.SessionSubscriber subscriber);
property @NonNull public static final com.google.firebase.sessions.FirebaseSessions instance;
field @NonNull public static final com.google.firebase.sessions.FirebaseSessions.Companion Companion;
}
Expand All @@ -16,4 +16,3 @@ package com.google.firebase.sessions {
}

}

Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,8 @@ class FirebaseSessionsTests {
}

@Test
fun mattDoesDayHi() {
// This will be replaced with real tests.
assertThat(FirebaseSessions.instance.greeting()).isEqualTo("Matt says hi!")
fun firebaseSessionsDoesInitialize() {
assertThat(FirebaseSessions.instance).isNotNull()
}

companion object {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,18 @@ package com.google.firebase.sessions

import android.app.Application
import android.util.Log
import androidx.annotation.Discouraged
import com.google.android.datatransport.TransportFactory
import com.google.firebase.FirebaseApp
import com.google.firebase.inject.Provider
import com.google.firebase.installations.FirebaseInstallationsApi
import com.google.firebase.ktx.Firebase
import com.google.firebase.ktx.app
import com.google.firebase.sessions.api.FirebaseSessionsDependencies
import com.google.firebase.sessions.api.SessionSubscriber
import com.google.firebase.sessions.settings.SessionsSettings
import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.launch

class FirebaseSessions
internal constructor(
Expand All @@ -47,9 +50,9 @@ internal constructor(
)
private val sessionGenerator = SessionGenerator(collectEvents = shouldCollectEvents())
private val eventGDTLogger = EventGDTLogger(transportFactoryProvider)
private val sessionCoordinator =
SessionCoordinator(firebaseInstallations, backgroundDispatcher, eventGDTLogger)
private val sessionCoordinator = SessionCoordinator(firebaseInstallations, eventGDTLogger)
private val timeProvider: TimeProvider = Time()
private val sessionStartScope = CoroutineScope(backgroundDispatcher)

init {
sessionSettings.updateSettings()
Expand All @@ -66,20 +69,53 @@ internal constructor(
}
}

@Discouraged(message = "This will be replaced with a real API.")
fun greeting(): String = "Matt says hi!"
/** Register the [subscriber]. This must be called for every dependency. */
fun register(subscriber: SessionSubscriber) {
FirebaseSessionsDependencies.register(subscriber)

Log.d(
TAG,
"Registering Sessions SDK subscriber with name: ${subscriber.sessionSubscriberName}, " +
"data collection enabled: ${subscriber.isDataCollectionEnabled}"
)
}

private fun initiateSessionStart() {
val sessionDetails = sessionGenerator.generateNewSession()

if (!sessionGenerator.collectEvents) {
Log.d(TAG, "Sessions SDK has sampled this session")
return
}
sessionStartScope.launch {
val subscribers = FirebaseSessionsDependencies.getRegisteredSubscribers()

sessionCoordinator.attemptLoggingSessionEvent(
SessionEvents.startSession(firebaseApp, sessionDetails, sessionSettings, timeProvider)
)
if (subscribers.isEmpty()) {
Log.d(
TAG,
"Sessions SDK did not have any dependent SDKs register as dependencies. Events will not be sent."
)
return@launch
}

if (subscribers.values.none { it.isDataCollectionEnabled }) {
Log.d(TAG, "Data Collection is disabled for all subscribers. Skipping this Session Event")
return@launch
}

Log.d(TAG, "Data Collection is enabled for at least one Subscriber")

subscribers.values.forEach { subscriber ->
if (subscriber.isDataCollectionEnabled) {
subscriber.onSessionChanged(SessionSubscriber.SessionDetails(sessionDetails.sessionId))
}
}

if (!sessionGenerator.collectEvents) {
Log.d(TAG, "Sessions SDK has sampled this session")
return@launch
}

sessionCoordinator.attemptLoggingSessionEvent(
SessionEvents.startSession(firebaseApp, sessionDetails, sessionSettings, timeProvider)
)
}
}

/** Calculate whether we should sample events using [sessionSettings] data. */
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ import com.google.android.datatransport.TransportFactory
import com.google.firebase.FirebaseApp
import com.google.firebase.annotations.concurrent.Background
import com.google.firebase.annotations.concurrent.Blocking
import com.google.firebase.components.*
import com.google.firebase.components.Component
import com.google.firebase.components.ComponentRegistrar
import com.google.firebase.components.Dependency
import com.google.firebase.components.Qualified.qualified
import com.google.firebase.components.Qualified.unqualified
import com.google.firebase.installations.FirebaseInstallationsApi
Expand All @@ -35,22 +37,14 @@ import kotlinx.coroutines.CoroutineDispatcher
internal class FirebaseSessionsRegistrar : ComponentRegistrar {
override fun getComponents() =
listOf(
Component.builder(firebaseSessionsEarly)
.name(EARLY_LIBRARY_NAME)
.factory { _ -> FirebaseSessionsEarly() }
.eagerInDefaultApp()
.build(),
Component.builder(FirebaseSessions::class.java)
.name(LIBRARY_NAME)
.add(Dependency.required(firebaseSessionsEarly))
.add(Dependency.required(firebaseApp))
.add(Dependency.required(firebaseInstallationsApi))
.add(Dependency.required(backgroundDispatcher))
.add(Dependency.required(blockingDispatcher))
.add(Dependency.requiredProvider(transportFactory))
.factory { container ->
// Make sure FirebaseSessionsEarly has started up
container.get(firebaseSessionsEarly)
FirebaseSessions(
container.get(firebaseApp),
container.get(firebaseInstallationsApi),
Expand All @@ -65,9 +59,7 @@ internal class FirebaseSessionsRegistrar : ComponentRegistrar {

companion object {
private const val LIBRARY_NAME = "fire-sessions"
private const val EARLY_LIBRARY_NAME = "fire-sessions-early"

private val firebaseSessionsEarly = unqualified(FirebaseSessionsEarly::class.java)
private val firebaseApp = unqualified(FirebaseApp::class.java)
private val firebaseInstallationsApi = unqualified(FirebaseInstallationsApi::class.java)
private val backgroundDispatcher =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,6 @@ package com.google.firebase.sessions

import android.util.Log
import com.google.firebase.installations.FirebaseInstallationsApi
import kotlin.coroutines.CoroutineContext
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.launch
import kotlinx.coroutines.tasks.await

/**
Expand All @@ -31,30 +28,26 @@ import kotlinx.coroutines.tasks.await
*/
internal class SessionCoordinator(
private val firebaseInstallations: FirebaseInstallationsApi,
context: CoroutineContext,
private val eventGDTLogger: EventGDTLoggerInterface,
) {
private val scope = CoroutineScope(context)

fun attemptLoggingSessionEvent(sessionEvent: SessionEvent) =
scope.launch {
sessionEvent.sessionData.firebaseInstallationId =
try {
firebaseInstallations.id.await()
} catch (ex: Exception) {
Log.e(TAG, "Error getting Firebase Installation ID: ${ex}. Using an empty ID")
// Use an empty fid if there is any failure.
""
}

suspend fun attemptLoggingSessionEvent(sessionEvent: SessionEvent) {
sessionEvent.sessionData.firebaseInstallationId =
try {
eventGDTLogger.log(sessionEvent)

Log.i(TAG, "Successfully logged Session Start event: ${sessionEvent.sessionData.sessionId}")
} catch (e: RuntimeException) {
Log.e(TAG, "Error logging Session Start event to DataTransport: ", e)
firebaseInstallations.id.await()
} catch (ex: Exception) {
Log.e(TAG, "Error getting Firebase Installation ID: ${ex}. Using an empty ID")
// Use an empty fid if there is any failure.
""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to allow dispatching events with empty fids? Can we stop from such events to go through? Empty FID could lead to invalid data ingested.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure. @samedson and I talked and thought this was better, we even updated iOS to do the same firebase/firebase-ios-sdk#11161 I am ok with dropping the event instead but we'll need to revert that iOS change to keep it consistent.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you remember the rationale as to why it is fine to dispatch an event with empty FID? Because, backend is going to drop it anyways since this event would be accounted as invalid.

}

try {
eventGDTLogger.log(sessionEvent)

Log.i(TAG, "Successfully logged Session Start event: ${sessionEvent.sessionData.sessionId}")
} catch (ex: RuntimeException) {
Log.e(TAG, "Error logging Session Start event to DataTransport: ", ex)
}
}

companion object {
private const val TAG = "SessionCoordinator"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,15 @@

package com.google.firebase.sessions.api

import androidx.annotation.Discouraged

/** [SessionSubscriber] is an interface that dependent SDKs must implement. */
interface SessionSubscriber {
/** [SessionSubscriber.Name]s are used for identifying subscribers. */
enum class Name {
CRASHLYTICS,
PERFORMANCE,
@Discouraged(message = "This is for testing purposes only.") MATT_SAYS_HI,
}

/** [SessionDetails] contains session data passed to subscribers whenever the session changes */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ class SessionCoordinatorTest {
val sessionCoordinator =
SessionCoordinator(
firebaseInstallations,
context = TestOnlyExecutors.background().asCoroutineDispatcher() + coroutineContext,
eventGDTLogger = fakeEventGDTLogger,
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ import com.google.firebase.FirebaseApp
import com.google.firebase.ktx.Firebase
import com.google.firebase.ktx.initialize
import com.google.firebase.sessions.FirebaseSessions
import com.google.firebase.sessions.api.FirebaseSessionsDependencies
import com.google.firebase.sessions.api.SessionSubscriber
import org.junit.After
import org.junit.Before
import org.junit.Test
Expand All @@ -41,16 +43,38 @@ class FirebaseSessionsTest {
}

@Test
fun initializeSession() {
fun initializeSessions_generatesSessionEvent() {
// Force the Firebase Sessions SDK to initialize.
assertThat(FirebaseSessions.instance.greeting()).isEqualTo("Matt says hi!")
assertThat(FirebaseSessions.instance).isNotNull()

// Add a fake dependency and register it, otherwise sessions will never send.
val fakeSessionSubscriber = FakeSessionSubscriber()
FirebaseSessions.instance.register(fakeSessionSubscriber)

// Wait for the session start event to send.
// TODO(mrober): Setup logger we can access from tests.
Thread.sleep(TIME_TO_LOG_SESSION)

// Assert that some session was generated and sent to the subscriber.
assertThat(fakeSessionSubscriber.sessionDetails).isNotNull()
}

companion object {
private const val TIME_TO_LOG_SESSION = 60_000L

init {
FirebaseSessionsDependencies.addDependency(SessionSubscriber.Name.MATT_SAYS_HI)
}
}

private class FakeSessionSubscriber(
override val isDataCollectionEnabled: Boolean = true,
override val sessionSubscriberName: SessionSubscriber.Name = SessionSubscriber.Name.MATT_SAYS_HI
) : SessionSubscriber {
var sessionDetails: SessionSubscriber.SessionDetails? = null
private set

override fun onSessionChanged(sessionDetails: SessionSubscriber.SessionDetails) {
this.sessionDetails = sessionDetails
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,12 @@ package com.google.firebase.testing.sessions
import android.os.Bundle
import android.widget.TextView
import androidx.appcompat.app.AppCompatActivity
import com.google.firebase.sessions.FirebaseSessions

class MainActivity : AppCompatActivity() {
override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
setContentView(R.layout.activity_main)

findViewById<TextView>(R.id.greeting_text).text = FirebaseSessions.instance.greeting()
findViewById<TextView>(R.id.greeting_text).text = getText(R.string.firebase_greetings)
}
}