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

introduce new chart for process states metrics #12305

Merged
merged 11 commits into from
Mar 8, 2022
Merged

introduce new chart for process states metrics #12305

merged 11 commits into from
Mar 8, 2022

Conversation

surajnpn
Copy link
Contributor

@surajnpn surajnpn commented Mar 3, 2022 •
edited
Loading

This commit introduces new chart for total number of processes in different states i.e running, sleeping_interruptible, sleeping_uninterruptible, zombie and stopped. This feature is implemented for Linux only. Closes #12139

12139: introduce new chart for process states metrics
6379c79 
This commit introduces new chart for total number of processes
in different states i.e running, sleeping, sleeping_d, zombie
and stopped.
@CLAassistant
Copy link

CLAassistant commented Mar 3, 2022 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot added area/collectors Everything related to data collection area/web labels Mar 3, 2022
@surajnpn surajnpn changed the title 12139: introduce new chart for process states metrics introduce new chart for process states metrics Mar 3, 2022
@surajnpn surajnpn removed the request for review from jacekkolasa March 3, 2022 10:11
@ilyam8
Copy link
Member

ilyam8 commented Mar 3, 2022

The current implementation creates 3 different charts with the same id

#/opt/netdata/usr/libexec/netdata/plugins.d# ./apps.plugin 2>/dev/null | grep "system.process_states"
CHART system.process_states '' 'Apps Process States' 'numbers' processes system.process_states line 601 1
CHART system.process_states '' 'Users Process States' 'numbers' processes system.process_states line 601 1
CHART system.process_states '' 'User Groups Process States' 'numbers' processes system.process_states line 601 1
BEGIN system.process_states 0
BEGIN system.process_states 0
BEGIN system.process_states 0

Should be only "Apps Process States".

@ilyam8
Copy link
Member

ilyam8 commented Mar 4, 2022 •
edited
Loading

@CodeGuru1 found another problem, we create the chart (the CHART line) on every data collection. We need to do it only once.

root@debian-s-1vcpu-1gb-fra1-01:/opt/netdata/usr/libexec/netdata/plugins.d# ./apps.plugin 2>/dev/null | grep "system.process_states"
CHART system.process_states '' 'Apps Process States' 'numbers' processes system.process_states line 601 1
BEGIN system.process_states 0
CHART system.process_states '' 'Apps Process States' 'numbers' processes system.process_states line 601 1
BEGIN system.process_states 1001073
CHART system.process_states '' 'Apps Process States' 'numbers' processes system.process_states line 601 1
BEGIN system.process_states 999863
CHART system.process_states '' 'Apps Process States' 'numbers' processes system.process_states line 601 1
BEGIN system.process_states 999706
CHART system.process_states '' 'Apps Process States' 'numbers' processes system.process_states line 601 1
BEGIN system.process_states 1000240

ilyam8
ilyam8 reviewed Mar 7, 2022
View reviewed changes
web/gui/dashboard_info.js Outdated Show resolved Hide resolved
collectors/apps.plugin/apps_plugin.c Outdated Show resolved Hide resolved
Suraj Neupane and others added 3 commits March 7, 2022 11:53
fixing comments
e7aef6e
@surajnpn @ilyam8
Update web/gui/dashboard_info.js
bd139e2 
Co-authored-by: Ilya Mashchenko <ilya@netdata.cloud>
@surajnpn @ilyam8
Update collectors/apps.plugin/apps_plugin.c
edbd79b 
Co-authored-by: Ilya Mashchenko <ilya@netdata.cloud>
ilyam8
ilyam8 reviewed Mar 7, 2022
View reviewed changes
collectors/apps.plugin/apps_plugin.c Outdated Show resolved Hide resolved
collectors/apps.plugin/apps_plugin.c Outdated Show resolved Hide resolved
@ilyam8
Copy link
Member

ilyam8 commented Mar 7, 2022

@CodeGuru1 let's change the order a bit: swap interruptible and uninterruptible. Because of the color - 2nd dimension is red.

