From 01f5b892785014db30db4d6b4b7c23e042c7ffe4 Mon Sep 17 00:00:00 2001 From: Nathan Scott Date: Wed, 7 Jul 2021 16:57:03 +1000 Subject: 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. --- pcp/PCPDynamicMeter.c | 137 +++++++++++++++++++++++++++++++++++--------------- pcp/meters/mysql | 2 - pcp/meters/postfix | 1 - pcp/meters/redis | 2 - pcp/meters/tcp | 1 - 5 files changed, 97 insertions(+), 46 deletions(-) (limited to 'pcp') diff --git a/pcp/PCPDynamicMeter.c b/pcp/PCPDynamicMeter.c index 511d964e..05b71d5c 100644 --- a/pcp/PCPDynamicMeter.c +++ b/pcp/PCPDynamicMeter.c @@ -42,6 +42,7 @@ static PCPDynamicMetric* PCPDynamicMeter_lookupMetric(PCPDynamicMeters* meters, metric = &meter->metrics[n-1]; memset(metric, 0, sizeof(PCPDynamicMetric)); metric->name = metricName; + metric->label = String_cat(name, ": "); metric->id = meters->offset + meters->cursor; meters->cursor++; @@ -65,13 +66,14 @@ static void PCPDynamicMeter_parseMetric(PCPDynamicMeters* meters, PCPDynamicMete /* use derived metrics in dynamic meters for simplicity */ char* error; if (pmRegisterDerivedMetric(metric->name, value, &error) < 0) { - char note[1024]; - xSnprintf(note, sizeof(note), - "failed to parse expression in %s at line %u\n%s\n%s", - path, line, error, pmGetProgname()); + char* note; + xAsprintf(¬e, + "%s: failed to parse expression in %s at line %u\n%s\n", + pmGetProgname(), path, line, error); free(error); errno = EINVAL; CRT_fatalError(note); + free(note); } } else { /* 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")) metric->color = DYNAMIC_WHITE; } 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")) { free_and_xStrdup(&metric->suffix, value); } @@ -112,12 +116,13 @@ static void PCPDynamicMeter_validateMeterName(char* key, const char *path, unsig if (end) { *end = '\0'; } else { - char note[1024]; - xSnprintf(note, sizeof(note), - "No closing brace on meter name at %s line %u\n\"%s\"", - path, line, key); + char* note; + xAsprintf(¬e, + "%s: no closing brace on meter name at %s line %u\n\"%s\"", + pmGetProgname(), path, line, key); errno = EINVAL; CRT_fatalError(note); + free(note); } while (*p) { @@ -131,12 +136,13 @@ static void PCPDynamicMeter_validateMeterName(char* key, const char *path, unsig p++; } if (*p != '\0') { /* badness */ - char note[1024]; - xSnprintf(note, sizeof(note), - "Invalid meter name at %s line %u\n\"%s\"", - path, line, key); + char* note; + xAsprintf(¬e, + "%s: invalid meter name at %s line %u\n\"%s\"", + pmGetProgname(), path, line, key); errno = EINVAL; CRT_fatalError(note); + free(note); } else { /* overwrite closing brace */ *p = '\0'; } @@ -182,7 +188,9 @@ static void PCPDynamicMeter_parseFile(PCPDynamicMeters* meters, const char* path PCPDynamicMeter_validateMeterName(key+1, path, lineno); meter = PCPDynamicMeter_new(meters, key+1); } 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")) { free_and_xStrdup(&meter->super.description, value); } else if (value && String_eq(key, "type")) { @@ -272,39 +280,66 @@ void PCPDynamicMeter_updateValues(PCPDynamicMeter* this, Meter* meter) { PCPDynamicMetric* metric = &this->metrics[i]; 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 */ 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) { case PM_TYPE_STRING: bytes += xSnprintf(buffer + bytes, size - bytes, "%s", atom.cp); free(atom.cp); break; 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; 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; 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; 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; 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; 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; default: break; } + if (saved != bytes && metric->suffix) + bytes += xSnprintf(buffer + bytes, size - bytes, "%s", metric->suffix); } if (!bytes) 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++) { PCPDynamicMetric* metric = &this->metrics[i]; const pmDesc* desc = Metric_desc(metric->id); - pmAtomValue atom; + pmAtomValue atom, raw; char buffer[64]; - int len; - if (!Metric_values(metric->id, &atom, 1, desc->type)) + if (!Metric_values(metric->id, &raw, 1, desc->type)) 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) RichString_appendnAscii(out, CRT_colors[metric->color], " ", 1); - if (metric->label) { - len = xSnprintf(buffer, sizeof(buffer), "%s ", metric->label); - RichString_appendnAscii(out, CRT_colors[METER_TEXT], buffer, len); - } + if (metric->label) + RichString_appendAscii(out, CRT_colors[METER_TEXT], metric->label); - /* TODO: pretty-print the values - pmConvScale, etc */ - len = 0; + int len = 0; switch (desc->type) { case PM_TYPE_STRING: len = xSnprintf(buffer, sizeof(buffer), "%s", atom.cp); free(atom.cp); break; 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; 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; 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; 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; 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; 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; default: break; } - if (len) + if (len) { RichString_appendnAscii(out, CRT_colors[metric->color], buffer, len); + if (metric->suffix) + RichString_appendAscii(out, CRT_colors[METER_TEXT], metric->suffix); + } } if (nodata) RichString_writeAscii(out, CRT_colors[METER_VALUE_ERROR], "no data"); diff --git a/pcp/meters/mysql b/pcp/meters/mysql index 07ae11e9..a9e75e46 100644 --- a/pcp/meters/mysql +++ b/pcp/meters/mysql @@ -6,10 +6,8 @@ caption = MySQL I/O recv.metric = mysql.status.bytes_received recv.color = green -recv.label = recv sent.metric = mysql.status.bytes_sent sent.color = blue -sent.label = sent [mysql_keys] caption = MySQL keys diff --git a/pcp/meters/postfix b/pcp/meters/postfix index 66876c5a..cda68377 100644 --- a/pcp/meters/postfix +++ b/pcp/meters/postfix @@ -18,4 +18,3 @@ bounce.color = red bounce.label = bnc hold.metric = sum(postfix.queues.hold) hold.color = yellow -hold.label = hold diff --git a/pcp/meters/redis b/pcp/meters/redis index b9c084be..a72f7278 100644 --- a/pcp/meters/redis +++ b/pcp/meters/redis @@ -13,10 +13,8 @@ caption = Redis mem description = Redis memory lua.metric = redis.used_memory_lua lua.color = magenta -lua.label = lua: used.metric = redis.used_memory used.color = blue -used.label = used: [redisclient] caption = Redis clients diff --git a/pcp/meters/tcp b/pcp/meters/tcp index cebc658d..c95736f3 100644 --- a/pcp/meters/tcp +++ b/pcp/meters/tcp @@ -13,7 +13,6 @@ active.color = blue active.label = act syn.metric = network.tcpconn.syn_sent + network.tcpconn.syn_recv + network.tcpconn.last_ack syn.color = cyan -syn.label = syn wait.metric = network.tcpconn.time_wait wait.color = red wait.label = tim -- cgit v1.2.3