Add site health check to detect blocked REST API and short-circuit optimization when Inaccessible #1762

b1ink0 · 2024-12-19T17:59:46Z

Summary

Relevant technical choices

Adds a site health check to verify the availability and status of the /optimization-detective/v1/url-metrics:store REST API endpoint. The process will short-circuit if the endpoint is inaccessible.
A site health check will run weekly and update the status.
Based on the status, a notice will be displayed on the plugins page.

Scenarios:

When the health check passes
When the REST API endpoint returns a forbidden error
When the REST API endpoint returns an unauthorized error
When other errors are encountered

westonruter

Should there be a plugin activation hook added as well which does add_option() for the new option and then also kicks off (or schedules) a REST API check? Ideally there would be a warning shown immediately after activating the plugin (e.g. on the plugins list table screen) whether the REST API is working so that the user doesn't have to discover it later via Site Health.

westonruter · 2024-12-19T18:26:02Z

plugins/optimization-detective/includes/site-health/load.php

Maybe put site-health in the root directory instead of inside includes since there are no other directories in there?

I thought in future if we added something like admin dashboard for managing URL metrics or any other admin dashboard related thing then it would be better to add that feature in includes/admin. But we can just refactor things later when we need it so moving site-health to root makes sence.

Yeah, let's put it in the root for now since all other directories are there.

if we added something like admin dashboard for managing URL metrics or any other admin dashboard related thing

Aside: I did put together a rough utility plugin for this: https://github.com/westonruter/od-admin-ui

westonruter · 2024-12-19T18:27:41Z

plugins/optimization-detective/includes/site-health/rest-api/helper.php

