Pretty-print values in the PCP DynamicMeter code

Several improvements to the way values are displayed in the
PCP platform DynamicMeter implementation:
- handle the initial 'caption' setting as with regular meters,
  this required a new meter callback because we no longer have
  just a single meter caption for the DynamicMeter case
- if no label is provided for a metric in a configuration file
  use the short form metric name as a fallback
- honour the suffix setting in the configuration file
- convert metric values to the canonical units for htop (kbyte
  and seconds), and use Meter_humanUnit when it makes sense to
  do so.

Also improves the handling of fatal string error messages in a
couple of places, thanks to BenBE for the review feedback.
This commit is contained in:
Nathan Scott 2021-07-07 16:57:03 +10:00
parent 149774209b
commit 01f5b89278
8 changed files with 129 additions and 54 deletions

View File

@ -70,12 +70,27 @@ static void DynamicMeter_display(const Object* cast, RichString* out) {
Platform_dynamicMeterDisplay(meter, out); Platform_dynamicMeterDisplay(meter, out);
} }
static const char* DynamicMeter_getCaption(const Meter* this) {
const ProcessList* pl = this->pl;
const DynamicMeter* meter = Hashtable_get(pl->dynamicMeters, this->param);
if (meter)
return meter->caption ? meter->caption : meter->name;
return this->caption;
}
static void DynamicMeter_getUiName(const Meter* this, char* name, size_t length) { static void DynamicMeter_getUiName(const Meter* this, char* name, size_t length) {
const ProcessList* pl = this->pl; const ProcessList* pl = this->pl;
const DynamicMeter* meter = Hashtable_get(pl->dynamicMeters, this->param); const DynamicMeter* meter = Hashtable_get(pl->dynamicMeters, this->param);
if (meter) { if (meter) {
const char* uiName = meter->caption ? meter->caption : meter->name; const char* uiName = meter->caption;
xSnprintf(name, length, "%s", uiName); if (uiName) {
int len = strlen(uiName);
if (len > 2 && uiName[len-2] == ':')
len -= 2;
xSnprintf(name, length, "%.*s", len, uiName);
} else {
xSnprintf(name, length, "%s", meter->name);
}
} }
} }
@ -87,6 +102,7 @@ const MeterClass DynamicMeter_class = {
}, },
.init = DynamicMeter_init, .init = DynamicMeter_init,
.updateValues = DynamicMeter_updateValues, .updateValues = DynamicMeter_updateValues,
.getCaption = DynamicMeter_getCaption,
.getUiName = DynamicMeter_getUiName, .getUiName = DynamicMeter_getUiName,
.defaultMode = TEXT_METERMODE, .defaultMode = TEXT_METERMODE,
.maxItems = 0, .maxItems = 0,

16
Meter.c
View File

