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.

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

Jump to bottom

Add a last sent date to heartbeat storage #6039

Merged
merged 5 commits into from
Mar 10, 2022
Merged

Add a last sent date to heartbeat storage #6039

merged 5 commits into from
Mar 10, 2022

Conversation

hsubox76
Copy link
Contributor

@hsubox76 hsubox76 commented Mar 1, 2022

Persist date of last sent heartbeat header, to prevent adding any more entries for that date. This is done by adding a lastSentHeartbeatDate to the heartbeat object stored in IndexedDB (and retained in app memory as this._heartbeatsCache).

@hsubox76 hsubox76 requested a review from allspain as a code owner March 1, 2022 00:09
@changeset-bot
Copy link

changeset-bot bot commented Mar 1, 2022 •
edited
Loading

🦋 Changeset detected

Latest commit: 303d844

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@firebase/app Patch
@firebase/app-compat Patch
firebase Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@hsubox76 hsubox76 requested a review from egilmorez as a code owner March 1, 2022 00:10
@google-oss-bot
Copy link
Contributor

google-oss-bot commented Mar 1, 2022 •
edited
Loading

Size Report 1

Affected Products

  • @firebase/app

    TypeBase (09e6243)Merge (0c6b56f)Diff
    browser13.9 kB13.8 kB-105 B (-0.8%)
    esm518.8 kB18.1 kB-708 B (-3.8%)
    main19.7 kB19.0 kB-761 B (-3.9%)
    module13.9 kB13.8 kB-105 B (-0.8%)

  • bundle

    43 size changes

    TypeBase (09e6243)Merge (0c6b56f)Diff
    analytics (logEvent)41.8 kB41.8 kB+47 B (+0.1%)
    app-check (CustomProvider)35.7 kB35.7 kB+45 B (+0.1%)
    app-check (ReCaptchaEnterpriseProvider)37.9 kB37.9 kB+45 B (+0.1%)
    app-check (ReCaptchaV3Provider)37.8 kB37.9 kB+45 B (+0.1%)
    auth (Anonymous)65.3 kB65.3 kB+45 B (+0.1%)
    auth (EmailAndPassword)69.3 kB69.4 kB+45 B (+0.1%)
    auth (GoogleFBTwitterGitHubPopup)89.1 kB89.2 kB+45 B (+0.1%)
    auth (GooglePopup)88.9 kB88.9 kB+45 B (+0.1%)
    auth (GoogleRedirect)89.1 kB89.1 kB+45 B (+0.1%)
    auth (Phone)75.3 kB75.4 kB+45 B (+0.1%)
    database (Append to a list of data)146 kB146 kB+46 B (+0.0%)
    database (Filtering data)144 kB144 kB+46 B (+0.0%)
    database (Listen for child events)160 kB161 kB+46 B (+0.0%)
    database (Listen for value events + Detach listeners)160 kB161 kB+46 B (+0.0%)
    database (Listen for value events)160 kB160 kB+46 B (+0.0%)
    database (Read data once)152 kB152 kB+46 B (+0.0%)
    database (Save data as transactions)162 kB162 kB+46 B (+0.0%)
    database (Sort data)146 kB146 kB+46 B (+0.0%)
    database (Write data)145 kB145 kB+46 B (+0.0%)
    firestore (Persistence)261 kB261 kB+46 B (+0.0%)
    firestore (Query Cursors)203 kB203 kB+47 B (+0.0%)
    firestore (Query)204 kB205 kB+47 B (+0.0%)
    firestore (Read data once)193 kB193 kB+47 B (+0.0%)
    firestore (Realtime updates)195 kB195 kB+47 B (+0.0%)
    firestore (Transaction)178 kB178 kB+47 B (+0.0%)
    firestore (Write data)177 kB177 kB+47 B (+0.0%)
    firestore-lite (Query Cursors)68.3 kB68.3 kB+45 B (+0.1%)
    firestore-lite (Query)71.3 kB71.4 kB+45 B (+0.1%)
    firestore-lite (Read data once)55.8 kB55.8 kB+45 B (+0.1%)
    firestore-lite (Transaction)73.1 kB73.2 kB+45 B (+0.1%)
    firestore-lite (Write data)58.6 kB58.7 kB+45 B (+0.1%)
    functions (call)29.6 kB29.6 kB+45 B (+0.2%)
    messaging (send + receive)44.9 kB44.9 kB+45 B (+0.1%)
    performance (trace)49.4 kB49.5 kB+45 B (+0.1%)
    remote-config (getAndFetch)44.1 kB44.2 kB+45 B (+0.1%)
    storage (getBytes)37.9 kB38.0 kB+45 B (+0.1%)
    storage (getDownloadURL)40.0 kB40.0 kB+45 B (+0.1%)
    storage (getMetadata)39.4 kB39.5 kB+45 B (+0.1%)
    storage (list + listAll)38.9 kB38.9 kB+45 B (+0.1%)
    storage (updateMetadata)39.7 kB39.8 kB+45 B (+0.1%)
    storage (uploadBytes)44.3 kB44.3 kB+45 B (+0.1%)
    storage (uploadBytesResumable)53.7 kB53.7 kB+45 B (+0.1%)
    storage (uploadString)44.5 kB44.5 kB+45 B (+0.1%)

  • firebase

    TypeBase (09e6243)Merge (0c6b56f)Diff
    firebase-app-compat.js28.2 kB28.3 kB+59 B (+0.2%)
    firebase-app.js84.2 kB84.3 kB+157 B (+0.2%)
    firebase-compat.js777 kB777 kB+33 B (+0.0%)
    firebase-performance-standalone-compat.es2017.js87.5 kB87.6 kB+112 B (+0.1%)
    firebase-performance-standalone-compat.js65.7 kB65.3 kB-365 B (-0.6%)

Test Logs

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

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Mar 1, 2022 •
edited
Loading

Size Analysis Report 1

This report is too large (154,095 characters) to be displayed here in a GitHub comment. Please use the below link to see the full report on Google Cloud Storage.

Test Logs

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

@egilmorez
Copy link
Contributor

iiuc, this has a lot of internally-viewable comments with "//" but no 3P-visible comments that would get parsed into reference docs. LMK if that's not true Christina, thanks!

allspain
allspain approved these changes Mar 8, 2022
View reviewed changes
packages/app/src/heartbeatService.ts
// If it's still null or the array is empty, there is no data to send.
if (
this._heartbeatsCache === null ||
this._heartbeatsCache.heartbeats.length === 0
Copy link

Choose a reason for hiding this comment

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

is it possible for this._heartbeatsCache.heartbeats to be undefined while this._heartbeatsCache is not null.

Might be worth an additional check on heartbeats

Copy link

Choose a reason for hiding this comment

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

Seeing below it's not super obvious how it's even possible

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Typescript should catch if there's ever an attempt to assign an object to this._heartbeatsCache that doesn't have a heartbeats property.

@allspain allspain assigned hsubox76 and unassigned allspain Mar 8, 2022
@hsubox76 hsubox76 merged commit 927c1af into master Mar 10, 2022
@hsubox76 hsubox76 deleted the ch-hb-once branch March 10, 2022 20:17
@google-oss-bot google-oss-bot mentioned this pull request Mar 16, 2022
Merged
@firebase firebase locked and limited conversation to collaborators Apr 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@allspain allspain allspain approved these changes

@egilmorez egilmorez Awaiting requested review from egilmorez

Assignees

@hsubox76 hsubox76

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@hsubox76 @google-oss-bot @egilmorez @allspain