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

Fix deployment count metric #8247

Merged
merged 3 commits into from
Dec 23, 2020
Merged

Conversation

mfpierre
Copy link
Contributor

What does this PR do?

Count the number of objects instead of summing up values for certain count metrics.
Otherwise the deployment count metric would sum up the number of deployment generations

Motivation

Fix deployment count metric

Additional Notes

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • PR title must be written as a CHANGELOG entry (see why)
  • Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
  • PR must have changelog/ and integration/ labels attached

@mfpierre mfpierre force-pushed the mfpierre/split-count-object-values branch from 5cd90a2 to 83544ff Compare December 23, 2020 13:08
@mfpierre mfpierre requested a review from a team December 23, 2020 14:01
xlucas
xlucas previously approved these changes Dec 23, 2020
Copy link
Member

@xlucas xlucas left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -869,7 +867,7 @@ def kube_limitrange(self, metric, scraper_config):
else:
self.log.error("Metric type %s unsupported for metric %s", metric.type, metric.name)

def count_objects_by_tags(self, metric, scraper_config):
def count_values_by_tags(self, metric, scraper_config):
""" Count objects by allowed tags and submit counts as gauges. """
Copy link
Member

Choose a reason for hiding this comment

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

Nit: could rephrase the comment to mention the difference between count_objects and count_values

Copy link
Contributor Author

Choose a reason for hiding this comment

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

renamed to sum_values_by_tags to be clearer

@mfpierre mfpierre force-pushed the mfpierre/split-count-object-values branch from a2cb1fb to 0cd89f6 Compare December 23, 2020 14:44
@mfpierre mfpierre requested a review from xlucas December 23, 2020 15:03
@ChristineTChen ChristineTChen merged commit 966da07 into master Dec 23, 2020
@ChristineTChen ChristineTChen deleted the mfpierre/split-count-object-values branch December 23, 2020 15:57
ChristineTChen pushed a commit that referenced this pull request Dec 23, 2020
* Split count_objects_by_tags from count_values_by_tags

* Assert on count values

* Update docstring
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.

3 participants