Skip to content

Server Timing - Allow description and make duration optional #2379

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.

Sign up for GitHub

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

Open
the-hercules wants to merge 6 commits into
base: trunk
Choose a base branch
from
Open

Server Timing - Allow description and make duration optional #2379

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3a2eefe
Add optional description support for Server-Timing metrics
the-hercules Feb 11, 2026
c91e96c
Sanitize description for HTTP header in Server-Timing metrics and add…
the-hercules Feb 11, 2026
41b677b
Use native type hint for set_description(), add @covers annotations, …
the-hercules Feb 23, 2026
ca2985e
chore: Remove redundant test for set_description() requiring a string.
the-hercules Feb 25, 2026
d5b4eec
chore: use str_replace instead of preg_replace for simple sanitazatio…
the-hercules Feb 26, 2026
83dd4e2
fix: update for name-only metrics support
the-hercules Feb 27, 2026
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
50 changes: 50 additions & 0 deletions plugins/performance-lab/includes/server-timing/class-perflab-server-timing-metric.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,14 @@ class Perflab_Server_Timing_Metric {
*/
private $before_value;

/**
* The metric description.
*
* @since n.e.x.t
* @var string|null
*/
private $description;

/**
* Constructor.
*
Expand Down Expand Up @@ -142,4 +150,46 @@ public function measure_after(): void {

$this->set_value( ( microtime( true ) - $this->before_value ) * 1000.0 );
}

/**
* Sets the metric description.
*
* @since n.e.x.t
*
* @param string|mixed $description The metric description.
*/
public function set_description( $description ): void {
if ( ! is_string( $description ) ) {
_doing_it_wrong(
__METHOD__,
/* translators: %s: PHP parameter name */
sprintf( esc_html__( 'The %s parameter must be a string.', 'performance-lab' ), '$description' ),
''
);
return;
}

if ( 0 !== did_action( 'perflab_server_timing_send_header' ) && ! doing_action( 'perflab_server_timing_send_header' ) ) {
_doing_it_wrong(
__METHOD__,
/* translators: %s: WordPress action name */
sprintf( esc_html__( 'The method must be called before or during the %s action.', 'performance-lab' ), 'perflab_server_timing_send_header' ),
''
);
return;
}
Comment on lines +161 to +170
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

set_description() adds a new “must be called before or during perflab_server_timing_send_header” guard (via _doing_it_wrong() and early return), but there is no test covering the late-call behavior (similar to test_set_value_prevents_late_measurement). Adding a unit test for the late-call path would help prevent regressions.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

@the-hercules the-hercules Feb 26, 2026

Choose a reason for hiding this comment

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

The late-call guard in set_description() uses the exact same
did_action() / doing_action() pattern as the existing set_value() method
(lines 91–99).

Adding a test for this would require calling:

do_action( 'perflab_server_timing_send_header' );

However, this triggers the global singleton to re-register default metrics
(e.g., before-template), which causes test isolation failures.


$this->description = $description;
}
Comment on lines +159 to +173
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

The PHPDoc declares $description as non-empty-string, but the method currently accepts and stores empty strings without any validation. Either enforce non-empty input (e.g. _doing_it_wrong() on empty string) or relax the docblock to string so the contract matches runtime behavior.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

@the-hercules the-hercules Feb 26, 2026

Choose a reason for hiding this comment

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

Leaving it as it is, relying on the type hint for the developer.


/**
* Gets the metric description.
*
* @since n.e.x.t
*
* @return string|null The metric description, or null if none set.
*/
public function get_description(): ?string {
return $this->description;
}
}
36 changes: 27 additions & 9 deletions plugins/performance-lab/includes/server-timing/class-perflab-server-timing.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,23 +290,41 @@ function ( string $output, ?int $phase ): string {
* @since 1.8.0
*
* @param Perflab_Server_Timing_Metric $metric The metric to format.
* @return string|null Segment for the Server-Timing header, or null if no value set.
* @return string|null Segment for the Server-Timing header, or null if neither value nor description is set.
*/
private function format_metric_header_value( Perflab_Server_Timing_Metric $metric ): ?string {
$value = $metric->get_value();
$value = $metric->get_value();
$description = $metric->get_description();

// If no value is set, make sure it's just passed through.
if ( null === $value ) {
// If neither value nor description is set, skip this metric.
if ( null === $value && null === $description ) {
return null;
}
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

The PR description mentions supporting a “name only” header format (e.g. wp-metric), but this implementation still skips metrics when both duration and description are null (returns null), so name-only metrics will never be emitted. If MDN compliance is the goal, consider returning the metric name when both are null and adding a corresponding test case.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

@the-hercules the-hercules Feb 26, 2026

Choose a reason for hiding this comment

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

I'll update it to remove the "name only" format since that was inaccurate.Metric with neither duration nor description provides no useful information in the header. This PR only adds description as a new optional field alongside the existing optional duration.

Copy link
Copy Markdown
Member

@westonruter westonruter Feb 26, 2026

Choose a reason for hiding this comment

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

Metric with neither duration nor description provides no useful information in the header.

Is this so? In the MDN docs, this example is provided:

// A metric with a name only
Server-Timing: missedCache

Copy link
Copy Markdown
Contributor Author

@the-hercules the-hercules Feb 27, 2026

Choose a reason for hiding this comment

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

@westonruter Added support for name only.


if ( is_float( $value ) ) {
$value = round( $value, 2 );
}

// See https://github.com/WordPress/performance/issues/955.
$name = preg_replace( '/[^!#$%&\'*+\-.^_`|~0-9a-zA-Z]/', '-', $metric->get_slug() );

return sprintf( 'wp-%1$s;dur=%2$s', $name, $value );
$parts = array( sprintf( 'wp-%s', $name ) );

if ( null !== $value ) {
if ( is_float( $value ) ) {
$value = round( $value, 2 );
}
$parts[] = sprintf( 'dur=%s', $value );
}

if ( null !== $description ) {
// Sanitize description for HTTP header quoted-string format.
// Remove control characters (CR/LF) and escape backslashes and quotes.
$sanitized_description = preg_replace( '/[\r\n]/', '', $description );
if ( null === $sanitized_description ) {
$sanitized_description = '';
}
$sanitized_description = addcslashes( $sanitized_description, '\\' );
$sanitized_description = addcslashes( $sanitized_description, '"' );
$parts[] = sprintf( 'desc="%s"', $sanitized_description );
}

return implode( ';', $parts );
}
}
16 changes: 16 additions & 0 deletions plugins/performance-lab/tests/includes/server-timing/test-perflab-server-timing-metric.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,4 +75,20 @@ public function test_measure_after_without_before(): void {

$this->assertNull( $this->metric->get_value() );
}

public function test_set_description_with_string(): void {
$this->metric->set_description( 'Database queries' );
$this->assertSame( 'Database queries', $this->metric->get_description() );
}

public function test_set_description_requires_string(): void {
$this->setExpectedIncorrectUsage( Perflab_Server_Timing_Metric::class . '::set_description' );

$this->metric->set_description( 123 );
$this->assertNull( $this->metric->get_description() );
}

public function test_get_description_returns_null_by_default(): void {
$this->assertNull( $this->metric->get_description() );
}
}
139 changes: 139 additions & 0 deletions plugins/performance-lab/tests/includes/server-timing/test-perflab-server-timing.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,4 +281,143 @@ public function test_use_output_buffer( callable $set_up, bool $expected ): void
$set_up();
$this->assertSame( $expected, $this->server_timing->use_output_buffer() );
}

/**
* @dataProvider data_get_header_with_description
*
* @phpstan-param array<string, mixed> $metrics
*/
public function test_get_header_with_description( string $expected, array $metrics ): void {
foreach ( $metrics as $metric_slug => $args ) {
$this->server_timing->register_metric( $metric_slug, $args );
}
$this->assertSame( $expected, $this->server_timing->get_header() );
}

/**
* @return array<string, mixed>
*/
public function data_get_header_with_description(): array {
$measure_with_description = static function ( Perflab_Server_Timing_Metric $metric ): void {
$metric->set_value( 100 );
$metric->set_description( 'Database queries' );
};
$measure_description_only = static function ( Perflab_Server_Timing_Metric $metric ): void {
$metric->set_description( 'Cache operations' );
};
$measure_duration_only = static function ( Perflab_Server_Timing_Metric $metric ): void {
$metric->set_value( 50 );
};

return array(
'metric with duration and description' => array(
'wp-db-query;dur=100;desc="Database queries"',
array(
'db-query' => array(
'measure_callback' => $measure_with_description,
'access_cap' => 'exist',
),
),
),
'metric with description only' => array(
'wp-cache-ops;desc="Cache operations"',
array(
'cache-ops' => array(
'measure_callback' => $measure_description_only,
'access_cap' => 'exist',
),
),
),
'metric with duration only' => array(
'wp-duration-only;dur=50',
array(
'duration-only' => array(
'measure_callback' => $measure_duration_only,
'access_cap' => 'exist',
),
),
),
'mixed metrics' => array(
'wp-with-both;dur=100;desc="Database queries", wp-desc-only;desc="Cache operations", wp-dur-only;dur=50',
array(
'with-both' => array(
'measure_callback' => $measure_with_description,
'access_cap' => 'exist',
),
'desc-only' => array(
'measure_callback' => $measure_description_only,
'access_cap' => 'exist',
),
'dur-only' => array(
'measure_callback' => $measure_duration_only,
'access_cap' => 'exist',
),
),
),
);
}

/**
* @dataProvider data_get_header_with_description_edge_cases
*
* @phpstan-param array<string, mixed> $metrics
*/
public function test_get_header_with_description_edge_cases( string $expected, array $metrics ): void {
foreach ( $metrics as $metric_slug => $args ) {
$this->server_timing->register_metric( $metric_slug, $args );
}
$this->assertSame( $expected, $this->server_timing->get_header() );
}

/**
* @return array<string, mixed>
*/
public function data_get_header_with_description_edge_cases(): array {
return array(
'description with double quote' => array(
'wp-quoted;desc="Say \\"hello\\""',
array(
'quoted' => array(
'measure_callback' => static function ( Perflab_Server_Timing_Metric $metric ): void {
$metric->set_description( 'Say "hello"' );
},
'access_cap' => 'exist',
),
),
),
'description with backslash' => array(
'wp-backslash;desc="path\\\\to\\\\file"',
array(
'backslash' => array(
'measure_callback' => static function ( Perflab_Server_Timing_Metric $metric ): void {
$metric->set_description( 'path\\to\\file' );
},
'access_cap' => 'exist',
),
),
),
'description with newline' => array(
'wp-newline;desc="Line 1Line 2"',
array(
'newline' => array(
'measure_callback' => static function ( Perflab_Server_Timing_Metric $metric ): void {
$metric->set_description( "Line 1\nLine 2" );
},
'access_cap' => 'exist',
),
),
),
'description with carriage return' => array(
'wp-cr;desc="BeforeAfter"',
array(
'cr' => array(
'measure_callback' => static function ( Perflab_Server_Timing_Metric $metric ): void {
$metric->set_description( "Before\rAfter" );
},
'access_cap' => 'exist',
),
),
),
);
}
}
Loading