Skip to content

NMS-20416: Make the custom graph builder filter-driven - #8932

Merged
dino2gnt merged 6 commits into
developfrom
dcy/customgraphspafilters
Oct 23, 2026
Merged

dino2gnt merged 6 commits into
developfrom
dcy/customgraphspafilters

Conversation

@dino2gnt

@dino2gnt dino2gnt commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

Each column accepts a filter. Nodes take an OpenNMS filter rule or a label. Resources and datasources take a wildcard pattern. The graph shows all matches unless the user selects items. Links store the filters and are evaluated again when opened.

Might could use additional docs around this. 🤷

External References

Each column accepts a filter. Nodes take an OpenNMS filter rule or a label.
Resources and datasources take a wildcard pattern. The graph shows all matches
unless the user selects items. Links store the filters and are evaluated again
when opened.
@christianpape

christianpape commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Problems / errors

  1. Series labels can change, which silently breaks expressions (AdhocGraphBuilder.vue:423). Generated labels are now rebuilt every time on purpose. Uniqueness comes from uniqueLabel, which gives the first series in order the plain name and later ones _2, _3. If a filter starts matching another node whose series sorts earlier, an existing series can go from ifHCInOctets_eth0 to ifHCInOctets_eth0_2, or the other way round. Any expression that used the old name then points at a different series or at nothing, with no warning. That is exactly what a link that "follows inventory" will do over time. Only labels the user edited are stable.
  2. The unbounded rule query is heavy on large installs (nodeService.ts, getNodesByFilterRule).
  • Typing nodeFilter() becomes IPADDR != '0.0.0.0' with limit: 0, so the server returns every node.
  • On the server, the v1 filterRule path does an IN (all ids) query and left-joins snmpInterfaces, ipInterfaces, categories, asset records and services with distinct. It then serializes full v1 OnmsNodes.
  • MAX_GRAPH_NODES (200) is only checked after the whole result has arrived in the browser.
  • Above about 32k nodes, the IN list will probably exceed the PostgreSQL JDBC limit on bind parameters.

It would be better to check the count first, or to request id and label only.

  1. The resource and datasource filters run on every keystroke with no debounce (AdhocGraphBuilder.vue:78).
  • datasourceCandidates is the product of nodes × resources × attributes. At 200 nodes, ×400 interfaces, ×~30 attributes, that is millions of objects.
  • Each keystroke rebuilds those objects, reconcileSeries runs again through a deep watcher (:710), and every SelectionColumn recomputes countLabel.

Expect UI jank. The deep: true watch is also unnecessary, because the computed returns a new array each time anyway.

  1. "Remove series" and "Select all" turn a dynamic filter into a fixed list (AdhocGraphBuilder.vue:474, SelectionColumn.vue:196).
  • Removing one series from a 150-series filter result writes the other 149 keys into pd=. The link stops following inventory, and it quickly goes over MAX_QUERY_LENGTH, which shows the "too many selections to share" message.
  • "Select all" on a rule that matches 2,000 nodes does the same with pn=.

Users aren't told about either.

  1. Clearing while a node filter is still debouncing (clearAll at :541 plus SelectionColumn.vue:161). If the user types into an empty node box and presses Clear within 500 ms:
  • The pending setNodeFilter("ab") still runs after the clear and fills the picker again.
  • The box keeps showing "ab", because the prop stayed '' and the watcher never fired.
  1. Old links with resources not tied to a node lose those series (adhocUrlState.ts legacy branch). nodeCriteriaOf() returns null for such a resource, so no node is picked. The resource is then never "reachable" and the series disappears. The old code graphed it straight from its resourceId. This is low impact, since the old picker was also node-driven.

  2. Rule detection can misread label searches (adhocGraphStore.ts:71, looksLikeFilterRule). A label search for like, isLab…, or anything containing ', ! or = (for example O'Brien) is sent to the filter engine. The user then sees "could not be parsed" instead of search results, and nothing explains why.

