Skip to content

Commit

Permalink
Merge pull request #1640 from WordPress/fix/od-with-page-caching-2
Browse files Browse the repository at this point in the history
Eliminate the detection time window
  • Loading branch information
westonruter authored Nov 13, 2024
2 parents e99b5b2 + 537fa3b commit f9e464b
Show file tree
Hide file tree
Showing 4 changed files with 1 addition and 46 deletions.
18 changes: 1 addition & 17 deletions plugins/optimization-detective/detect.js
Original file line number Diff line number Diff line change
Expand Up @@ -232,8 +232,6 @@ function extendElementData( xpath, properties ) {
* Detects the LCP element, loaded images, client viewport and store for future optimizations.
*
* @param {Object} args Args.
* @param {number} args.serveTime The serve time of the page in milliseconds from PHP via `microtime( true ) * 1000`.
* @param {number} args.detectionTimeWindow The number of milliseconds between now and when the page was first generated in which detection should proceed.
* @param {string[]} args.extensionModuleUrls URLs for extension script modules to import.
* @param {number} args.minViewportAspectRatio Minimum aspect ratio allowed for the viewport.
* @param {number} args.maxViewportAspectRatio Maximum aspect ratio allowed for the viewport.
Expand All @@ -248,8 +246,6 @@ function extendElementData( xpath, properties ) {
* @param {Object} [args.urlMetricGroupCollection] URL Metric group collection, when in debug mode.
*/
export default async function detect( {
serveTime,
detectionTimeWindow,
minViewportAspectRatio,
maxViewportAspectRatio,
isDebug,
Expand All @@ -263,22 +259,10 @@ export default async function detect( {
webVitalsLibrarySrc,
urlMetricGroupCollection,
} ) {
const currentTime = getCurrentTime();

if ( isDebug ) {
log( 'Stored URL Metric group collection:', urlMetricGroupCollection );
}

// Abort running detection logic if it was served in a cached page.
if ( currentTime - serveTime > detectionTimeWindow ) {
if ( isDebug ) {
warn(
'Aborted detection due to being outside detection time window.'
);
}
return;
}

// Abort if the current viewport is not among those which need URL Metrics.
if ( ! isViewportNeeded( win.innerWidth, urlMetricGroupStatuses ) ) {
if ( isDebug ) {
Expand Down Expand Up @@ -330,7 +314,7 @@ export default async function detect( {
// As an alternative to this, the od_print_detection_script() function can short-circuit if the
// od_is_url_metric_storage_locked() function returns true. However, the downside with that is page caching could
// result in metrics missed from being gathered when a user navigates around a site and primes the page cache.
if ( isStorageLocked( currentTime, storageLockTTL ) ) {
if ( isStorageLocked( getCurrentTime(), storageLockTTL ) ) {
if ( isDebug ) {
warn( 'Aborted detection due to storage being locked.' );
}
Expand Down
17 changes: 0 additions & 17 deletions plugins/optimization-detective/detection.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,6 @@
* @param OD_URL_Metric_Group_Collection $group_collection URL Metric group collection.
*/
function od_get_detection_script( string $slug, OD_URL_Metric_Group_Collection $group_collection ): string {
/**
* Filters the time window between serve time and run time in which loading detection is allowed to run.
*
* This is the allowance of milliseconds between when the page was first generated (and perhaps cached) and when the
* detect function on the page is allowed to perform its detection logic and submit the request to store the results.
* This avoids situations in which there is missing URL Metrics in which case a site with page caching which
* also has a lot of traffic could result in a cache stampede.
*
* @since 0.1.0
* @todo The value should probably be something like the 99th percentile of Time To Last Byte (TTLB) for WordPress sites in CrUX.
*
* @param int $detection_time_window Detection time window in milliseconds.
*/
$detection_time_window = apply_filters( 'od_detection_time_window', 5000 );

$web_vitals_lib_data = require __DIR__ . '/build/web-vitals.asset.php';
$web_vitals_lib_src = add_query_arg( 'ver', $web_vitals_lib_data['version'], plugin_dir_url( __FILE__ ) . 'build/web-vitals.js' );

Expand All @@ -49,8 +34,6 @@ function od_get_detection_script( string $slug, OD_URL_Metric_Group_Collection $

$current_url = od_get_current_url();
$detect_args = array(
'serveTime' => microtime( true ) * 1000, // In milliseconds for comparison with `Date.now()` in JavaScript.
'detectionTimeWindow' => $detection_time_window,
'minViewportAspectRatio' => od_get_minimum_viewport_aspect_ratio(),
'maxViewportAspectRatio' => od_get_maximum_viewport_aspect_ratio(),
'isDebug' => WP_DEBUG,
Expand Down
4 changes: 0 additions & 4 deletions plugins/optimization-detective/readme.txt
Original file line number Diff line number Diff line change
Expand Up @@ -132,10 +132,6 @@ Filters the freshness age (TTL) for a given URL Metric. The freshness TTL must b
add_filter( 'od_url_metric_freshness_ttl', '__return_zero' );
`

**Filter:** `od_detection_time_window` (default: 5 seconds)

Filters the time window between serve time and run time in which loading detection is allowed to run. This amount is the allowance between when the page was first generated (and perhaps cached) and when the detect function on the page is allowed to perform its detection logic and submit the request to store the results. This avoids situations in which there are missing URL Metrics in which case a site with page caching which also has a lot of traffic could result in a cache stampede.

**Filter:** `od_minimum_viewport_aspect_ratio` (default: 0.4)

Filters the minimum allowed viewport aspect ratio for URL Metrics.
Expand Down
8 changes: 0 additions & 8 deletions plugins/optimization-detective/tests/test-detection.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,12 @@ public function data_provider_od_get_detection_script(): array {
'unfiltered' => array(
'set_up' => static function (): void {},
'expected_exports' => array(
'detectionTimeWindow' => 5000,
'storageLockTTL' => MINUTE_IN_SECONDS,
'extensionModuleUrls' => array(),
),
),
'filtered' => array(
'set_up' => static function (): void {
add_filter(
'od_detection_time_window',
static function (): int {
return 2500;
}
);
add_filter(
'od_url_metric_storage_lock_ttl',
static function (): int {
Expand All @@ -45,7 +38,6 @@ static function ( array $urls ): array {
);
},
'expected_exports' => array(
'detectionTimeWindow' => 2500,
'storageLockTTL' => HOUR_IN_SECONDS,
'extensionModuleUrls' => array( home_url( '/my-extension.js', 'https' ) ),
),
Expand Down

0 comments on commit f9e464b

Please sign in to comment.