Hi,
On 2015-12-14 13:24, Marko Myllynen wrote:
> On 2015-12-14 06:38, Nathan Scott wrote:
>>>
>>> related to the earlier raw/rate patches, the patch below renames -R to
>>> -W in case we'd want to support requesting rates also for non-counters
>>> in future (-W is not as good as -R for runtime but OTOH having -r/-R
>>> available for raw/rate is nice).
>>
>> Hmm, this looks alot like -T with a relative end point. So, we should
>> not need neither of these (-R / -W) options ...?
>
> hmm, good point, I saw messages in the past time window options not
> being supported with other than archives but I was probably using
> something like -S or -T @10:00 at that time and mistakenly thought -T 2s
> is not supported either. -R is used by pmcollectl and -T by pmnewlog for
> this so -W is certainly not optimal.
>
>> Perhaps the right thing to do here is to just remove this command
>> line option at this stage, and implement -T more completely next
>> release
>
> Yeah, I have a look at this later today, I'll either send a two-liner to
> drop -R and, if truly trivial, a patch to replace -R with -T.
the patch below changes two relevant lines of code and updates the
documentation accordingly. In my testing I don't see any negative
side-effects. Looking at this overall, it reminds me of potential clean
up opportunities in few other places (e.g., writing the extended
header) but those are certainly for another release cycle.
>From 0b2cd199fb33b2ba443367f01f5634e716636f1f Mon Sep 17 00:00:00 2001
From: Marko Myllynen <myllynen@xxxxxxxxxx>
Date: Mon, 14 Dec 2015 17:59:40 +0200
Subject: [PATCH] no more -R
---
src/pmrep/pmrep.1 | 63
+++++++++++++++++++++-----------------------------
src/pmrep/pmrep.conf | 1 -
src/pmrep/pmrep.conf.5 | 11 ---------
src/pmrep/pmrep.py | 13 ++++-------
4 files changed, 31 insertions(+), 57 deletions(-)
diff --git a/src/pmrep/pmrep.1 b/src/pmrep/pmrep.1
index 2165136..2294344 100644
--- a/src/pmrep/pmrep.1
+++ b/src/pmrep/pmrep.1
@@ -35,7 +35,6 @@
[\f3\-O\f1 \f2origin\f1]
[\f3\-P\f1 \f2precision\f1]
[\f3\-q\f1 \f2count-scale\f1]
-[\f3\-R\f1 \f2runtime\f1]
[\f3\-s\f1 \f2samples\f1]
[\f3\-S\f1 \f2starttime\f1]
[\f3\-t\f1 \f2interval\f1]
@@ -436,39 +435,6 @@ See also
Output raw metric values, do not convert cumulative counters to rates.
This option \fIwill\fR override possible per-metric specifications.
.TP
-.B \-R
-The argument
-.I runtime
-defines the time
-.B pmrep
-will run before exiting.
-If no
-.I samples
-is given (see
-.BR \-s )
-then the number of reported samples depends on
-.I interval
-(see
-.BR \-t ).
-If
-.I samples
-is given then
-.I interval
-will be adjusted to allow reporting of
-.I samples
-during
-.IR runtime .
-In case all of
-.BR \-R ,
-.BR \-s ,
-and
-.B \-t
-are given,
-.I runtime
-determines the actual time
-.B pmrep
-will run.
-.TP
.B \-s
The argument
.I samples
@@ -482,7 +448,7 @@ is not specified,
will sample and report continuously (in real time mode) or until the end
of the PCP archive (in archive mode).
See also
-.BR \-R .
+.BR \-T .
.TP
.B \-S
When reporting archived metrics, the report will be restricted to those
@@ -504,7 +470,7 @@ argument follows the syntax described in
and in the simplest form may be an unsigned integer (the implied units
in this case are seconds).
See also the
-.B \-R
+.B \-T
option.
.TP
.B \-T
@@ -515,6 +481,29 @@ Refer to
.BR PCPIntro (1)
for a complete description of the syntax for
.IR endtime .
+.RS
+.PP
+When used to define the runtime before \fBpmrep\fR will exit,
+if no \fIsamples\fR is given (see \fB-s\fR) then the number of
+reported samples depends on \fIinterval\fR (see \fB-t\fR).
+If
+.I samples
+is given then
+.I interval
+will be adjusted to allow reporting of
+.I samples
+during runtime.
+In case all of
+.BR \-T ,
+.BR \-s ,
+and
+.B \-t
+are given,
+.I endtime
+determines the actual time
+.B pmrep
+will run.
+.RE
.TP
.B \-u
When reporting archived metrics, by default values are reported
@@ -664,7 +653,7 @@ archive
.RS +1
.ft CW
.nf
-$ pmrep -o archive -F ./a -t 5s -R 5m ds389 xfs kernel.all.cpu disk mem
+$ pmrep -o archive -F ./a -t 5s -T 5m ds389 xfs kernel.all.cpu disk mem
.fi
.ft P
.RE
diff --git a/src/pmrep/pmrep.conf b/src/pmrep/pmrep.conf
index ddcdb51..bd8d375 100644
--- a/src/pmrep/pmrep.conf
+++ b/src/pmrep/pmrep.conf
@@ -14,7 +14,6 @@
#timestamp = no
#samples =
#interval = 1s
-#runtime =
#delay = no
#raw = no
#width =
diff --git a/src/pmrep/pmrep.conf.5 b/src/pmrep/pmrep.conf.5
index c1eaa35..deb77e6 100644
--- a/src/pmrep/pmrep.conf.5
+++ b/src/pmrep/pmrep.conf.5
@@ -169,17 +169,6 @@ parameter is \fB-o\fR. Follows the time syntax
described in
Defaults to \fB1s\fR.
.RE
.P
-runtime (string)
-.RS 4
-Indicates the time
-.B pmrep
-will run before exiting. Corresponding command line parameter is
-\fB-R\fR. Follows the time syntax described in
-.BR PCPIntro (1).
-Undefined by default (thus runtime will be determined by the number of
-samples and interval).
-.RE
-.P
delay (bool)
.RS 4
Indicates whether to pause between samples when replaying from an
diff --git a/src/pmrep/pmrep.py b/src/pmrep/pmrep.py
index 3437370..fc56613 100644
--- a/src/pmrep/pmrep.py
+++ b/src/pmrep/pmrep.py
@@ -168,7 +168,7 @@ class PMReporter(object):
# Configuration directives
self.keys = ('source', 'output', 'derived', 'header', 'unitinfo',
- 'globals', 'timestamp', 'samples', 'interval',
'runtime',
+ 'globals', 'timestamp', 'samples', 'interval',
'delay', 'raw', 'width', 'precision', 'delimiter',
'extheader', 'repeat_header', 'timefmt', 'interpol',
'count_scale', 'space_scale', 'time_scale', 'version',
@@ -300,7 +300,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:R: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:zdrw:P:l:xE:f:uq:b:y:")
opts.pmSetShortUsage("[option...] metricspec [...]")
opts.pmSetLongOptionHeader("General options")
@@ -329,7 +329,6 @@ class PMReporter(object):
opts.pmSetLongOptionOrigin() # -O/--origin
opts.pmSetLongOptionSamples() # -s/--samples
opts.pmSetLongOptionInterval() # -t/--interval
- opts.pmSetLongOption("runtime", 1, "R", "N", "runtime duration
(overrides -t or -s)")
opts.pmSetLongOptionTimeZone() # -Z/--timezone
opts.pmSetLongOptionHostZone() # -z/--hostzone
opts.pmSetLongOption("delay", 0, "d", "", "delay, pause between
updates for archive replay")
@@ -386,8 +385,6 @@ class PMReporter(object):
self.globals = 0
elif opt == 'p':
self.timestamp = 1
- elif opt == 'R':
- self.runtime = optarg
elif opt == 'd':
self.delay = 1
elif opt == 'r':
@@ -574,9 +571,9 @@ class PMReporter(object):
sys.stderr.write("zabbix_server, zabbix_port, and
zabbix_host must be defined with Zabbix.\n")
sys.exit(1)
- # Runtime overrides samples/interval/endtime
- if self.runtime != -1:
- self.runtime = int(pmapi.timeval.fromInterval(self.runtime))
+ # Runtime overrides samples/interval
+ if self.opts.pmGetOptionFinishOptarg():
+ self.runtime = int(float(self.opts.pmGetOptionFinish()) -
float(self.opts.pmGetOptionStart()))
if self.opts.pmGetOptionSamples():
self.samples = self.opts.pmGetOptionSamples()
if self.samples < 2:
Thanks,
--
Marko Myllynen
|