Skip to content

Offer latest as a time aggregation, and fix the state-units predicate - #232

Merged
novykh merged 7 commits into
mainfrom
feat/latest-time-aggregation
Aug 4, 2026
Merged

Offer latest as a time aggregation, and fix the state-units predicate#232
novykh merged 7 commits into
mainfrom
feat/latest-time-aggregation

Conversation

@novykh

@novykh novykh commented Aug 3, 2026

Copy link
Copy Markdown
Member

Why

State metrics (units: state | status | boolean) are one-hot: exactly one dimension is 1 at a time. Every existing time aggregation breaks that invariant when a point spans more than one database sample — average lands between states (0.5), min collapses every dimension to zero, max reports several states as simultaneously active. time_group=latest returns the last database value in each point instead of combining them, and is the only option that keeps a one-hot set consistent.

Two commits, reviewable independently.

1. Move the state-units predicate into helpers/units and fix its coverage

isStateUnits / stateUnits lived in helpers/stepped.js — named after their first caller (camelizePayload's stepPlot) rather than their content. They now sit beside isRateUnit and stripRateUnit in helpers/units, which is also where getAlias lives.

Resolving through getAlias closes two coverage gaps:

  • {boolean} was missing. It is a defined unit (units/all.js), and httpcheck.status, portcheck.status and x509check.revocation_status all declare Units: "boolean" in their collectors — so the three most obvious state contexts were not recognised.
  • Aliased spellings were missed. "app pool status" and "exit status" resolve to {status} in the units table but were absent from the raw set.

Case handling moves into the helper, so getAggregateMethod no longer probes both the raw and lowercased unit.

Behaviour change to note: boolean contexts are now treated as state by both existing consumers — getAggregateMethod returns sum instead of avg, and camelizePayload sets stepPlot: true. Both are correct for a one-hot set (sum across instances is a count of entities per state; a step plot is right for discrete states), but it is a visible rendering change for httpcheck / portcheck / x509check charts.

2. Offer latest as a time aggregation option

Added after sum, behind a description-only separator, so the everyday functions keep their position at the top of the list. Older agents parse an unknown time_group as average (time_grouping_parse(value, RRDR_GROUPING_AVERAGE)), which the option description calls out, so this is safe to send to any agent.

useMenuItems is exported so the list can be asserted directly.

Tests

156 suites, 1540 passed, 2 skipped, 0 failures on this branch. 8 new cases cover the predicate (curly forms, alias resolution, case/padding, counters and rates rejected, array every semantics, empty/non-string input); 4 cover the dropdown entry, its ordering, and the separator.

novykh added 7 commits August 3, 2026 14:08
isStateUnits and stateUnits lived in helpers/stepped.js, named after their
first caller (camelizePayload's stepPlot) rather than their content. They
belong next to isRateUnit and stripRateUnit in helpers/units, which is also
where getAlias lives.

Two coverage gaps fixed by resolving through getAlias:

- {boolean} was missing, so httpcheck.status, portcheck.status and
  x509check.revocation_status were not recognised as state metrics; all
  three declare boolean units in their collectors.
- Aliased spellings such as "app pool status" and "exit status" resolve to
  {status} in the units table but were absent from the raw set.

Case handling moves into the helper, so getAggregateMethod no longer needs
to probe both the raw and lowercased unit.
The agent supports time_group=latest, which returns the last database value
in each point instead of combining them. It is the only time aggregation
that keeps a one-hot state set consistent: min collapses every dimension to
zero, max reports several states at once, and average lands between states.

Placed after sum, behind a description-only separator, so the everyday
functions keep their position at the top of the list. Older agents parse an
unknown time_group as average, which the description calls out.

Exports useMenuItems so the list can be asserted directly.
…rrectly

Celsius conversion was unreachable. getMethod returned "original" for any
non-scalable unit, and Cel is declared is_scalable: false, so the Cel -> [degF]
converter never ran in any configuration. Widen the guard to let conversable
units through while keeping Cel and [degF] non-scalable, since flipping that
flag degrades their labels to "" and invents K/M/B/T temperature scales.

Explicit unit selections were also mislabelled. The conversable branch returned
the target in the prefix slot with an empty base, so getUnitsString composed
labels like "usns", "sms" and "h:mm:sss", and a Fahrenheit selection still read
"°C". Conversable candidates now put the target in base and carry a full label
mode through the conversion attributes into getUnitSign, which bypasses the
prefix-only "scale" rendering that would otherwise blank out the label for
min/h/d/wk/mo/a sources. getUnitsString itself is unchanged; it already accepted
a mode override.

With the prefix slot empty, getConversionAttributes now returns before its
exponential-notation check, so a selected unit is no longer silently replaced by
an auto-scaled candidate on large ranges.

Adds the reverse [degF] -> Cel converter, and leaves temperature unset by
default so charts render whatever the agent reported until a preference is
chosen. Changing temperature or secondsAsTime now recomputes conversion instead
of waiting for the next axis change. The Scale dropdown offers temperature
targets and labels them by symbol.

Covers all 85 selectable unit/scale pairs with expected labels, the three
temperature states in both directions, magnitude independence, and both
recompute paths.
A conversable conversion changes the base unit and leaves the prefix alone, but
four consumers were keyed only on the prefix or on attributes that no longer
exist, so switching a unit preference did not reach the screen until the next
fetch or a page reload.

- dygraph redrew only on unitsConversionPrefix, which a conversable conversion
  never changes; it now redraws on base too.
- useValueUnitAttributes recomputed its memo without watching temperature, so
  table cells kept a stale unit and value.
- useValue listened to "unitsConversion", an attribute nothing writes and which
  is absent from initialAttributes, leaving successFetch as its only unit-related
  trigger.
- useUnits watched the prefix alone, matching useUnitSign now.

These were unreachable until temperature conversion started working, since a
conversable conversion previously could not happen mid-session.
Temperature charts get a single units control offering "Follow preference (°X)",
the source unit and the converted unit. It writes desiredUnits, so it follows the
same inherit-then-override path as groupingMethod: the root carries the user's
setting, a chart inherits it on append, and a per-chart change is recorded in
pristine for saving and resetting. desiredUnits was already tracked there and
already listed in the Display tab reset keys, so no new plumbing was needed.

A separate preference select next to the scale control was tried first and
dropped: an explicit desiredUnits wins inside getMethod, so picking Fahrenheit in
one and °C in the other silently rendered °C.

The label for the inherited option is resolved by asking the converter's own
check(), so it cannot drift from the conversion rules, and the unit predicate is
exported from conversableUnits rather than hardcoded in the component.
The settings panel kept its own copy of the time aggregation option list, so
adding "latest" to the filter toolbox left the Data tab without it. The two lists
were identical apart from that entry, so the duplicate is gone in favour of the
already-exported useMenuItems and the lists can no longer drift.
@novykh
novykh merged commit 5a69d34 into main Aug 4, 2026
2 checks passed
@novykh
novykh deleted the feat/latest-time-aggregation branch August 4, 2026 11:55
novykh added a commit that referenced this pull request Aug 4, 2026
Main #232 made conversion recompute mid-session (a temperature or
seconds-as-time preference change now re-runs onConvert without a refetch) and
bound dygraph's handler to both unitsConversionPrefix and unitsConversionBase.
uPlot listened only to the prefix, and its handler called u.redraw() with no
args, which leaves shouldConvergeSize false so uPlot repaints cached tick
strings. Both halves were broken: a conversable conversion moves the base and
never the prefix, so it was ignored entirely, and even a prefix change did not
refresh the labels.

Re-derive on both attributes. Rebuild rather than redraw(true, true) because
getAxes captures secondsAsTime/units/durationAxis at create time, so a
conversion that flips duration ticks needs fresh axis config - dygraph gets
this by re-applying makeChartTypeOptions on every conversion change.

Tests assert the y-axis labels actually recompute, replacing a not.toThrow()
that passed even while the redraw was ineffective.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
novykh added a commit that referenced this pull request Aug 4, 2026
getPaths() resolves the stepped-vs-smooth builder inside getSeries(), which
only runs at create time, and render() otherwise just calls u.setData unless
the series count changed. With no stepPlot listener, a mid-session flip left
uPlot drawing with the previously resolved builder while dygraph switched -
dygraph re-applies makeChartTypeOptions on every render, re-reading stepPlot
and re-picking the plotter.

stepPlot arrives with the payload (camelizePayload derives it from
isStateUnits, which main #232 widened), so this is reachable in production.
drawStacked reads stepPlot live, so the gap was line/area only.

A listener is sufficient: stepPlot is only readable via getAttribute, so it
cannot change without an attribute update firing this listener.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants