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

Kotlin: Set required client_info fields first #1633

Merged
merged 1 commit into from
May 11, 2021
Merged

Conversation

badboy
Copy link
Member

@badboy badboy commented May 11, 2021

There's a chance that the app is killed at any point.
If that happens while setting core metrics, on next start a ping might
be sent, which is then missing data.

Some of this data is required; and if missing the pipeline rejects the
ping during validation. By moving these metrics first we increase the
chance for them to be set.

This is a very desperate attempt on mitigating missing data.


I've spent all day reading the code over and over again, starting Fenix, trying to capture any ping without the app_build data.

I've only found one tiny (forced) edge case:

When adding a delay (30s) right before setting app_build, then killing Fenix when it's waiting there, then starting it again, will trigger a ping that's missing the app build as expected.
Now in production we of course don't have that delay.
But there's theoretically a teeny tiny window in which this could still happen.
For the lack of better approaches I propose to land this, get it into a release and land it in Fenix to gather some nightly data.
By just rearranging the data collection we merely change what would be affected by early app kills/crashes.
If we see a drop of missing app_build data after this it would underline that theory.

@auto-assign auto-assign bot requested a review from Dexterp37 May 11, 2021 14:18
Copy link
Contributor

@Dexterp37 Dexterp37 left a comment

Choose a reason for hiding this comment

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

This is a very desperate attempt on mitigating missing data.

Desperate r+wc!

Can you add a changelog entry?

There's a chance that the app is killed at any point.
If that happens while setting core metrics, on next start a ping might
be sent, which is then missing data.

Some of this data is required; and if missing the pipeline rejects the
ping during validation. By moving these metrics first we increase the
chance for them to be set.

This is a very desperate attempt on mitigating missing data.
@badboy badboy force-pushed the set-appbuild-early branch from 55818bb to 82f9a0a Compare May 11, 2021 15:10
@badboy badboy merged commit 01a62a0 into main May 11, 2021
@badboy badboy deleted the set-appbuild-early branch May 11, 2021 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants