Add Site Health check for source code/tag status

[mxbclang]

Feature Description

The support team regularly gets topics from users who are confused about why a particular module in Site Kit isn't working. One of the first things that they often ask is if Site Kit is properly "connected," e.g. if the necessary code snippet was properly added. As a result, the support team spends a lot of time verifying source code placement on each site and letting the user know that the code was properly placed (it almost always is) and that the issue must be elsewhere.

We do have a guide about what code Site Kit places and where, but many of our users have difficulty using the inspector to verify that the code has been added to their site.

Per our discussion in the August 15 meeting, it would be helpful to have a Site Health check that confirms that code snippets have been properly added and placed for the Site Kit modules that are connected on a site. Ideally, this would appear in both the Status and Info screens so that this information is an easy check for users in Status but is also included in their Site Health Info, as many users share this information with support for troubleshooting purposes.

cc @jamesozzie @adamdunnage @marrrmarrr @felixarntz

---

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

Acceptance criteria

• Site Health information should display whether a tag is placed for a connected module that supports tags.
• The tag placement should appear in Site Health in WordPress. It should always reflect the current state of the site, with no caching of the frontend request.
• The request should check for any compatible tag for the relevant Site Kit module, eg. use the tag-matchers.js file for the relevant module.
• The request should also check for the HTML comments inserted by Site Kit when Site Kit places a tag. (See: https://github.com/google/site-kit-wp/blob/5f0b0643992d4ac5a172c4a129bb91811d0fa70b/includes/Modules/Analytics_4/Web_Tag.php#L167-L168 for an example of the HTML tags output by Site Kit).
• The request should always be made unauthenticated, to ensure the tag appears as it would to a non-authenticated user.
• If no tag is detected for a module, we should output: "No tag detected."
  • Optional: If the "Site Kit should place snippet/tag" setting is true for that module, we should surface a warning in the Site Health info tab. (If this is a lot of effort/better left to a separate issue, please flag in the IB/Execution and we can create a follow-up)
• If a tag is detected but we could not detect the "placed by Site Kit" HTML, we should output: "Tag detected but could not verify that Site Kit placed the tag."
• If a tag is detected with the Site Kit HTML comment, we should output: "Tag detected and placed by Site Kit."
• If a module is not connected, it should not display the tag status.

For example: if Analytics and Tag Manager are connected, then Site Health should display the status of whether the Analytics and Tag Manager tag is placed on the frontend.

Implementation Brief

• Update includes/Core/Util/Debug_Data.php:
  • In register method:
  • Hook into googlesitekit_rest_routes filter, to register a new GET REST route, say core/site/data/tags-placement-test. You can use this example as starting point
  • Hook into site_status_tests filter to add tests. You can follow the example here. Include a callback method, say tag_tests.
  • Add tag_tests method:
    • Use the passed $tests variable, to include custom Site Kit test to the array. Check for WordPress version, if it is higher than 5.6, put it into async tests group instead of direct, to leverage asynchronous handling without adding new javascript, as WordPress core handles the response and updates the UI. But if WP version is less than 5.6, than keep it as direct, like in the example.
  • Add tags_placement_test method:
    • If it is WP version bellow 5.6, return the description This feature requires WordPress version 5.6 or higher
    • Otherwise fetch the homepage, you can see an example here
    • Add new variable $active_modules, and assign it a value of active modules, e.g $this->modules->get_active_modules(), and filter only for AdSense, Tag Manager, and Analytics modules
    • You can add new variable that would dynamically store the string output, based on the match results. It can be formatted like $description .= '<li>{MODULE_NAME}: Tag detected and placed by Site Kit.</li>', etc based on the messaging provided in AC.
    • If there is no active module (from the mentioned 3 above), then $description can output Tag status not available: AdSense, Tag Manager, and Analytics modules are not connected.
    • Loop through $active_modules and using the module slug, search through fetched response body of the homepage, for Google $module_name snippet added by Site Kit. Note that modules slug needs to be modified for module name, as Tag Manager slug is tagmanager, and snippet comment will be Google Tag Manager snippet added by Site Kit
    • If match is found, you can join the text for the module to the $description
    • If match is not found, search for the module specific tags, you can adapt the js tag-matchers regex to a php regex, example, for the modules (AdSense, Tag Manager and Analytics).
      • If tag is found, then add text defined in AC (when no Site Kit comment is found, but there is a tag match) to the $description.
    • If neither Site Kit inline comment, or tag are found, add text defined in AC to the $description.
    • Format the test result data
    • For label under badge use Site Kit
    • For color under badge use blue
    • For status, use recommended, so that tab always stays visible, and does not go under Passed Tests dropdown, so users can always spot it right away.
    • For description, use the final $description variable output
    • You can check this doc for more info on parameters, and for more info, this is the core method for getting the tests array. And this core class for one of the REST endpoint callbacks (pointed out above), as well as update announcement where site health switched to REST callback instead of AJAX callback

Test Coverage

• You can introduce new tests for Google\Site_Kit\Tests\Core\Util\Debug_DataTest that would check for new changes

QA Brief

• Setup Site Kit
• Visit Tools -> Site Health. Under the first Status tab, within 5-10sec, a new accordion will Appear on the list which will show the status messages as described in AC. Wihtout any of the three modules active - Analytics, AdSense and Tag Manager, or any of them being active, etc. You can play with options, for example in Analytics module settings, disable option to place tag automatically, and check the health status, it should report that no tag is detected, etc

Changelog entry

• Add information about code/tag placement statuses to Site Health information.

[mxbclang]

@jamesozzie This is an issue that I filed that last time that we discussed this issue back in August. When I mentioned this on our call earlier this week, you said that we already have something in the plugin like this. Can you please take a look at this issue and let me know if this is something that we already have, and if so, if it should be tweaked in any way to make this easier for us/users? Thanks!

[jamesozzie]

@bethanylang We have the snippet placement status printed to a users Site Health information, within the Site Kit section. We also have it output on the AdSense settings page. Screenshots below of each.

One tweak I think would be useful would be to display the snippet status when viewing the connected services, and not just display the status when editing the AdSense settings. There is a GitHub issue for this (https://github.com/google/site-kit-wp/issues/1292). An enhancement to this would also be to display not just "Snippet inserted via Site Kit" but a "Standard" or "Modified" suffix, to help users to determine a third party modification (https://github.com/google/site-kit-wp/issues/2817). I say this as consent, optimization and other plugins can impact the standard snippet.

Site Health information:

AdSense settings page:

[jamesozzie]

@bethanylang I've proposed some UX improvements over on another issue (https://github.com/google/site-kit-wp/issues/5693#issuecomment-1295162491) which I feel would would address user confusion for when their site is not yet approved on the AdSense side.

When it comes to further improvements I think that displaying the snippet status next to the connected status (as per https://github.com/google/site-kit-wp/issues/1292) would cover this particular issue. Should we revisit that one? Alternatively we can add this for internal discussion next week?

[tofumatt]

ACs here are good, moving to IB 👍🏻

[tofumatt]

search through fetched response body for placed tags using module slug

Do you mean just search through the HTML for adsense? I'm not sure what this means exactly—just searching for the module slug is pretty broad and would lead to false positives. For instance, we have tag matcher code here: https://github.com/google/site-kit-wp/blob/5f0b0643992d4ac5a172c4a129bb91811d0fa70b/assets/js/modules/tagmanager/util/tag-matchers.js#L2, we might want to be more specific to ensure the code is output.

tag is found push it to the $tags array, using module slug as a key, and true for value

This isn't in the ACs, but it could be nice to distinguish between a tag output by Site Kit (which would have HTML tags like this: https://github.com/google/site-kit-wp/blob/5f0b0643992d4ac5a172c4a129bb91811d0fa70b/assets/js/modules/tagmanager/datastore/__factories__/html-with-tag.js#L30-L36) and just any Tag Manager/other tag on the page. Maybe we could differentiate with:

• No tag found (nothing output)
• Google $module_name snippet found on website
• Google $module_name snippet added by Site Kit

Of course, for some users with HTML optimisation plugins, our HTML comments might be stripped. 🤔

Hook into admin_init, and check if parse_homepage_tags_transient exists, and if it doesn't invoke the parse_homepage_tags method

This would trigger the homepage HTTP request when the user accesses any Admin page. Given the slowdown this would cause I think that's excessive; it'd be better to add a hook that triggers this behaviour when loading the Site Health page or similar.

I'd also caution against saving this debug information as a 24 hour transient, because it means if the user changes/removes the tag on the frontend somehow, the debug information will be out-of-date for 24 hours. That would be a highly confusing debugging experience; we'd be showing the user very technical information on a very technical page that was out of date. We're better off either:

• making the request via PHP every time, regardless of the slowdown, but only on the Site Health page
• making the request in the background after loading Site Health and triggering an update of the page after the request is complete

[zutigrm]

@tofumatt Thanks for the feedback.

Do you mean just search through the HTML for adsense? I'm not sure what this means exactly—just searching for the module slug is pretty broad and would lead to false positives

It is meant to be used as $module_name in the string that will be searched for, mentioned in the IB: Google $module_name snippet added by Site Kit. This is to ensure we only look for output made by Site Kit. The AdSense and Tag Manager are mentioned in the context of their module slugs needing modified assignment to $module_name variable that will be part of the searched inline comment.

For instance, we have tag matcher code here:

• Tag matching patterns. we might want to be more specific to ensure the code is output

Thanks for pointing this out to me, this can be helpful. But detecting just that snippet is present as you also mentioned it, can't tell us if it was done from Site Kit, by user, or any other plugin. That's why I suggested searching for comment (see above) in the IB. Although comments can be stripped by optimisation plugins, this is still the safest way to confirm if snippet is added by Site Kit.

Hook into admin_init, and check if parse_homepage_tags_transient exists, and if it doesn't invoke the parse_homepage_tags method

This would trigger the homepage HTTP request when the user accesses any Admin page. Given the slowdown this would cause I think that's excessive; it'd be better to add a hook that triggers this behaviour when loading the Site Health page or similar.

I'd also caution against saving this debug information as a 24 hour transient, because it means if the user changes/removes the tag on the frontend somehow, the debug information will be out-of-date for 24 hours. That would be a highly confusing debugging experience; we'd be showing the user very technical information on a very technical page that was out of date. We're better off either:

• making the request via PHP every time, regardless of the slowdown, but only on the Site Health page
• making the request in the background after loading Site Health and triggering an update of the page after the request is complete

admin_init is the earliest hook for site health also, I will update IB to include additional check if we are on Site Health page to make a request. But I would still suggest using transient vs making request every time. User can change snippet usage, by activating/deactivating the module that inserts the snippet, but that's why I included that transient should be deleted on activation and deactivation method in the modules. I can also include to reset this transient when useSnippet setting change?

Let me know what you think?

[tofumatt]

Sounds good regarding searching for the Site Kit HTML snippet. Let's make sure we include the HTML comments as well, but that sounds good. 👍🏻

---

User can change snippet usage, by activating/deactivating the module that inserts the snippet

That's true, but they can also change a lot of other things, like a plugin's optimisation to strip HTML comments, that would show/hide the Site Kit comments in HTML output. Also: if they weren't outputting the snippet via Site Kit, went to the admin to set the transient, and then set Site Kit to output the snippet, with this approach, Site Health would still claim that the snippet is not output.

If you want to go the transient route, it would be good to at least offer the user two things:

1. when the site was last checked
2. a button to force a refresh of the tag

---

Given this feature is for a subset of Site Kit users who need to see its status in Site Health, we shouldn't be slowing down the entire WordPress admin by a minimum of a few hundred ms (though I'd guess it might take a second or two to make the request in total). Site Kit sometimes gets mischaracterised as slowing down user's sites, which we really don't do, but adding this kind of request on every admin page for something a subset of users will use, and running it once a day, would actually give the idea that we slow down users' sites some weight 😆. So let's not do that.

I don't like the transient because it thoroughly decreases the accuracy and thus usefulness of this feature. The whole point is that this should save time and clarify whether Site Kit is successfully outputting tags, in part for Support. If we add the feature but make it laggy, that might just confuse the support flow further because it might say that Site Kit it outputting the tag, but then a user changes a setting in an already activated plugin, and then the output is stripped. But the Site Health would still claim it's there.

I think the info needs to be refreshed every time. I'd be fine if it were done asynchronously, but I think that's the only way this feature meets its intended use case.

[tofumatt]

@zutigrm I've updated the ACs here to be more explicit about the scenarios we expect and how this should behave. I've left @techanvil's link to https://make.wordpress.org/core/2019/04/25/site-health-check-in-5-2/ as well, in case it helps implementing the async status checks/info.

Should be ready for an updated IB 🙂

@aaemnnosttv noted that this is somewhat related to #7962, so just referencing it here 🙂

[zutigrm]

@tofumatt Thank you!

I updated the IB with new approach

[tofumatt]

@zutigrm This looks good, if I'm understanding the async approach! 🙂 Just one question: how often will the data be refreshed. Is it every request, in the background? If so, that seems good. But I'm not very familiar with the async approach so I wasn't sure (is there any WordPress documentation you can point to beyond the blog post?).

Then we should be good to move to IB, this looks quite straightforward 👍🏻

[tofumatt]

Oh, oops, I mixed up our PHP requirements and WordPress requirements: these APIs are only available in WordPress 5.6+, but our minimum required WordPress version is currently 5.2.

What will happen if a user is using WP 5.2-5.5? Is there a straightforward fallback? I'd probably say the checks running non-async for older versions would be a fine compromise, but I'd prefer if we offered them to 5.2+ users.

That said, it might be worth just outputting that the check requires WP 5.6 or later, because I don't think it's worth implementing a worse approach that will take on technical debt when we upgrade our minimum version. 🤔

[zutigrm]

Hi @tofumatt ,

Just one question: how often will the data be refreshed. Is it every request, in the background?

It works like regular async request, after Site Health page is visited, it is going to fetch the data and update it in the tab. It is not interval request like health check if that's what you mean. It makes request every time Site health page is visited, refreshed, or switched back to the status tab.

But I'm not very familiar with the async approach so I wasn't sure (is there any WordPress documentation you can point to beyond the blog post?).

This part of functionality is not covered much, and only the links I included in the IB are available. So part of the functionality can be examined from the wp core example that is included as well. We have enough of data to implement it based on all the links.

What will happen if a user is using WP 5.2-5.5? Is there a straightforward fallback? I'd probably say the checks running non-async for older versions would be a fine compromise, but I'd prefer if we offered them to 5.2+ users.

That said, it might be worth just outputting that the check requires WP 5.6 or later, because I don't think it's worth implementing a worse approach that will take on technical debt when we upgrade our minimum version.

Hm, this functionality was introduced in 5.2 actually, but async tests work with ajax callback, from 5.6 it was implemented to work with REST endpoint as well, which is recommended approach. So technically we can implement this using AJAX callback (which we do not use anywhere else, everything is implemented via REST), or as you suggested we can show a message in the description that this feature works only with WordPress 5.6+. In my opinion maybe it is better to stick to the message, since users that are still on 5.2 are probably in small numbers, and WordPress might depreciate AJAX request in favour of REST endpoint in the future releases.

Let me know what you think

[tofumatt]

Ahhh, I see. Yeah, let's stick with the REST request approach.

I'd be fine with limiting the feature to users with 5.6+, agreed 👍🏻

I think for 5.2-5.5 users we can have that output something that says "Requires WP 5.6+ to check". Can you add that to the IB? Then this is good 😄

[zutigrm]

@tofumatt Thanks, IB updated

[tofumatt]

IB ✅

[wpdarren]

QA Update: ⚠️

@zutigrm I have concerns about the placement of this information in WordPress Site Health. As someone who checks this regularly on my site, I am not sure placing it under ' recommended improvements' makes sense. Usually, an admin would have to undertake a specific task to improve the site's health, and then the recommendation would disappear.

This would always appear for Site Kit, and what we are displaying is not an improvement that the user has to take action on.

Am I missing something? I am interested in your thoughts.

[zutigrm]

@wpdarren The reason it was added under recommendations is because that one is always visible, it is not going under collapsed accordions. It is a specific case of showing it always (not putting under critical section, which would be more confusing) so user can easily find it when support asks for this info

[mxbclang]

Reassigning this over to @aaemnnosttv for next steps.

[aaemnnosttv]

I agree with @wpdarren that the tag being placed as expected should be a passed test rather than a recommendation. We shouldn't use that section just for enhanced visibility because there is in fact no recommendation made.

As an aside, I wanted to highlight the deviation from the initial definition that we took here from putting this in the info tab due to the runtime nature of the check. This was an intentional change because manipulating the Site Health info does not support this kind of async check as well. WP core does do this though, so I think we can follow up on this one with another issue to add it there as well which will be useful for our support team.

One more important observation I made is regarding the way tags are checked is by generating a 404 page using a random URL. This isn't really necessary but it's also inconsistent with how SK checks tags normally which looks at the home_url. We can simply include a query parameter like we do for normal tag checks to ensure we don't get a cached response. More importantly however, this approach actually breaks the detection of the AdSense tag, as it is intentionally not output on 404 pages.

Finally, the detected statuses seem to report improperly for sites using AMP, as the AMP plugin strips HTML comments from the output so all tags report as "could not verify that Site Kit placed the tag" even though SK does place them. This could be okay, but we could also potentially add a data attribute for the purpose of identification. This isn't the most important thing though and something we could enhance in a follow-up. It's possible other AMP implementations don't strip the comments, I'm not sure.

I'm going to have a look at the PR as well, but at a minimum this will need a follow-up to address the use of the 404 page as noted above.

[aaemnnosttv]

I've left a secondary review on the PR here so sending back to @zutigrm to follow up 👍

[aaemnnosttv]

@zutigrm moving to MR since I ran out of time to give this a more thorough review but overall LGTM. Someone else can give it another look over before merging 👍

[mohitwp]

QA Update ⚠️

@zutigrm As per AC - The request should also check for the HTML comments inserted by Site Kit. If we disable the AdSense code on site then it makes no changes under Tag placement status of AdSense under Site Health. Is this expected and correct ?

[zutigrm]

Hey @mohitwp , thank you for spotting that. It is not supposed to detect a tag, unless tag is present in the source code. Which is kinda weird, I checked this locally, and when disabling AdSense tag, it was still present. Can you verify if this is the case on your website you are testing this on. Can you go to Homepage, right click somewhere and select Inspect from the dropdown to see the source code. And then search for <!-- Google AdSense snippet added by Site Kit -->, let me know if you still see this after placing AdSense tag is disabled in option

[zutigrm]

@mohitwp I see what's the issue, we are outputting the comment for AdSense above meta tags as well, not only tag snippet. I will consult with others if I can change the comment content for meta tags. You can return ticket back to me

[mohitwp]

@zutigrm I'm using oi.ie site to test this.

Case 1 : When AdSense code placed on site.

Case 2 : When AdSense code not placed on site.

No change in status.

[zutigrm]

@mohitwp Yes, currently AdSense meta tags have the same comment, which leads to false positive, when AdSense tag is disabled, since meta tags are still added, including the same comment that "snippet" is placed, even it is not.

[techanvil]

Back to you for another pass, @mohitwp.

[mohitwp]

QA Update ⚠️

• Tested on main environment.
• Verified Tag placement status under site health when modules are connected or disconnected.
• Verified when Analytics 4, Tag manager and AdSense Set up is not complete.
• Verified when Analytics 4, Tag manager and AdSense Set up code is not placed in site using option available under settings.
• Verified Tag placement status is appearing as per AC.

@zutigrm when we cancel the setup of module or do not complete the module setup. Then in this case Tag Placement Status of AdSense is not same as Analytics 4 and Tag manager module. Notice at set up screen display that _"Site Kit has placed AdSense code on your site to connect your site to AdSense and help you get the most out of ads" so even if user do not configure AdSense still code is placed on the site so tag is detected. But do not want to assume and want to confirm that current tag placement status for AdSense in this case is expected or we want to display different status in this case ?

Pass Cases Screenshots

Analytics connected and code placed  No modules are connected  Analytics connected but code is not placed.  Tag manager connected and code placed  Tag manager connected but code is not placed  AdSense connected and code is placed.  AdSense connected but code is not placed 

[zutigrm]

@mohitwp Thanks for your observation. The tag placement check does a single purpose task - checking if tag is present or not. In this case, I just checked, we do place a tag when connecting the AdSense, even we do not configure the next step. It is correct for tag placement test to report that tag is placed, since it is present.

Now, it is a different question if this is originally intended behaviour for AdSense to place snippet at this point in the setup. Let's see if @aaemnnosttv has some input regarding that part

[techanvil]

@mohitwp, it's an interesting observation. The current behaviour is intentional, but as you've pointed out it's inconsistent and I think there is room for improvement.

Essentially, having clicked "Set up Analytics", gone through the OAuth flow and landed on this screen, Site Kit has retrieved the AdSense clientID which is what's needed to render the tag.

However, AdSense is still not connected at this point, because its connected status depends on accountSetupComplete and siteSetupComplete being set to a value other than false. These values are only set to true upon clicking the Configure Analytics CTA. So until the CTA has been successfully actioned, the tag is placed but the module is still not connected.

https://github.com/google/site-kit-wp/blob/b1fc4ee268ee5ebd7168080baabacb115ba33ffb/includes/Modules/AdSense.php#L175-L183

This is in contrast to modules like Analytics and Tag Manager, where the connected state and the tag placement condition both depend on the same values being set. For example, Analytics won't be able to place its tag until measurementID is set, and it's not connected until measurementID is set either (as well as propertyID and webDataStreamID, but these are all set at the same time in the setup flow). So the tag placement and connected state are inherently in sync.

I would suggest we could improve things for AdSense by not placing the tag until the module is connected. We could potentially manage this by changing the copy on the above page to indicate the code is not placed yet (e.g. the mockup below), and modify the conditions for placing the tag to include a check for accountSetupComplete and siteSetupComplete being set. Or, we may take an alternative, yet to be determined approach.

This is of course out of scope for the current issue, so I would suggest raising a new issue where we can consider this more thoroughly.

[mohitwp]

QA Update ✅

Thank you @techanvil @zutigrm .

• For the observation reported above I created a ticket https://github.com/google/site-kit-wp/issues/8187 .
• Tested on main environment.
• Verified Tag placement status under site health when modules are connected or disconnected.
• Verified when Analytics 4, Tag manager and AdSense Set up is not complete.
• Verified when Analytics 4, Tag manager and AdSense Set up code is not placed in site using option available under settings.
• Verified Tag placement status is appearing as per AC.

Analytics connected and code placed  No modules are connected  Analytics connected but code is not placed.  Tag manager connected and code placed  Tag manager connected but code is not placed  AdSense connected and code is placed.  AdSense connected but code is not placed 

[aaemnnosttv]

@tofumatt @zutigrm This seems to be working well, but I noticed that it is a bit odd if we only say "Not tag detected" when the module's snippet is turned off in the settings. This is mentioned in the AC as something we could improve in a follow-up, which seems worth improving. Also, the fact that we cannot reliably determine if the tag is placed by SK in an AMP context (due to missing comments) could also be improved with a notice for context. Nice work!

[zutigrm]

@aaemnnosttv Thanks, I opened follow-up issue #8206

[aaemnnosttv]

This is in contrast to modules like Analytics and Tag Manager, where the connected state and the tag placement condition both depend on the same values being set. For example, Analytics won't be able to place its tag until measurementID is set, and it's not connected until measurementID is set either (as well as propertyID and webDataStreamID, but these are all set at the same time in the setup flow). So the tag placement and connected state are inherently in sync.

I would suggest we could improve things for AdSense by not placing the tag until the module is connected.

@techanvil @mohitwp I missed this conversation during approval but it didn't affect the implementation here, so there's no problem with this issue, however AdSense is unique in that the tag needs to be placed as part of the approval process for an AdSense account (see https://support.google.com/adsense/answer/7402256). Currently, the connected status requires that the account setup is completed, so we can't bypass placing the tag until connected given the current logic.

We could consider decoupling the connected status from the account and site statuses, but I'm not really sure it's necessary or would be beneficial.

[techanvil]

@aaemnnosttv, thanks for clarifying this.

Seeing as a few of us got confused here I wonder if we can do something to improve the UX. Maybe we can we determine if the account is approved and skip the Configure AdSense step if so. And, that being the case, we could clarify the copy on the screen with the Configure AdSense CTA to indicate that the tag is in place in order for the account to be approved. WDYT?

[aaemnnosttv]

Sure, there's probably room for improvement in the flow. It doesn't seem like a high priority but we could open an issue to seek input from @marrrmarrr and @sigal-teller about how to revise it.