Hi,
Specifying instances to report is hugely helpful with many metrics, the
best example is probably proc.* metrics where there's lots of instances
and the user might be interested in only one or two of them.
The patch below adds support to pmrep(1) to specify the instances to
report, either using globally defined list or instances per metric.
The implementation is relatively straightforward, a bit of gymnastics
needed to parse the command line vs config file right and to pass more
than one parameter to pmTraversePMNS().
QA updated with lots of tests, all current and new QA passing, shell
completions updated while at it, man pages updated as well.
There are two things that might require further thought:
1) Instance specification can actually be a regex but I'm not yet 100%
sure it's a good idea so for now I've left it undocumented, I think
it's best we gather a bit of experience and then either document it or
remove the regex support.
2) Reporting to archive ignore this in the spirit of pmlogger(1) but it
might make sense to respect the instance specification with archives as
well. There's been some discussion about this wrt pmlogger earlier,
perhaps we could try to find a common consensus for both:
https://bugzilla.redhat.com/show_bug.cgi?id=1345821
---
qa/1069 | 23 ++++++++++++
qa/1069.out | 51 ++++++++++++++++++++++++++
src/bashrc/pcp_completion.sh | 2 +-
src/pmrep/TODO | 6 ++--
src/pmrep/pmrep.1 | 65 ++++++++++++++++++++++++++++++---
src/pmrep/pmrep.conf | 1 +
src/pmrep/pmrep.conf.5 | 10 +++++-
src/pmrep/pmrep.py | 86 +++++++++++++++++++++++++++++++++++++++++---
src/zshrc/_pcp | 3 +-
9 files changed, 232 insertions(+), 15 deletions(-)
diff --git a/qa/1069 b/qa/1069
index 41955fc..c313901 100755
--- a/qa/1069
+++ b/qa/1069
@@ -160,6 +160,29 @@ pmrep -s 1 pmcd.version | sed -e
"s/$pcp_version/PCP_VERSION/g"
echo "== divide-by-zero error handling"
pmrep -t .01 -s 1 -e 'fail = sample.long.one / 0' fail
+echo "== user-requested instance handling"
+pmrep -s 1 -i wrong $log2 disk.dev.read
+pmrep -s 1 -i wrong $log2 mem.util.free
+pmrep -s 1 -i wrong $log2 disk.dev.read mem.util.free
+pmrep -s 1 -i wrong,sda $log2 disk.dev.read
+pmrep -s 1 -i wrong,sda,wrong $log2 disk.dev.read
+pmrep -s 1 -i '"1 minute",wrong,sda,"5 minute","still,wrong"' $log2
disk.dev.read kernel.all.load
+pmrep -s 1 -i '"1 minute",wrong,"5 minute"' $log2 kernel.all.load,,"1
minute",,,16
+pmrep -s 1 -i "1 minute" -i wrong -i "5 minute" $log2 kernel.all.load
+pmrep -s 1 $log2 disk.dev.read,,sda
+pmrep -s 1 $log2 disk.dev.read,,sda,,,16
+pmrep -s 1 $log2 disk.dev.read,,"'sda','sdb'"
+pmrep -s 1 $log2 disk.dev.read,,"'sda','sdb'",,,16
+echo '[test]' > $tmp.pconfig
+echo 'instances = wrong' >> $tmp.pconfig
+echo 'sysfork = kernel.all.sysfork' >> $tmp.pconfig
+echo 'mem.util.free = free,,,,16' >> $tmp.pconfig
+# We want: kernel.all.load = load,"'1 minute','5 minute','15 min'",,,16
+echo "kernel.all.load = load,\"'1 minute','5 minute','15 min'\",,,16" >>
$tmp.config
+echo 'disk.dev.read = read,sda,,,16' >> $tmp.config
+echo 'disk.dev.write = write,wrong,,,16' >> $tmp.config
+pmrep -s 1 $log2 :test
+
echo "== exercise option priority"
echo '[options]' > $tmp.pconfig
echo 'header = yes' >> $tmp.pconfig
diff --git a/qa/1069.out b/qa/1069.out
index f1b93d3..28169fb 100644
--- a/qa/1069.out
+++ b/qa/1069.out
@@ -252,6 +252,57 @@ Incompatible configuration file version (read v99, need
v1).
fail
inf
+== user-requested instance handling
+No matching instances found.
+
+Requested instances:
+wrong
+ m.u.free
+ Kbyte
+ 38220
+ m.u.free
+ Kbyte
+ 38220
+ d.d.read
+ sda
+ count/s
+ N/A
+ d.d.read
+ sda
+ count/s
+ N/A
+ d.d.read k.a.load k.a.load
+ sda 1 minute 5 minute
+ count/s
+ N/A 0.820 0.700
+ k.a.load
+ 1 minute
+
+ 0.820
+ k.a.load k.a.load
+ 1 minute 5 minute
+
+ 0.820 0.700
+ d.d.read
+ sda
+ count/s
+ N/A
+ d.d.read
+ sda
+ count/s
+ N/A
+ d.d.read d.d.read
+ sda sdb
+ count/s count/s
+ N/A N/A
+ d.d.read d.d.read
+ sda sdb
+ count/s count/s
+ N/A N/A
+ k.a.sysfork free load load read
+ 1 minute 5 minute sda
+ count/s Kbyte count/s
+ N/A 38220 0.820 0.700 N/A
== exercise option priority
m.u.free
Kbyte
diff --git a/src/bashrc/pcp_completion.sh b/src/bashrc/pcp_completion.sh
index cb33687..4531590 100644
--- a/src/bashrc/pcp_completion.sh
+++ b/src/bashrc/pcp_completion.sh
@@ -41,7 +41,7 @@ _pcp_complete()
;;
pmrep)
- opt_regex="-[AabcEeFfhKlOoPqSsTtwyZ]"
+ opt_regex="-[AaBbcCdDeEfFGhHiKLloOpPqQrsStTuUVwxyYzZ]"
curpos_expand=1
;;
diff --git a/src/pmrep/TODO b/src/pmrep/TODO
index 23f6cb4..317b4a7 100644
--- a/src/pmrep/TODO
+++ b/src/pmrep/TODO
@@ -1,4 +1,3 @@
-- allow defining instances to display [-i]
- opt to write cols per row with stdout output [-k]
- opt to swap cols and rows (e.g., compare ps vs sar)
- -Q/-B/-Y to override per-metric settings unit/scale
@@ -19,6 +18,7 @@
- includedir config file support (?)
- generalised daemon startup support (?)
- multi-sheet support for xlsx output (?)
+- consider using the csv module for csv output (?)
- add option to prevent truncating string output (?)
- generalized zabbix_interval (limit writes to archive) (?)
- support for multiple output targets (e.g. archive+stdout) (?)
@@ -27,10 +27,10 @@
-----
In use / reserved cmd line options:
-AaBbcCdDeEfFGhHKLloOpPqQrsStTuUVwxyYzZ
+AaBbcCdDeEfFGhHiKLloOpPqQrsStTuUVwxyYzZ
Tentatively planned:
-IikMmR
+IkMmR
Available:
gJjNnXvW
diff --git a/src/pmrep/pmrep.1 b/src/pmrep/pmrep.1
index 0489953..11b67dc 100644
--- a/src/pmrep/pmrep.1
+++ b/src/pmrep/pmrep.1
@@ -32,6 +32,7 @@
[\f3\-f\f1 \f2format\f1]
[\f3\-F\f1 \f2outfile\f1]
[\f3\-h\f1 \f2host\f1]
+[\f3\-i\f1 \f2instances\f1]
[\f3\-K\f1 \f2spec\f1]
[\f3\-l\f1 \f2delimiter\f1]
[\f3\-o\f1 \f2output\f1]
@@ -47,6 +48,22 @@
[\f3\-Z\f1 \f2timezone\f1]
\f2metricspec [...]\f1
.SH DESCRIPTION
+.de EX
+.in +0.5i
+.ie t .ft CB
+.el .ft B
+.ie t .sp .5v
+.el .sp
+.ta \\w' 'u*8
+.nf
+..
+.de EE
+.fi
+.ie t .sp .5v
+.el .sp
+.ft R
+.in
+..
.B pmrep
is a customizable performance metrics reporting tool.
Any available performance metric, live or archived, system and/or application,
@@ -149,7 +166,12 @@ and
see below).
The optional
.I instance
-definition is currently unimplemented.
+definition restricts
+.I csv
+and
+.I stdout
+reporting to the specified instances (so non-matching instances
+will be filtered).
An optional
.I unit/scale
is applicable for dimension-compatible, non-string, and (currently)
@@ -204,7 +226,7 @@ kernel.all.sysfork,forks,,,,8
.ft R
.in
.P
-The third form of a metricspec is valid only in
+The third form of a metricspec is described and valid only in
.BR pmrep.conf (5).
.P
Derived metrics are specified like PMNS leaf node metrics.
@@ -347,6 +369,43 @@ rather than the default localhost.
.B \-H
Do not print any headers.
.TP
+.B \-i
+Report only the listed
+.I instances
+(if present).
+By default all current instances are reported.
+This is a global option that is used for all metrics unless a
+metric-specific instance definition is provided as part of a
+.IR metricspec .
+Metrics without instances are reported as usual.
+.RS
+.PP
+The list may consist of one or more comma-separated instances.
+The instance name may be quoted with single (') or double (")
+quotes for those cases where the instance name contains commas
+or white space. Note that on the command line when specifying
+more than one instance, all the names must be quoted.
+.PP
+Multiple
+.B \-i
+options are allowed as an alternative way of specifying more than
+one instance of interest.
+.PP
+As an example, the following would report the same instances:
+.EX
+$ pmrep \-i "'1 minute','5 minute'" kernel.all.load
+$ pmrep \-i '"1 minute","5 minute"' kernel.all.load
+$ pmrep \-i "'1 minute'" -i "'5 minute'" kernel.all.load
+$ pmrep kernel.all.load,,"'1 minute','5 minute'"
+$ pmrep kernel.all.load,,'"1 minute","5 minute"'
+.EE
+.PP
+However, this would report only the 1-minute instance:
+.EX
+$ pmrep \-i '"1 minute","5 minute"' kernel.all.load,,"1 minute"
+.EE
+.RE
+.TP
.B \-K
When fetching metrics from a local context (see
.BR \-L ),
@@ -705,8 +764,6 @@ System provided configuration file.
.PD
.SH BUGS
No command line option can follow metrics.
-
-Specifying instances is not yet supported.
.SH "PCP ENVIRONMENT"
Environment variables with the prefix
.B PCP_
diff --git a/src/pmrep/pmrep.conf b/src/pmrep/pmrep.conf
index d31e483..fcdd8e5 100644
--- a/src/pmrep/pmrep.conf
+++ b/src/pmrep/pmrep.conf
@@ -17,6 +17,7 @@
#interval = 1s
#delay = no
#type = default
+#instances =
#width =
#precision = 3
#delimiter =
diff --git a/src/pmrep/pmrep.conf.5 b/src/pmrep/pmrep.conf.5
index a2d0bb7..48550bc 100644
--- a/src/pmrep/pmrep.conf.5
+++ b/src/pmrep/pmrep.conf.5
@@ -195,6 +195,13 @@ Corresponding command line option is \fB-r\fR. Allowed
values are
\fBdefault\fR or \fBraw\fR.
.RE
.P
+instances (string)
+.RS 4
+Defines the instances to be reported. Corresponding command line
+option is \fB-i\fR. Undefined (all current instances are reported)
+by default.
+.RE
+.P
width (integer)
.RS 4
Indicates the width of stdout output columns. Corresponding command line
@@ -383,7 +390,8 @@ see
.BR pmRegisterDerived (3).
.TP 2
.I instance
-This specifier is currently recognized but not implemented.
+Defines the instances to be reported for the metric. For details, see
+.BR pmrep (1).
.TP 2
.I unit
Defines the unit/scale conversion for the metric. Needs to be
diff --git a/src/pmrep/pmrep.py b/src/pmrep/pmrep.py
index 977cf7b..d35561b 100755
--- a/src/pmrep/pmrep.py
+++ b/src/pmrep/pmrep.py
@@ -58,6 +58,7 @@ import errno
import time
import math
import copy
+import csv
import sys
import os
import re
@@ -173,7 +174,7 @@ class PMReporter(object):
'extheader', 'repeat_header', 'timefmt', 'interpol',
'count_scale', 'space_scale', 'time_scale', 'version',
'zabbix_server', 'zabbix_port', 'zabbix_host',
'zabbix_interval',
- 'speclocal')
+ 'speclocal', 'instances')
# Special command line switches
self.arghelp = ('-?', '--help', '-V', '--version')
@@ -204,6 +205,7 @@ class PMReporter(object):
self.runtime = -1
self.delay = 0
self.type = 0
+ self.instances = []
self.width = 0
self.precision = 3 # .3f
self.delimiter = None
@@ -223,12 +225,14 @@ class PMReporter(object):
self.pmfg_ts = None
# Corresponding config file metric specifiers
- self.metricspec = ('label', 'instance', 'unit', 'type', 'width',
'formula')
+ self.metricspec = ('label', 'instances', 'unit', 'type', 'width',
'formula')
self.pmids = []
self.descs = []
self.insts = []
+ self.tmp = []
+
# Zabbix integration
self.zabbix_server = None
self.zabbix_port = ZBXPORT
@@ -242,7 +246,7 @@ class PMReporter(object):
opts = pmapi.pmOptions()
opts.pmSetOptionCallback(self.option)
opts.pmSetOverrideCallback(self.option_override)
-
opts.pmSetShortOptions("a:h:LK:c:Co:F:e:D:V?HUGpA:S:T:O:s:t:Z:zdrw:P:l:xE:f:uq:b:y:")
+
opts.pmSetShortOptions("a:h:LK:c:Co:F:e:D:V?HUGpA:S:T:O:s:t:Z:zdri:w:P:l:xE:f:uq:b:y:")
opts.pmSetShortUsage("[option...] metricspec [...]")
opts.pmSetLongOptionHeader("General options")
@@ -276,6 +280,7 @@ class PMReporter(object):
opts.pmSetLongOptionHostZone() # -z/--hostzone
opts.pmSetLongOption("delay", 0, "d", "", "delay, pause between
updates for archive replay")
opts.pmSetLongOption("raw", 0, "r", "", "output raw counter values (no
rate conversion)")
+ opts.pmSetLongOption("instances", 1, "i", "STR", "instances to report
(default: all current)")
opts.pmSetLongOption("width", 1, "w", "N", "default column width")
opts.pmSetLongOption("precision", 1, "P", "N", "N digits after the
decimal separator (if width enough)")
opts.pmSetLongOption("delimiter", 1, "l", "STR", "delimiter to
separate csv/stdout columns")
@@ -340,6 +345,8 @@ class PMReporter(object):
self.delay = 1
elif opt == 'r':
self.type = 1
+ elif opt == 'i':
+ self.instances = self.instances + self.parse_instances(optarg)
elif opt == 'w':
self.width = int(optarg)
elif opt == 'P':
@@ -388,6 +395,16 @@ class PMReporter(object):
break
return config
+ def parse_instances(self, instances):
+ """ Parse user-supplied instances string """
+ insts = []
+ reader = csv.reader([instances])
+ for i, inst in enumerate(list(reader)[0]):
+ if inst.startswith('"') or inst.startswith("'"):
+ inst = inst[1:-1]
+ insts.append(inst)
+ return insts
+
def set_attr(self, name, value):
""" Helper to apply config file settings properly """
if value in ('true', 'True', 'y', 'yes', 'Yes'):
@@ -418,6 +435,8 @@ class PMReporter(object):
def read_config(self):
""" Read options from configuration file """
+ if not self.config:
+ return
config = ConfigParser.SafeConfigParser()
config.read(self.config)
if not config.has_section('options'):
@@ -436,12 +455,41 @@ class PMReporter(object):
if pmapi.c_api.pmGetOptionsFromList(sys.argv):
raise pmapi.pmUsageErr()
+ def parse_metric_spec_instances(self, spec):
+ """ Parse instances from metric spec """
+ insts = [None]
+ if spec.count(",") < 2:
+ return spec + ",,", insts
+ # User may supply quoted or unquoted instance specification
+ # Conf file preservers outer quotes, command line does not
+ # We need to detect which is the case here. What a mess.
+ quoted = 0
+ s = spec.split(",")[2]
+ if s and (s[1] == "'" or s[1] == '"'):
+ quoted = 1
+ if spec.count('"') or spec.count("'"):
+ inststr = spec.partition(",")[2].partition(",")[2]
+ q = inststr[0]
+ inststr = inststr[:inststr.rfind(q)+1]
+ if quoted:
+ insts = self.parse_instances(inststr[1:-1])
+ else:
+ insts = self.parse_instances(inststr)
+ spec = spec.replace(inststr, "")
+ else:
+ insts = [s]
+ if spec.count(",") < 2:
+ spec += ",,"
+ return spec, insts
+
def parse_metric_info(self, metrics, key, value):
""" Parse metric information """
# NB. Uses the config key, not the metric, as the dict key
if ',' in value:
# Compact / one-line definition
- metrics[key] = (key + "," + value).split(",")
+ spec, insts = self.parse_metric_spec_instances(key + "," + value)
+ metrics[key] = spec.split(",")
+ metrics[key][2] = insts
else:
# Verbose / multi-line definition
if not '.' in key or key.rsplit(".", 1)[1] not in self.metricspec:
@@ -492,7 +540,9 @@ class PMReporter(object):
if metric.startswith(":"):
tempmet[metric[1:]] = None
else:
- m = metric.split(",")
+ spec, insts = self.parse_metric_spec_instances(metric)
+ m = spec.split(",")
+ m[2] = insts
tempmet[m[0]] = m[1:]
# Get config and set details for configuration file metric sets
@@ -646,6 +696,18 @@ class PMReporter(object):
mtype == PM_TYPE_DOUBLE or
mtype == PM_TYPE_STRING):
raise pmapi.pmErr(PM_ERR_TYPE)
+ instances = self.instances if not self.tmp[0] else self.tmp
+ if instances and inst[1][0]:
+ found = [[], []]
+ for r in instances:
+ cr = re.compile('\A' + r + '\Z')
+ for i, s in enumerate(inst[1]):
+ if re.match(cr, s):
+ found[0].append(inst[0][i])
+ found[1].append(inst[1][i])
+ if not found[0]:
+ return
+ inst = found
self.pmids.append(pmid)
self.descs.append(desc)
self.insts.append(inst)
@@ -703,7 +765,12 @@ class PMReporter(object):
for metric in metrics:
try:
l = len(self.pmids)
+ if len(metrics[metric]) > 1:
+ self.tmp = metrics[metric][1]
+ if not 'append' in dir(self.tmp):
+ self.tmp = [None]
self.context.pmTraversePMNS(metric, self.check_metric)
+ self.tmp = []
if len(self.pmids) == l + 1:
# Leaf
if metric == self.context.pmNameID(self.pmids[l]):
@@ -721,6 +788,15 @@ class PMReporter(object):
sys.stderr.write("Invalid metric %s (%s).\n" % (metric,
str(error)))
sys.exit(1)
+ # Exit if no metrics with specified instances found
+ if not self.insts:
+ sys.stderr.write("No matching instances found.\n")
+ # Try to help the user to get the instance specifications right
+ if self.instances:
+ print("\nRequested global instances:")
+ print("\n".join(self.instances))
+ sys.exit(1)
+
# Finalize the metrics set
for i, metric in enumerate(self.metrics):
# Fill in all fields for easier checking later
diff --git a/src/zshrc/_pcp b/src/zshrc/_pcp
index 33c4678..b625164 100644
--- a/src/zshrc/_pcp
+++ b/src/zshrc/_pcp
@@ -439,6 +439,7 @@ _pcp () {
"(-p --timestamps $exargs)"{-p,--timestamps}'[print timestamps]' \
"(-d --delay --container -h --host -L --local-PMDA -K --spec-local -u
--no-interpol $exargs)"{-d,--delay}'[delay between updates in archive mode]' \
"(-r --raw $exargs)"{-r,--raw}'[report raw values]' \
+ "($exargs)"\*{-i+,--instances}'[specify instances to
report]:instances:->instances' \
"(-w --width $exargs)"{-w+,--width}'[set default column width]:width:' \
"(-P --precision $exargs)"{-P+,--precision}'[set floating point
precision]:precision:' \
"(-l --delimiter $exargs)"{-l+,--delimiter}'[set column
delimiter]:delimiter:' \
@@ -483,7 +484,7 @@ _pcp () {
"(-h --host $exargs)"\*{-K+,--spec-local}'[define PMDA spec for local
DSO PMDAs]:spec:(add del clear)' \
"(-n --namespace $exargs)"{-n+,--namespace}'[specify alternative
PMNS]:pmnsfile:_files' \
"(-f --force $exargs)"{-f,--force}'[store value even if unset]' \
- "($exargs)"{-i+,--insts}'[specify instances to store
to]:instances:->instances' \
+ "($exargs)"\*{-i+,--insts}'[specify instances to store
to]:instances:->instances' \
'1:metric:->metrics' \
&& return 0
;;
Thanks,
--
Marko Myllynen
|