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

Provide a config knob to disable collecting root Cgroup stats #2259 #2283

Merged
merged 1 commit into from
Aug 11, 2019
Merged

Provide a config knob to disable collecting root Cgroup stats #2259 #2283

merged 1 commit into from
Aug 11, 2019

Conversation

sashankreddya
Copy link
Collaborator

@sashankreddya sashankreddya changed the title Provide a config knob to disable collecting root Cgroup stats #2259: Provide a config knob to disable collecting root Cgroup stats Aug 7, 2019
@sashankreddya sashankreddya changed the title #2259: Provide a config knob to disable collecting root Cgroup stats Provide a config knob to disable collecting root Cgroup stats #2259 Aug 7, 2019
@sashankreddya
Copy link
Collaborator Author

@dashpole : Looks like some test eco system issue in pulling changes (pull-cadvisor-e2e).

@sashankreddya
Copy link
Collaborator Author

/retest

@dashpole
Copy link
Collaborator

dashpole commented Aug 9, 2019 

The following files are not properly formatted:
container/raw/factory.go container/raw/factory.go

@sashankreddya sashankreddya force-pushed the disable_root_cgroup_knobs branch from 0e4ee62 to 03661bb Compare August 9, 2019 18:26
@sashankreddya sashankreddya force-pushed the disable_root_cgroup_knobs branch from 03661bb to 249a1d5 Compare August 9, 2019 18:28
@sashankreddya
Copy link
Collaborator Author 

The following files are not properly formatted:
container/raw/factory.go container/raw/factory.go

Fixed it.

@dashpole dashpole self-assigned this Aug 9, 2019
dashpole
dashpole reviewed Aug 9, 2019
View reviewed changes
Copy link
Collaborator

@dashpole dashpole left a comment

Choose a reason for hiding this comment

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

This is probably fine to do, although I am still not entirely sure what functionality we lose when this is set. As a follow-up, please add documentation for the flag and include an assessment of which metrics stop working when this is set.

container/raw/handler.go Show resolved Hide resolved
dashpole
dashpole approved these changes Aug 11, 2019
View reviewed changes
Copy link
Collaborator

@dashpole dashpole left a comment

Choose a reason for hiding this comment

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

lgtm. Please follow this up with documentation for the flag.

@dashpole dashpole merged commit d200c9f into google:master Aug 11, 2019
@trallnag
Copy link
Contributor

@dashpole documentation on disable_root_cgroup_stats option is still missing in https://github.com/google/cadvisor/blob/master/docs/runtime_options.md

trallnag pushed a commit to trallnag/cadvisor that referenced this pull request Sep 29, 2020
Add missing flag to runtime_options.md
ca75d4d 
Flag was merged in google#2283, but never added to the docs
@trallnag trallnag mentioned this pull request Sep 29, 2020
Merged
@dashpole dashpole mentioned this pull request Sep 29, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dashpole dashpole dashpole approved these changes

Assignees

@dashpole dashpole

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sashankreddya @dashpole @trallnag