Skip to content

fpm: Use recommended order for OpenMetrics stat metadata - #23871

Merged
TimWolla merged 3 commits into
php:masterfrom
1ma:fix-openmetrics-nit
Sep 25, 2026
Merged

TimWolla merged 3 commits into
php:masterfrom
1ma:fix-openmetrics-nit

Conversation

@1ma

@1ma 1ma commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

Extracted from #23808. I was asked by @TimWolla on Discord to submit this bit separately.

@1ma
1ma requested a review from bukka as a code owner September 23, 2026 22:26

@TimWolla TimWolla left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looking at https://prometheus.io/docs/specs/om/open_metrics_spec/#metricfamily-metadata:

The ordering SHOULD be TYPE, UNIT, HELP.

The order for phpfpm_listen_queue_length is the expected order and all the others are wrong.

And given this is a SHOULD, it is a master-only fix. I misunderstood that as part of the other PR.

@1ma
1ma changed the base branch from PHP-8.4 to master September 24, 2026 10:08
@1ma 1ma changed the title nit: fix label order of phpfpm_listen_queue_length metric nit: canonical order of OpenMetrics stats Sep 24, 2026
@1ma
1ma force-pushed the fix-openmetrics-nit branch from c349652 to 6a32172 Compare September 24, 2026 10:08
@1ma

1ma commented Sep 24, 2026

Copy link
Copy Markdown
Contributor Author

My bad, I should have checked the authoritative source like you did. Order fixed and target branch switched to master.

@1ma
1ma requested a review from TimWolla September 24, 2026 10:10
@TimWolla TimWolla changed the title nit: canonical order of OpenMetrics stats fpm: Use recommended order for OpenMetrics stat metadata Sep 25, 2026
@TimWolla
TimWolla merged commit ae9d9cd into php:master Sep 25, 2026
18 checks passed
@1ma
1ma deleted the fix-openmetrics-nit branch September 25, 2026 07:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants