-
Notifications
You must be signed in to change notification settings - Fork 486
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
feat(exporter/windows): expose physical_disk collector #5927
feat(exporter/windows): expose physical_disk collector #5927
Conversation
Signed-off-by: hainenber <[email protected]>
Signed-off-by: hainenber <[email protected]>
Signed-off-by: hainenber <[email protected]>
Signed-off-by: hainenber <[email protected]>
We should decide a policy going forward on it. Technically we have said that changing metrics is not a breaking change. Do we have a feel for how many metrics it creates? |
There are 11 new metrics if this is enabled by default. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor doc suggestion
docs/sources/static/configuration/integrations/windows-exporter-config.md
Outdated
Show resolved
Hide resolved
docs/sources/static/configuration/integrations/windows-exporter-config.md
Outdated
Show resolved
Hide resolved
…r-config.md Co-authored-by: Clayton Cornell <[email protected]>
…r-config.md Co-authored-by: Clayton Cornell <[email protected]>
Looks good enabled automerge once conflicts are resolved. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@mattdurham Automerge can't complete... there are conflicts. I've tried a quick resolve. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need a comma
adding a comma
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets check now
* feat(exporter/windows): expose physical_disk collector Signed-off-by: hainenber <[email protected]> * fix(exporter/windows): correct river attr name for physical_disk Signed-off-by: hainenber <[email protected]> * feat(exporter/windows): set default args for physical_disk attr Signed-off-by: hainenber <[email protected]> * feat(exporter/windows): update unit test + integration doc Signed-off-by: hainenber <[email protected]> * Update docs/sources/static/configuration/integrations/windows-exporter-config.md Co-authored-by: Clayton Cornell <[email protected]> * Update docs/sources/static/configuration/integrations/windows-exporter-config.md Co-authored-by: Clayton Cornell <[email protected]> --------- Signed-off-by: hainenber <[email protected]> Co-authored-by: Clayton Cornell <[email protected]>
PR Description
Which issue(s) this PR fixes
Fixes #5882
Notes to the Reviewer
The new collector is set as default by
windows_exporter
. Should we follow the course or keep it as non-default to keep it simpler?PR Checklist