Copilot commented on code in PR #24502:
URL: https://github.com/apache/camel/pull/24502#discussion_r3539116049
##########
dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/OverviewTab.java:
##########
@@ -504,11 +504,16 @@ public void render(Frame frame, Rect area) {
for (long v : mergedTotal) {
rawMax = Math.max(rawMax, v);
}
- long maxTp = roundUpNice(rawMax);
+ long maxTp = MetricsCollector.niceMax(rawMax);
Review Comment:
`roundUpNice(...)` in this class appears to be unused after switching to
`MetricsCollector.niceMax(rawMax)`. Keeping the unused helper makes it harder
to know which axis-scaling implementation is authoritative; consider deleting
`roundUpNice` (or delegating it to `MetricsCollector.niceMax`) to avoid dead
code.
##########
dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/OverviewTab.java:
##########
@@ -504,11 +504,16 @@ public void render(Frame frame, Rect area) {
for (long v : mergedTotal) {
rawMax = Math.max(rawMax, v);
}
- long maxTp = roundUpNice(rawMax);
+ long maxTp = MetricsCollector.niceMax(rawMax);
long curTp = mergedTotal[renderPoints - 1];
long curFailed = mergedFailed[renderPoints - 1];
long curOk = Math.max(0, curTp - curFailed);
+ // Format throughput values unscaled for display
+ String curTpFmt = MetricsCollector.formatThroughput(curTp);
+ String curOkFmt = MetricsCollector.formatThroughput(curOk);
+ String curFailFmt = MetricsCollector.formatThroughput(curFailed);
Review Comment:
The chart title can show an inconsistent ok/fail breakdown when `curFailed`
exceeds `curTp` (now possible because total throughput comes from EWMA while
failed throughput is still delta-based). The bar groups clamp failed to total
(`Math.min(mergedFailed[i], mergedTotal[i])`), but the title formats the
unclamped `curFailed`, so it can display e.g. throughput 0.20 with fail 0.50.
Clamp the failed value for the title (and derive ok from the same clamped
value) so the title matches the chart data.
##########
dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/MetricsCollector.java:
##########
@@ -403,4 +416,44 @@ private void removeByPrefix(String prefix, Map<String,
?>... maps) {
map.keySet().removeIf(k -> k.startsWith(prefix));
}
}
+
+ // --- Shared throughput formatting utilities ---
+
+ /**
+ * Round a scaled throughput value up to a nice chart-axis maximum.
Returns values that are multiples of 1, 2, or 5
+ * at the appropriate magnitude, ensuring the Y-axis labels are
human-readable.
+ */
+ static long niceMax(long rawMax) {
+ if (rawMax <= 0) {
+ return THROUGHPUT_SCALE;
+ }
+ int[] steps = { 1, 2, 5 };
+ long multiplier = THROUGHPUT_SCALE;
+ while (true) {
+ for (int s : steps) {
+ long candidate = s * multiplier;
+ if (candidate >= rawMax) {
+ return candidate;
+ }
+ }
+ multiplier *= 10;
+ }
+ }
Review Comment:
`niceMax(...)` can overflow `multiplier` (and `candidate = s * multiplier`)
for very large `rawMax`, which can lead to negative values and an infinite
loop. Adding overflow guards avoids pathological behavior if an unexpectedly
large throughput value is encountered.
##########
dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/MetricsCollector.java:
##########
@@ -262,8 +275,8 @@ private void recordEndpointSample(
long[] oldest = samples.get(0);
long[] newest = samples.get(samples.size() - 1);
long deltaMs = newest[0] - oldest[0];
- long inRate = deltaMs > 0 ? (newest[1] - oldest[1]) * 1000 /
deltaMs : 0;
- long outRate = deltaMs > 0 ? (newest[2] - oldest[2]) * 1000 /
deltaMs : 0;
+ long inRate = deltaMs > 0 ? (newest[1] - oldest[1]) * 1000 *
THROUGHPUT_SCALE / deltaMs : 0;
+ long outRate = deltaMs > 0 ? (newest[2] - oldest[2]) * 1000 *
THROUGHPUT_SCALE / deltaMs : 0;
Review Comment:
`recordEndpointSample(...)` now scales computed rates by `THROUGHPUT_SCALE`,
which affects *all* callers (including `updateCbHistory`, which feeds
`cbSuccessHistory`/`cbFailHistory`). `CircuitBreakerTab` currently formats
these values as plain integers ("ok:%-4d", "fail:%-4d msg/s") and
`DualSparkline.showYAxis(true)` will also label the scaled values, so the
Circuit Breaker tab (and endpoint sparklines’ Y axis) will be off by 100x after
this change. Consider either (1) keeping circuit-breaker histories unscaled
while scaling only endpoint/per-endpoint histories, or (2) updating
CircuitBreakerTab + sparkline Y-axis formatting to use `formatThroughput` (or
disable Y-axis labels until tamboui provides a formatter).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
