hipages / php-fpm_exporter

A prometheus exporter for PHP-FPM.
Apache License 2.0
604 stars 123 forks source link

Return all combinations for phpfpm_process_state metric #123

Open matejzero opened 3 years ago

matejzero commented 3 years ago

At the moment, the phpfpm_process_state only exports the metric for the currently active state. That works OK in Prometheus, but has problems with VictoriaMetrics, which expects all metrics to be exported all the time.

Prometheus on missing metrics:

Avoid missing metrics Time series that are not present until something happens are difficult to deal with, as the usual simple operations are no longer sufficient to correctly handle them. To avoid this, export 0 (or NaN, if 0 would be misleading) for any time series you know may exist in advance. Most Prometheus client libraries (including Go, Java, and Python) will automatically export a 0 for you for metrics with no labels.

A good case is also presented by SuperQue.

This is easy with php-fpm, as we know all the states a process can be in.

But I think https://github.com/hipages/php-fpm_exporter/issues/110 needs to be solved before that. If not, it will make cardinality 5x worse.

matejzero commented 3 years ago

Now that https://github.com/hipages/php-fpm_exporter/pull/124 is merged, would you accept a PR for this issue?

This is something we need in order to be able to use php-fpm with VictoriaMetrics.

estahn commented 3 years ago

Now that #124 is merged, would you accept a PR for this issue?

@matejzero Yes, would be great if you can work on this. Otherwise, I may have a look over the coming week.

matejzero commented 3 years ago

My idea was to simply add a struct to exporter.go and raise the number based on state value. After that, iterate over struct keys and yield all metrics.

But since we already have all possible process states in php-fpm.go, we could work with these maybe?

I don't know, I'm not a coder and second part is over my head:) I'll do a PoC for the first case anyway, then I leave it up to you to decide what is the best and cleanest way to do that, as you have more experience:)

estahn commented 3 years ago

@matejzero I suggest counting the values within the loop you linked above and then add then below that loop. There is already a function doing the counting: CountProcessState

At the moment, the phpfpm_process_state only exports the metric for the currently active state.

Can you provide some examples? The exporter should export all states already: https://github.com/hipages/php-fpm_exporter/blame/1181deb197819a570e295890ec817b5823e37a27/phpfpm/exporter.go#L216

Are you saying you would like to see this:

phpfpm_process_state{child="0",state="idle"} 1
phpfpm_process_state{child="0",state="active"} 0
phpfpm_process_state{child="0",state="running"} 0
matejzero commented 3 years ago

I pushed an initial implementation.

Yes, I would like to see all states for all childs in the output. That way all metrics are exposed all the time which enables some calculations that depend on having those metrics available as makes VictoriaMetrics draw correct graphs as it doesn't have stallness markers like Prometheus.

estahn commented 3 years ago

@matejzero Thanks, I will have a look later today. Do you mind if I'm changing it to be more inline with how I would implement it?

matejzero commented 3 years ago

Please, be my guest!

matejzero commented 3 years ago

Is there any news regarding this issue? Thanks

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

matejzero commented 3 years ago

This would still be nice to implement. I'm using a custom fork from https://github.com/hipages/php-fpm_exporter/pull/128 to achieve this, but @estahn wanted a better implementation, which I'm not able to provide as I'm no coder:/

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

itcsoft54 commented 1 year ago

@estahn the PR #173 fix this issue, so we can close it, but bot has mark it at "wontfix"