Skip to content
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.

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

Enable configuration of prometheus metrics namespace #4722

Merged

Conversation

jacobtomlinson
Copy link
Member

This PR adds the ability to configure the prometheus metric namespace prefix.

All applications should prefix their metrics with a consistent name. In most of our metrics we prefix them with dask_ but not always.

Adding a config option distributed.dashboard.prometheus.namespace which defaults to dask (the _ is added automatically) but allows users to override this if they have a system-specific requirement for this.

Given there are three collector classes which have had this added it may make sense to base class these and consolidate duplicated code in the future.

@jacobtomlinson
Copy link
Member Author

jacobtomlinson commented Apr 21, 2021

I've done a little more housekeeping here.

  • Added distributed.http.prometheus.PrometheusCollector base class for other collectors to use
  • Added a much simpler replacement of _build_full_name to the base class
  • Restructured some files/names/attributes to be consistent with each other
  • Moved large sections of code out of __init__.py files
  • Updated tests

@fjetter
Copy link
Member

fjetter commented Apr 21, 2021

cc @StephanErb @fgebhart this will break your dashboards ;)

@jacobtomlinson
Copy link
Member Author

jacobtomlinson commented Apr 21, 2021

Metrics being renamed in this PR are:

semaphore_max_leases                   -> dask_semaphore_max_leases
semaphore_active_leases                -> dask_semaphore_active_leases
semaphore_pending_leases               -> dask_semaphore_pending_leases
semaphore_acquire                      -> dask_semaphore_acquire
semaphore_release                      -> dask_semaphore_release
semaphore_average_pending_lease_time_s -> dask_semaphore_average_pending_lease_time_s

For backward compatibility folks could rename them to use the original names in the scrape config. If I were better at regex I expect you could do this in one rule.

- source_labels: [__name__]
  regex:  'dask_semaphore_max_leases'
  target_label: __name__
  replacement: 'semaphore_max_leases'
- source_labels: [__name__]
  regex:  'dask_semaphore_active_leases'
  target_label: __name__
  replacement: 'semaphore_active_leases'
- source_labels: [__name__]
  regex:  'dask_semaphore_pending_leases'
  target_label: __name__
  replacement: 'semaphore_pending_leases'
- source_labels: [__name__]
  regex:  'dask_semaphore_acquire'
  target_label: __name__
  replacement: 'semaphore_acquire'
- source_labels: [__name__]
  regex:  'dask_semaphore_release'
  target_label: __name__
  replacement: 'semaphore_release'
- source_labels: [__name__]
  regex:  'dask_semaphore_average_pending_lease_time_s'
  target_label: __name__
  replacement: 'semaphore_average_pending_lease_time_s'

@jacobtomlinson
Copy link
Member Author

CI failures appear related to #4719.

This is ready for final review and merge.

@jacobtomlinson
Copy link
Member Author

Many thanks for your input here @fjetter. I've updated those two.

Copy link
Member

@fjetter fjetter left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @jacobtomlinson . This is really nice

@jacobtomlinson jacobtomlinson merged commit 42e3f22 into dask:main Apr 22, 2021
@jacobtomlinson jacobtomlinson deleted the prometheus-configure-namespace branch April 22, 2021 15:33
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