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.

Sign up for GitHub

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

Jump to bottom

Update string parsing of allocatable cpu cores in kube_inventory #8512

Merged
merged 6 commits into from
Dec 10, 2020
Merged

Update string parsing of allocatable cpu cores in kube_inventory #8512

merged 6 commits into from
Dec 10, 2020

Conversation

j03wang
Copy link
Contributor

@j03wang j03wang commented Dec 3, 2020 •
edited
Loading

The Allocatable cpu value returned from Kubernetes can have a milli suffix, which gets parsed as 0 when stored in allocatable_cpu_cores.

This change updates the metric to be millicpus to prevent truncation of decimals.

Required for all PRs:

  • Signed CLA.
  • Associated README.md updated.
  • Has appropriate unit tests.

@srebhan srebhan self-assigned this Dec 4, 2020
srebhan
srebhan approved these changes Dec 4, 2020
View reviewed changes
Copy link
Member

@srebhan srebhan left a comment

Choose a reason for hiding this comment

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

Looks good to me. I cannot test, so I take your word on it. :-)

@j03wang
Copy link
Contributor Author

j03wang commented Dec 4, 2020 •
edited
Loading

Now that I think about it, would it make sense to update the metric itself to be "allocatable_millicpu_cores" and scale the value by 1000? Otherwise decimal values would be rounded/truncated.

@sjwang90 sjwang90 added area/k8s feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin labels Dec 8, 2020
srebhan
srebhan approved these changes Dec 9, 2020
View reviewed changes
Copy link
Member

@srebhan srebhan left a comment

Choose a reason for hiding this comment

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

Also fine with me. :-)

@srebhan srebhan added the ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review. label Dec 9, 2020
@j03wang j03wang requested a review from reimda December 10, 2020 01:00
@reimda reimda merged commit 99287d8 into influxdata:master Dec 10, 2020
ssoroka pushed a commit that referenced this pull request Dec 16, 2020
@j03wang @ssoroka
Update string parsing of allocatable cpu cores in kube_inventory (#8512)
12a28b1 
(cherry picked from commit 99287d8)
arstercz pushed a commit to arstercz/telegraf that referenced this pull request Mar 5, 2023
@j03wang
Update string parsing of allocatable cpu cores in kube_inventory (inf…
33b3784 
…luxdata#8512)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@srebhan srebhan srebhan approved these changes

@reimda reimda reimda approved these changes

Assignees

@srebhan srebhan

Labels
area/k8s feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@j03wang @reimda @srebhan @sjwang90