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

Implement logic to measure specific hook execution time with Server-Timing controlled by a WP Admin screen #784

Merged
merged 22 commits into from
Aug 9, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
a1fd89c
Implement logic to measure specific hook execution time with Server-T…
felixarntz Jul 18, 2023
1c21d08
Merge branch 'trunk' into add/server-timing-hook-ui
felixarntz Jul 21, 2023
ec9708b
Fix newly exposed lint violations.
felixarntz Jul 21, 2023
6a1205f
Use more appropriate workaround when PHP_INT_MIN is not defined.
felixarntz Jul 21, 2023
08d79c3
Merge branch 'trunk' into add/server-timing-hook-ui
felixarntz Aug 3, 2023
694a6d1
Reuse logic to measure actions and filters, and provide inline docume…
felixarntz Aug 3, 2023
6b5d12a
Clarify how the hooks are measured in WP Admin UI.
felixarntz Aug 3, 2023
61f5ccc
Ensure that no duplicate hooks can be provided.
felixarntz Aug 3, 2023
585911d
Add tests for server timing setting registration.
felixarntz Aug 3, 2023
6db1e85
Add tests for server timing admin UI.
felixarntz Aug 3, 2023
a8d141f
Remove XHTML syntax from WP Admin forms.
felixarntz Aug 7, 2023
400c0a1
Modify hook measuring logic to use all hook.
felixarntz Aug 7, 2023
c5fe384
Replace sanitize_key for hook name sanitization with custom regex to …
felixarntz Aug 7, 2023
447f078
Have hook callbacks self-remove to only measure first occurrence.
felixarntz Aug 7, 2023
c48d206
Remove novalidate attribute.
felixarntz Aug 7, 2023
b85959e
Loosen hook name sanitization to rely on sanitize_text_field(), while…
felixarntz Aug 7, 2023
124a7a6
Update test to cover whitespace in hookname.
felixarntz Aug 7, 2023
7f99021
Use allowlist for Server-Timing option keys.
felixarntz Aug 7, 2023
05d849e
Fix tests.
felixarntz Aug 7, 2023
fd41db1
Add note on hook limitation unless output buffering is enabled.
felixarntz Aug 7, 2023
f484a7d
Use placeholders for non-translatable strings.
felixarntz Aug 8, 2023
7c94b9e
Add missing early return on top of procedural file.
felixarntz Aug 8, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
131 changes: 131 additions & 0 deletions admin/server-timing.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
<?php
/**
* Server-Timing API admin integration file.
*
* @package performance-lab
*/

/**
* Adds the Server-Timing page to the Tools menu.
*
* @since n.e.x.t
*/
function perflab_add_server_timing_page() {
$hook_suffix = add_management_page(
__( 'Server-Timing', 'performance-lab' ),
__( 'Server-Timing', 'performance-lab' ),
'manage_options',
PERFLAB_SERVER_TIMING_SCREEN,
'perflab_render_server_timing_page'
);

// Add the following hooks only if the screen was successfully added.
if ( false !== $hook_suffix ) {
add_action( "load-{$hook_suffix}", 'perflab_load_server_timing_page' );
}

return $hook_suffix;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are you returning this for? This is hooked into an action right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, this would only be useful for test coverage, which I'm planning to add.

}
add_action( 'admin_menu', 'perflab_add_server_timing_page' );

