Skip to content

Conversation

@bryanatkinson
Copy link
Contributor

… the FirelogPublisher handle the full SessionEvent creation since that's only relevant to Firelog.

@google-oss-bot
Copy link
Contributor

1 Warning
⚠️ Did you forget to add a changelog entry? (Add the 'no-changelog' label to the PR to silence this warning.)

Generated by 🚫 Danger

@github-actions
Copy link
Contributor

github-actions bot commented Oct 16, 2023

Unit Test Results

0 tests   0 ✔️  0s ⏱️
0 suites  0 💤
0 files    0

Results for commit 583cc09.

♻️ This comment has been updated with latest results.

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Oct 16, 2023

Size Report 1

Affected Products

  • base

    TypeBase (83d407e)Merge (94ac226)Diff
    apk (aggressive)?8.39 kB? (?)
    apk (release)?8.66 kB? (?)
  • firebase-installations

    TypeBase (83d407e)Merge (94ac226)Diff
    aar?58.8 kB? (?)
    apk (aggressive)?127 kB? (?)
    apk (release)?1.67 MB? (?)
  • firebase-installations-interop

    TypeBase (83d407e)Merge (94ac226)Diff
    aar?8.17 kB? (?)
    apk (aggressive)?65.2 kB? (?)
    apk (release)?652 kB? (?)
  • firebase-sessions

    TypeBase (83d407e)Merge (94ac226)Diff
    aar?148 kB? (?)
    apk (aggressive)?370 kB? (?)
    apk (release)?2.08 MB? (?)

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/wrahU0sCAr.html

* Determines if the foregrounding that occurred at the given time should trigger a new session
* because the app has been idle for too long.
*/
private fun isSessionRestart(foregroundTimeMs: Long) =
Copy link
Contributor

Choose a reason for hiding this comment

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

The old SessionInitiator used Durations not Longs for this check. The WallClock has a better clock for measuring durations that won't be affected by funny things like the user changing the time or anything with daylight savings etc. See my comment #5420 (comment)

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 getting the time out of the messages themselves where we don't actually know which clock was used to generate them other than that the framework did. Switching to use our own clock would mean that we'd be keeping track of timestamps based on when the service processes each message as opposed to when the event occurred. It's true that manipulating the system clock might cause a session to be dropped or created when it shouldn't have, but IMO that's a much less important or likely scenario than delays in message delivery.

… the FirelogPublisher handle the full SessionEvent creation since that's only relevant to Firelog.
@bryanatkinson bryanatkinson force-pushed the sessions-firelog-integrate branch from 9714062 to a0b5558 Compare October 16, 2023 19:25
@google-oss-bot
Copy link
Contributor

google-oss-bot commented Oct 16, 2023

Coverage Report 1

Affected Products

No changes between base commit (bcca211) and merge commit (d9ceb21).

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/y4yPOHZFS9.html

@bryanatkinson bryanatkinson merged commit 771ffb7 into sessions-nine Oct 17, 2023
@bryanatkinson bryanatkinson deleted the sessions-firelog-integrate branch October 17, 2023 14:21
@firebase firebase locked and limited conversation to collaborators Nov 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants