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.

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

apps: disable reporting min/avg/max group uptime by default #11609

Merged
merged 5 commits into from
Oct 4, 2021

Conversation

ilyam8
Copy link
Member

@ilyam8 ilyam8 commented Oct 1, 2021

Summary

The min/avg/max group uptime charts are not really useful (unlike "carried over time uptime"), I think we need to disable them by default.

This PR disables reporting those charts by default. They can be enabled using with-detailed-uptime command-line option.

Component Name

collectors/apps.plugin

Test Plan

Run the plugin w/ and w/o with-detailed-uptime.

Additional Information

@github-actions github-actions bot added the area/collectors Everything related to data collection label Oct 1, 2021
@ilyam8 ilyam8 marked this pull request as ready for review October 1, 2021 16:35
@ilyam8 ilyam8 requested a review from vlvkobal as a code owner October 1, 2021 16:35
@ilyam8 ilyam8 requested a review from thiagoftsm October 1, 2021 17:08
@thiagoftsm
Copy link
Contributor

@ilyam8 code is working as expected, but before I approve this PR, I will suggest we change apps.plugin documentation saying what happens when we enable this option.

@ilyam8
Copy link
Member Author

ilyam8 commented Oct 1, 2021

@thiagoftsm I've checked the apps.plugin documentation (did it before marking the PR ready for review) and I see we don't really describe all the available options. We just briefly mention that there are some, use --help for more details.

See the end of the Configuration section. Moreover, as I mentioned in the OP, I consider min/avg/max uptime functionality very questionable and not really useful.

@thiagoftsm
Copy link
Contributor

@thiagoftsm I've checked the apps.plugin documentation (did it before marking the PR ready for review) and I see we don't really describe all the available options. We just briefly mention that there are some, use --help for more details.

See the end of the Configuration section. Moreover, as I mentioned in the OP, I consider min/avg/max uptime functionality very questionable and not really useful.

Yes you are right, apps.plugin is going in a different direction that we have for ebpf.plugin. @DShreve2 probably this is something we will need to change too.

thiagoftsm
thiagoftsm previously approved these changes Oct 1, 2021
Copy link
Contributor

@thiagoftsm thiagoftsm left a comment

Choose a reason for hiding this comment

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

Everything worked as expected. LGTM!

@ilyam8 ilyam8 requested a review from thiagoftsm October 4, 2021 08:35
@ilyam8 ilyam8 merged commit 28e65ca into netdata:master Oct 4, 2021
@ilyam8 ilyam8 deleted the apps_disable_details_uptime branch October 4, 2021 11:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/collectors Everything related to data collection
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants