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

Cleanup P3A clent code / get rid of PROCHLO ? #14408

Closed
iefremov opened this issue Mar 1, 2021 · 8 comments · Fixed by brave/brave-core#10853
Closed

Cleanup P3A clent code / get rid of PROCHLO ? #14408

iefremov opened this issue Mar 1, 2021 · 8 comments · Fixed by brave/brave-core#10853

Comments

@iefremov
Copy link
Contributor

iefremov commented Mar 1, 2021

Initially, the P3A system was designed to utilise PROCHLO for additional privacy, however PROCHLO backend seems abandoned and P3A flies as is. We should consider removing PROCHLO bits from the client side to drop unused legacy and craft better pings (e.g. drop ascii string -> protobuf -> base64 conversion).

@iefremov iefremov added dev-concern dev-experience OS/Android Fixes related to Android browser functionality features/P3A OS/Desktop labels Mar 1, 2021
@iefremov iefremov self-assigned this Mar 1, 2021
@iefremov
Copy link
Contributor Author

iefremov commented Mar 1, 2021

cc @too4words @keur @porteron

@iefremov
Copy link
Contributor Author

looking forward for resurrecting PROCHLO, reopen if PROCHLO is not in production in 2022 :)

@stephendonner
Copy link

@iefremov / @rillian: can you help QA by providing a test plan we can run & maybe key off of? Thanks!

@rillian
Copy link

rillian commented Nov 6, 2021

My understanding is that this is just changing internal code paths for P3A message generation, so observable behaviour should be the same, or at least similar. Helpful QA might be:

  • Verify (e.g. with a network monitor) that the same P3A message contents are sent for a particular profile. This need not be exhaustive. Spot checks and message counts without changing any preferences will validate the code changes.
  • Verify that metadata tags are still stripped appropriately: On linux the browser should not report a referral code, a browser installed in a minority locale (Singapore? Nigeria? New Zealand?) should not report a region, and so on.

@LaurenWags
Copy link
Member

Labelling QA/Blocked until discussion in https://bravesoftware.slack.com/archives/CDNJ9SVUL/p1637170701059200 is resolved

@rillian
Copy link

rillian commented Nov 18, 2021

Unblocking and setting QA/No based on discussions. It would be helpful to have some spot testing of what the browser is sending, but data coming into the backend looks similar, so I think we can defer QA until #15967 is complete.

@LaurenWags
Copy link
Member

Sounds good @rillian - will proceed with this unless we hear differently from @iefremov 👍🏻

@GeetaSarvadnya
Copy link

Performed a spot check on the client-side and ensured things are working as expected and nothing is regressed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants