-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
[receiver/k8sclusterreceiver] Fix k8s node and container cpu metrics not being reported properly #8245
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dmitryax
reviewed
Mar 5, 2022
Rebased this feature branch and pulled the latest changes from main, all my changes for PR responses ended up getting merged into the first commit during the rebase. |
…to be reported as double values
…ith the feature gate enabled
dmitryax
reviewed
Mar 9, 2022
dmitryax
reviewed
Mar 10, 2022
dmitryax
approved these changes
Mar 10, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
A bug has been reported where the k8sclusterreceiver has been reporting k8s node and container metrics improperly. The metrics k8s.container.cpu_request, k8s.container.cpu_limit, and k8s.node.allocatable_cpu should be reported as double values according to opentelemetry cpu metric specifications .
Fixing these type of bugs is likely a breaking change to some users, so the code fixes have been added behind a feature gate that is disabled by default. In the future, we will enable this feature gate by default and eventually remove the feature gate entirely.
Link to tracking Issue:
#8115
Testing:
Unit tests have been added. Manually testing in a Kubernetes cluster was successful.
Documentation:
A configuration entry was add to the k8sclusterreceiver README.md.
An entry was added to the CHANGELOG.md. For this PR since the feature gate is disabled by default, so we don't have to include a "Breaking changes" entry. In a future PR when the feature gate is enable by default, we will include a "Breaking changes" entry in the CHANGELOG.md.
Logging was added to notify users about the new feature gate and how the k8sclusterreceiver is being transitioned.
Notes:
I recommend looking at the first commit in this PR to see the main changes. The later commits are for adding unit tests that required refactoring some test util methods.