[OpenNMS/opennms] 7dbe35: chore: ignore Claude Code and local triage working...

  Branch: refs/heads/ronny-poc/victorialogs-flows
  Home:   https://github.com/OpenNMS/opennms
  Commit: 7dbe354b1a2eaa77789d5edba8bb0110ecbf5960
      https://github.com/OpenNMS/opennms/commit/7dbe354b1a2eaa77789d5edba8bb0110ecbf5960
  Author: Ronny Trommer <[email protected]>
  Date:   2026-07-30 (Thu, 30 Jul 2026)

  Changed paths:
    M .gitignore

  Log Message:
  -----------
  chore: ignore Claude Code and local triage working directories

.claude/ is an AI tool working directory and triage/ holds local drafts,
benchmark runs and release announcements. Neither is repository content.

Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Ronny Trommer <[email protected]>


  Commit: 3548a327506022b052d581ba7d4e0c1a88ed497e
      https://github.com/OpenNMS/opennms/commit/3548a327506022b052d581ba7d4e0c1a88ed497e
  Author: Ronny Trommer <[email protected]>
  Date:   2026-07-30 (Thu, 30 Jul 2026)

  Changed paths:
    M features/flows/itests/pom.xml
    A features/flows/itests/src/test/java/org/opennms/netmgt/flows/elastic/FlowQueryReferenceCaptureIT.java

  Log Message:
  -----------
  test(flows): capture the Elasticsearch and Drift query reference

FlowQueryReferenceCaptureIT records what Elasticsearch plus the Drift
plugin answer for every FlowQueryService method -- 61 cases over 19
methods and 4 filter sets -- into flow-query-reference.json, together
with the eight indexed documents that produced them.

This is an oracle, not a test of the Elasticsearch path. The
proportionally attributed byte series are the output of a closed-source
Elasticsearch plugin, so agreement with the recorded answers is the only
way an alternative backend can be shown to be correct. The corpus is
defined in this class rather than shared with FlowQueryIT on purpose: an
oracle whose inputs can be changed from another file is not an oracle.
The spot checks at the end mirror FlowQueryIT's own expectations so that
a corpus or enrichment change fails loudly here instead of quietly
recording the wrong answers.

Every time range is a fixed constant and table rows and columns are
emitted in sorted order, so the output is byte-identical across runs.

The testcontainers version is taken from the property the rest of the
build already uses; the two pinned 1.17.6 declarations predate it and
cannot start a container against a current Docker.

Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Ronny Trommer <[email protected]>


  Commit: 1c80fe4cb43923b44e87de180ba5a05909019d6b
      https://github.com/OpenNMS/opennms/commit/1c80fe4cb43923b44e87de180ba5a05909019d6b
  Author: Ronny Trommer <[email protected]>
  Date:   2026-07-30 (Thu, 30 Jul 2026)

  Changed paths:
    M container/features/src/main/resources/features-sentinel.xml
    M container/features/src/main/resources/features.xml
    M features/flows/pom.xml
    A features/flows/victorialogs/pom.xml
    A features/flows/victorialogs/src/main/java/org/opennms/netmgt/flows/victorialogs/ElasticFuzziness.java
    A features/flows/victorialogs/src/main/java/org/opennms/netmgt/flows/victorialogs/FlowJsonSerializer.java
    A features/flows/victorialogs/src/main/java/org/opennms/netmgt/flows/victorialogs/IngestStats.java
    A features/flows/victorialogs/src/main/java/org/opennms/netmgt/flows/victorialogs/LogsQlFilterVisitor.java
    A features/flows/victorialogs/src/main/java/org/opennms/netmgt/flows/victorialogs/ProportionalSeriesService.java
    A features/flows/victorialogs/src/main/java/org/opennms/netmgt/flows/victorialogs/ProportionalSumQuery.java
    A features/flows/victorialogs/src/main/java/org/opennms/netmgt/flows/victorialogs/VictoriaLogsClient.java
    A features/flows/victorialogs/src/main/java/org/opennms/netmgt/flows/victorialogs/VictoriaLogsClientConfig.java
    A features/flows/victorialogs/src/main/java/org/opennms/netmgt/flows/victorialogs/VictoriaLogsException.java
    A features/flows/victorialogs/src/main/java/org/opennms/netmgt/flows/victorialogs/VictoriaLogsFlowQueryService.java
    A features/flows/victorialogs/src/main/java/org/opennms/netmgt/flows/victorialogs/VictoriaLogsFlowRepository.java
    A features/flows/victorialogs/src/main/java/org/opennms/netmgt/flows/victorialogs/VictoriaLogsHealthCheck.java
    A features/flows/victorialogs/src/main/resources/OSGI-INF/blueprint/blueprint.xml
    A features/flows/victorialogs/src/test/java/org/opennms/netmgt/flows/victorialogs/ElasticFuzzinessTest.java
    A features/flows/victorialogs/src/test/java/org/opennms/netmgt/flows/victorialogs/FlowJsonSerializerTest.java
    A features/flows/victorialogs/src/test/java/org/opennms/netmgt/flows/victorialogs/LogsQlFilterVisitorTest.java
    A features/flows/victorialogs/src/test/java/org/opennms/netmgt/flows/victorialogs/ProportionalSumIT.java
    A features/flows/victorialogs/src/test/java/org/opennms/netmgt/flows/victorialogs/ProportionalSumQueryTest.java
    A features/flows/victorialogs/src/test/java/org/opennms/netmgt/flows/victorialogs/ReferenceComparisonIT.java
    A features/flows/victorialogs/src/test/java/org/opennms/netmgt/flows/victorialogs/TestFlow.java
    A features/flows/victorialogs/src/test/java/org/opennms/netmgt/flows/victorialogs/VictoriaLogsFlowRepositoryIT.java
    A features/flows/victorialogs/src/test/java/org/opennms/netmgt/flows/victorialogs/VictoriaLogsFlowRepositoryTest.java
    A features/flows/victorialogs/src/test/java/org/opennms/netmgt/flows/victorialogs/VictoriaLogsHealthCheckTest.java
    A features/flows/victorialogs/src/test/java/org/opennms/netmgt/flows/victorialogs/WriterReaderContractTest.java
    A features/flows/victorialogs/src/test/resources/flow-query-reference.json
    M opennms-full-assembly/pom.xml

  Log Message:
  -----------
  feat(flows): add a VictoriaLogs flow backend