ilyam8
ilyam8 reviewed Mar 7, 2022
View reviewed changes
web/gui/dashboard_info.js Outdated Show resolved Hide resolved
@ilyam8 @underhood
Update collectors/apps.plugin/apps_plugin.c
f3ef601 
Co-authored-by: Timotej S. <6674623+underhood@users.noreply.github.com>
ilyam8
ilyam8 previously approved these changes Mar 7, 2022
View reviewed changes
@ilyam8 ilyam8 requested a review from kickoke March 7, 2022 15:20
thiagoftsm
thiagoftsm previously approved these changes Mar 7, 2022
View reviewed changes
kickoke
kickoke reviewed Mar 7, 2022
View reviewed changes
Copy link
Contributor

@kickoke kickoke left a comment

Choose a reason for hiding this comment

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

I know that we are trying to have a short style in the descriptions, but omitting the subject of the sentence could lead to confusion.

Also, if we should ever truly become a "global" product and start to translate our UI, complete sentences are key to cost-efficient translation. While translation is still to be discussed, we should try to deliver high quality text the first time around. 😄

Thank you @ilyam8 for including me in these reviews!

web/gui/dashboard_info.js Outdated Show resolved Hide resolved
web/gui/dashboard_info.js Outdated Show resolved Hide resolved
web/gui/dashboard_info.js Outdated Show resolved Hide resolved
web/gui/dashboard_info.js Outdated Show resolved Hide resolved
web/gui/dashboard_info.js Outdated Show resolved Hide resolved
@ilyam8 @kickoke
Apply suggestions from code review
e57ed5d 
Co-authored-by: Tina Luedtke <kickoke@users.noreply.github.com>
@ilyam8 ilyam8 dismissed stale reviews from thiagoftsm and themself via e57ed5d March 7, 2022 19:36
thiagoftsm
thiagoftsm approved these changes Mar 7, 2022
View reviewed changes
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.

I am approving without to test, because only documentation was merged.

kickoke
kickoke approved these changes Mar 7, 2022
View reviewed changes
Copy link
Contributor

@kickoke kickoke left a comment

Choose a reason for hiding this comment

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

Docs: LGTM

@ilyam8
Copy link
Member

ilyam8 commented Mar 7, 2022

Close/open to retrigger stuck build jobs.

@ilyam8 ilyam8 closed this Mar 7, 2022
@ilyam8 ilyam8 reopened this Mar 7, 2022
@surajnpn surajnpn merged commit 6437515 into netdata:master Mar 8, 2022
@surajnpn surajnpn self-assigned this Mar 18, 2022
bmwiedemann pushed a commit to bmwiedemann/openSUSE that referenced this pull request May 17, 2022
@bmwiedemann
Update netdata to version 1.34.1 / rev 33 via SR 977544
7627c1b 
https://build.opensuse.org/request/show/977544
by user mia + dimstar_suse
- Update to 1.34.1 (go.d.plugin 0.32.3)
Collectors
  * New collectors
    + Add CPU throttling charts (cgroups.plugin)
      (gh#netdata/netdata#12591)
    + Add clock status chart (timex.plugin)
      (gh#netdata/netdata#12501)
    + Add Asterisk configuration file with synthetic charts
      (statsd.plugin)
      (gh#netdata/netdata#12381)
    + Add new chart for process states metrics (apps.plugin)
      (gh#netdata/netdata#12305)
    + Add thermal zone metrics collection (go.d/wmi)
      (gh#netdata/netdata#667)
    + Add SNMP data collector (go.d/snmp)
      (gh#netdata/netdata#644)
  * Improvements
    + Add 'locust' to apps_groups.conf
      (gh#netdata/netdata#12498)
    + Enable timex plugin for non-linux systems (timex.plugin)
      (gh#netdata/netdata#12489)
    + Prefer 'blkio.*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kickoke kickoke kickoke approved these changes

@ilyam8 ilyam8 ilyam8 approved these changes

@thiagoftsm thiagoftsm thiagoftsm approved these changes

@vlvkobal vlvkobal Awaiting requested review from vlvkobal

@underhood underhood Awaiting requested review from underhood

Assignees

@surajnpn surajnpn

Labels
area/collectors Everything related to data collection area/web Q1 GOAL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Feat]: Group processes based on state
7 participants
@surajnpn @CLAassistant @ilyam8 @underhood @kickoke @thiagoftsm @cpipilas