Side effects

  • Some nodes are left out. nodeFilter() becomes IPADDR != '0.0.0.0', which excludes nodes with no IP interfaces. That doesn't really mean "every node".
  • Resource picks are deleted on first narrowing. In setPickedNodes (:576), going from zero node picks to one prunes resource picks under every other node that matched the filter. The comment describes this as "deselecting", but it is really narrowing, and the picks are gone for good.
  • Failed node lookups aren't reported.
  • A failed getResourceForNode leaves that node out of the candidates without any message. It is fetched again on every evaluation, because failures aren't cached.
  • A failed label search (getNodes returning false) shows an empty list instead of an error.
  • Misleading message on link open. The "this link now matches N series" snackbar also appears when the difference comes from a temporary fetch failure.
  • Bigger URLs. Old links decode as complete overrides for every field, so they get noticeably longer once re-encoded.
  • Any UI user can run filter rules. The UI now sends free-form filter rules to the server for every UI user. The endpoint already accepted them, so this adds no new exposure, but rules are far more reachable now. Point 2 is the main concern there.

Count rule matches before fetching, and fetch no more than the node limit.
Debounce the resource and datasource boxes. Tell the user when a selection
pins a filtered column. Cancel a pending filter on Clear. Keep selections
when a filter narrows, drop them only on deselection. Report failed lookups.

@marshallmassengill marshallmassengill left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this is good to go. Probably worth some others providing input but overall, much more usable.

@christianpape

Copy link
Copy Markdown
Contributor

Errors / likely bugs

  1. Picks that are currently hidden get thrown away, contrary to the PR's own design. The store keeps picks under nodes the filter no longer matches (reachablePickedResources/reachablePickedDatasources, "kept, not dropped"). But the columns only show the reachable subset, and any click writes that subset back through setPickedResources/setPickedDatasources. removeSeries does the same. So one click while a filter is narrowed silently deletes the hidden picks.

  2. Shared links no longer reproduce colors. Unedited series colors are no longer written into the link; only overrides are. On restore, expressions get palette slots 0..k-1 (seriesColor(index)), and series then start after them. In the original session, series were colored first and expressions after (addExpression uses series.length + expressions.length). So the recipient sees different colors than the sender.

  3. Edits vanish from the URL while a series is filtered out. linkState() only carries overrides for series currently in config.series. When a filter temporarily excludes a series, or the 200-series cap empties config.series, its label/color/style edits drop out of the URL. They survive in memory, but are lost on reload or share.

  4. A label fragment is treated as a paged filter. Any non-empty text that isn't a rule makes effectiveNodes = nodeMatches, which is the first 100 label matches. So typing r graphs an arbitrary, alphabetically truncated 100 nodes, and a link carrying it re-evaluates to a different page as inventory changes. A truncated page shouldn't count as "the filter matched".

  5. Very large rule matches get the wrong error. The comment admits that a match too big to bind as an IN list makes the probe itself fail. The user then sees "could not be evaluated" instead of the "too many nodes" message, which is exactly the case where the limit should explain itself.

Side effects

  • Server load: every settled keystroke in the node box (500 ms debounce) can trigger up to 100 (label search) or 200 (rule) fornode calls for the full subtree, each reading RRD/JRB attribute metadata. Before this PR, resources were only fetched for nodes the user explicitly selected. Every rule evaluation also hits the filter engine twice (count probe, then fetch).
  • The cache never refreshes: nodeResourceCache is only cleared by Clear all. New interfaces or resources stay invisible until reload, and a transient failure is cached as null for the rest of the session.
  • Links blow up: removing one series from a filter-driven graph pins all the remaining datasource keys (up to 199) into pd. A short filter link becomes one that probably exceeds MAX_QUERY_LENGTH, and it also stops following inventory. "Select all" on a large resource list has the same effect.
  • "Select all" changed meaning: it used to add the visible items to the selection; it now replaces the selection with everything listed. It also fires the "now a fixed selection" snackbar, even though with a filter active it's semantically the same as selecting nothing.
  • Race on concurrent loads: concurrent loadResources calls can fetch the same uncached node twice, since the cache is only written after a batch finishes.

@dino2gnt

Copy link
Copy Markdown
Contributor Author

@christianpape Run it again :)

@christianpape

Copy link
Copy Markdown
Contributor