Adds features/flows/victorialogs, which both persists flows to
VictoriaLogs and answers flow queries from it, as an alternative to
Elasticsearch plus the OpenNMS Drift plugin.

The write path plugs in at org.opennms.integration.api.v1.flows
.FlowRepository, an OIA v1 extension point, so it ships alongside the
Elasticsearch persister rather than replacing it. persist() never does
I/O: it buffers and returns, and a ScheduledExecutorService owns all
sending. PipelineImpl walks its persisters serially on the enrichment
thread, so an HTTP call there would put this backend's latency in front
of Elasticsearch's and a merely slow backend would throttle flow
persistence for everyone. Ingestion is /insert/jsonline rather than an
Elasticsearch _bulk emulation: _bulk would let the existing Jest writer
be repointed by configuration alone, but only by carrying Jest 5.3.4
forward, which is the abandoned client this exists to retire.

The read path reproduces the Drift plugin's proportional_sum aggregation
in LogsQL, pushed down to the server. A flow rarely lines up with the
buckets a chart wants, so it must contribute to each bucket in
proportion to the overlap; LogsQL has no sequence generator, so the
fan-out is a literal index array expanded by unroll, indexed from the
flow's own first bucket rather than the window's so that the row count
per flow depends only on how long a flow may last and not on how wide
the query is. Removing the need for a native Elasticsearch plugin that
must be installed on every node and rebuilt for every Elasticsearch
version is the point of the module.

Not everything survives translation. Elasticsearch matches applications
with a fuzzy query and hosts and conversations with Lucene's anchored
regexp; LogsQL has no fuzzy operator and its ~ is unanchored RE2, so
ElasticFuzziness reimplements Damerau-Levenshtein with Elasticsearch's
AUTO schedule and the regex paths compile to an anchored java.util
.regex.Pattern. Those listings therefore cannot be pushed down and pull
distinct values back under a ceiling that logs when it truncates.

VictoriaLogsFlowQueryService is deliberately not published from
blueprint.xml: every method still blocks the caller's thread, so wiring
it up is a separate step.

ReferenceComparisonIT ingests the recorded corpus and re-asks all 61
questions FlowQueryReferenceCaptureIT recorded. All 19 methods agree,
with four known divergences that are reported on every run and whose
list is kept honest -- a case listed there that starts agreeing fails
the build, so a stale excuse cannot sit in it unnoticed.

Nothing depends on this module; it is named only from features.xml, so
opennms-full-assembly needs an explicit dependency or its bundle never
reaches system/ and 'feature:install flow' fails to resolve, taking all
flow persistence down rather than just this backend.

Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Ronny Trommer <[email protected]>


  Commit: 7e5fe95246bd379d8b90e08011c45387047a94ab
      https://github.com/OpenNMS/opennms/commit/7e5fe95246bd379d8b90e08011c45387047a94ab
  Author: Ronny Trommer <[email protected]>
  Date:   2026-07-30 (Thu, 30 Jul 2026)

  Changed paths:
    M features/flows/elastic/src/main/resources/org/opennms/netmgt/flows/elastic/common.ftl
    M features/flows/itests/src/test/java/org/opennms/netmgt/flows/elastic/FlowQueryReferenceCaptureIT.java
    M features/flows/victorialogs/src/test/resources/flow-query-reference.json

  Log Message:
  -----------
  fix(flows): guard the direction script against absent SNMP interface fields

The Painless script in common.ftl resolves an undetermined flow direction
against the interface being queried, and read doc['netflow.input_snmp']
.value without first checking the field exists. Reading .value off a
field a document does not have is an error in Painless rather than a
miss, so it failed the shard -- and one such document failed the whole
search with HTTP 400.

Both conditions come from the exporter and both are ordinary. A missing
direction field maps to UNKNOWN in Netflow9MessageBuilder, and the two
interface fields are mapped independently, so a record can carry an
egress interface and no ingress one. A record only carries the interface
its direction implies, which makes an unknown-direction flow naming just
its output exactly the shape that fails.

Measured against Elasticsearch 8.18.2 with Drift 2.0.7: filtering on an
interface whose matched set contains such a record turned every summary
and series method into a 400, while getFlowCount -- which uses no script
-- still answered. Excluding that one record by DSCP made the same
queries succeed, which isolates the cause to the absent field rather
than to the undetermined direction itself.

Guarding every access with size() also settles what the record resolves
to: ingress when the interface is its input, egress when its output,
either way regardless of whether the other field is present. That is what
filter_snmp_interface.ftl already documents as the intent. Falling off
the end stays unreachable, because that filter only admits an
unknown-direction record when the interface is one of the two.

The reference corpus grows three undetermined-direction flows to cover
this, one per branch of the script, each with its own DSCP so a filter
can select it alone. They sit on a second SNMP interface, which every
pre-existing filter set pins away from, so all 61 recorded answers are
unchanged -- verified byte-identical rather than assumed.

Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Ronny Trommer <[email protected]>


  Commit: 487c0ec344d260d4c8aa7c29dfffa6c679a7708a
      https://github.com/OpenNMS/opennms/commit/487c0ec344d260d4c8aa7c29dfffa6c679a7708a
  Author: Ronny Trommer <[email protected]>
  Date:   2026-07-30 (Thu, 30 Jul 2026)

  Changed paths:
    M features/flows/victorialogs/src/main/java/org/opennms/netmgt/flows/victorialogs/ProportionalSumQuery.java
    M features/flows/victorialogs/src/main/java/org/opennms/netmgt/flows/victorialogs/VictoriaLogsFlowQueryService.java
    M features/flows/victorialogs/src/test/java/org/opennms/netmgt/flows/victorialogs/ProportionalSumIT.java

  Log Message:
  -----------
  fix(flows): resolve undetermined flow directions in the VictoriaLogs backend

netflow.direction has three values and Directional has two, and every
record that was not ingress had been counted as egress. For an exporter
that omits the direction field -- which Netflow9MessageBuilder maps to
UNKNOWN, so every one of its records is affected -- that reported all
inbound traffic as outbound.

Records are now resolved the way common.ftl does: the interface the query
filtered on was the flow's input, so it was inbound, or its output, so it
was outbound. Where no interface was filtered on there is nothing to
resolve against and the records are excluded, which is what the series
templates do. Both are driven off the SnmpInterfaceIdFilter, so the two
interface fields join the grouping only when there is an interface to
compare them against and the cheaper query is unchanged.

Note what deliberately does not exclude them: top_n_terms.ftl carries no
direction constraint, so these records still count toward the Top-N
ranking while their bytes go unreported, and flow_count.ftl still counts
them. Both are quirks of the reference that the recorded answers depend
on.

Also fixes a bucketing edge the extended corpus exposed. A flow ending
exactly on a bucket boundary reaches the bucket beginning there without
occupying any of it, and that bucket was dropped for having no overlap.
The reference reports it as zero, and the difference is visible rather
than cosmetic: an absent cell becomes NaN once rows are aligned, so a
chart drew a gap where the reference draws a zero. The bucket rather than
the overlap is now what the query filters on, which leaves the leading
boundary alone -- a flow starting on a boundary does occupy the bucket it
starts in. ProportionalSumIT recorded the old behaviour and is updated;
byte conservation is unaffected.

Reference comparison is 72/72 with the four known hostname divergences.

Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Ronny Trommer <[email protected]>


  Commit: 5cb580028b246aa3aac9f8a51637fbd28f8a5955
      https://github.com/OpenNMS/opennms/commit/5cb580028b246aa3aac9f8a51637fbd28f8a5955
  Author: Ronny Trommer <[email protected]>
  Date:   2026-07-30 (Thu, 30 Jul 2026)

  Changed paths:
    M features/flows/victorialogs/src/main/java/org/opennms/netmgt/flows/victorialogs/LogsQlFilterVisitor.java
    R features/flows/victorialogs/src/main/java/org/opennms/netmgt/flows/victorialogs/ProportionalSeriesService.java
    M features/flows/victorialogs/src/main/java/org/opennms/netmgt/flows/victorialogs/ProportionalSumQuery.java
    M features/flows/victorialogs/src/main/java/org/opennms/netmgt/flows/victorialogs/VictoriaLogsFlowQueryService.java
    M features/flows/victorialogs/src/test/java/org/opennms/netmgt/flows/victorialogs/LogsQlFilterVisitorTest.java
    A features/flows/victorialogs/src/test/java/org/opennms/netmgt/flows/victorialogs/ProportionalSeriesService.java
    M features/flows/victorialogs/src/test/java/org/opennms/netmgt/flows/victorialogs/ProportionalSumQueryTest.java
    A features/flows/victorialogs/src/test/java/org/opennms/netmgt/flows/victorialogs/VictoriaLogsFlowQueryServiceTest.java

  Log Message:
  -----------
  fix(flows): address code review of the VictoriaLogs read path

Twelve findings from an adversarial review of the query path. The three
that matter most were invisible to the reference comparison, which is the
point worth recording: it passes 72/72 over eleven flows with no duplicate
labels, no self-flows, and row order normalised away before comparing.

Byte totals were truncated per result row before being summed. Adding the
SNMP interface fields to the grouping -- which the direction fix needed --
turned one row per entity and direction into many, so what had matched
Elasticsearch exactly now lost up to a byte per row and made a summary
disagree with the sum of its own series. Accumulated as a double and
rounded once, at the end.

Top-N series lost their ranking. RawFlowQueryService ends with
TableUtils.sortTableByRowKeys; nothing here did, so rows came back in hash
order -- and FlowRestServiceImpl builds a chart's columns straight from
rowKeySet(), so the busiest entity landed anywhere and moved between two
identical calls. ReferenceComparisonIT sorts rows before comparing and so
could never have caught it.

An entity whose name collided with a label was mishandled twice over. One
literally named Unknown was ranked as two entities, took two of the N
slots, and had its bytes reported under Other; Elasticsearch merges the
two buckets before ranking via the terms aggregation's "missing" parameter, so topN
now over-fetches and merges to match. One named Other was silently added
to the complement in the series path, and then crashed relabel() on a
duplicate row key.

Also: a flow whose two endpoints are the same host was counted twice,
where FlowDocument.hosts is a Set and Elasticsearch counts it once; an
empty entity set with includeOther answered a request naming nothing with
a chart of everything; ecnFlags truncated at ten thousand groups and
reported the remainder as having seen no congestion, without the warning
its two siblings carry; N was used as an ArrayList capacity straight from
an unbounded REST parameter; control characters went unescaped into
quoted LogsQL tokens, with the escaping rule duplicated in two classes;
and a parse failure escaped as a bare NumberFormatException from methods
declaring VictoriaLogsException.

ProportionalSeriesService moves to test scope. It was public production
code that nothing called, implementing the same aggregation with the old
naive direction handling -- wiring it up would have silently produced
different numbers from the code the reference validates.

One reported finding is not fixed because it was wrong: NodeCriteria was
said to admit a half-populated state rendering a literal null into the
query. All three of its constructors either requireNonNull the node id or
set the foreign source and id, so that state cannot be built. The guard
was written, then removed once verified; the javadoc now records why
there is deliberately none.

Three comments asserted behaviour the code does not have, including one
claiming LogsQL sorts numerals as text -- the recorded Top-N answers rank
2310 above 300, so it does not.

Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Ronny Trommer <[email protected]>


  Commit: a6d8edd038a02aa28cd74aae49766004c9686db5
      https://github.com/OpenNMS/opennms/commit/a6d8edd038a02aa28cd74aae49766004c9686db5
  Author: Ronny Trommer <[email protected]>
  Date:   2026-07-30 (Thu, 30 Jul 2026)

  Changed paths:
    M features/flows/victorialogs/src/main/java/org/opennms/netmgt/flows/victorialogs/FlowJsonSerializer.java
    M features/flows/victorialogs/src/main/java/org/opennms/netmgt/flows/victorialogs/VictoriaLogsClient.java
    M features/flows/victorialogs/src/main/java/org/opennms/netmgt/flows/victorialogs/VictoriaLogsClientConfig.java
    M features/flows/victorialogs/src/main/java/org/opennms/netmgt/flows/victorialogs/VictoriaLogsFlowRepository.java
    M features/flows/victorialogs/src/test/java/org/opennms/netmgt/flows/victorialogs/FlowJsonSerializerTest.java
    M features/flows/victorialogs/src/test/java/org/opennms/netmgt/flows/victorialogs/TestFlow.java
    A features/flows/victorialogs/src/test/java/org/opennms/netmgt/flows/victorialogs/VictoriaLogsClientConfigTest.java
    M features/flows/victorialogs/src/test/java/org/opennms/netmgt/flows/victorialogs/VictoriaLogsFlowRepositoryIT.java
    M features/flows/victorialogs/src/test/java/org/opennms/netmgt/flows/victorialogs/VictoriaLogsFlowRepositoryTest.java

  Log Message:
  -----------
  fix(flows): address code review of the VictoriaLogs write path

Twelve findings from an adversarial review of the ingestion path. Unlike
the query path, this code is reachable today -- the module is wired into
features.xml and the repository is published from blueprint.xml -- so
these are live rather than latent.

An Error was enough to end all flushing for the life of the process.
flushSafely caught Exception, and scheduleWithFixedDelay cancels a task
that throws, so an OutOfMemoryError -- which serialization can raise, as
it holds the NDJSON string, its UTF-8 bytes and the gzipped copy at once
-- would silently cancel the schedule and leave a repository that accepts
flows forever and sends none. Nothing was logged, because the executor
buries the cause in a future nobody reads. Both the send and the
scheduled task now catch Throwable; the send counts the batch as dropped
so the promise that flowsPersisted plus flowsDropped covers every flow
handed over survives an Error too. Writing the test for this is what
showed the first attempt only fixed half of it.

Shutdown could hang the container. stop() drained the whole buffer
synchronously with no deadline, and a backend that is slow rather than
broken makes that a hundred sequential requests at thirty seconds each on
the blueprint destroy thread. A failing backend was always fine, since
the first failure ends the loop. The drain now has its own deadline,
separate from the executor join it was being confused with.

A NaN or infinite sampling interval produced output that is not JSON.
Gson's lenient writer emits the bare token, VictoriaLogs cannot parse the
line, skips it, and still answers 2xx -- so the flow was lost silently
and counted as sent. The value is a boxed Double taken from an
exporter-supplied protobuf field, so this is reachable input. Non-finite
numbers are now omitted, as nulls already were.

One unrenderable flow no longer costs its whole chunk. Serialization ran
inside the send's try, so a single bad record failed the batch and all
999 others were counted as dropped. Each line is independent by design;
the bad one is now skipped and counted, and the rest go out.

Also: maxBufferedFlows was the one setter without validation, and zero --
a plausible reading of "unlimited" -- silently dropped every flow while
blaming VictoriaLogs; a url with no scheme was accepted at construction
because URI.create finds it a valid relative URI, then failed every send
forever; half a credential pair downgraded to anonymous and produced a
401 per batch; flushQueued was cleared on task entry rather than
completion, so the "only one flush is queued" invariant was false, and a
task discarded by shutdownNow left it latched true; an interrupted
shutdown reported the backend as unreachable; empty categories were
omitted where NodeDocument emits an empty array, breaking the
byte-comparability the serializer is built around.

Documentation corrected where it overstated: maxBufferedFlows bounds a
count of retained references, not heap; readTimeout is a total-exchange
deadline rather than a socket read timeout; timeField is deliberately not
a Config Admin property because the serializer writes _time specifically.

flowsPersisted is now documented for what it is -- flows handed to a
request VictoriaLogs accepted, not flows stored -- and the IT that
demonstrates the difference now asserts it, so the day someone wires up
reconciliation against fetchIngestStats the test says where to look.

Three reported findings are not fixed because they were wrong. Emitting
JSON null for flow_records, flow_seq_num, node_id and interface_id is
impossible: javap confirms all four are primitives. A null timestamp
falling back to epoch 0 does not diverge from Elasticsearch; FlowDocument
does exactly the same. And persist() buffering while disabled is already
prevented upstream by SwitchedFlowRepository.

Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Ronny Trommer <[email protected]>


  Commit: 36a434038e85b0158f0686ba4d1a04ed04f2cb61
      https://github.com/OpenNMS/opennms/commit/36a434038e85b0158f0686ba4d1a04ed04f2cb61
  Author: Ronny Trommer <[email protected]>
  Date:   2026-07-30 (Thu, 30 Jul 2026)

  Changed paths:
    M features/flows/elastic/src/main/resources/org/opennms/netmgt/flows/elastic/common.ftl
    M features/flows/itests/src/test/java/org/opennms/netmgt/flows/elastic/FlowQueryReferenceCaptureIT.java
    M features/flows/processing/src/test/java/org/opennms/netmgt/flows/processing/FlowBuilder.java
    M features/flows/victorialogs/src/main/java/org/opennms/netmgt/flows/victorialogs/ProportionalSumQuery.java
    M features/flows/victorialogs/src/test/java/org/opennms/netmgt/flows/victorialogs/ReferenceComparisonIT.java
    M features/flows/victorialogs/src/test/resources/flow-query-reference.json

  Log Message:
  -----------
  fix(flows): close the oracle's blind spots and apply the sampling multiplier

An adversarial review of the reference capture -- the artifact that
defines what "correct" means for the whole VictoriaLogs backend -- found
that a wrong implementation could reproduce every recorded answer along
several dimensions. Extending the corpus to cover them immediately caught
a real defect.

netflow.sampling_interval was absent from all eleven corpus documents,
and it is the fourth field the Drift plugin's proportional_sum reads. An
exporter sampling 1-in-N reports only the bytes it saw, so the stored
figure has to be multiplied back up; this backend never did, and the
oracle could not tell. The new corpus records a 500-byte flow at an
interval of 10 as 5000 bytes, and the comparison failed on it in four
cases at once. Sampled export is the normal case on the hardware this
targets, so every such exporter's traffic was under-reported by its
sampling rate. Applied in ProportionalSumQuery only: top_n_terms.ftl
ranks on a plain unscaled sum, so the Top-N ranking deliberately does not
scale even though the totals it feeds do.

Four other blind spots are now covered and, reassuringly, already
correct: delta_switched equal to first_switched in every document, so
attributing from the wrong one was invisible; no document where the
direction field and the interface disagree, so an implementation that
ignored netflow.direction and inferred direction from an interface match
agreed everywhere; ecn zero throughout, leaving congestionEncountered
false in all sixty-three recorded rows; and no undetermined-direction
record with the queried interface at both ends, so which of the script's
two tests wins was never pinned down.

Covering the direction-disagreement case needed FlowBuilder to be able to
set both SNMP interfaces on a directed flow, which it could not: it wrote
only the interface a direction implies, which is precisely why the shape
was unreachable. Added as an opt-in alongside a sampling-interval setter,
so no existing caller changes.

The extension is additive. The five new flows sit on a third interface
and, more importantly, in a disjoint time window -- every pre-existing
filter set is bounded by the original window, including the one with no
interface filter at all -- so all seventy-two existing answers are
byte-identical, verified rather than assumed.

The capture itself had three ways to publish a wrong oracle. It wrote the
file before running its own spot checks, so a failed check still left a
complete-looking artifact for someone to copy out. recordSafely, added
earlier so one bad query could not destroy a whole capture, turned a
failed query into a recorded answer that cases.size() counted as a case;
a regressed direction script would have produced a green, published
reference whose direction cases were all error strings. And nothing
asserted what was actually recorded -- the spot checks re-issued a live
query, leaving every serializer unverified, while ReferenceComparisonIT
carries a textually identical copy of the same methods, so a transposition
would have cancelled out on both sides and still reported agreement. All
three are closed, and the file now declares its own case and corpus
counts for the comparison to check on load, since it is copied between
modules by hand.

One newly reachable case is a permanent divergence rather than a defect:
Elasticsearch evaluates the ECN flags once per direction bucket and lets
the last bucket processed overwrite the earlier one, so a group whose
ingress and egress records disagree reports whichever direction its terms
aggregation happened to return last. This backend reports a flag when any
record in the group carries it. Recorded in KNOWN_DIVERGENCES, which
fails the run if a listed case ever starts agreeing.

Also: common.ftl now guards with containsKey as well as size, matching
the ECN scripts in the same templates -- size covers a document without
the field, containsKey covers an index whose mapping lacks it, where
doc['x'] throws before size is reached. The DSCP constants are derived
from their TOS values rather than restated, the filter sets built from
them assert they select the flows they were built to select, the
before-each cleanliness check now uses a window the corpus actually
occupies instead of a zero-width one that passed on a full index, and
params() refuses an odd argument list instead of silently dropping a key.

Reference comparison is 77/77 with five known divergences. FlowQueryIT
still passes against real Elasticsearch.

Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Ronny Trommer <[email protected]>


  Commit: 17fdafcfa3d3a109351d6a81e6a88a2f2debec7e
      https://github.com/OpenNMS/opennms/commit/17fdafcfa3d3a109351d6a81e6a88a2f2debec7e
  Author: Ronny Trommer <[email protected]>
  Date:   2026-07-30 (Thu, 30 Jul 2026)

  Changed paths:
    M features/flows/victorialogs/src/main/java/org/opennms/netmgt/flows/victorialogs/VictoriaLogsFlowQueryService.java
    M features/flows/victorialogs/src/main/resources/OSGI-INF/blueprint/blueprint.xml
    M features/flows/victorialogs/src/test/java/org/opennms/netmgt/flows/victorialogs/ReferenceComparisonIT.java
    M features/flows/victorialogs/src/test/java/org/opennms/netmgt/flows/victorialogs/VictoriaLogsFlowQueryServiceTest.java

  Log Message:
  -----------
  feat(flows): answer flow queries off the caller's thread, and publish the service

Two steps that only make sense together: the query service could not be
published while every method blocked its caller, and there was no point
fixing the threading while nothing could reach it.

supply() completed its future inline, so the CompletableFuture in every
signature was decorative. Answering a summary takes three to five round
trips and a series three to four, and they are sequential by nature -- a
Top-N ranks before it totals -- so each one held whichever thread the REST
layer handed over for the whole exchange. A slow VictoriaLogs would have
tied up request threads rather than merely answering slowly. Work now runs
on a bounded pool the service owns and releases on shutdown.

The queue is bounded and rejection is reported rather than absorbed. An
unbounded queue turns a backend that has stopped answering into heap
exhaustion, and the obvious alternative -- running rejected work on the
calling thread -- quietly restores the exact behaviour this removes. The
task body catches Throwable, because a future nobody completes is worse
than one that fails: an Error escaping the worker would leave the caller
waiting for a result that can never arrive.

The service is registered at a negative ranking, and that is the load-
bearing part. FlowRepository tolerates several registrations because every
persister is used, but FlowQueryService is consumed through a singleton
reference in features/flows/rest/impl, which binds to exactly one
implementation. features/flows/elastic registers its own with no ranking,
i.e. zero. Two equally-ranked services would leave the choice to
service.id, so which backend answered the entire flow UI would depend on
bundle start order and could differ between restarts. At -1 the outcome is
deterministic: where the Elasticsearch bundle is present it keeps serving
and nothing changes; VictoriaLogs answers only if that bundle is absent or
stopped. Raising it above zero is what hands the UI over, and that is left
as a deliberate one-line change.

The ranking is a literal rather than a property placeholder, following the
opennms-icmp pinger bundles, which are the existing precedent for
several implementations of one interface.

maxFlowDurationMs becomes a configuration property. It bounds the
query-side fan-out and is not a cap -- a flow running longer is attributed
only to its first buckets and the rest is silently uncounted -- so an
install whose exporters use a longer active timeout needs to raise it.

Not verified in a running container: there is no Karaf in this build and
the test suite does not exercise blueprint. What is checked is that the
XML is well formed, the bundle resolves, and bnd imports
org.opennms.netmgt.flows.api. The ranking behaviour above is reasoned from
the registrations, not observed.

Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Ronny Trommer <[email protected]>


  Commit: c5e28cf6cf560f3decad0c4d70c105f9161e6f68
      https://github.com/OpenNMS/opennms/commit/c5e28cf6cf560f3decad0c4d70c105f9161e6f68
  Author: Ronny Trommer <[email protected]>
  Date:   2026-07-30 (Thu, 30 Jul 2026)

  Changed paths:
    M features/flows/itests/src/test/java/org/opennms/netmgt/flows/elastic/FlowQueryReferenceCaptureIT.java
    M features/flows/victorialogs/src/main/java/org/opennms/netmgt/flows/victorialogs/VictoriaLogsFlowQueryService.java
    M features/flows/victorialogs/src/test/java/org/opennms/netmgt/flows/victorialogs/FlowJsonSerializerTest.java
    M features/flows/victorialogs/src/test/java/org/opennms/netmgt/flows/victorialogs/ProportionalSumIT.java
    M features/flows/victorialogs/src/test/java/org/opennms/netmgt/flows/victorialogs/ProportionalSumQueryTest.java
    A features/flows/victorialogs/src/test/java/org/opennms/netmgt/flows/victorialogs/QueryPostProcessingTest.java
    M features/flows/victorialogs/src/test/java/org/opennms/netmgt/flows/victorialogs/ReferenceComparisonIT.java
    M features/flows/victorialogs/src/test/java/org/opennms/netmgt/flows/victorialogs/TestFlow.java
    M features/flows/victorialogs/src/test/java/org/opennms/netmgt/flows/victorialogs/VictoriaLogsFlowQueryServiceTest.java
    M features/flows/victorialogs/src/test/java/org/opennms/netmgt/flows/victorialogs/VictoriaLogsFlowRepositoryIT.java
    M features/flows/victorialogs/src/test/java/org/opennms/netmgt/flows/victorialogs/VictoriaLogsFlowRepositoryTest.java

  Log Message:
  -----------
  test(flows): make the suite able to fail, and cover the fixes that shipped unverified

An adversarial review of the test suite itself, prompted by three earlier
rounds finding real defects in code the suite reported as green. The
headline is that it reported 77/77 as a print statement rather than an
assertion.

ReferenceComparisonIT could not fail when a query method stopped working.
A method throwing UnsupportedOperationException was counted as skipped and
continued past; TOTAL_QUERY_METHODS, compared and skipped existed only
inside the System.out.println, and the sole assertion was compared > 0. So
eighteen of the nineteen methods could regress to throwing and the
acceptance test would stay green on one trivial case. Worse, that
exception was never a reliable marker for "not implemented yet": sorting
a List.of, or mutating a Set.of, throws exactly it, so a genuine
regression was indistinguishable from progress not yet made. The skip path
is gone -- every method is implemented, so the exception is now a failure
like any other -- and the tally is asserted: every recorded case compared,
every method exercised. The scaffolding was right while the work was in
progress and became a hole the moment it finished.

KNOWN_DIVERGENCES claimed to be kept honest in both directions and was
honest in only one. An entry whose case agreed failed the run; an entry
matching no case at all sat there indefinitely, which is what happens when
the reference is regenerated with a renamed parameter or without the case.
Both directions are now enforced.

Four fixes from earlier rounds had shipped with nothing pinning them, and
none of it was visible from the suite: series row order is sorted away
before comparison, the corpus yields exactly one result row per group so
per-row truncation matched by accident, no corpus flow has the same host
at both ends, and no corpus value is ever literally "Other" or "Unknown".
QueryPostProcessingTest covers all four through a stubbed client, which
makes the input exact and reaches inputs the corpus cannot express.

Writing it found a real bug in one of those fixes. Asking for a host
called "Other" together with includeOther puts that name in the requested
order twice, so orderRows emitted its rows twice and the immutable table
rejected the duplicate cell -- the same crash the relabel merge was
written to prevent, one layer further up, and reached by a different
route. Rows are now emitted once however often their name is asked for.

TestFlow gained setters for addresses, ports, application and hostnames.
Their absence was not cosmetic: with no way to set an address no test
could build a self-flow, so the double-count guard was unverifiable, and
the stream-cardinality test's fixture varied only the conversation key
while its javadoc claimed addresses and ports -- meaning it could not have
detected src_addr or src_port reaching _stream_fields, which is the one
thing it exists to catch.

That test also passed when nothing had been ingested at all, since
awaitIngestion returns rather than failing on timeout and zero flows
create zero streams. It now asserts the rows arrived.

Also: ReferenceComparisonIT waited on an instance-wide row count, so a
shared VictoriaLogs -- a documented mode -- satisfied the wait
immediately with another test's rows and compared against a
partly-indexed corpus; it now waits relative to a baseline. The capture IT
gained a spot check on the series renderer to match the one on summaries,
since both are duplicated verbatim in the comparison and a matching
mistake would cancel out on both sides. A test whose name promised the
scheduled flush path never called start(), so the scheduler, requestFlush
and flushSafely had no coverage whatsoever; one now drives them, with a
javadoc that is explicit about what it still does not pin rather than
claiming the guard it cannot reach.

Smaller: an assertion that could not fail (a substring taken up to the
first newline cannot contain one), an over-long-flow test asserting a
bound that total data loss also satisfies rather than the exact 500 bytes
two buckets of fan-out produce, bit-exact Double comparison where every
sibling uses a delta, a poison-flow fixture unrenderable for several
reasons besides the intended one, a VLAN assertion that could not tell a
JSON string from a number, a cap asserted against itself, a Top-N test
that swallowed every exception and asserted only that some query
happened, setBulkFlushMs(0) in a setUp where the setter refuses
non-positive values, a per-call HttpClient never released, and two dead
imports.

One reported finding is not fixed because it was wrong: the unit tests do
not call setDisabled(false), which was read as either exercising a
disabled repository or contradicting the IT. Neither -- disabled gates
only the scheduler, and those tests drive sending through flushNow().

Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Ronny Trommer <[email protected]>


  Commit: eebb4bd837a7048419145c970d88427a1a255c14
      https://github.com/OpenNMS/opennms/commit/eebb4bd837a7048419145c970d88427a1a255c14
  Author: Ronny Trommer <[email protected]>
  Date:   2026-07-30 (Thu, 30 Jul 2026)

  Changed paths:
    M .gitignore
    M features/flows/victorialogs/pom.xml
    M features/flows/victorialogs/src/main/java/org/opennms/netmgt/flows/victorialogs/VictoriaLogsFlowQueryService.java
    A features/flows/victorialogs/src/main/java/org/opennms/netmgt/flows/victorialogs/VictoriaLogsQueryServiceRegistrar.java
    M features/flows/victorialogs/src/main/resources/OSGI-INF/blueprint/blueprint.xml
    A features/flows/victorialogs/src/test/java/org/opennms/netmgt/flows/victorialogs/VictoriaLogsQueryServiceRegistrarTest.java
    M opennms-full-assembly/pom.xml

  Log Message:
  -----------
  fix(flows): register the query service only when it is asked for

The service ranking added in the previous commit does not do what its
comment claimed, and the claim was flagged there as reasoned rather than
observed. Reading the Aries implementation shows why the reasoning was
incomplete.

A plain blueprint <reference> binds reluctantly. ReferenceRecipe.track
re-binds only for a greedy reference; untrack re-binds to the best of
whatever remains. So the sequence that matters is not a contest of
rankings but a departure. features/flows/elastic uses
update-strategy="reload", so editing any property in its .cfg destroys
that container and unregisters its FlowQueryService; the REST layer then
re-binds to the only candidate left, which is this backend at any ranking
because ranking cannot break a field of one; Elasticsearch re-registers
moments later and nothing moves back. From then until a restart the whole
flow UI is served by a backend still pointed at localhost:9428, while
Elasticsearch keeps ingesting normally and nothing in the log says why.
features/datachoices flips the same way and starts reporting a flow count
of zero.

A cold boot has the same shape for a different reason: the Elasticsearch
blueprint waits on eight mandatory references before it publishes and
this module waits on none, so it would ordinarily win the race outright.

Not being in the registry is the only state that cannot be bound by
accident, so registration is now programmatic and gated. Querying gets its
own flag rather than sharing the persistence one: writing is additive and
affects only this backend, while answering queries takes over what the
whole UI shows, and evaluating ingestion without handing the UI over is
the normal intermediate state. The residual limitation is written down
rather than hidden -- enabling it on a running system will not move a
reference that has already bound Elasticsearch, for the same reluctant
damping reason.

Shutdown left callers waiting forever. shutdownNow() discards queued
tasks, and a discarded task never runs its body, so the futures handed out
for those queries were neither completed nor failed --
FlowRestServiceImpl.waitForFuture calls get() with no timeout, so each one
held a request thread for the life of the process. Outstanding futures are
now tracked and failed on stop.

The justification for the opennms-full-assembly dependency was false.
karaf-maven-plugin's features-add-to-repository resolves the whole closure
of the opennms-flows feature, which is in that plugin's feature list, so
the bundle reaches system/ regardless -- as flows.elastic and flows.api
demonstrate by shipping without being dependencies there. The entry is
still worth keeping, for reactor ordering, and now says so.

Also: gson is pinned via gsonVersion rather than the jestGsonVersion
alias, which exists to track the client this module is meant to retire;
the explicit Bundle-Version instruction is removed, and bnd now emits
37.0.0.SNAPSHOT rather than being handed the raw Maven 37.0.0-SNAPSHOT,
which is not a valid OSGi version; the cfg-facing timeout properties carry
the Ms their setters do; the two defaults that are silently wrong when
mis-set say concretely what to set them to; a dead placeholder id is gone;
and triage/ moves out of the shared .gitignore, being one contributor's
directory rather than repository policy.

Several XML comments contained a double hyphen, which is not legal inside
an XML comment. Maven's parser tolerated it; a strict one does not.

Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Ronny Trommer <[email protected]>


  Commit: b2e5233347872116a7dfdeb931f6e1e9a466a78a
      https://github.com/OpenNMS/opennms/commit/b2e5233347872116a7dfdeb931f6e1e9a466a78a
  Author: Ronny Trommer <[email protected]>
  Date:   2026-07-30 (Thu, 30 Jul 2026)

  Changed paths:
    M features/flows/victorialogs/src/main/java/org/opennms/netmgt/flows/victorialogs/VictoriaLogsClient.java
    M features/flows/victorialogs/src/main/java/org/opennms/netmgt/flows/victorialogs/VictoriaLogsHealthCheck.java
    M features/flows/victorialogs/src/main/resources/OSGI-INF/blueprint/blueprint.xml
    M features/flows/victorialogs/src/test/java/org/opennms/netmgt/flows/victorialogs/VictoriaLogsClientConfigTest.java

  Log Message:
  -----------
  fix(flows): report a misconfigured VictoriaLogs instead of failing the container

Of the three risks held back from the wiring review as too consequential
to leave for later, two were real and one was not.

A bad url or half a credential pair threw from the VictoriaLogsClient
constructor. Blueprint constructs that bean unconditionally, and a
constructor that throws fails the whole container -- so the FlowRepository
registration, the metric set and, self-defeatingly, the health check whose
entire job is to say this backend is unwell all disappeared together. An
operator who blanked the url got no VictoriaLogs entry in
opennms:health-check at all, and the diagnosis lived only in a startup log
line they had no reason to look for.

The fault is now remembered rather than thrown: logged at ERROR on the way
past, reported by isHealthy(), named by getConfigurationError(), and
turned into a VictoriaLogsException by ingest, query and fetchIngestStats.
The health check reports the specific fault rather than collapsing it into
"not reachable", because a misconfiguration and an unreachable server need
different things looked at. Nothing is silently tolerated; it fails where
someone is looking.

httpCompression defaults to on here and off on the Elasticsearch side.
That is the better default and it stays, but it is also a difference
between the two backends that has nothing to do with either storage
engine, and comparing them is part of why this module exists. Running the
comparison with gzip on one side confounds it: some of the difference in
CPU and in bytes on the wire is the compression. Said so where the
property is set, rather than leaving it to be discovered in the results.

The third risk was reported as "update-strategy=reload discards up to
100000 buffered flows on any config edit". It does not. stop() drains
first, bounded by SHUTDOWN_DRAIN_SECONDS, and discards only what could not
be sent inside that window -- deliberate, and already documented on the
method. Verified before changing anything; no change made.

Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Ronny Trommer <[email protected]>


  Commit: 26bc1eddddf92e28e062994b96eab51f9aea3160
      https://github.com/OpenNMS/opennms/commit/26bc1eddddf92e28e062994b96eab51f9aea3160
  Author: Ronny Trommer <[email protected]>
  Date:   2026-07-30 (Thu, 30 Jul 2026)

  Changed paths:
    M features/flows/victorialogs/src/main/java/org/opennms/netmgt/flows/victorialogs/VictoriaLogsClient.java
    M features/flows/victorialogs/src/main/java/org/opennms/netmgt/flows/victorialogs/VictoriaLogsFlowQueryService.java
    M features/flows/victorialogs/src/main/java/org/opennms/netmgt/flows/victorialogs/VictoriaLogsHealthCheck.java
    M features/flows/victorialogs/src/main/java/org/opennms/netmgt/flows/victorialogs/VictoriaLogsQueryServiceRegistrar.java
    M features/flows/victorialogs/src/main/resources/OSGI-INF/blueprint/blueprint.xml
    A features/flows/victorialogs/src/test/java/org/opennms/netmgt/flows/victorialogs/BlueprintWiringTest.java
    M features/flows/victorialogs/src/test/java/org/opennms/netmgt/flows/victorialogs/VictoriaLogsFlowQueryServiceTest.java
    M features/flows/victorialogs/src/test/java/org/opennms/netmgt/flows/victorialogs/VictoriaLogsHealthCheckTest.java
    M features/flows/victorialogs/src/test/java/org/opennms/netmgt/flows/victorialogs/VictoriaLogsQueryServiceRegistrarTest.java
    M opennms-full-assembly/pom.xml

  Log Message:
  -----------
  fix(flows): stop the misconfiguration fix from handing the UI to a broken backend

Reviewing the previous two commits found that they were individually
reasonable and jointly worse than what they replaced.

Registering the query service was gated on the enable flag alone. Since a
misconfiguration no longer failed the container, the container now started,
the registrar published FlowQueryService at ranking 1000, the singleton
reference in the REST layer bound it in preference to Elasticsearch, and
every flow query threw -- from a backend that had logged at startup that it
would not be used. The same typo previously failed the container, published
nothing, and left Elasticsearch serving untouched. A loud startup failure
had been turned into a silent takeover. The registrar now takes the client
and declines to publish one it knows cannot work.

The health check was gated on the persistence flag while the same commit
introduced a separate flag for querying and argued at length that the two
must be independent. In the asymmetric state that argument describes --
ingest off, querying on -- VictoriaLogs answered every flow query in the UI
while the health check reported Success, "Not configured", and the
misconfiguration branch added for exactly this purpose was unreachable. It
now considers both.

And the fail-fast hole was only half closed: buildInsertUri ran on the line
before validate, and URI.create throws for a url that is syntactically
illegal rather than merely relative. A stray space or an unbracketed IPv6
literal still failed the whole container and still took the health check
with it. Construction now happens inside the recorded-fault path.

The rest were smaller but of the same kind. The invalid-url message echoed
the raw url, so a url carrying userinfo -- exactly the mistake that reaches
that path -- put the secret in the ERROR log, in getConfigurationError, in
the operator-facing health text and in every exception; it is redacted now.
validate reported only the first fault, costing a diagnose-fix-reload cycle
per mistake. stop()'s comment claimed a queued-versus-running distinction
the loop does not make. A submission after shutdown was reported as "the
executor is saturated; the backend is not keeping up", sending the operator
to look at VictoriaLogs capacity when the cause was a stopped bundle.
ingest checked for an empty body before checking it could reach a server.

Four things now have tests that had none: the registrar refusing a
misconfigured client, the health check naming a fault, the health check
seeing query-only mode, and a queued query being failed rather than left
unsettled at shutdown. BlueprintWiringTest pins what no Java test can see
-- that FlowQueryService is never registered declaratively, that both
opt-ins default to off, and that the health check receives both flags. It
earned its place immediately by catching a property that had silently
landed nowhere, because the edit that was supposed to add it matched no
line and said nothing.

Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Ronny Trommer <[email protected]>


Compare: https://github.com/OpenNMS/opennms/compare/e2cd36e80518...26bc1eddddf9

To unsubscribe from these emails, change your notification settings at https://github.com/OpenNMS/opennms/settings/notifications


_______________________________________________
Please read the OpenNMS Mailing List FAQ:
http://www.opennms.org/wiki/index.php?page=MailingListFaq
opennms-cvs mailing list

To *unsubscribe* or change your subscription options, see the bottom of this page:
https://lists.sourceforge.net/lists/listinfo/opennms-cvs