Skip to content

Sync the FPM openmetrics status with php-fpm_exporter #7291

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

Merged
merged 1 commit into from
Jul 21, 2021
Merged

Sync the FPM openmetrics status with php-fpm_exporter #7291

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 23 additions & 27 deletions sapi/fpm/fpm/fpm_status.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ int fpm_status_handle_request(void) /* {{{ */
struct fpm_scoreboard_proc_s proc;
char *buffer, *time_format, time_buffer[64];
time_t now_epoch;
int full, encode, is_start_time_str;
int full, encode, has_start_time;
char *short_syntax, *short_post;
char *full_pre, *full_syntax, *full_post, *full_separator;
zend_string *_GET_str;
Expand Down Expand Up @@ -223,7 +223,7 @@ int fpm_status_handle_request(void) /* {{{ */
short_syntax = short_post = NULL;
full_separator = full_pre = full_syntax = full_post = NULL;
encode = 0;
is_start_time_str = 1;
has_start_time = 1;

/* HTML */
if (fpm_php_get_string_from_table(_GET_str, "html")) {
Expand Down Expand Up @@ -398,21 +398,18 @@ int fpm_status_handle_request(void) /* {{{ */
"# HELP phpfpm_up Could pool %s using a %s PM on PHP-FPM be reached?\n"
"# TYPE phpfpm_up gauge\n"
"phpfpm_up 1\n"
"# HELP phpfpm_start_time When FPM has started.\n"
"# TYPE phpfpm_start_time gauge\n"
"phpfpm_start_time %lld\n"
"# HELP phpfpm_start_since_total The number of seconds since FPM has started.\n"
"# TYPE phpfpm_start_since_total counter\n"
"phpfpm_start_since_total %lu\n"
"# HELP phpfpm_accepted_connections_total The number of requests accepted by the pool.\n"
"# TYPE phpfpm_accepted_connections_total counter\n"
"phpfpm_accepted_connections_total %lu\n"
"# HELP phpfpm_start_since The number of seconds since FPM has started.\n"
"# TYPE phpfpm_start_since counter\n"
"phpfpm_start_since %lu\n"
"# HELP phpfpm_accepted_connections The number of requests accepted by the pool.\n"
"# TYPE phpfpm_accepted_connections counter\n"
"phpfpm_accepted_connections %lu\n"
"# HELP phpfpm_listen_queue The number of requests in the queue of pending connections.\n"
"# TYPE phpfpm_listen_queue gauge\n"
"phpfpm_listen_queue %d\n"
"# HELP phpfpm_max_listen_queue_total The maximum number of requests in the queue of pending connections since FPM has started.\n"
"# TYPE phpfpm_max_listen_queue_total counter\n"
"phpfpm_max_listen_queue_total %d\n"
"# HELP phpfpm_max_listen_queue The maximum number of requests in the queue of pending connections since FPM has started.\n"
"# TYPE phpfpm_max_listen_queue counter\n"
"phpfpm_max_listen_queue %d\n"
Comment on lines -413 to +412
Copy link
Contributor

@WyriHaximus WyriHaximus Jul 22, 2021

Choose a reason for hiding this comment

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

Hey @bukka I get the reasoning for this change, the reason I went for these names instead of matching them with that exporter is to match the Prometheus naming conversion: https://prometheus.io/docs/practices/naming/

The main question that is going around in my mind right now, is do we want to match up with that exporter, or set our own naming following the Prometheus naming convention?

Copy link
Member Author

@bukka bukka Jul 22, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Well you agreed about the sync: #5723 (comment) so I did it that way. I think there's some value to have a compatibility mode with the exporter as it makes the migration pretty easy. Although after reading the conventions I feel the right way would be to follow them. Maybe we should later introduce some kind of versioning with a format using correct conventions and labels. Now it's too late to change anything as beta1 has been just released.

Copy link
Contributor

@WyriHaximus WyriHaximus Jul 22, 2021

Choose a reason for hiding this comment

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

Well you agreed about the sync: #5723 (comment) so I did it that way.

Correct, and I'm on the fence about it now. Some of them actually make sense to me, like phpfpm_max_active_processes, but might not follow that convention.

I think there's some value to have a compatibility mode with the exporter as it makes the migration pretty easy. Although after reading the conventions I feel the right way would be to follow them. Maybe we should later introduce some kind of versioning with a format using correct conventions and labels.

How would versioning work? An additional query field to version that?

Now it's too late to change anything as beta1 has been just released.

shrug it's not a very big deal, just think it would be nice if we did it perfectly 😎 . Having this out there is a massive win already 🎉 .

Copy link
Member Author

@bukka bukka Jul 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

How would versioning work? An additional query field to version that?

I was thinking about making it more query based with the same keys and different values and not differentiated by keys. For example something like format=openmetrics&version=2.

Copy link
Member Author

@bukka bukka Jul 25, 2021

Choose a reason for hiding this comment

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

We would of course keep the old queries working for quite some time so it doesn't break between the versions.

Copy link
Contributor

@WyriHaximus WyriHaximus Jul 25, 2021

Choose a reason for hiding this comment

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

That would work, assuming that will come in PHP 8.2 then?

Copy link
Member Author

@bukka bukka Jul 25, 2021

Choose a reason for hiding this comment

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

Yeah hopefully :)

Copy link
Contributor

@WyriHaximus WyriHaximus Jul 25, 2021

Choose a reason for hiding this comment

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

Well then let me pot an reminder for that in my calendar :D

"# TYPE phpfpm_listen_queue_length gauge\n"
"# HELP phpfpm_listen_queue_length The size of the socket queue of pending connections.\n"
"phpfpm_listen_queue_length %u\n"
Expand All @@ -425,17 +422,17 @@ int fpm_status_handle_request(void) /* {{{ */
"# HELP phpfpm_total_processes The number of idle + active processes.\n"
"# TYPE phpfpm_total_processes gauge\n"
"phpfpm_total_processes %d\n"
"# HELP phpfpm_max_active_processes_total The maximum number of active processes since FPM has started.\n"
"# TYPE phpfpm_max_active_processes_total counter\n"
"phpfpm_max_active_processes_total %d\n"
"# HELP phpfpm_max_children_reached_total The number of times, the process limit has been reached, when pm tries to start more children (works only for pm 'dynamic' and 'ondemand').\n"
"# TYPE phpfpm_max_children_reached_total counter\n"
"phpfpm_max_children_reached_total %u\n"
"# HELP phpfpm_slow_requests_total The number of requests that exceeded your 'request_slowlog_timeout' value.\n"
"# TYPE phpfpm_slow_requests_total counter\n"
"phpfpm_slow_requests_total %lu\n";

is_start_time_str = 0;
"# HELP phpfpm_max_active_processes The maximum number of active processes since FPM has started.\n"
"# TYPE phpfpm_max_active_processes counter\n"
"phpfpm_max_active_processes %d\n"
"# HELP phpfpm_max_children_reached The number of times, the process limit has been reached, when pm tries to start more children (works only for pm 'dynamic' and 'ondemand').\n"
"# TYPE phpfpm_max_children_reached counter\n"
"phpfpm_max_children_reached %u\n"
"# HELP phpfpm_slow_requests The number of requests that exceeded your 'request_slowlog_timeout' value.\n"
"# TYPE phpfpm_slow_requests counter\n"
"phpfpm_slow_requests %lu\n";

has_start_time = 0;
if (!full) {
short_post = "";
} else {
Expand Down Expand Up @@ -487,7 +484,7 @@ int fpm_status_handle_request(void) /* {{{ */
}

