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

Improves tagging compliancy #5105

Merged
merged 5 commits into from
Nov 29, 2019

Conversation

clamoriniere
Copy link
Contributor

@clamoriniere clamoriniere commented Nov 28, 2019

What does this PR do?

This change aims to introduce the kubernetes check tag set in the kubernetes_state
check.

To ease the user migration to the new tag names. This commit also introduces a new
parameter ignore_autodiscovery_tags to have both tag sets on kubernetes_state metrics.
Currently, ignore_autodiscovery_tags is set to True by default.

Motivation

The kubernetes_state and kubernetes checks were using a different set of tag names
to convey the same information.

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

@clamoriniere clamoriniere self-assigned this Nov 28, 2019
@clamoriniere clamoriniere requested a review from a team November 28, 2019 10:22
@clamoriniere clamoriniere marked this pull request as ready for review November 28, 2019 10:33
@clamoriniere clamoriniere requested a review from a team as a code owner November 28, 2019 10:33
@clamoriniere clamoriniere requested a review from therve November 28, 2019 10:33
@clamoriniere clamoriniere force-pushed the clamoriniere/ksm-tags branch 2 times, most recently from 89b809f to c0c308b Compare November 28, 2019 11:05
The `kubernetes_state` and `kubernetes` checks were using a different set of tag names
to convey the same information.

This change aims to introduce the `kubernetes` check tag set in the `kubernetes_state`
check.

To ease the user migration to the new tag names. This commit also introduces a new
parameter `keep_ksm_labels` to have both tag sets on `kubernetes_state` metrics.
Currently, `keep_ksm_labels` is set to `True` by default.

Add `ignore_listener_tags` in `auto_conf` file to remove the autodiscovery listener
tags from the check config.
hkaj
hkaj previously approved these changes Nov 28, 2019
Copy link
Contributor

@therve therve left a comment

Choose a reason for hiding this comment

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

A few minor nits.

Redefine this method to allow labels duplication, during migration phase
"""
custom_tags = scraper_config['custom_tags']
_tags = list(custom_tags)
Copy link
Contributor

Choose a reason for hiding this comment

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

tags.

Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't look like it? :). No worries though.

@clamoriniere
Copy link
Contributor Author

@therve thank you for your review, I addressed all your comments. It would be great ff you can do a second review.

…f.yaml

Co-Authored-By: Haïssam Kaj <hkaj@users.noreply.github.com>
@clamoriniere clamoriniere requested a review from therve November 29, 2019 12:57
Redefine this method to allow labels duplication, during migration phase
"""
custom_tags = scraper_config['custom_tags']
_tags = list(custom_tags)
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't look like it? :). No worries though.

@clamoriniere clamoriniere merged commit 119baea into DataDog:master Nov 29, 2019
@clamoriniere clamoriniere deleted the clamoriniere/ksm-tags branch November 29, 2019 13:09
@therve therve changed the title [kubernetes_state] improves tagging compliancy Improves tagging compliancy Dec 2, 2019
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