Skip to content

Conversation

roidelapluie
Copy link

Here are a few changes that should bring this exporter closer to Prometheus best practices.

  • phpfpm_scrape_failures is a counter and should end with _total. It has been renamed to phpfpm_scrape_failures_total (https://prometheus.io/docs/instrumenting/writing_exporters/#naming)
  • phpfpm_start_since has been renamed to phpfpm_start_time_seconds to match standard process_start_time_seconds and contain unit. It has been changed to a gauge (https://www.robustperception.io/are-increasing-timestamps-counters-or-gauges).
  • phpfpm_accepted_connections is a counter and should end with _total. It has been renamed to phpfpm_accepted_connections_total.
  • phpfpm_idle_processes, phpfpm_active_processes have been changed to phpfpm_processes{state="active"} and phpfpm_processes{state="idle"}.
  • phpfpm_total_processes has been dropped because it can be found as sum without(state) phpfpm_processes.
  • phpfpm_max_children_reached is a counter and should end with _total, so it has been renamed to phpfpm_max_children_reached_total.
  • process_last_request_memory should contain the unit, so it has been renamed to process_last_request_memory_bytes.
  • process_request_duration has been normalized to seconds and renamed phpfpm_process_last_request_duration_seconds.

@itcsoft54
Copy link
Contributor

Rename metric create breaking change with existing use of phpfpm_exporter.
Need to deprecate old metric instead of remove (or rename) it

@roidelapluie
Copy link
Author

Certainly. The main problem is process_request_duration which now uses the normalized second (https://prometheus.io/docs/practices/naming/#base-units).

For the rest, we could offer recording rules or relabel configs to ease the transition and release a 3.0 maybe.

@stale
Copy link

stale bot commented Jan 8, 2022

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.

@stale stale bot added the wontfix This will not be worked on label Jan 8, 2022
@gerardnico
Copy link

All max metrics such as phpfpm_max_children_reached should be a gauge, no?
I don't understand why they are set as counter. They are not going up for every request and they can go down.

@stale stale bot removed the wontfix This will not be worked on label Sep 20, 2024
@gerardnico
Copy link

Note that on remote_write, the type of metrics metadata seems to be not send,
you can rename them:

- url: https://metric-api.newrelic.com/prometheus/v1/write?X-License-Key=...
  write_relabel_configs:
  - source_labels: [__name__]
    regex: ^my_counter$
    target_label: newrelic_metric_type
    replacement: "counter"
    action: replace

Ref

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants