[receiver/dockerstats] Disable container.cpu.usage.system by default, and expand description#14558
Merged
dmitryax merged 2 commits intoOct 15, 2022
Merged
Conversation
Contributor
Author
bogdandrutu
reviewed
Sep 28, 2022
bogdandrutu
reviewed
Oct 7, 2022
bogdandrutu
left a comment
Member
There was a problem hiding this comment.
@open-telemetry/collector-contrib-approvers how do you want to do this?
Contributor
|
@bogdandrutu , not an approver but as I understand that this change is a low risk due to the fact that receiver has yet to adopt the version 2 of the scraper as the default. (Version 2 captures metrics using the generated metrics file) |
Member
|
IMHO since the stability of this feature gate is alpha we just need to list the change as breaking and move on. The whole point of feature gates is to allow doing breaking changes more quickly, we should make clear that this is breaking but that's about it. |
Member
|
If it's behind a feature gate that is disabled by default, I believe we can move on |
dmitryax
approved these changes
Oct 15, 2022
shalper2
pushed a commit
to shalper2/opentelemetry-collector-contrib
that referenced
this pull request
Dec 6, 2022
… and expand description (open-telemetry#14558)
Closed
This file contains hidden or 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
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:
container.cpu.usage.systemis no longer a default metric.My rationale behind disabling this as a default metric, is that it has the potential to be misleading unless the user has explicitly read the description. It also duplicates system CPU metrics emitted by the
hostmetricsreceiver. I thought about completely removing it, but I thought I better leave it in since it is something that docker reports.Change is technically breaking, but only behind the non-default featuregate. I don't expect it will impact any users.
Link to tracking Issue: #9794
Testing:
Tests were updated to test for the updated description
Documentation:
Go generate / mdatagen updates the docs.