Errors

  1. A test fails depending on locale. AdhocGraphBuilder.test.ts ("says when a label search is only showing its first page") expects 2,310. On a German-locale machine toLocaleString() gives 2.310, so the test fails. tooManySeriesMessage, nodeNote, the snackbars and SelectionColumn all depend on the locale the same way. Either pin the locale in the tests or pass an explicit locale.
  2. Hidden picks can silently empty the graph. Say datasources (or resources) were picked under node A, and then the node filter stops matching A. reachablePickedDatasources becomes [], but pickedDatasources.length > 0, so effectiveDatasources returns [] and ignores the datasource filter. The column then shows 0 selected and "all N matching", yet nothing is graphed. The only way out is to press Clear in that column without knowing why. Separately, deselecting a node prunes its resource picks but not the datasource picks under them, so orphaned picks build up and also travel in links.
  3. Series and expression colors can collide. Series now take palette slots from 0 and ignore expressions (seriesColor(next.length)). addExpression still uses series.length + expressions.length, and restore uses seriesColor(index). A series added later, when the filter matches one more item, can get the same color as an existing expression.

Side effects

  • Server load. A label fragment with ≤100 hits now becomes the effective node set. So typing a fragment fires up to 100 fornode requests (full resource trees) after each 500 ms debounce. A rule can do the same for up to 200 nodes. The cache softens this, but Refresh clears the whole cache and re-fetches every node, every time.
  • Large rules fail with a generic message. The backend turns the rule into an IN (...) list of every matching id before it applies limit, so a rule matching a huge estate gets a 500. The PR acknowledges this, but the user only sees the generic "could not be evaluated" message. limit=201 also still serializes 201 full nodes just to learn "too many".
  • Exclusions are invisible. excludedDatasourceKeys is never pruned. It survives filter changes, goes into links (xd), and nothing in the UI shows it. It is only cleared by picking datasources or by Clear all.
  • Edits are never dropped either. overrides and the generatedLabels map also grow until Clear all, and every override is written into the URL. Links get longer over a session and hit MAX_QUERY_LENGTH sooner.
  • Removing the last series doesn't cancel a pending filter. removeSeries clears the datasource filter but doesn't bump filterGeneration, so a debounced filter still in flight can bring it back.
  • Legacy links can lose their edits. On a server where storeByForeignSource has changed since the link was made, old node[7]… keys no longer match the generated nodeSource[…] keys. Labels and colors are orphaned, and expressions that refer to the old labels break.

Print counts as plain digits.
Treat a selection the filters cannot reach as no selection, show it as set aside, and drop datasource picks with their node.
Give series and expressions the first free palette colour.
Show and prune removed series, prune stale edits, cancel a settling filter when the last series is removed, and refresh only the effective nodes.

@marshallmassengill marshallmassengill left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Blocker now and something else worth looking at I think:

Blocker: a failed load deletes the edits of a link.

  • Commit 710d2b5 causes this problem. Prior one doesn't have it.
  • If the rule fails, the rule matches too many nodes, or a fornode request fails, the page deletes the label and color edits.
  • After the user corrects the problem, the edits do not come back.
  • After 400 ms, the page writes the address bar again without o=. A reload or a bookmark does not restore the edits.
  • Possible fix: do not delete edits after a load that has failures, a rule error, or too many matches.

Finding: Unchecking the last visible pick deletes the hidden picks.

  • The column sends [], and the page clears all picks.
  • The PR test at AdhocGraphBuilder.test.ts:378-381 says that this must not occur, but it tests that it does occur.

Prune edits only after a load with nothing missing, so a failed rule or
node lookup does not delete a link's edits. Make Clear its own event, so
unchecking the last listed item keeps the selections set aside. Enable
Refresh whenever there are nodes to reload.
@dino2gnt

Copy link
Copy Markdown
Contributor Author

Mmm... yeah. That should be fixed now.

@marshallmassengill marshallmassengill left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good enough for me!

@dino2gnt

Copy link
Copy Markdown
Contributor Author

Chandra said to just merge it. Full send!

@marshallmassengill marshallmassengill left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ship it!

@dino2gnt
dino2gnt merged commit 5afcc6d into develop Oct 23, 2026
5 of 8 checks passed
@dino2gnt
dino2gnt deleted the dcy/customgraphspafilters branch October 23, 2026 20:32
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.

3 participants