Skip to content

Conversation

lmchilton
Copy link
Contributor

Resolves git issue #2017

Updated pcp dstat tool to not rely on hard coded mappings between instance filtering command line options and the specific plugin they apply to

Updated pcp dstat tool to not rely on hard coded mappings between
instance filtering command line options and the specific plugin they
apply to
@wcohen
Copy link
Contributor

wcohen commented May 7, 2025

Read #2017 and #2016 to get an understanding what the patch is addressing. The patch looks sane. It would be really good to have a test in qa directory to verify that the some filtering is working. It looks like qa/1187 would be a likely place to include such a test. Ideally, would check that #2016 is addressed, but that would require some additional archived data. To keep it simple maybe filter existing archive data for cpu usage.

One thing I noticed is that the filtertype ended up being the plugin file name. Are there going to be any cases where the filtertype is going to be different that the plugin name?

@natoscott
Copy link
Member

| One thing I noticed is that the filtertype ended up being the plugin file name.
| Are there going to be any cases where the filtertype is going to be different that the plugin name?

I think this is a safe assumption to make at this stage. Only future plugin/filter I can think of is for GPUs, but I expect they'll each require a separate file (a bit like disk vs dm vs md I guess).

@wcohen
Copy link
Contributor

wcohen commented May 21, 2025

For testing it would be good to build an RPM with this patch installed, install the generated RPMs, and test out on the dstat test group with:

cd /var/lib/pcp/testsuite/
sudo ./check -g dstat

Looking through the patch it seems odd that the plugins have "filtertype" but the pcp-dstat.py is looking for "filter". Is that correct? When trying to try this out for future extentions to support dstat monitoring of GPUs (#2095) changed pcp-dstat.py to look for "filtertype", but got:

$ dstat -c
Traceback (most recent call last):
  File "/usr/bin/dstat", line 614, in prepare_plugins
    found = config.read(paths)
  File "/usr/lib64/python3.13/configparser.py", line 735, in read
    self._read(fp, filename)
    ~~~~~~~~~~^^^^^^^^^^^^^^
  File "/usr/lib64/python3.13/configparser.py", line 1050, in _read
    ParsingError._raise_all(self._read_inner(fp, fpname))
                            ~~~~~~~~~~~~~~~~^^^^^^^^^^^^
  File "/usr/lib64/python3.13/configparser.py", line 1079, in _read_inner
    self._handle_rest(st, line, fpname)
    ~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^
  File "/usr/lib64/python3.13/configparser.py", line 1103, in _handle_rest
    raise MissingSectionHeaderError(fpname, st.lineno, line)
configparser.MissingSectionHeaderError: File contains no section headers.
file: '/etc/pcp/dstat/GNUmakefile', line: 15
'TOPDIR = ../../../..\n'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/bin/dstat", line 1933, in <module>
    dstat = DstatTool(sys.argv[1:])
  File "/usr/bin/dstat", line 537, in __init__
    configs = self.prepare_plugins()
  File "/usr/bin/dstat", line 616, in prepare_plugins
    sys.stderr.write("Config parse failure: %s\n" % cfgerr.message())
                                                    ~~~~~~~~~~~~~~^^
TypeError: 'str' object is not callable

@natoscott
Copy link
Member

| sudo ./check -g dstat

Just for info, the testsuite installs a 'pcpqa' user and some tests do not correctly as root - I tend to "su - pcpqa" (which also conveniently lands you in /var/lib/pcp/testsuite) and from there it's just "./check ..."

@wcohen
Copy link
Contributor

wcohen commented May 29, 2025

The earlier error message was due to a GNUMakefile in the pcp dstat plugin directory. Eliminating that issue uncovered some additional things that need to be addressed in the patch:

  • The attribute used in the plugins is "filtertype", but the check in prepare_metrics() uses is 'filter'. Changed in pcp-dstat.py to 'filtertype' to make things agree.
  • There is a ordering issue where the filter (or filtertype) attribute in the plugin and grouptype options both set the grouptype member. In the prepatched code the filtering is set after the grouptype option is set. However, in the patch the filter gets overwritten by the later grouptype option in the plugins. This can be observed when using "dstat --cpu --full" and the printout just has the total rather than individual CPU entries.
  • There should be some update to documentation in src/pcp/dstat/pcp-dstat.5 describing the filter option.

@lmchilton
Copy link
Contributor Author

See new PR #2218

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants