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

prometheus-emitter: add extraLabels parameter #14728

Merged
merged 4 commits into from
Aug 29, 2023

Conversation

yianni
Copy link
Contributor

@yianni yianni commented Aug 1, 2023

Description

Building on previous work (#14659), this PR introduces the ability to add additional labels to Prometheus metrics, providing more flexibility for data management and identification. This change is particularly useful when managing various types of data across multiple Druid clusters.

The additional labels can be set using a new optional configuration parameter (druid.emitter.prometheus.extraLabels) added to the Prometheus emitter.

Changes Made

  • Added a new configuration parameter (druid.emitter.prometheus.extraLabels) to the Prometheus emitter.
  • Modified the PrometheusEmitter to include the extra labels as Prometheus labels if the configuration parameter is set.
  • Modified the unit tests to include and test this new functionality.

Release note

The Prometheus emitter now supports a new optional configuration parameter, druid.emitter.prometheus.extraLabels. This addition offers users the flexibility to add arbitrary extra labels to their Prometheus metrics, providing more granular control in managing and identifying data across multiple Druid clusters or other dimensions.


Key changed/added classes in this PR:

  • PrometheusEmitter
  • PrometheusEmitterConfig
  • PrometheusEmitterTest
  • PrometheusEmitterConfigTest

This PR has:

  • been self-reviewed.
  • added documentation for new or modified features or behaviors.
  • added a release note entry in the PR description.
  • added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
  • added integration tests.
  • been tested in a test Druid cluster.

@TSFenwick
Copy link
Contributor

Took a quick glance and it looks good, but need to actually review it tomorrow :)

Copy link
Contributor

@ektravel ektravel left a comment

Choose a reason for hiding this comment

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

Changes to prometheus.md look good.

@yianni
Copy link
Contributor Author

yianni commented Aug 11, 2023

@TSFenwick Thanks. Whenever you can dive deeper, would be great. Cheers!

Copy link
Contributor

@abhishekrb19 abhishekrb19 left a comment

Choose a reason for hiding this comment

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

The approach looks good to me. I left a few suggestions.

@yianni
Copy link
Contributor Author

yianni commented Aug 28, 2023

The approach looks good to me. I left a few suggestions.

Thanks @abhishekrb19, I made the changes.

Copy link
Contributor

@abhishekrb19 abhishekrb19 left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you for this patch, @yianni!

@abhishekrb19 abhishekrb19 merged commit d201ea0 into apache:master Aug 29, 2023
74 checks passed
jakubmatyszewski pushed a commit to jakubmatyszewski/druid that referenced this pull request Sep 8, 2023
* prometheus-emitter: add extraLabels parameter

* prometheus-emitter: update readme to include the extraLabels parameter

* prometheus-emitter: remove nullable and surface label name issues

* remove import to make linter happy
@LakshSingla LakshSingla added this to the 28.0 milestone Oct 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants