Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

Investigate into null values in startup metrics #19147

Closed
rocketsroger opened this issue Apr 20, 2021 · 8 comments
Closed

Investigate into null values in startup metrics #19147

rocketsroger opened this issue Apr 20, 2021 · 8 comments
Assignees
Labels
E2 Estimation Point: easy, half a day to 2 days Feature:Telemetry needs:investigation
Milestone

Comments

@rocketsroger
Copy link
Contributor

rocketsroger commented Apr 20, 2021

Currently there are large number of startup metrics are reported as null. Investigate into how startup metrics are reported to understand the cause/solution of the issue.

Example query: https://sql.telemetry.mozilla.org/queries/79505/

┆Issue is synchronized with this Jira Task

@rocketsroger rocketsroger self-assigned this Apr 20, 2021
@github-actions github-actions bot added the needs:triage Issue needs triage label Apr 20, 2021
@amedyne amedyne removed the needs:triage Issue needs triage label Apr 20, 2021
@wlach
Copy link
Contributor

wlach commented Apr 20, 2021

@rocketsroger when you've diagnosed this issue and have a fix, could you also provide a list of affected metrics? This would be a handy thing to have in the commentary section in the glean dictionary so people looking at the data are aware of this issue / gotcha. See the commentary section here, for example: https://dictionary.protosaur.dev/apps/firefox_ios/metrics/app_opened_as_default_browser

@rocketsroger
Copy link
Contributor Author

Glean team understand the current limitation and has opened an issue for this as well. https://bugzilla.mozilla.org/show_bug.cgi?id=1706637

@rocketsroger rocketsroger added the E2 Estimation Point: easy, half a day to 2 days label Apr 22, 2021
@rocketsroger
Copy link
Contributor Author

As a workaround suggestion in https://bugzilla.mozilla.org/show_bug.cgi?id=1706637. I will move any startup metrics operations to right after Glean initialization. I will create another issue to track the follow up.

@rocketsroger
Copy link
Contributor Author

follow up issue created: #19251

rocketsroger added a commit to rocketsroger/fenix that referenced this issue Apr 26, 2021
@rocketsroger
Copy link
Contributor Author

While moving the implementation I've noticed that it is using incorrect types. Found a issue tracking this #12131.

@mcomella
Copy link
Contributor

mcomella commented May 5, 2021

I analyzed the perf impact of this and it's very large: 114ms or -8.07% in time to visual completeness on the Moto G5. This is a very large regression so I don't think we should keep this change and have recommendations of alternative implementations: #19395 (comment)

@gabrielluong gabrielluong added this to the 90 milestone May 5, 2021
@rocketsroger
Copy link
Contributor Author

I analyzed the perf impact of this and it's very large: 114ms or -8.07% in time to visual completeness on the Moto G5. This is a very large regression so I don't think we should keep this change and have recommendations of alternative implementations: #19395 (comment)

Agreed. I was expecting performance degradation but I didn't think it was this much. Since this is a workaround until Glean team's investigation. I will create a PR to move the metrics setup off the main thread. If moving metrics setup off the main thread causes the workaround to not work consistently then there's nothing we can do.

@rocketsroger rocketsroger reopened this May 5, 2021
@rocketsroger
Copy link
Contributor Author

Looking at the query we have about 20% null values still. This is probably the best we can do without affecting performance. Once Glean team's investigation is done I will use the follow up issue #19251 to fix the root cause. Closing this as done.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
E2 Estimation Point: easy, half a day to 2 days Feature:Telemetry needs:investigation
Projects
None yet
Development

No branches or pull requests

5 participants