/**
* Initializes settings sections and fields for the Server-Timing page.
*
* @since n.e.x.t
*/
function perflab_load_server_timing_page() {
add_settings_section(
'benchmarking',
__( 'Benchmarking', 'performance-lab' ),
static function() {
?>
<p>
<?php esc_html_e( 'In this section, you can provide hook names to include measurements for them in the Server-Timing header.', 'performance-lab' ); ?>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the Server-Timing header is not something that should be translated, I'd recommend using sprintf with a placeholder here (+ a translator comment of course).

</p>
<?php
},
PERFLAB_SERVER_TIMING_SCREEN
);

/*
* For all settings fields, the field slug, option sub key, and label
* suffix have to match for the rendering in the callback to be
* semantically correct.
*/
add_settings_field(
'benchmarking_actions',
__( 'Actions', 'performance-lab' ),
static function() {
perflab_render_server_timing_page_field( 'benchmarking_actions' );
},
PERFLAB_SERVER_TIMING_SCREEN,
'benchmarking',
array( 'label_for' => 'server_timing_benchmarking_actions' )
);
add_settings_field(
'benchmarking_filters',
__( 'Filters', 'performance-lab' ),
static function() {
perflab_render_server_timing_page_field( 'benchmarking_filters' );
},
PERFLAB_SERVER_TIMING_SCREEN,
'benchmarking',
array( 'label_for' => 'server_timing_benchmarking_filters' )
);
}

/**
* Renders the Server-Timing page.
*
* @since n.e.x.t
*/
function perflab_render_server_timing_page() {
?>
<div class="wrap">
<?php settings_errors(); ?>
<h1>
<?php esc_html_e( 'Server-Timing', 'performance-lab' ); ?>
</h1>

<form action="options.php" method="post" novalidate="novalidate">
felixarntz marked this conversation as resolved.
Show resolved Hide resolved
<?php settings_fields( PERFLAB_SERVER_TIMING_SCREEN ); ?>
<?php do_settings_sections( PERFLAB_SERVER_TIMING_SCREEN ); ?>
<?php submit_button(); ?>
</form>
</div>
<?php
}

