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

New daily pixel for history save failures #2533

Merged
merged 3 commits into from
Apr 9, 2024
Merged

Conversation

tomasstrba
Copy link
Contributor

Task/Issue URL: https://app.asana.com/0/0/1206920829163329/f

Description:
Adding new daily pixel for history save failures to better understand how many devices are affected by a potential issue

Steps to test this PR:

  1. Checkout the main branch, run the app and turn on the pixel logging: Debug -> Logging -> Pixel
  2. Checkout the tom/history-pixel branch
  3. Modify EncryptedHistoryStore.swift on line 237 by adding return .failure(HistoryStoreError.savingFailed) (right after beginning of the insertNewVisits(..) method)
  4. Run the app, navigate to a website and make sure the pixelm.mac.debug.history.save.failed.dailyis only fired once on the first history save attempt. Any subsequent save failures only trigger should only trigger m.mac.debug.history.save.failed

Internal references:

Pull Request Review Checklist
Software Engineering Expectations
Technical Design Template
Pull Request Documentation

@github-actions github-actions bot added the bot: not in app board Added by automation for pull requests with tasks not added to macOS App Board Asana project label Apr 2, 2024
@tomasstrba tomasstrba requested review from a team and THISISDINOSAUR and removed request for a team April 2, 2024 09:51
@tomasstrba tomasstrba removed the bot: not in app board Added by automation for pull requests with tasks not added to macOS App Board Asana project label Apr 2, 2024
@duckduckgo duckduckgo deleted a comment from github-actions bot Apr 2, 2024
Copy link
Contributor

@THISISDINOSAUR THISISDINOSAUR left a comment

Choose a reason for hiding this comment

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

So far I'm not seeing the pixel at all, is it included in the reset option in the debug menu?

@tomasstrba
Copy link
Contributor Author

Nope, I'll add it to the menu today and let you know when it is ready for another review.

@tomasstrba
Copy link
Contributor Author

@THISISDINOSAUR , I added the pixel and made sure it works as expected. It's ready for another review. If you don't see the m.mac.debug.history.save.failed pixel too, make sure history saving is returning the error correctly (the step 3 from testing steps)

Copy link
Contributor

@THISISDINOSAUR THISISDINOSAUR left a comment

Choose a reason for hiding this comment

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

LGTM!

@tomasstrba tomasstrba merged commit 0d7c9de into main Apr 9, 2024
17 checks passed
@tomasstrba tomasstrba deleted the tom/history-pixel branch April 9, 2024 13:31
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