now_epoch = time(NULL);
if (is_start_time_str) {
if (has_start_time) {
strftime(time_buffer, sizeof(time_buffer) - 1, time_format, localtime(&scoreboard.start_epoch));
spprintf(&buffer, 0, short_syntax,
scoreboard.pool,
Expand All @@ -508,7 +505,6 @@ int fpm_status_handle_request(void) /* {{{ */
spprintf(&buffer, 0, short_syntax,
scoreboard.pool,
PM2STR(scoreboard.pm),
(unsigned long long) scoreboard.start_epoch,
(unsigned long) (now_epoch - scoreboard.start_epoch),
scoreboard.requests,
scoreboard.lq,
Expand Down
39 changes: 18 additions & 21 deletions sapi/fpm/tests/status.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,21 +209,18 @@ class Status
$pattern = "|# HELP phpfpm_up Could pool " . $fields['pool'] . " using a " . $fields['process manager'] . " PM on PHP-FPM be reached\?\n" .
"# TYPE phpfpm_up gauge\n" .
"phpfpm_up 1\n" .
"# HELP phpfpm_start_time When FPM has started\.\n" .
"# TYPE phpfpm_start_time gauge\n" .
"phpfpm_start_time \d+\n" .
"# HELP phpfpm_start_since_total The number of seconds since FPM has started\.\n" .
"# TYPE phpfpm_start_since_total counter\n" .
"phpfpm_start_since_total " . $fields['start since'] . "\n" .
"# HELP phpfpm_accepted_connections_total The number of requests accepted by the pool\.\n" .
"# TYPE phpfpm_accepted_connections_total counter\n" .
"phpfpm_accepted_connections_total " . $fields['accepted conn'] . "\n" .
"# HELP phpfpm_start_since The number of seconds since FPM has started\.\n" .
"# TYPE phpfpm_start_since counter\n" .
"phpfpm_start_since " . $fields['start since'] . "\n" .
"# HELP phpfpm_accepted_connections The number of requests accepted by the pool\.\n" .
"# TYPE phpfpm_accepted_connections counter\n" .
"phpfpm_accepted_connections " . $fields['accepted conn'] . "\n" .
"# HELP phpfpm_listen_queue The number of requests in the queue of pending connections\.\n" .
"# TYPE phpfpm_listen_queue gauge\n" .
"phpfpm_listen_queue " . $fields['listen queue'] . "\n" .
"# HELP phpfpm_max_listen_queue_total The maximum number of requests in the queue of pending connections since FPM has started\.\n" .
"# TYPE phpfpm_max_listen_queue_total counter\n" .
"phpfpm_max_listen_queue_total " . $fields['max listen queue'] . "\n" .
"# HELP phpfpm_max_listen_queue The maximum number of requests in the queue of pending connections since FPM has started\.\n" .
"# TYPE phpfpm_max_listen_queue counter\n" .
"phpfpm_max_listen_queue " . $fields['max listen queue'] . "\n" .
"# TYPE phpfpm_listen_queue_length gauge\n" .
"# HELP phpfpm_listen_queue_length The size of the socket queue of pending connections\.\n" .
"phpfpm_listen_queue_length " . $fields['listen queue len'] . "\n" .
Expand All @@ -236,15 +233,15 @@ class Status
"# HELP phpfpm_total_processes The number of idle \+ active processes\.\n" .
"# TYPE phpfpm_total_processes gauge\n" .
"phpfpm_total_processes " . $fields['total processes'] . "\n" .
"# HELP phpfpm_max_active_processes_total The maximum number of active processes since FPM has started\.\n" .
"# TYPE phpfpm_max_active_processes_total counter\n" .
"phpfpm_max_active_processes_total " . $fields['max active processes'] . "\n" .
"# HELP phpfpm_max_children_reached_total The number of times, the process limit has been reached, when pm tries to start more children \(works only for pm 'dynamic' and 'ondemand'\)\.\n" .
"# TYPE phpfpm_max_children_reached_total counter\n" .
"phpfpm_max_children_reached_total " . $fields['max children reached'] . "\n" .
"# HELP phpfpm_slow_requests_total The number of requests that exceeded your 'request_slowlog_timeout' value\.\n" .
"# TYPE phpfpm_slow_requests_total counter\n" .
"phpfpm_slow_requests_total " . $fields['slow requests'] . "|";
"# HELP phpfpm_max_active_processes The maximum number of active processes since FPM has started\.\n" .
"# TYPE phpfpm_max_active_processes counter\n" .
"phpfpm_max_active_processes " . $fields['max active processes'] . "\n" .
"# HELP phpfpm_max_children_reached The number of times, the process limit has been reached, when pm tries to start more children \(works only for pm 'dynamic' and 'ondemand'\)\.\n" .
"# TYPE phpfpm_max_children_reached counter\n" .
"phpfpm_max_children_reached " . $fields['max children reached'] . "\n" .
"# HELP phpfpm_slow_requests The number of requests that exceeded your 'request_slowlog_timeout' value\.\n" .
"# TYPE phpfpm_slow_requests counter\n" .
"phpfpm_slow_requests " . $fields['slow requests'] . "|";

if (!preg_match($pattern, $body)) {
echo "ERROR: Expected body does not match pattern\n";
Expand Down