@ -153,11 +153,12 @@ ListItem* Meter_toListItem(const Meter* this, bool moving) {
/* ---------- TextMeterMode ---------- */ /* ---------- TextMeterMode ---------- */
static void TextMeterMode_draw(Meter* this, int x, int y, int w) { static void TextMeterMode_draw(Meter* this, int x, int y, int w) {
const char* caption = Meter_getCaption(this);
attrset(CRT_colors[METER_TEXT]); attrset(CRT_colors[METER_TEXT]);
mvaddnstr(y, x, this->caption, w - 1); mvaddnstr(y, x, caption, w - 1);
attrset(CRT_colors[RESET_COLOR]); attrset(CRT_colors[RESET_COLOR]);
int captionLen = strlen(this->caption); int captionLen = strlen(caption);
x += captionLen; x += captionLen;
w -= captionLen; w -= captionLen;
if (w <= 0) if (w <= 0)
@ -174,10 +175,11 @@ static void TextMeterMode_draw(Meter* this, int x, int y, int w) {
static const char BarMeterMode_characters[] = "|#*@$%&."; static const char BarMeterMode_characters[] = "|#*@$%&.";
static void BarMeterMode_draw(Meter* this, int x, int y, int w) { static void BarMeterMode_draw(Meter* this, int x, int y, int w) {
const char* caption = Meter_getCaption(this);
w -= 2; w -= 2;
attrset(CRT_colors[METER_TEXT]); attrset(CRT_colors[METER_TEXT]);
int captionLen = 3; int captionLen = 3;
mvaddnstr(y, x, this->caption, captionLen); mvaddnstr(y, x, caption, captionLen);
x += captionLen; x += captionLen;
w -= captionLen; w -= captionLen;
attrset(CRT_colors[BAR_BORDER]); attrset(CRT_colors[BAR_BORDER]);
@ -306,9 +308,10 @@ static void GraphMeterMode_draw(Meter* this, int x, int y, int w) {
GraphMeterMode_pixPerRow = PIXPERROW_ASCII; GraphMeterMode_pixPerRow = PIXPERROW_ASCII;
} }
const char* caption = Meter_getCaption(this);
attrset(CRT_colors[METER_TEXT]); attrset(CRT_colors[METER_TEXT]);
int captionLen = 3; int captionLen = 3;
mvaddnstr(y, x, this->caption, captionLen); mvaddnstr(y, x, caption, captionLen);
x += captionLen; x += captionLen;
w -= captionLen; w -= captionLen;
@ -393,8 +396,9 @@ static void LEDMeterMode_draw(Meter* this, int x, int y, ATTR_UNUSED int w) {
#endif #endif
y + 2; y + 2;
attrset(CRT_colors[LED_COLOR]); attrset(CRT_colors[LED_COLOR]);
mvaddstr(yText, x, this->caption); const char* caption = Meter_getCaption(this);
int xx = x + strlen(this->caption); mvaddstr(yText, x, caption);
int xx = x + strlen(caption);
int len = RichString_sizeVal(out); int len = RichString_sizeVal(out);
for (int i = 0; i < len; i++) { for (int i = 0; i < len; i++) {
int c = RichString_getCharVal(out, i); int c = RichString_getCharVal(out, i);

View File

@ -53,6 +53,7 @@ typedef void(*Meter_Done)(Meter*);
typedef void(*Meter_UpdateMode)(Meter*, int); typedef void(*Meter_UpdateMode)(Meter*, int);
typedef void(*Meter_UpdateValues)(Meter*); typedef void(*Meter_UpdateValues)(Meter*);
typedef void(*Meter_Draw)(Meter*, int, int, int); typedef void(*Meter_Draw)(Meter*, int, int, int);
typedef const char*(*Meter_GetCaption)(const Meter*);
typedef void(*Meter_GetUiName)(const Meter*, char*, size_t); typedef void(*Meter_GetUiName)(const Meter*, char*, size_t);
typedef struct MeterClass_ { typedef struct MeterClass_ {
@ -62,6 +63,7 @@ typedef struct MeterClass_ {
const Meter_UpdateMode updateMode; const Meter_UpdateMode updateMode;
const Meter_UpdateValues updateValues; const Meter_UpdateValues updateValues;
const Meter_Draw draw; const Meter_Draw draw;
const Meter_GetCaption getCaption;
const Meter_GetUiName getUiName; const Meter_GetUiName getUiName;
const int defaultMode; const int defaultMode;
const double total; const double total;
@ -84,6 +86,8 @@ typedef struct MeterClass_ {
#define Meter_updateValues(this_) As_Meter(this_)->updateValues((Meter*)(this_)) #define Meter_updateValues(this_) As_Meter(this_)->updateValues((Meter*)(this_))
#define Meter_getUiNameFn(this_) As_Meter(this_)->getUiName #define Meter_getUiNameFn(this_) As_Meter(this_)->getUiName
#define Meter_getUiName(this_,n_,l_) As_Meter(this_)->getUiName((const Meter*)(this_),n_,l_) #define Meter_getUiName(this_,n_,l_) As_Meter(this_)->getUiName((const Meter*)(this_),n_,l_)
#define Meter_getCaptionFn(this_) As_Meter(this_)->getCaption
#define Meter_getCaption(this_) (Meter_getCaptionFn(this_) ? As_Meter(this_)->getCaption((const Meter*)(this_)) : (this_)->caption)
#define Meter_defaultMode(this_) As_Meter(this_)->defaultMode #define Meter_defaultMode(this_) As_Meter(this_)->defaultMode
#define Meter_attributes(this_) As_Meter(this_)->attributes #define Meter_attributes(this_) As_Meter(this_)->attributes
#define Meter_name(this_) As_Meter(this_)->name #define Meter_name(this_) As_Meter(this_)->name

View File

@ -42,6 +42,7 @@ static PCPDynamicMetric* PCPDynamicMeter_lookupMetric(PCPDynamicMeters* meters,
metric = &meter->metrics[n-1]; metric = &meter->metrics[n-1];
memset(metric, 0, sizeof(PCPDynamicMetric)); memset(metric, 0, sizeof(PCPDynamicMetric));
metric->name = metricName; metric->name = metricName;
metric->label = String_cat(name, ": ");
metric->id = meters->offset + meters->cursor; metric->id = meters->offset + meters->cursor;
meters->cursor++; meters->cursor++;
@ -65,13 +66,14 @@ static void PCPDynamicMeter_parseMetric(PCPDynamicMeters* meters, PCPDynamicMete
/* use derived metrics in dynamic meters for simplicity */ /* use derived metrics in dynamic meters for simplicity */
char* error; char* error;
if (pmRegisterDerivedMetric(metric->name, value, &error) < 0) { if (pmRegisterDerivedMetric(metric->name, value, &error) < 0) {
char note[1024]; char* note;
xSnprintf(note, sizeof(note), xAsprintf(&note,
"failed to parse expression in %s at line %u\n%s\n%s", "%s: failed to parse expression in %s at line %u\n%s\n",
path, line, error, pmGetProgname()); pmGetProgname(), path, line, error);
free(error); free(error);
errno = EINVAL; errno = EINVAL;
CRT_fatalError(note); CRT_fatalError(note);
free(note);
} }
} else { } else {
/* this is a property of a dynamic metric - the metric expression */ /* this is a property of a dynamic metric - the metric expression */
@ -97,7 +99,9 @@ static void PCPDynamicMeter_parseMetric(PCPDynamicMeters* meters, PCPDynamicMete
else if (String_eq(value, "white")) else if (String_eq(value, "white"))
metric->color = DYNAMIC_WHITE; metric->color = DYNAMIC_WHITE;
} else if (String_eq(p, "label")) { } else if (String_eq(p, "label")) {
free_and_xStrdup(&metric->label, value); char* label = String_cat(value, ": ");
free_and_xStrdup(&metric->label, label);
free(label);
} else if (String_eq(p, "suffix")) { } else if (String_eq(p, "suffix")) {
free_and_xStrdup(&metric->suffix, value); free_and_xStrdup(&metric->suffix, value);
} }
@ -112,12 +116,13 @@ static void PCPDynamicMeter_validateMeterName(char* key, const char *path, unsig
if (end) { if (end) {
*end = '\0'; *end = '\0';
} else { } else {
char note[1024]; char* note;
xSnprintf(note, sizeof(note), xAsprintf(&note,
"No closing brace on meter name at %s line %u\n\"%s\"", "%s: no closing brace on meter name at %s line %u\n\"%s\"",
path, line, key); pmGetProgname(), path, line, key);
errno = EINVAL; errno = EINVAL;
CRT_fatalError(note); CRT_fatalError(note);
free(note);
} }
while (*p) { while (*p) {
@ -131,12 +136,13 @@ static void PCPDynamicMeter_validateMeterName(char* key, const char *path, unsig
p++; p++;
} }
if (*p != '\0') { /* badness */ if (*p != '\0') { /* badness */
char note[1024]; char* note;
xSnprintf(note, sizeof(note), xAsprintf(&note,
"Invalid meter name at %s line %u\n\"%s\"", "%s: invalid meter name at %s line %u\n\"%s\"",
path, line, key); pmGetProgname(), path, line, key);
errno = EINVAL; errno = EINVAL;
CRT_fatalError(note); CRT_fatalError(note);
free(note);
} else { /* overwrite closing brace */ } else { /* overwrite closing brace */
*p = '\0'; *p = '\0';
} }
@ -182,7 +188,9 @@ static void PCPDynamicMeter_parseFile(PCPDynamicMeters* meters, const char* path
PCPDynamicMeter_validateMeterName(key+1, path, lineno); PCPDynamicMeter_validateMeterName(key+1, path, lineno);
meter = PCPDynamicMeter_new(meters, key+1); meter = PCPDynamicMeter_new(meters, key+1);
} else if (value && String_eq(key, "caption")) { } else if (value && String_eq(key, "caption")) {
free_and_xStrdup(&meter->super.caption, value); char* caption = String_cat(value, ": ");
free_and_xStrdup(&meter->super.caption, caption);
free(caption);
} else if (value && String_eq(key, "description")) { } else if (value && String_eq(key, "description")) {
free_and_xStrdup(&meter->super.description, value); free_and_xStrdup(&meter->super.description, value);
} else if (value && String_eq(key, "type")) { } else if (value && String_eq(key, "type")) {
@ -272,39 +280,66 @@ void PCPDynamicMeter_updateValues(PCPDynamicMeter* this, Meter* meter) {
PCPDynamicMetric* metric = &this->metrics[i]; PCPDynamicMetric* metric = &this->metrics[i];
const pmDesc* desc = Metric_desc(metric->id); const pmDesc* desc = Metric_desc(metric->id);
pmAtomValue atom; pmAtomValue atom, raw;
if (!Metric_values(metric->id, &atom, 1, desc->type)) { if (!Metric_values(metric->id, &raw, 1, desc->type)) {
bytes--; /* clear the separator */ bytes--; /* clear the separator */
continue; continue;
} }
/* TODO: pretty-print the values - pmConvScale, etc */
pmUnits conv = desc->units; /* convert to canonical units */
if (conv.dimSpace)
conv.scaleSpace = PM_SPACE_KBYTE;
if (conv.dimTime)
conv.scaleTime = PM_TIME_SEC;
if (desc->type == PM_TYPE_STRING)
atom = raw;
else if (pmConvScale(desc->type, &raw, &desc->units, &atom, &conv) < 0) {
bytes--; /* clear the separator */
continue;
}
size_t saved = bytes;
switch (desc->type) { switch (desc->type) {
case PM_TYPE_STRING: case PM_TYPE_STRING:
bytes += xSnprintf(buffer + bytes, size - bytes, "%s", atom.cp); bytes += xSnprintf(buffer + bytes, size - bytes, "%s", atom.cp);
free(atom.cp); free(atom.cp);
break; break;
case PM_TYPE_32: case PM_TYPE_32:
bytes += xSnprintf(buffer + bytes, size - bytes, "%d", atom.l); bytes += conv.dimSpace ?
Meter_humanUnit(buffer + bytes, atom.l, size - bytes) :
xSnprintf(buffer + bytes, size - bytes, "%d", atom.l);
break; break;
case PM_TYPE_U32: case PM_TYPE_U32:
bytes += xSnprintf(buffer + bytes, size - bytes, "%u", atom.ul); bytes += conv.dimSpace ?
Meter_humanUnit(buffer + bytes, atom.ul, size - bytes) :
xSnprintf(buffer + bytes, size - bytes, "%u", atom.ul);
break; break;
case PM_TYPE_64: case PM_TYPE_64:
bytes += xSnprintf(buffer + bytes, size - bytes, "%lld", (long long) atom.ll); bytes += conv.dimSpace ?
Meter_humanUnit(buffer + bytes, atom.ll, size - bytes) :
xSnprintf(buffer + bytes, size - bytes, "%lld", (long long) atom.ll);
break; break;
case PM_TYPE_U64: case PM_TYPE_U64:
bytes += xSnprintf(buffer + bytes, size - bytes, "%llu", (unsigned long long) atom.ull); bytes += conv.dimSpace ?
Meter_humanUnit(buffer + bytes, atom.ull, size - bytes) :
xSnprintf(buffer + bytes, size - bytes, "%llu", (unsigned long long) atom.ull);
break; break;
case PM_TYPE_FLOAT: case PM_TYPE_FLOAT:
bytes += xSnprintf(buffer + bytes, size - bytes, "%f", (double) atom.f); bytes += conv.dimSpace ?
Meter_humanUnit(buffer + bytes, atom.f, size - bytes) :
xSnprintf(buffer + bytes, size - bytes, "%.2f", (double) atom.f);
break; break;
case PM_TYPE_DOUBLE: case PM_TYPE_DOUBLE:
bytes += xSnprintf(buffer + bytes, size - bytes, "%f", atom.d); bytes += conv.dimSpace ?
Meter_humanUnit(buffer + bytes, atom.d, size - bytes) :
xSnprintf(buffer + bytes, size - bytes, "%.2f", atom.d);
break; break;
default: default:
break; break;
} }
if (saved != bytes && metric->suffix)
bytes += xSnprintf(buffer + bytes, size - bytes, "%s", metric->suffix);
} }
if (!bytes) if (!bytes)
xSnprintf(buffer, size, "no data"); xSnprintf(buffer, size, "no data");
@ -316,52 +351,74 @@ void PCPDynamicMeter_display(PCPDynamicMeter* this, ATTR_UNUSED const Meter* met
for (unsigned int i = 0; i < this->totalMetrics; i++) { for (unsigned int i = 0; i < this->totalMetrics; i++) {
PCPDynamicMetric* metric = &this->metrics[i]; PCPDynamicMetric* metric = &this->metrics[i];
const pmDesc* desc = Metric_desc(metric->id); const pmDesc* desc = Metric_desc(metric->id);
pmAtomValue atom; pmAtomValue atom, raw;
char buffer[64]; char buffer[64];
int len;
if (!Metric_values(metric->id, &atom, 1, desc->type)) if (!Metric_values(metric->id, &raw, 1, desc->type))
continue; continue;
nodata = 0;
pmUnits conv = desc->units; /* convert to canonical units */
if (conv.dimSpace)
conv.scaleSpace = PM_SPACE_KBYTE;
if (conv.dimTime)
conv.scaleTime = PM_TIME_SEC;
if (desc->type == PM_TYPE_STRING)
atom = raw;
else if (pmConvScale(desc->type, &raw, &desc->units, &atom, &conv) < 0)
continue;
nodata = 0; /* we will use this metric so *some* data will be added */
if (i > 0) if (i > 0)
RichString_appendnAscii(out, CRT_colors[metric->color], " ", 1); RichString_appendnAscii(out, CRT_colors[metric->color], " ", 1);
if (metric->label) { if (metric->label)
len = xSnprintf(buffer, sizeof(buffer), "%s ", metric->label); RichString_appendAscii(out, CRT_colors[METER_TEXT], metric->label);
RichString_appendnAscii(out, CRT_colors[METER_TEXT], buffer, len);
}
/* TODO: pretty-print the values - pmConvScale, etc */ int len = 0;
len = 0;
switch (desc->type) { switch (desc->type) {
case PM_TYPE_STRING: case PM_TYPE_STRING:
len = xSnprintf(buffer, sizeof(buffer), "%s", atom.cp); len = xSnprintf(buffer, sizeof(buffer), "%s", atom.cp);
free(atom.cp); free(atom.cp);
break; break;
case PM_TYPE_32: case PM_TYPE_32:
len = xSnprintf(buffer, sizeof(buffer), "%d", atom.l); len = conv.dimSpace ?
Meter_humanUnit(buffer, atom.l, sizeof(buffer)) :
xSnprintf(buffer, sizeof(buffer), "%d", atom.l);
break; break;
case PM_TYPE_U32: case PM_TYPE_U32:
len = xSnprintf(buffer, sizeof(buffer), "%u", atom.ul); len = conv.dimSpace ?
Meter_humanUnit(buffer, atom.ul, sizeof(buffer)) :
xSnprintf(buffer, sizeof(buffer), "%u", atom.ul);
break; break;
case PM_TYPE_64: case PM_TYPE_64:
len = xSnprintf(buffer, sizeof(buffer), "%lld", (long long) atom.ll); len = conv.dimSpace ?
Meter_humanUnit(buffer, atom.ll, sizeof(buffer)) :
xSnprintf(buffer, sizeof(buffer), "%lld", (long long) atom.ll);
break; break;
case PM_TYPE_U64: case PM_TYPE_U64:
len = xSnprintf(buffer, sizeof(buffer), "%llu", (unsigned long long) atom.ull); len = conv.dimSpace ?
Meter_humanUnit(buffer, atom.ull, sizeof(buffer)) :
xSnprintf(buffer, sizeof(buffer), "%llu", (unsigned long long) atom.ull);
break; break;
case PM_TYPE_FLOAT: case PM_TYPE_FLOAT:
len = xSnprintf(buffer, sizeof(buffer), "%f", (double)atom.f); len = conv.dimSpace ?
Meter_humanUnit(buffer, atom.f, sizeof(buffer)) :
xSnprintf(buffer, sizeof(buffer), "%.2f", (double) atom.f);
break; break;
case PM_TYPE_DOUBLE: case PM_TYPE_DOUBLE:
len = xSnprintf(buffer, sizeof(buffer), "%f", atom.d); len = conv.dimSpace ?
Meter_humanUnit(buffer, atom.d, sizeof(buffer)) :
xSnprintf(buffer, sizeof(buffer), "%.2f", atom.d);
break; break;
default: default:
break; break;
} }
if (len) if (len) {
RichString_appendnAscii(out, CRT_colors[metric->color], buffer, len); RichString_appendnAscii(out, CRT_colors[metric->color], buffer, len);
if (metric->suffix)
RichString_appendAscii(out, CRT_colors[METER_TEXT], metric->suffix);
}
} }
if (nodata) if (nodata)
RichString_writeAscii(out, CRT_colors[METER_VALUE_ERROR], "no data"); RichString_writeAscii(out, CRT_colors[METER_VALUE_ERROR], "no data");

View File

@ -6,10 +6,8 @@
caption = MySQL I/O caption = MySQL I/O
recv.metric = mysql.status.bytes_received recv.metric = mysql.status.bytes_received
recv.color = green recv.color = green
recv.label = recv
sent.metric = mysql.status.bytes_sent sent.metric = mysql.status.bytes_sent
sent.color = blue sent.color = blue
sent.label = sent
[mysql_keys] [mysql_keys]
caption = MySQL keys caption = MySQL keys

View File

@ -18,4 +18,3 @@ bounce.color = red
bounce.label = bnc bounce.label = bnc
hold.metric = sum(postfix.queues.hold) hold.metric = sum(postfix.queues.hold)
hold.color = yellow hold.color = yellow
hold.label = hold

View File

@ -13,10 +13,8 @@ caption = Redis mem
description = Redis memory description = Redis memory
lua.metric = redis.used_memory_lua lua.metric = redis.used_memory_lua
lua.color = magenta lua.color = magenta
lua.label = lua:
used.metric = redis.used_memory used.metric = redis.used_memory
used.color = blue used.color = blue
used.label = used:
[redisclient] [redisclient]
caption = Redis clients caption = Redis clients

View File

@ -13,7 +13,6 @@ active.color = blue
active.label = act active.label = act
syn.metric = network.tcpconn.syn_sent + network.tcpconn.syn_recv + network.tcpconn.last_ack syn.metric = network.tcpconn.syn_sent + network.tcpconn.syn_recv + network.tcpconn.last_ack
syn.color = cyan syn.color = cyan
syn.label = syn
wait.metric = network.tcpconn.time_wait wait.metric = network.tcpconn.time_wait
wait.color = red wait.color = red
wait.label = tim wait.label = tim