site-kit-wp: Prevent user quota errors and timeouts for users with many Analytics accounts

Bug Description

During the Analytics setup flow and in Analytics settings, Site Kit tries to find the user’s correct Analytics property to choose for the current site, based on matching the site URL. This works correctly overall, however it can cause problems if a user has many Analytics accounts in their Google account: For every account, an individual request is being made, which e.g. with 20 accounts results in 20 GET:properties-profiles requests. This can result in rate limits being exceeded, and theoretically at some point it could also result in server timeouts.

This lookup should be made more scalable. Some ideas:

  • If one of the requests fails with an error, cancel the process and simply return as if there had not been a match found.
  • Limit the number of accounts that the logic runs for to some threshold, e.g. 10. If it’s more, just don’t run the matching logic (again return as if there had not been a match found).
  • Combine all requests into a single batch request (would not help with quota errors, but with potential timeouts).

Let’s discuss which of these ideas make the most sense to implement in order to improve this functionality’s reliability.

Related: #341 (but it’s not worth it to make client-side adjustments for this, like multiple requests etc.)


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

  • When iterating over accounts to match a property in the Analytics GET:accounts-properties-profiles datapoint, errors should be handled gracefully: An error in that logic should not cause the request to fail, it should only result in no property being matched. If possible, the properties and profiles for the first account should be returned in the response then.
    1. Before running the loop, we should query properties-profiles for the first account in the array of accounts.
    2. If that succeeds and it includes matchedProperty, it should be returned. Otherwise, we should store it in a variable, to use as a fallback if no matchedProperty is found. If an error, we should simply return without any properties and profiles.
    3. Then we should iterate through the remaining accounts and query properties-profiles. Doing this in reverse order (as happening now) is no longer necessary since we’ll always get properties for the first account first.
    4. As soon as one of the properties-profiles requests from that loop fails, no more such requests should be made, and that error should be ignored. If so, the properties and profiles for the first account (as received prior) should be returned.
  • The number of accounts to iterate over should be limited to 25. In other words, we should limit searching for a matchedProperty to only the first 25 accounts to avoid excessive requests. Users that have that many accounts are likely power users anyway.

Implementation Brief

Update the Analytics::parse_data_response function in the includes/Modules/Analytics.php file for the GET:accounts-properties-profiles case:

  1. Before the foreach ( array_reverse( $accounts ) as $account ) { loop add call to get the property for the first account id in the list and assign it to a variable (ie. $first_account) we can use later using $this->get_data('properties-profiles':...
  2. If the $first_account variable is an error return the accounts array without any properties and profiles.
  3. If the $first_account variable is not an error and result has matchedProperty attribute return the $first_account.
  4. Limit the foreach loop to the next 25 accounts after the first account selected above by using array_slice($accounts, 1, 25); - make sure to remove the first element from the array (using the offset in the array_slice as ie. or using array_shift) so that the first profile is not fetched twice.
  5. Remove the reverse function from the loop on line 1225 as it’s no longer required.
  6. Add a new condition within the loop to catch a returned WP_Error and immediately return the $first_account. Finally remove the redundant ! is_wp_error check.

Test Coverage

  • N/A

Visual Regression Changes

  • N/A

QA Brief

  • Check the Analytics module to make sure that it doesn’t send excessive requests to GET:properties-profiles when it handles the GET:accounts-properties-profiles endpoint:
    • It should stop as soon as it finds the first property with a non-empty matchedProperty key;
    • It should fallback to the first response if no properties have the aforementioned key;
    • It should check the first 25 accounts only;
    • It should return empty properties if at least one response failed with an error.

Changelog entry

  • Improve Analytics property matching logic so that users with many Analytics accounts do not run into user quota errors.

About this issue

  • Original URL
  • State: closed
  • Created 4 years ago
  • Comments: 18 (2 by maintainers)

Most upvoted comments

Yes, this one is tough to test. I have added the QA brief and QA: Eng tag because I think QA engineers won’t be able to verify it anyway.

This just needs a QA brief and then can be moved to Merge Review 👍🏻

Thanks @felixarntz, I’ve updated the IB based on your comments and removed blocks of example code in favour of in line suggestions. Finally, I’ve added and estimate.