/**
* Renders a field for the given Server-Timing option.
*
* @since n.e.x.t
*
* @param string $slug Slug of the field and sub-key in the Server-Timing option.
*/
function perflab_render_server_timing_page_field( $slug ) {
$options = (array) get_option( PERFLAB_SERVER_TIMING_SETTING, array() );

// Value for the sub-key is an array of hook names.
$value = '';
if ( isset( $options[ $slug ] ) ) {
$value = implode( "\n", $options[ $slug ] );
}

$field_id = "server_timing_{$slug}";
$field_name = PERFLAB_SERVER_TIMING_SETTING . '[' . $slug . ']';
$description_id = "{$field_id}_description";

?>
<textarea
id="<?php echo esc_attr( $field_id ); ?>"
name="<?php echo esc_attr( $field_name ); ?>"
aria-describedby="<?php echo esc_attr( $description_id ); ?>"
class="large-text code"
rows="8"
felixarntz marked this conversation as resolved.
Show resolved Hide resolved
><?php echo esc_textarea( $value ); ?></textarea>
<p id="<?php echo esc_attr( $description_id ); ?>" class="description">
<?php esc_html_e( 'Enter a single hook name per line.', 'performance-lab' ); ?>
</p>
<?php
}
1 change: 1 addition & 0 deletions load.php
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,7 @@ function perflab_maybe_remove_object_cache_dropin() {
// Only load admin integration when in admin.
if ( is_admin() ) {
require_once PERFLAB_PLUGIN_DIR_PATH . 'admin/load.php';
require_once PERFLAB_PLUGIN_DIR_PATH . 'admin/server-timing.php';
}

/**
Expand Down
83 changes: 83 additions & 0 deletions server-timing/defaults.php
Original file line number Diff line number Diff line change
Expand Up @@ -207,3 +207,86 @@ static function( $acc, $query ) {
}
}
add_action( 'wp_loaded', 'perflab_register_default_server_timing_template_metrics' );

/**
* Registers additional Server-Timing metrics as configured in the setting.
*
* These metrics should be registered as soon as possible. They can be added
* and modified in the "Tools > Server-Timing" screen.
*
* @since n.e.x.t
*/
function perflab_register_additional_server_timing_metrics_from_setting() {
$options = (array) get_option( PERFLAB_SERVER_TIMING_SETTING, array() );

if ( isset( $options['benchmarking_actions'] ) ) {
foreach ( $options['benchmarking_actions'] as $action ) {
$metric_slug = 'action-' . $action;

$measure_callback = static function( $metric ) use ( $action ) {
$metric->measure_before();
add_action( $action, array( $metric, 'measure_after' ), PHP_INT_MAX, 0 );
felixarntz marked this conversation as resolved.
Show resolved Hide resolved
};

add_action(
$action,
static function() use ( $metric_slug, $measure_callback ) {
perflab_server_timing_register_metric(
$metric_slug,
array(
'measure_callback' => $measure_callback,
'access_cap' => 'exist',
)
);
},
// phpcs:ignore PHPCompatibility.Constants.NewConstants.php_int_minFound
defined( 'PHP_INT_MIN' ) ? PHP_INT_MIN : ~PHP_INT_MAX
);
}
}

if ( isset( $options['benchmarking_filters'] ) ) {
foreach ( $options['benchmarking_filters'] as $filter ) {
$metric_slug = 'filter-' . $filter;

$measure_callback = static function( $metric ) use ( $filter ) {
$metric->measure_before();
add_filter(
$filter,
static function( $passthrough ) use ( $metric ) {
$metric->measure_after();
return $passthrough;
},
PHP_INT_MAX
);
};

add_filter(
$filter,
static function( $passthrough ) use ( $metric_slug, $measure_callback ) {
perflab_server_timing_register_metric(
$metric_slug,
array(
'measure_callback' => $measure_callback,
'access_cap' => 'exist',
)
);
return $passthrough;
},
// phpcs:ignore PHPCompatibility.Constants.NewConstants.php_int_minFound
defined( 'PHP_INT_MIN' ) ? PHP_INT_MIN : ~PHP_INT_MAX
);
}
}
}

/*
* If this file is loaded from the Server-Timing logic in the object-cache.php
* drop-in, it must not call this function right away since otherwise the cache
* will not be loaded yet.
*/
if ( ! did_action( 'muplugins_loaded' ) ) {
add_action( 'muplugins_loaded', 'perflab_register_additional_server_timing_metrics_from_setting' );
} else {
perflab_register_additional_server_timing_metrics_from_setting();
}
48 changes: 48 additions & 0 deletions server-timing/load.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,54 @@
* @since 1.8.0
*/

define( 'PERFLAB_SERVER_TIMING_SETTING', 'perflab_server_timing_settings' );
define( 'PERFLAB_SERVER_TIMING_SCREEN', 'perflab-server-timing' );
felixarntz marked this conversation as resolved.
Show resolved Hide resolved

/**
* Registers the Server-Timing setting.
*
* @since n.e.x.t
*/
function perflab_register_server_timing_setting() {
register_setting(
PERFLAB_SERVER_TIMING_SCREEN,
PERFLAB_SERVER_TIMING_SETTING,
array(
'type' => 'object',
'sanitize_callback' => 'perflab_sanitize_server_timing_setting',
'default' => array(),
)
);
felixarntz marked this conversation as resolved.
Show resolved Hide resolved
}
add_action( 'init', 'perflab_register_server_timing_setting' );

/**
* Sanitizes the Server-Timing setting.
*
* @since n.e.x.t
*
* @param mixed $value Server-Timing setting value.
* @return array Sanitized Server-Timing setting value.
*/
function perflab_sanitize_server_timing_setting( $value ) {
if ( ! is_array( $value ) ) {
return array();
}

// Ensure that every element is an indexed array of hook names.
return array_filter(
array_map(
static function( $hooks ) {
if ( ! is_array( $hooks ) ) {
$hooks = explode( "\n", $hooks );
}
return array_filter( array_map( 'sanitize_key', $hooks ) );
},
$value
)
);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could do with a unit test IMO.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I haven't added unit tests yet, but definitely planning to do so.


/**
* Provides access the Server-Timing API.
*
Expand Down