Skip to content
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

Add check if Sessions is disabled #5061

Merged
merged 3 commits into from
Jun 20, 2023
Merged

Conversation

mrober
Copy link
Contributor

@mrober mrober commented Jun 7, 2023

No description provided.

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Jun 7, 2023

Coverage Report 1

Affected Products

  • firebase-sessions

    Overall coverage changed from ? (0da1dd2) to 71.21% (258c357) by ?.

    20 individual files with coverage change

    FilenameBase (0da1dd2)Merge (258c357)Diff
    ApplicationInfo.kt?100.00%?
    AutoSessionEventEncoder.java?100.00%?
    EventGDTLogger.kt?75.00%?
    FirebaseSessions.kt?0.00%?
    FirebaseSessionsDependencies.kt?91.30%?
    FirebaseSessionsRegistrar.kt?0.00%?
    LocalOverrideSettings.kt?88.46%?
    RemoteSettings.kt?86.25%?
    RemoteSettingsFetcher.kt?3.03%?
    SessionCoordinator.kt?75.00%?
    SessionEvent.kt?100.00%?
    SessionEvents.kt?96.97%?
    SessionGenerator.kt?91.67%?
    SessionInitiateListener.kt?0.00%?
    SessionInitiator.kt?74.19%?
    SessionsSettings.kt?90.00%?
    SessionSubscriber.kt?75.00%?
    SettingsCache.kt?94.12%?
    SettingsProvider.kt?0.00%?
    Time.kt?0.00%?

Test Logs

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

@github-actions
Copy link
Contributor

github-actions bot commented Jun 7, 2023

Unit Test Results

     79 files       79 suites   4m 33s ⏱️
1 022 tests 1 021 ✔️ 0 💤 1
1 084 runs  1 083 ✔️ 0 💤 1

For more details on these failures, see this check.

Results for commit b55f399.

♻️ This comment has been updated with latest results.

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Jun 7, 2023

Size Report 1

Affected Products

  • base

    TypeBase (0da1dd2)Merge (258c357)Diff
    apk (aggressive)?8.39 kB? (?)
    apk (release)?8.66 kB? (?)
  • firebase-sessions

    TypeBase (0da1dd2)Merge (258c357)Diff
    aar?114 kB? (?)
    apk (aggressive)?364 kB? (?)
    apk (release)?2.06 MB? (?)

Test Logs

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

@mrober mrober changed the title Do not send session id to subscribers if Sessions is disabled Add check if Sessions is disabled Jun 9, 2023
Copy link
Contributor

@samedson samedson left a comment

Choose a reason for hiding this comment

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

🚢

Copy link
Contributor

@visumickey visumickey left a comment

Choose a reason for hiding this comment

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

Do we have tests to validate this change? Basically I'm surprised that we did not have tests where we validate that we do not sent events when sessions SDK is disabled. We should add tests to ensure that this scenario is tested.

@mrober
Copy link
Contributor Author

mrober commented Jun 20, 2023

Do we have tests to validate this change? Basically I'm surprised that we did not have tests where we validate that we do not sent events when sessions SDK is disabled. We should add tests to ensure that this scenario is tested.

I am adding these tests in a new PR.

@mrober mrober merged commit 70d86f4 into firebase-sessions Jun 20, 2023
@mrober mrober deleted the sessions-disable branch June 20, 2023 13:57
davidmotson pushed a commit that referenced this pull request Jun 20, 2023
* Do not send session id to subscribers if Sessions is disabled

