Skip to content

Fix encoding for Session Events #4870

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

Merged
merged 4 commits into from
Apr 11, 2023
Merged

Fix encoding for Session Events #4870

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5117a07
Fix encoding for other fields
samedson Apr 6, 2023
5547487
Better
samedson Apr 10, 2023
26dee18
Update tests
samedson Apr 10, 2023
482c401
Fix tests
samedson Apr 10, 2023
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
8 changes: 6 additions & 2 deletions firebase-sessions/src/main/kotlin/com/google/firebase/sessions/EventGDTLogger.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.google.firebase.sessions

import android.util.Log
import com.google.android.datatransport.*
import com.google.android.datatransport.TransportFactory
import com.google.firebase.inject.Provider
Expand Down Expand Up @@ -53,11 +54,14 @@ internal class EventGDTLogger(private val transportFactoryProvider: Provider<Tra
}

private fun encode(value: SessionEvent): ByteArray {
return SessionEvents.SESSION_EVENT_ENCODER.encode(value).toByteArray()
val jsonEvent = SessionEvents.SESSION_EVENT_ENCODER.encode(value)
Log.d(TAG, "Encoded Session Start event $jsonEvent")
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious if you want to leave this as Debug or Verbose?

Copy link
Contributor Author

@samedson samedson Apr 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I'm down for Verbose - I wasn't sure when we use debug vs verbose

return jsonEvent.toByteArray()
}

companion object {
// TODO What do we put for the AQS Log Source
private const val TAG = "EventGDTLogger"
Copy link
Contributor

Choose a reason for hiding this comment

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

How would the log message look with this TAG? Would it contain FirebaseSessions anywhere? If not, I would recommend prefixing the tag with the library name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks like: V/EventGDTLogger: Encoded Session Start event {"event_type":1...

We've been doing it like this so I'll update this file and others


private const val AQS_LOG_SOURCE = "FIREBASE_APPQUALITY_SESSION"
}
}
2 changes: 1 addition & 1 deletion firebase-sessions/src/main/kotlin/com/google/firebase/sessions/SessionCoordinator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ internal class SessionCoordinator(
try {
eventGDTLogger.log(sessionEvent)

Log.i(TAG, "Logged Session Start event: $sessionEvent")
Log.i(TAG, "Logged Session Start event: ${sessionEvent.sessionData.sessionId}")
} catch (e: RuntimeException) {
Log.w(TAG, "Failed to log Session Start event: ", e)
}
Expand Down
35 changes: 35 additions & 0 deletions firebase-sessions/src/main/kotlin/com/google/firebase/sessions/SessionEvent.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,41 @@ internal data class SessionInfo(
/** What order this Session came in this run of the app. For the first Session this will be 0. */
val sessionIndex: Int,

/** Tracks when the event was initiated */
var eventTimestampUs: Long,

/** Data collection status of the dependent product SDKs. */
var dataCollectionStatus: DataCollectionStatus = DataCollectionStatus(),

/** Identifies a unique device+app installation: go/firebase-installations */
var firebaseInstallationId: String = "",
)

/** Contains the data collection state for all dependent SDKs and sampling info */
internal data class DataCollectionStatus(
val performance: DataCollectionState = DataCollectionState.COLLECTION_ENABLED,
val crashlytics: DataCollectionState = DataCollectionState.COLLECTION_ENABLED,
Comment on lines +74 to +75
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the default COLLECTION_ENABLED (for now?)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I'm putting it like this so it looks right in our data but we'll need to change this when we integrate with the product SDKs

val sessionSamplingRate: Double = 1.0,
Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting to have sessionSamplingRate inside the DataCollectionStatus. I was thinking DataCollectionStatus is for external dependencies and used sessionSamplingRate as a remote config. But this looks a bit different from that intention. Curious to hear your rationale to have session sampling rate inside DataCollectionStatus.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is how it is in our proto -
I wanted to be consistent with how it's defined: https://source.corp.google.com/piper///depot/google3/logs/proto/firebase/appquality/sessions/session_event.proto;l=67?q=DataCollectionStatus

)

/** Enum denoting different data collection states. */
internal enum class DataCollectionState(override val number: Int) : NumberedEnum {
COLLECTION_UNKNOWN(0),

// This product SDK is not present in this version of the app.
COLLECTION_SDK_NOT_INSTALLED(1),

// The product SDK is present and collecting all product-level events.
COLLECTION_ENABLED(2),

// The product SDK is present but data collection for it has been locally
// disabled.
COLLECTION_DISABLED(3),

// The product SDK is present but data collection has been remotely disabled.
COLLECTION_DISABLED_REMOTE(4),

// Indicates that the product SDK is present, but session data is being
// collected, but the product-level events are not being uploaded.
COLLECTION_SAMPLED(5),
}
44 changes: 43 additions & 1 deletion firebase-sessions/src/main/kotlin/com/google/firebase/sessions/SessionEvents.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ internal object SessionEvents {
run {
ctx.add(FieldDescriptor.of("event_type"), sessionEvent.eventType)
ctx.add(FieldDescriptor.of("session_data"), sessionEvent.sessionData)
ctx.add(FieldDescriptor.of("application_info"), sessionEvent.applicationInfo)
}
}

Expand All @@ -53,6 +54,42 @@ internal object SessionEvents {
FieldDescriptor.of("firebase_installation_id"),
sessionInfo.firebaseInstallationId
)
ctx.add(FieldDescriptor.of("event_timestamp_us"), sessionInfo.eventTimestampUs)
ctx.add(FieldDescriptor.of("data_collection_status"), sessionInfo.dataCollectionStatus)
}
}

it.registerEncoder(DataCollectionStatus::class.java) {
dataCollectionStatus: DataCollectionStatus,
ctx: ObjectEncoderContext ->
run {
ctx.add(FieldDescriptor.of("performance"), dataCollectionStatus.performance)
ctx.add(FieldDescriptor.of("crashlytics"), dataCollectionStatus.crashlytics)
ctx.add(
FieldDescriptor.of("session_sampling_rate"),
dataCollectionStatus.sessionSamplingRate
)
}
}

it.registerEncoder(ApplicationInfo::class.java) {
applicationInfo: ApplicationInfo,
ctx: ObjectEncoderContext ->
run {
ctx.add(FieldDescriptor.of("app_id"), applicationInfo.appId)
ctx.add(FieldDescriptor.of("device_model"), applicationInfo.deviceModel)
ctx.add(FieldDescriptor.of("session_sdk_version"), applicationInfo.sessionSdkVersion)
ctx.add(FieldDescriptor.of("log_environment"), applicationInfo.logEnvironment)
ctx.add(FieldDescriptor.of("android_app_info"), applicationInfo.androidAppInfo)
}
}

it.registerEncoder(AndroidApplicationInfo::class.java) {
androidAppInfo: AndroidApplicationInfo,
ctx: ObjectEncoderContext ->
run {
ctx.add(FieldDescriptor.of("package_name"), androidAppInfo.packageName)
ctx.add(FieldDescriptor.of("version_name"), androidAppInfo.versionName)
}
}
}
Expand All @@ -63,14 +100,19 @@ internal object SessionEvents {
*
* Some mutable fields, e.g. firebaseInstallationId, get populated later.
*/
fun startSession(firebaseApp: FirebaseApp, sessionDetails: SessionDetails) =
fun startSession(
firebaseApp: FirebaseApp,
sessionDetails: SessionDetails,
currentTimeUs: Long = System.currentTimeMillis() * 1000
) =
SessionEvent(
eventType = EventType.SESSION_START,
sessionData =
SessionInfo(
sessionDetails.sessionId,
sessionDetails.firstSessionId,
sessionDetails.sessionIndex,
currentTimeUs,
),
applicationInfo = getApplicationInfo(firebaseApp)
)
Expand Down
3 changes: 2 additions & 1 deletion firebase-sessions/src/test/kotlin/com/google/firebase/sessions/EventGDTLoggerTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ class EventGDTLoggerTest {
val sessionEvent =
SessionEvents.startSession(
FakeFirebaseApp.fakeFirebaseApp(),
TestSessionEventData.TEST_SESSION_DETAILS
TestSessionEventData.TEST_SESSION_DETAILS,
TestSessionEventData.TEST_SESSION_TIMESTAMP_US,
)
val fakeTransportFactory = FakeTransportFactory()
val fakeTransportFactoryProvider = FakeProvider(fakeTransportFactory as TransportFactory)
Expand Down
22 changes: 6 additions & 16 deletions firebase-sessions/src/test/kotlin/com/google/firebase/sessions/SessionCoordinatorTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ package com.google.firebase.sessions
import androidx.test.ext.junit.runners.AndroidJUnit4
import com.google.common.truth.Truth.assertThat
import com.google.firebase.sessions.testing.FakeEventGDTLogger
import com.google.firebase.sessions.testing.FakeFirebaseApp
import com.google.firebase.sessions.testing.FakeFirebaseInstallations
import com.google.firebase.sessions.testing.TestSessionEventData
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.test.StandardTestDispatcher
import kotlinx.coroutines.test.runCurrent
Expand All @@ -42,22 +44,10 @@ class SessionCoordinatorTest {

// Construct an event with no fid set.
val sessionEvent =
SessionEvent(
eventType = EventType.SESSION_START,
sessionData =
SessionInfo(
sessionId = "id",
firstSessionId = "first",
sessionIndex = 3,
),
applicationInfo =
ApplicationInfo(
appId = "",
deviceModel = "",
sessionSdkVersion = "",
logEnvironment = LogEnvironment.LOG_ENVIRONMENT_PROD,
androidAppInfo = AndroidApplicationInfo(packageName = "", versionName = "")
)
SessionEvents.startSession(
FakeFirebaseApp.fakeFirebaseApp(),
TestSessionEventData.TEST_SESSION_DETAILS,
TestSessionEventData.TEST_SESSION_TIMESTAMP_US,
)

sessionCoordinator.attemptLoggingSessionEvent(sessionEvent)
Expand Down
96 changes: 63 additions & 33 deletions firebase-sessions/src/test/kotlin/com/google/firebase/sessions/SessionEventEncoderTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,48 +18,61 @@ package com.google.firebase.sessions

import androidx.test.ext.junit.runners.AndroidJUnit4
import com.google.common.truth.Truth.assertThat
import com.google.firebase.FirebaseApp
import com.google.firebase.sessions.SessionEvents.SESSION_EVENT_ENCODER
import com.google.firebase.sessions.testing.FakeFirebaseApp
import com.google.firebase.sessions.testing.TestSessionEventData
import org.junit.After
import org.junit.Test
import org.junit.runner.RunWith

@RunWith(AndroidJUnit4::class)
class SessionEventEncoderTest {

@After
fun cleanUp() {
FirebaseApp.clearInstancesForTest()
}

@Test
fun sessionEvent_encodesToJson() {
val sessionEvent =
SessionEvent(
eventType = EventType.SESSION_START,
sessionData =
SessionInfo(
sessionId = "id",
firstSessionId = "first",
sessionIndex = 9,
firebaseInstallationId = "fid"
),
applicationInfo =
ApplicationInfo(
appId = "",
deviceModel = "",
sessionSdkVersion = "",
logEnvironment = LogEnvironment.LOG_ENVIRONMENT_PROD,
AndroidApplicationInfo(packageName = "", versionName = ""),
)
SessionEvents.startSession(
FakeFirebaseApp.fakeFirebaseApp(),
TestSessionEventData.TEST_SESSION_DETAILS,
TestSessionEventData.TEST_SESSION_TIMESTAMP_US,
)

val json = SESSION_EVENT_ENCODER.encode(sessionEvent)

assertThat(json)
.isEqualTo(
"""
{
"event_type":1,
"session_data":{
"session_id":"id",
"first_session_id":"first",
"session_index":9,
"firebase_installation_id":"fid"
{
"event_type":1,
"session_data":{
"session_id":"a1b2c3",
"first_session_id":"a1a1a1",
"session_index":3,
"firebase_installation_id":"",
"event_timestamp_us":12340000,
"data_collection_status":{
"performance":2,
"crashlytics":2,
"session_sampling_rate":1.0
}
},
"application_info":{
"app_id":"1:12345:android:app",
"device_model":"",
"session_sdk_version":"0.1.0",
"log_environment":3,
"android_app_info":{
"package_name":"com.google.firebase.sessions.test",
"version_name":"1.0.0"
}
}
}
}
"""
.lines()
.joinToString("") { it.trim() }
Expand All @@ -76,6 +89,7 @@ class SessionEventEncoderTest {
sessionId = "",
firstSessionId = "",
sessionIndex = 0,
eventTimestampUs = 0,
),
applicationInfo =
ApplicationInfo(
Expand All @@ -92,15 +106,31 @@ class SessionEventEncoderTest {
assertThat(json)
.isEqualTo(
"""
{
"event_type":0,
"session_data":{
"session_id":"",
"first_session_id":"",
"session_index":0,
"firebase_installation_id":""
{
"event_type":0,
"session_data":{
"session_id":"",
"first_session_id":"",
"session_index":0,
"firebase_installation_id":"",
"event_timestamp_us":0,
"data_collection_status":{
"performance":2,
"crashlytics":2,
"session_sampling_rate":1.0
}
},
"application_info":{
"app_id":"",
"device_model":"",
"session_sdk_version":"",
"log_environment":3,
"android_app_info":{
"package_name":"",
"version_name":""
}
}
}
}
"""
.lines()
.joinToString("") { it.trim() }
Expand Down
3 changes: 2 additions & 1 deletion firebase-sessions/src/test/kotlin/com/google/firebase/sessions/SessionEventTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ class SessionEventTest {
val sessionEvent =
SessionEvents.startSession(
FakeFirebaseApp.fakeFirebaseApp(),
TestSessionEventData.TEST_SESSION_DETAILS
TestSessionEventData.TEST_SESSION_DETAILS,
TestSessionEventData.TEST_SESSION_TIMESTAMP_US,
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason why the event timestamp is configurable? Can we leave it as a constant?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

startSession is also used by the SDK. When it leaves off the parameter it uses the current time. Since we can't have the time be different for every test I made it optional to just set the time

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is fine for now, but we need a proper time solution at some point. We can't just use go/javatime because it requires Android API level 26+. Maybe we can add another function in WallClock and try to abstract this away?

)

assertThat(sessionEvent).isEqualTo(TestSessionEventData.EXPECTED_DEFAULT_SESSION_EVENT)
Expand Down
4 changes: 4 additions & 0 deletions ...ase-sessions/src/test/kotlin/com/google/firebase/sessions/testing/TestSessionEventData.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ internal object TestSessionEventData {
sessionIndex = 3,
)

const val TEST_SESSION_TIMESTAMP_US: Long = 12340000

val EXPECTED_DEFAULT_SESSION_EVENT =
SessionEvent(
eventType = EventType.SESSION_START,
Expand All @@ -37,6 +39,8 @@ internal object TestSessionEventData {
sessionId = "a1b2c3",
firstSessionId = "a1a1a1",
sessionIndex = 3,
firebaseInstallationId = "",
eventTimestampUs = TestSessionEventData.TEST_SESSION_TIMESTAMP_US,
),
applicationInfo =
ApplicationInfo(
Expand Down