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

Remove P3A protobuf reporting #23902

Closed
GeetaSarvadnya opened this issue Jul 6, 2022 · 11 comments
Closed

Remove P3A protobuf reporting #23902

GeetaSarvadnya opened this issue Jul 6, 2022 · 11 comments

Comments

@GeetaSarvadnya
Copy link

Description

Reverting P3A protobuf submission is done via the PR brave/brave-core#14015 and which resolves the issue #23754 and the same is verified in 1.41.x #23754 (comment) and ensured P3A and P2A work as it works in 1.40.x

The actual fix for Remove P3A protobuf reporting is merged in 1.42.x and 1.43.x via the PR brave/brave-core#14017 hence created this issue to address Remove P3A protobuf reporting in 1.42.x

Test Plan:

Run browser under a network proxy for traffic inspection

  • With a fresh profile, wait 10-15 minutes for P3A reports to be sent.
  • Confirm messages all go the p3a-json.brave.com and are formatted as readable json. There should be no traffic to p3a.brave.com.
  • P2A messages are unaffected and may go to p2a.brave.com, p2a-json.brave.com or both, depending on the version. This change is only about Brave.P3A.* reports.

Miscellaneous Information:

Discussed with @kjozwiak and created the issue.

cc: @brave/legacy_qa @rillian @mattmcalister @rebron

@GeetaSarvadnya
Copy link
Author

@rillian I need clarification on p2a metrics. p2a metrics are recorded only when rewards is enabled and view an ad. It's behaving the same in both 1.41 and 1.42. But earlier it was not like that, we used to get the p2a metrics after 15 or 20 mins of browser load (p2a metrics records verified in 1.36 #15967) Is this a recent change?

@GeetaSarvadnya
Copy link
Author

GeetaSarvadnya commented Jul 13, 2022

Verification PASSED on

Brave | 1.42.64 Chromium: 103.0.5060.114 (Official Build) beta (64-bit)
-- | --
Revision | a1c2360c5b02a6d4d6ab33796ad8a268a6128226-refs/branch-heads/5060@{#1124}
OS | Windows 10 Version 21H2 (Build 19044.1806)

  • Confirmed traffic is not sent to the old endpoint p3a.brave.com
  • Confirmed that all the P3A metrics are sent to the json format to p3a-json.brave.com endpoint
  • Confirmed brave://local-state file has 47 records for p3a and the same is shown in p3a-json.brave.com endpoint
  • Confirmed P2A metrics data sent correctly in both old p2a.brave.com and new json p2a.json.brave.com formats
  • Confirmed brave://local-state file has 6 records for p2a and the same is shown in both p2a-json.brave.com and p2a.brave.com endpoints

P3A

Example Example Example
image (2) image (1) image

P2A

P2A Json format P2A encoded binary format
image image (1)

@rillian
Copy link

rillian commented Jul 14, 2022

I need clarification on p2a metrics. p2a metrics are recorded only when rewards is enabled and view an ad. It's behaving the same in both 1.41 and 1.42. But earlier it was not like that, we used to get the p2a metrics after 15 or 20 mins of browser load (p2a metrics records verified in 1.36 #15967) Is this a recent change?

@GeetaSarvadnya are you saying in 1.36 you see P2A metrics in the first ~10 minutes without having to enable rewards and browse long enough to trigger an ad?

I'm not aware of anything changing the in P2A scheduling, but the ad code which reports opportunities and impressions has changed, particularly with Brave News, so maybe that's the origin of the change?

@tmancey you refactored the p2a reporting recently. Is this an expected change between 1.36 and 1.41?

@tmancey
Copy link
Contributor

tmancey commented Jul 14, 2022

@rillian p2a metrics have not changed, just moved in the code. However, users can join Brave News ads without joining rewards and P2A are sent for ad impressions for those ads. So it could be that Brave News has changed and the ads are viewed much sooner.

@GeetaSarvadnya
Copy link
Author

GeetaSarvadnya commented Jul 14, 2022

are you saying in 1.36 you see P2A metrics in the first ~10 minutes without having to enable rewards and browse long enough to trigger an ad? -

@rillian Yes

@tmancey Right now P2A metrics are shown only when rewards is enabled and the ad is viewed. I have not checked p2a records after enabling brave news, let me check that as well

@rillian
Copy link

rillian commented Jul 14, 2022

At least it sounds like an improvement instead of a regression.

@GeetaSarvadnya
Copy link
Author

@tmancey Enabled brave news and viewed brave news cards in 1.42.65, P2A records are not shown under brave://local-state file and network monitoring tool (Charles). Currently, p2a records are shown only when rewards is enabled and the AD is viewed.

cc: @rillian

@tmancey
Copy link
Contributor

tmancey commented Jul 15, 2022

@GeetaSarvadnya I do not believe this is a regression.

@GeetaSarvadnya
Copy link
Author

GeetaSarvadnya commented Jul 15, 2022

@tmancey Pretty sure when we verified the P2A records in #15967 in 1.36.x, we haven't enabled rewards.

@tmancey
Copy link
Contributor

tmancey commented Jul 15, 2022

@GeetaSarvadnya nothing has changed with regards to p2a in ads lib for quite a while. More than happy to jump on screen-share to go over your results. Thanks

@srirambv
Copy link
Contributor

srirambv commented Aug 2, 2022

Verification passed on Oppo Reno 5 with Android 12 running 1.42.85 x64 build

  • Verified traffic is not sent to old endpoint p3a.brave.com
  • Verified that all the P3A metrics are sent in JSON format to p3a-json.brave.com
  • Verified around 17 metric that are sent for P3A is available in brave://local-state
  • Verified no P2A metrics are sent (known issue)
image image

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

No branches or pull requests

5 participants