Skip to content

NMS-19712: Strafeping overlay fix#29

Merged
marshallmassengill merged 4 commits intoOpenNMS:masterfrom
marshallmassengill:strafeping-overlay-fix
Apr 20, 2026
Merged

NMS-19712: Strafeping overlay fix#29
marshallmassengill merged 4 commits intoOpenNMS:masterfrom
marshallmassengill:strafeping-overlay-fix

Conversation

@marshallmassengill
Copy link
Copy Markdown
Contributor

@marshallmassengill marshallmassengill commented Apr 17, 2026

This will address the consecutive data median line drawing issue for NMS-19712. I have tested this against data from several different instances of OpenNMS and I've targeted this specifically around the StrafePing data series. We will need to do a build and release for this to get it into a release of OpenNMS.

This does also technically introduce the ability to create step graphs as well, though that will require additional upstream changes to enable fully and it is disabled by default (just now passed through correctly to enable it as an option).

Screenshot showing before:
image

Screenshot showing after:
image

Two additional after shots that are Zoomed in:
image

Assisted-by: Claude Code:Opus 4.6 & 4.7

External References

rrdtool conventionally uses 8-digit hex with a 00 alpha channel (e.g.
#00000000) to declare a fully transparent color on AREA/LINE/STACK
commands. Previously _getColor returned the literal 8-digit string,
which reached the renderer as an opaque-ish fill in browsers that don't
honor CSS 8-digit hex, and left the "no-color" path unused.

Treat any 8-digit hex ending in 00 as undefined so the renderer's
existing no-color handling (no fill, no auto-palette line) takes over.
Colorless commands (e.g. `AREA:meL0`) already produced undefined and
are unchanged. Opaque 8-digit hex (alpha != 00) still passes through
as-is for browsers that support RGBA hex.
Adds support for StrafePing-style overlays — transparent AREA + colored
STACK pairs that rrdtool uses to paint the median line in a loss-colored
segment — and handles both the original OpenNMS strafeping definition
and the updated one that uses alpha-transparent hex and diff-stacked
gray smoke bands.

* New `step` option. When enabled, lines and areas render with
  rrdgraph-style stair-stepping. Stack-type series always render smooth
  regardless, because Flot's step-mode stacked fills leave triangular
  gaps between adjacent layers (top/bottom step transitions don't align
  across stacks).

* _mergeOverlaySegments post-processor. Detects transparent-AREA +
  colored-labeled-STACK pairs (6+ required, colored STACK must carry a
  legend) and rewrites them as contiguous per-color line segments. Each
  individual overlay series is mostly NaN (valid only inside its loss
  range), which Flot renders as isolated dots; the merge joins them into
  a single colored line across the timeline. Original pairs are hidden
  but retained in flotSeries so the data table still aligns.

* Transparent series (color undefined) now also suppress the line, not
  just the fill. Flot otherwise auto-assigns a palette color and paints
  a visible line along the series' data.

Verified against every graph definition in graph-defs/: the overlay
merge only triggers on strafeping (no other definition has the required
transparent-AREA + labeled-STACK pattern at that count), and the only
colorless AREA/LINE/STACK commands in the tree are the strafeping
overlay bases.
* Guard the overlay merge against mismatched series lengths: step 2
  reads stackSeries.data[j] for every j in [0, numValues). Every series
  built in drawChart has exactly numValues points, but if a caller ever
  hands us mismatched lengths we now bail out instead of reading OOB.

* Rename `seriesSteps` to `useStepRendering` — it's a per-series
  step-rendering flag, not a Flot config named `steps`.

* Document the behavior change introduced when colorless series started
  suppressing their line: `LINE:foo` without a color used to render in
  Flot's auto-assigned palette color and now renders as nothing. No
  shipped OpenNMS graph definition uses that form; the comment captures
  the rationale for future readers who hit a regression.

* Clarify the scope of `_getColor`'s normalization: only 8-digit hex
  with a 00 alpha is treated as transparent. 3-, 4-, and 6-digit values
  are passed through as-is.

* Drop the "6+" magic number from the `_mergeOverlaySegments` docstring.
  The threshold is already commented where it's enforced; having it in
  two places invited drift.
cgorantla
cgorantla previously approved these changes Apr 17, 2026
Copy link
Copy Markdown

@cgorantla cgorantla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

synqotik
synqotik previously approved these changes Apr 20, 2026
Copy link
Copy Markdown
Contributor

@synqotik synqotik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! just a couple small nits before merging.

Comment thread src/Backshift.Utilities.RrdGraphVisitor.js Outdated
Comment thread src/Backshift.Graph.Flot.js Outdated
@marshallmassengill marshallmassengill dismissed stale reviews from synqotik and cgorantla via 0e2fa23 April 20, 2026 17:47
Copy link
Copy Markdown
Contributor

@synqotik synqotik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@marshallmassengill marshallmassengill merged commit 1eac694 into OpenNMS:master Apr 20, 2026
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.

5 participants