Skip to content

Rename and fix metric names and types #182

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

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Rename and fix metric names and types #182

wants to merge 1 commit into from

Conversation

@roidelapluie
Copy link

@roidelapluie roidelapluie commented Oct 12, 2021

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

itcsoft54 commented Oct 12, 2021

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

roidelapluie commented Oct 12, 2021

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

gerardnico commented Sep 20, 2024

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

gerardnico commented Sep 20, 2024

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

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@roidelapluie @itcsoft54 @gerardnico