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

Introduce Server-Timing API as part of infrastructure #551

Closed
felixarntz opened this issue Oct 3, 2022 · 7 comments · Fixed by #553
Closed

Introduce Server-Timing API as part of infrastructure #551

felixarntz opened this issue Oct 3, 2022 · 7 comments · Fixed by #553
Assignees
Labels
Infrastructure Issues for the overall performance plugin infrastructure [Type] Feature A new feature within an existing module

Comments

@felixarntz
Copy link
Member

Context

Common web performance metric and auditing tools (e.g. Lighthouse, PageSpeed Insights, WebPageTest) only focus on client-side performance in detail. Server-side performance gets reduced to a single metric (TTFB), simply because these tools cannot look "behind the scenes". This means we have no clear way to figure out how e.g. a feature in the Performance Lab plugin affects server-side performance in the lab or in the field.

The Server-Timing header is a standardized way to expose server-side metrics to the client-side. This way, performance tools can use and evaluate such data, which was previously hidden from them. Of course the server has to somehow measure and expose the results though, and that is what this issue is about.

Proposal

The Performance Lab plugin should come with a PHP API to centrally control the Server-Timing header which, most importantly, allows developers to pass identifiers for a metric and their values to the API, with the API then managing the header output.

This API should be part of the plugin infrastructure, i.e. not a module. The reason for that is that this API should be usable by any module to measure certain aspects of its performance impact, which would not be reliable if it was a module. Another reason to not make the API a module is that modules should come with functionality on their own, which would not be the case for this API. Last but not least, unlike the purpose of modules, this API should not be proposed for inclusion in WordPress core (at least not in the near term) - its main purpose is to allow us to gather server-side metrics in the lab and in the field, which we have not been able to so far.

Defaults

Primarily, individual modules of the plugin should use the API to measure specific parts of their execution relevant to the feature they implement, in order to see how they affect PHP performance. However, a single default is worth including, which is the overall WordPress execution time, which WordPress core already tracks itself using its timer_start() function.

High-level spec

  • Class Perflab_Server_Timing:
    • register_metric( string $metric_slug, callable $measure_callback, string|callable $access ): Registers a metric and its measuring callback, to be added to the list of metrics. The measuring callback will only be executed if the access is satisfied by the current request.
      • Potentially for a more flexible API the signature of this method should use array $args as second parameter, but the exact parameters are outlined above for clarity.
    • get_header_value() : string: Returns the full Server-Timing header value, based on all registered metrics which have a value set.
    • add_header(): Adds the Server-Timing header, using get_header_value().
  • Function perflab_server_timing(): Initializes the main instance of the Perflab_Server_Timing class and fires an action hook for any extension to integrate, passing the instance to the action as a parameter.
  • Function perflab_wrap_server_timing( callable $callback, string $metric_slug, string|callable $access ) : callable: Wraps a callback (e.g. of an add_action() or add_filter() call) into this function, which uses Perflab_Server_timing::register_metric() internally to simply measure execution time of the $callback function and add it to the Server-Timing header value.
    • This little utility will simplify usage of the API around hook callbacks and is inspired by this article.

Quality attributes (performance, privacy, security, accessibility)

  • Header length: While it is perfectly acceptable to include various metrics and values in the Server-Timing header, it should not become too long in order to minimize the HTTP overhead. For that reason, the API should include validation to ensure the metric names are kept short and concise (exact character limit per metric TBD), and it should furthermore limit the overall number of metrics that can be included in the header as well.
  • Timing overhead: While measuring time in PHP is generally considered harmless, an excessive number of timers may cause a performance regression. For example, WordPress core conditionally measures the time every individual database request takes, though it does so only conditionally if the SAVEQUERIES flag is set. Note that this has some overlap with the header length consideration above and will potentially already be satisfied by the solution for that.
  • Sensitive values: The values for each metric should be numeric, e.g. integers or floats. However, since the Server-Timing header is technically a string, any other value could also be rendered, which opens room for abuse, where code in the WordPress site could leak sensitive information as part of that value. To avoid that, the API should require all timing values to be numeric.
  • Sensitive metrics: There are certain metrics which a site may want to measure but keep internal. For this purpose, the API should require for each metric to be registered with a certain access level definition, so that not every metric is public. This could either be a WordPress capability check or a shorthand such as “public” or “admin”.
@felixarntz felixarntz added [Type] Feature A new feature within an existing module Infrastructure Issues for the overall performance plugin infrastructure Needs Discussion Anything that needs a discussion/agreement labels Oct 3, 2022
@felixarntz felixarntz self-assigned this Oct 3, 2022
@johnbillion
Copy link
Member

The primary problem I ran into when attempting to implement this in Query Monitor is that headers need to be sent before any output, and a lot of processing that happens in WordPress that would ideally be measured occurs after the output begins. Short of wrapping everything in an output buffer, this means the header has limited use.

I looked into HTTP trailers but these are only supported when the request includes a header indicating support for it, which IIRC only happens in Firefox.

@tillkruss
Copy link
Member

Right, I ran into this as well in the past. The only way around it was output buffering 😬

@felixarntz
Copy link
Member Author

@johnbillion I've added a POC with some testing instructions including a Gist with examples in #553. Would love to get your feedback on it.

Regarding usage of an output buffer, I think this is definitely something we should consider, given that this is a plugin. We can make it filterable like it is in the POC PR, but potentially we may even want to enable it by default.

@tillkruss
Copy link
Member

Given that my plug-in already measures timings, how can I supply them to this module at the last possible moment?

@felixarntz
Copy link
Member Author

@tillkruss Great question, I was just thinking that we should probably introduce an action like perflab_server_timing_send_header which could be used as the last possible point to add metrics. That'll abstract away the need to have to know the internals on when the header is sent.

@ecotechie
Copy link

Why not use Xdebug + webgrind as mentioned in the Slack channel?
https://wordpress.slack.com/archives/C02KGN5K076/p1666106999180819

I've been using Xdebug + webgrind - and pushed a few tweaks on Core the past couple of weeks as a result of those profiling tests  Setup instructions for wp-env can be found on WordPress/wordpress-develop#3418 if anyone's interested to get started easily

@felixarntz
Copy link
Member Author

@ecotechie Tools like those could certainly be used, but IMO that falls outside the scope of this issue, which is mainly about a way to expose performance metrics data to the client. For how the measurement happens, I think here the simplest starting point is to simply measure time in PHP.

Obviously there are more sophisticated ways to measure PHP performance like those you're mentioning, but those could always be added in a separate issue. The proposed API here would be compatible with them as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Infrastructure Issues for the overall performance plugin infrastructure [Type] Feature A new feature within an existing module
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants