Export http_host tag with PHP-FPM metrics #3074
Conversation
The current PHP-FPM integration provides no mechanism in which to group by anything beyond `host` and `ping_url` for the `php_fpm.can_ping` service check. Allow clients to also group by the `http_host` in circumstances where the `ping_url` is identical (e.g. localhost) on the same host (server).
The current PHP-FPM mechanism provides no mechanism to associate a metric with the `http_host` that was specified to enact the check. This can be useful in circumstances when a single server has multiple FPM pools. Notably this data can be discerned from the extant `pool` tag, however it is quite possible that the pool name and the host header can be entirely different.
|
Hey @toksvaeth! Thanks for the PR, this change looks useful. Regarding the question you had about the conditional statement for submitting the We're currently transitioning our checks to a new SDK-based model for the agent and have imposed a freeze on the dd-agent repo. I see there is a small merge conflict, so I took the liberty of creating #3165 with these changes. I will close this PR and merge #3165 so the change will be included when this check is moved to the new SDK repo. Fyi, the SDK consists of two repositories integrations-core and integrations-extras, where all of our checks will reside. Thanks again for your contribution! |
What does this PR do?
This PR builds upon #2779.
It exports the newly added
http_hostoption as a tag for both the PHP-FPMservice check and the metric data.
Motivation
The author's use case in the original PR was for a single site behind a load
balancer that may be exposed by only a distinct host URL that wasn't
localhost.My current use case differs slightly. I have a host with multiple vhosts, each
with its own FPM pool defined. The FPM status page for each vhost listens on
http://localhost/php-fpm-status. The newly added
http_hostoption allows meto define multiple instances that each talk to a different vhost/pool by way of
the host header whilst hitting the same
ping_url.The current integration allows you to group checks based on
ping_url. In mycircumstance, this is value identical for every pool. This means that if just
one of the pool checks fails, the check hits its critical threshold without any
indication to the operator as to which pool or instance failed. Allowing the
option to group by the
http_hosttag allows the operator to identify thesource of the problem easily.
The first commit exposes this tag via the service check.
The second commit exposes this tag with the metric data also. I have made this
amendment for consistency such that an operator can query on this tag for
relevant metrics in future.
Testing Guidelines
I have tested this as a custom service check however have not executed your
test harness.
Additional Notes
I am unsure as to the conditional statements and am willing to amend if
necessary.
I felt it better omit the tag rather than supply one with a null value. I am
unsure what preferred practice is.
I am also aware that the
pooltag is already exported with the metrics.Whilst an operator can potentially discern the right pool exhibiting a problem
by way of the
pooltag, I felt it wise to include the extra tag for thefollowing reasons:
pooltag is not always populated. This tag is gathered from theresponse that the FPM extended status page returns. If the request to the
status page fails (an error condition I am facing now), the tag is exported
with no value.
http_hosttag allows for easier metric queries later thatare consistent between the service check data and metric data.
Additionally, I am unsure if the Datadog UI will require any amendments because
of this change. The PHP-FPM integration page appears to lock down the
'grouping' clauses to
hostandping_urlwith no option for adjustment.