+			'<p>%s</p>',
+			esc_html__( 'The Optimization Detective endpoint could not be reached. This might mean the REST API is disabled or blocked.', 'optimization-detective' )
+		);
+		update_option(


This is the first PR that adds an option to to Optimization Detective. We'll need to make sure that the relevant delete_option() calls get added to the plugin's uninstall.php.

westonruter · 2024-12-19T18:29:24Z

plugins/optimization-detective/optimization.php

+	// Disable detection if the REST API is disabled.
+	$od_rest_api_info = get_option( 'od_rest_api_info' );
+	if ( is_array( $od_rest_api_info ) && isset( $od_rest_api_info['available'] ) ) {
+		$needs_detection = (bool) $od_rest_api_info['available'];
+	}
+


Actually, this check wouldn't make sense here. It should rather be done in od_maybe_add_template_output_buffer_filter() to short-circuit if the REST API it is not available.

westonruter · 2024-12-19T18:32:01Z

plugins/optimization-detective/includes/site-health/rest-api/helper.php

+		update_option(
+			'od_rest_api_info',
+			array(
+				'status'    => 'error',


Should the $error->get_message() and maybe $error->get_code() be stored here?

westonruter · 2024-12-19T18:34:40Z

plugins/optimization-detective/includes/site-health/rest-api/helper.php

+		update_option(
+			'od_rest_api_info',
+			array(
+				'status'    => 'forbidden',


Instead of storing the string, what about storing the $status_code instead?

westonruter · 2024-12-19T18:35:07Z

plugins/optimization-detective/includes/site-health/rest-api/helper.php

+				'available' => false,
+			)
+		);
+	}


The else condition should be added as an error result as well. Here especially the $status_code could be used.

westonruter · 2024-12-19T18:38:03Z

plugins/optimization-detective/includes/site-health/rest-api/helper.php

+		&& count( $expected_params ) === count( array_intersect( $data['data']['params'], $expected_params ) )
+	) {
+		// The REST API endpoint is available.
+		update_option(


Instead of having update_option() appearing in multiple places, each condition could populate an $info variable which is then sent into update_option() once at the end of the function.

…ndition

westonruter · 2024-12-20T21:13:28Z

plugins/optimization-detective/site-health/rest-api/helper.php

+		wp_schedule_event( time(), 'hourly', 'od_rest_api_health_check_event' );
+	}
+}
+add_action( 'wp', 'od_schedule_rest_api_health_check' );


Is this a best practice? Should it rather go in admin_init to avoid frontend writes? I'm not sure what others do here.

I think scheduling on plugin activation hook will be better than admin_init.

westonruter · 2024-12-20T21:13:42Z

plugins/optimization-detective/site-health/rest-api/helper.php

+ */
+function od_schedule_rest_api_health_check(): void {
+	if ( ! (bool) wp_next_scheduled( 'od_rest_api_health_check_event' ) ) {
+		wp_schedule_event( time(), 'hourly', 'od_rest_api_health_check_event' );


I think hourly is too much. Maybe weekly would make sense.

I currently have it set to run weekly, but that might be too infrequent. If a configuration change disables the REST API, it could take an entire week for user to detect the issue. I believe running it daily would be a better choice.

…ck fails

…tion

github-actions · 2024-12-24T14:12:43Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: b1ink0 <b1ink0@git.wordpress.org>
Co-authored-by: westonruter <westonruter@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

b1ink0 added 7 commits December 19, 2024 18:34

Add REST API health check for Optimization Detective

88952a5

Improve error message of site health check

4bff9fe

Add scheduled health check for Optimization Detective REST API

56d769a

Save site health check status for Optimization Detective REST API

ba911e1

Add test for when REST API is available

12a229c

Add tests for REST API unauthorized error handling

a0f460d

Add test for REST API forbidden error handling

c29cb7c

b1ink0 changed the title ~~Add/site health check for od rest api~~ Optimization Detective can be blocked from working due to sites disabling the REST API Dec 19, 2024

westonruter reviewed Dec 19, 2024

View reviewed changes

westonruter added this to the optimization-detective n.e.x.t milestone Dec 19, 2024

westonruter added [Type] Enhancement A suggestion for improvement of an existing feature [Plugin] Optimization Detective Issues for the Optimization Detective plugin labels Dec 19, 2024

b1ink0 added 6 commits December 20, 2024 18:04

Refactor to move site-health directory to plugin root directory

8d8ae3e

Clear site health check data during plugin uninstallation

422a5bf

Move REST API availability check to appropriate function

0bc74f7

Add error message and error code to option

36042fd

Refactor to store status code instead of string, Add fallback else co…

ad21df8

…ndition

Refactor to only use update_option once

0bba468

b1ink0 changed the title ~~Optimization Detective can be blocked from working due to sites disabling the REST API~~ Add site health check to detect blocked REST API and short-circuit optimization when Inaccessible Dec 20, 2024

westonruter reviewed Dec 20, 2024

View reviewed changes

b1ink0 added 7 commits December 23, 2024 19:07

Add activation hook to initialize option value

e1b9004

Move scheduling to activation hook

dbede65

Change health check scheduling from hourly to weekly

2b2ca3e

Run REST API health check on plugin activation, Display notice if che…

e7c1001

…ck fails

Move activation logic to separate function

7e4e057

Refactor activation logic to directly call REST API health check func…

a2baa65

…tion

Merge branch 'trunk' into add/site-health-check-for-od-rest-api

e1ec10c

b1ink0 marked this pull request as ready for review December 24, 2024 14:12

b1ink0 requested a review from felixarntz as a code owner December 24, 2024 14:12

b1ink0 requested a review from westonruter December 24, 2024 14:12

Improve site health status messages for clarity and consistency

48c83a5

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add site health check to detect blocked REST API and short-circuit optimization when Inaccessible #1762

Add site health check to detect blocked REST API and short-circuit optimization when Inaccessible #1762

b1ink0 commented Dec 19, 2024 •

edited

Loading

westonruter left a comment

westonruter Dec 19, 2024

b1ink0 Dec 19, 2024

westonruter Dec 19, 2024

westonruter Dec 19, 2024

westonruter Dec 19, 2024

westonruter Dec 19, 2024

westonruter Dec 19, 2024

westonruter Dec 19, 2024

westonruter Dec 19, 2024

westonruter Dec 20, 2024

b1ink0 Dec 23, 2024

westonruter Dec 20, 2024

b1ink0 Dec 24, 2024

github-actions bot commented Dec 24, 2024

Add site health check to detect blocked REST API and short-circuit optimization when Inaccessible #1762

Are you sure you want to change the base?

Add site health check to detect blocked REST API and short-circuit optimization when Inaccessible #1762

Conversation

b1ink0 commented Dec 19, 2024 • edited Loading

Summary

Relevant technical choices

Scenarios:

westonruter left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Dec 24, 2024

b1ink0 commented Dec 19, 2024 •

edited

Loading