* Move the Sessions settings disabled check to after subscrubers
mrober added a commit that referenced this pull request Jun 26, 2023
* Initial commit for Firebase Sessions SDK (#4713)

* Initial commit for Firebase Sessions SDK

* Add copyright notice

* Remove redundant dependency

* Revert 'Remove redundant dependency' to avoid using api

* Generate api.txt file

* Implement SessionInitiator and hook it up to lifecycle events (#4723)

* Implement SessionInitiator and hook it up to lifecycle events

* Add a warning when failed to register lifecycle callbacks

* Fix formatting

* Use kotlin.time.Duration instead of Long for time intervals (#4733)

* Use kotlin.time.Duration instead of Long for time intervals

* Adjust test constants

* Simplify elapsedRealtime to always be millis precision

* Add SessionGenerator to generate the Session ID (#4747)

* Add SessionEvent data class (#4763)

* Add SessionEvent data class

* Add enum test case

* Remove serialization (this will be handled in encoders) and rename SessionState

* Generate a SessionEvent based on the SessionState

* Rename SessionState to SessionDetails

* Firebase Session Event

* Add Firebase Sessions Test App (#4775)

* Add Firebase Sessions Test App

* Remove boilerplate

* Add JSON encoder for SessionEvents (#4778)

* Use the private setter pattern on SessionGenerator.currentSession (#4785)

* Use the private setter pattern on SessionGenerator.currentSession

* Comments

* Integrate with FirebaseInstallations. (#4796)

* Populate firebaseInstallationId field in session (#4801)

* Package session start event's application information. (#4829)

* Fix workflows for Firebase Sessions (#4841)

* Use googleServices.gradle

* Move package level function to object to fix api surface check

* Add android manifest file with versionName set to fix android tests

* Update pinned deps

* Make getApplicationInfo look leaner

* Setup configuration layer for the library. (#4853)

* Fix Firebase Sessions test app temporarily (#4858)

* Add GDT to Firebase Sessions (#4851)

* Add README files (#4857)

* Fix test app and add README's

* Undo app changes

* Undo app changes

* Add description and formatting to README file

* Add google-services.json file to unit tests (#4868)

* Setup sessions e2e workflow (#4859)

* Start setting up sessions e2e workflow

* Remove schedule

* Format

* Setup gcloud

* Set FIREBASE_APP_CHECK_DEBUG_SECRET

* Set FTL_RESULTS_BUCKET

* Add schedule

* Add 20 min

* Implement local manifest override configs (#4877)

* Fix encoding for Session Events (#4870)

* Use test app for Sessions e2e test (#4881)

* Use test app for Sessions e2e test

* Add test lab plugin

* Use firebase-sessions branch

* Increase time to log session to 60 seconds

* Move currentTimeUs into WallClock (#4890)

* Populate sampling rate in session event (#4891)

* Populate sampling rate in session event

* Fix test

* Fix merge

* Add TimeProvider interface to abstract away the clock (#4905)

* Add TimeProvider interface to abstract away the clock

* Add comment about default elapsed time being zero

* Fill in device_model field for AQS (#4892)

* Implement sampling (#4896)

* Implement sampling

* return

* Remove collectEvents from SessionDetails, use sessionGenerator.collectEvents

* Don't generate whole Session Start event for sampled event

* Fix log message

* Add Sessions to benchmark config (#4914)

* Introduce remote config cache for storing configs fetched remotely. (#4917)

* Make logging consistent with Apple SDK (#4919)

* Pass a CoroutineContext, not CoroutineDispatcher, to SessionCoordinator (#4950)

* Implement remote config fetcher to fetch and cache configs. (#4967)

* Add os_version, app_build_version, device_manufacturer for Sessions #11222 (#4984)

* Add App information for config fetching. (#4992)

* Update the header params for network request. (#4995)

* Add thread safe FirebaseSessionsDependencies (#4983)

* Add thread safe FirebaseSessionsDependencies

* Remove debug line

* Format

* Address comments

* Rename to getRegisteredSubscribers

* Add register api to FirebaseSessions so products can integrate (#4989)

* Add thread safe FirebaseSessionsDependencies

* Remove debug line

* Format

* Add register api to FirebaseSessions so products can integrate

* Fix Android tests and api txt file

* Address comments

* Address comments

* Rename to getRegisteredSubscribers

* Fix callsite

* Add Sessions to Crashlytics (#4993)

* Add thread safe FirebaseSessionsDependencies

* Remove debug line

* Format

* Add register api to FirebaseSessions so products can integrate

* Fix Android tests and api txt file

* Add Sessions to Crashlytics

* Address comments

* Address comments

* Rename to getRegisteredSubscribers

* Fix callsite

* Add todo

* Fix gradle command in README

* Use firebase-encoders-processor instead of manually building encoder (#5030)

* Fix behaviour with multiple subscribers (#5031)

* Fix behaviour with multiple subscribers

* Fix format

* Integrate Firebase Performance with Firebase Sessions. (#5020)

* Clean up how we launch suspend functions (#5028)

* Make `initiateSessionStart` a suspend function

* Notify subscribers regardless of sampling and data collection state

* Add timestamp to SessionDetails

* Fix format

* Generate the session details on main thread

* Fix tests

* Call it sessionStartTimestampUs for clarity sake

* Add and populate appQualitySessionId in Crashlytics reports (#5045)

* Add and populate appQualitySessionId in Crashlytics reports

* Bump Crashlytics ktx target sdk to 33, same as Java sdk

* Log warning when missing AQS session id

* Pin firebase-datatransport version (#5070)

* Use AQS provided sessionId for Fireperf events. (#5060)

* Fix firebase-crashlytics-ktx instrumented tests (#5068)

* Fix firebase-crashlytics-ktx deps

* Fix CrashlyticsTests and clean up build file

* Make FirebaseCrashlyticsKtxRegistrar internal

* Bump health metrics compile sdk

* Add versionName version

* Add check if Sessions is disabled (#5061)

* Do not send session id to subscribers if Sessions is disabled

* Move the Sessions settings disabled check to after subscrubers

* Add fid to Crashlytics report (#5052)

* Add fid to Crashlytics report

* Format

* Handle IOExceptions when fetching remote settings (#5092)

* Handle IOExceptions went fetching remote settings

* Add failure test case for RemoteSettingsFetcher

* Make remote settings fetch non-blocking (#5095)

* Ensure the settings cache has loaded from disk before reading any values (#5103)

* Make remote settings fetch non-blocking

* Ensure the settings cache has loaded from disk before reading any values

* Fix runtime exception with a load-bearing empty block

* Inline the definition of metadata to avoid needing an empty block

* Comments

* Fetch remote settings on session, not on launch (#5107)

* Make remote settings fetch non-blocking

* Ensure the settings cache has loaded from disk before reading any values

* Fix runtime exception with a load-bearing empty block

* Inline the definition of metadata to avoid needing an empty block

* Fetch remote settings on session, not on launch

* Log warning when we fail to fetch the fid

---------

Co-authored-by: Sam Edson <samedson@google.com>
Co-authored-by: Visu <visumickey@gmail.com>
@firebase firebase locked and limited conversation to collaborators Jul 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants