Skip to content

Conversation

@njam
Copy link
Contributor

@njam njam commented Feb 21, 2018

Copy link
Contributor

@brian-brazil brian-brazil left a comment

Choose a reason for hiding this comment

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

Why are you unregistering the processes and Go collectors? You still want to monitor the exporter itself.

Version information should not be added as a label on gearman_up, that'd be quite annoying to deal with. Put the label on a separate metric.

gearman_status_total is a gauge, and should thus not end in _total. Something like gearman_jobs_queued would be a better name.

* [MQTT blackbox exporter](https://github.com/inovex/mqtt_blackbox_exporter)
* [RabbitMQ exporter](https://github.com/kbudde/rabbitmq_exporter)
* [RabbitMQ Management Plugin exporter](https://github.com/deadtrickster/prometheus_rabbitmq_exporter)
* [Gearman exporter](https://github.com/bakins/gearman-exporter)
Copy link
Contributor

Choose a reason for hiding this comment

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

Keep it sorted.

@njam
Copy link
Contributor Author

njam commented Feb 21, 2018

Thanks for the feedback! I will fix it and then get back to you.

@njam
Copy link
Contributor Author

njam commented Feb 21, 2018

gearman_jobs_queued could be a bit misleading, as the metric actually represents all non-finished jobs, the ones queued for execution as well as the ones currently being executed.
I would suggest the following names:

  • gearman_running_jobs
  • gearman_total_jobs (while I don't like "total" very much, it is well established from gearman)
  • gearman_workers

@brian-brazil wdyt?

njam added a commit to njam/gearman-exporter that referenced this pull request Feb 21, 2018
@brian-brazil
Copy link
Contributor

gearman_total_jobs (while I don't like "total" very much, it is well established from gearman)

Everything in metrics is some form of total, it rarely adds anything to a metric name.

_jobs and _jobs_running would be the usual way to organise that.

njam added a commit to njam/gearman-exporter that referenced this pull request Feb 21, 2018
njam added a commit to njam/gearman-exporter that referenced this pull request Feb 24, 2018
@njam
Copy link
Contributor Author

njam commented Feb 24, 2018

@brian-brazil I went with your suggestion. Could you take another look?

@brian-brazil brian-brazil merged commit 00a294c into prometheus:master Feb 26, 2018
@brian-brazil
Copy link
Contributor

You still have the version label on up, that should be moved to a different metric.

@njam
Copy link
Contributor Author

njam commented Feb 26, 2018

I split it into two metrics, as you recommend in your blogpost:

gearman_up 1
gearman_version_info{version="1.1.18"} 1

Did I miss some place to update it?

@brian-brazil
Copy link
Contributor

Odd, it was missing when I had last looked.

aylei pushed a commit to aylei/docs that referenced this pull request Oct 28, 2019
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.

2 participants