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 region support for Ads as CDN now controls catalog for each region #3752

Closed
tmancey opened this issue Mar 16, 2019 · 4 comments · Fixed by brave/brave-core#1983
Closed

Comments

@tmancey
Copy link
Contributor

tmancey commented Mar 16, 2019

Remove region support for Ads including --brave-ads-locale command-line argument as CDN now controls catalog for each region

@tmancey tmancey self-assigned this Mar 16, 2019
@tmancey tmancey changed the title Remove region locale support for Ads as CDN now controls catalog for each region Remove region support for Ads as CDN now controls catalog for each region Mar 16, 2019
@tmancey tmancey added the QA/Yes label Mar 16, 2019
@tmancey tmancey added this to the 0.64.x - Nightly milestone Mar 16, 2019
@tmancey tmancey modified the milestones: 0.64.x - Dev, 0.65.x - Nightly Apr 1, 2019
tmancey added a commit to brave/brave-core that referenced this issue Apr 2, 2019
tmancey added a commit to brave/brave-core that referenced this issue Apr 2, 2019
tmancey added a commit to brave/brave-core that referenced this issue Apr 2, 2019
@tmancey tmancey added the priority/P2 A bad problem. We might uplift this to the next planned release. label Apr 2, 2019
@GeetaSarvadnya
Copy link

GeetaSarvadnya commented Apr 10, 2019

Verification passed on

Brave 0.63.34 Chromium: 74.0.3729.40 (Official Build) beta (64-bit)
Revision 278d6a0d856d8314f36fb9a68b6e1d85cd0d14e7-refs/branch-heads/3729@{#470}
OS Windows 10 OS Build 17134.523
`Region - US`
   "transaction_history": {
    "transactions": [
      {
        "confirmation_type": "view",
        "estimated_redemption_value": 0.05,
        "timestamp_in_seconds": "13199427604"
      },
      {
        "confirmation_type": "click",
        "estimated_redemption_value": 0,
        "timestamp_in_seconds": "13199427608"
      },
      {
        "confirmation_type": "view",
        "estimated_redemption_value": 0.05,
        "timestamp_in_seconds": "13199428624"
      },
      {
        "confirmation_type": "click",
        "estimated_redemption_value": 0,
        "timestamp_in_seconds": "13199428631"
      },
      {
        "confirmation_type": "view",
        "estimated_redemption_value": 0.05,
        "timestamp_in_seconds": "13199430367"
      },
      {
        "confirmation_type": "view",
        "estimated_redemption_value": 0.05,
        "timestamp_in_seconds": "13199431196"
      }
    ]
  },
`Region - India`

Verification PASSED on macOS 10.14.4 x64 using the following build:

Brave 0.63.44 Chromium: 74.0.3729.75 (Official Build) beta(64-bit)
Revision fdb7915642fef8cf997beac2554709d148e3c187-refs/branch-heads/3729@{#754}
OS Mac OS X
  • Connected to the following locations and ensured I didn't receive any ads
    • Brussels, Belgium, Tel Aviv, Israel, Saint Petersburg, Russian
  • Checked Default/ads_service/catalog.json and ensure there was no region information
[16788:775:0417/155656.119589:INFO:ads_impl.cc(685)] Notification not made: No ads found in "arts & entertainment-comics" category, trying again with "arts & entertainment" category
[16788:775:0417/155656.120426:INFO:ads_impl.cc(699)] Notification not made: No ads found in "arts & entertainment" category

[16788:775:0417/160005.248862:INFO:ads_impl.cc(685)] Notification not made: No ads found in "personal finance-investing" category, trying again with "personal finance" category
[16788:775:0417/160005.249666:INFO:ads_impl.cc(699)] Notification not made: No ads found in "personal finance" category
  • ensured that I was receiving ads in the following regions
    • "geoTargets":[{"code":"FR","name":"France"}]
    • "geoTargets":[{"code":"DE","name":"Germany"}]
    • "geoTargets":[{"code":"CA","name":"Canada"}]

Verificatoin passed on

Brave 0.63.45 Chromium: 74.0.3729.75 (Official Build) beta(64-bit)
Revision fdb7915642fef8cf997beac2554709d148e3c187-refs/branch-heads/3729@{#754}
OS Linux
  • Verified following console log shown for unsupported region (Russia/India/Singapore)
[1:1:0418/191835.859662:INFO:ads_impl.cc(208)] Browser state changed to idle
context mismatch in svga_sampler_view_destroy
[1:1:0418/191848.877062:INFO:ads_impl.cc(215)] Browser state changed to unidle
[1:1:0418/191848.880675:INFO:ads_impl.cc(647)] Notification for category technology & computing-software
[1:1:0418/191848.881231:INFO:client.cc(267)] Successfully saved client state
[1:1:0418/191848.882073:INFO:ads_impl.cc(685)] Notification not made: No ads found in "technology & computing-software" category, trying again with "technology & computing" category
[1:1:0418/191848.882976:INFO:ads_impl.cc(699)] Notification not made: No ads found in "technology & computing" category
  • Verified ads shown for supported regions (US/CA/FR)

@kjozwiak
Copy link
Member

@tmancey would it be possible to basically add something into the logging that specifies that the CDN check has failed? Right now, QA is basically reliant on the UI via #3967. However, #3967 is currently broken so there's no way for QA to check if ads are not appearing due to being in an incorrect region or something else might have regressed. If the UI breaks in future builds, QA won't be able to double check if the CDN check actually failed etc.. Having something within logging would be super helpful and useful. Thoughts?

@tmancey
Copy link
Contributor Author

tmancey commented Apr 17, 2019

@tmancey would it be possible to basically add something into the logging that specifies that the CDN check has failed? Right now, QA is basically reliant on the UI via #3967. However, #3967 is currently broken so there's no way for QA to check if ads are not appearing due to being in an incorrect region or something else might have regressed. If the UI breaks in future builds, QA won't be able to double check if the CDN check actually failed etc.. Having something within logging would be super helpful and useful. Thoughts?

@kjozwiak Sure if you could raise a ticket and we can triage with Jimmy as once we generate the bundle from the catalog we can log information such as the amount of campaigns. Also we can log the IsRegionSupported call to show what region was detected.

@kjozwiak
Copy link
Member

Thanks @tmancey 👍 For now, we can just take a look at the catalog.json and if it's empty, it's probably a good indicator that you're in an unsupported region.

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