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 kubernetes_state avoid tags collision #4149

Merged
merged 1 commit into from
Jul 19, 2019

Conversation

clamoriniere
Copy link
Contributor

What does this PR do?

Avoid tags collision for the "namespace"

Motivation

What inspired you to submit this pull request?

Additional Notes

Anything else we should know when reviewing?

Review checklist (to be filled by reviewers)

  • 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
  • Feature or bugfix must have tests
  • Git history must be clean
  • If PR adds a configuration option, it must be added to the configuration file.

@clamoriniere clamoriniere requested a review from a team July 18, 2019 16:16
@clamoriniere clamoriniere requested a review from a team as a code owner July 18, 2019 16:16
@clamoriniere clamoriniere force-pushed the clamoriniere/fix_kubernetes_state_telemetry branch from 72fe446 to 3dca574 Compare July 18, 2019 16:29
@codecov
Copy link

codecov bot commented Jul 18, 2019

Codecov Report

Merging #4149 into master will decrease coverage by 3.9%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #4149      +/-   ##
==========================================
- Coverage   86.71%   82.81%   -3.91%     
==========================================
  Files         743        4     -739     
  Lines       39233      384   -38849     
  Branches     4677       76    -4601     
==========================================
- Hits        34020      318   -33702     
+ Misses       3942       45    -3897     
+ Partials     1271       21    -1250

Copy link
Member

@hkaj hkaj left a comment

Choose a reason for hiding this comment

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

Is it ever a problem? The problem is that namespace is generic and may end up conflicting with a host tag, or a tag from the cloud provider.

CharlyF
CharlyF previously approved these changes Jul 18, 2019
@clamoriniere
Copy link
Contributor Author

@hkaj
we already use the kube_namespace tag for knowing on which kubernetes namespace the ksm pod is running.
So at least we do not conflict with one of our internal tag

@clamoriniere clamoriniere force-pushed the clamoriniere/fix_kubernetes_state_telemetry branch from 3dca574 to be47667 Compare July 19, 2019 08:37
@clamoriniere clamoriniere merged commit 43562c6 into master Jul 19, 2019
@ahmed-mez ahmed-mez deleted the clamoriniere/fix_kubernetes_state_telemetry branch July 19, 2019 09:34
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.

4 participants