NMS-20027: Compute flow proportional sums with Painless scripts#8638
Draft
marshallmassengill wants to merge 1 commit into
Draft
NMS-20027: Compute flow proportional sums with Painless scripts#8638marshallmassengill wants to merge 1 commit into
marshallmassengill wants to merge 1 commit into
Conversation
…e drift plugin Replace the proportional_sum aggregation provided by the elasticsearch-drift-plugin with an equivalent scripted_metric aggregation built from inline Painless scripts. This removes the need to install a plugin build that exactly matches the Elasticsearch version, and makes flow queries work against stock Elasticsearch and OpenSearch clusters. The scripted_metric state is a flat double[] indexed by bucket when the window/step combination yields at most 4096 buckets (any UI-driven query), which benchmarked 2.3x faster than the plugin on whole-window series queries at 40M flows; a HashMap-based form with identical output covers arbitrarily fine-grained windows, since the dense state is allocated per terms cell up front. Field names are inlined into the script source: per-document params lookups benchmarked ~1.5x slower. The previous plugin-based queries remain available by setting proportionalSumStrategy=plugin in org.opennms.features.flows.persistence.elastic.cfg for clusters that disallow inline scripting and already run the plugin. The script-based implementation intentionally does not reproduce the defect tracked as NMS-20001: for query windows whose start is not a multiple of the step (the common case), the plugin mixes two bucket grids in series results and stops counting window totals at the first epoch-grid boundary after the window start, undercounting totals. Totals may therefore increase after this change. FlowQueryIT and AggregatedFlowQueryIT now run against a stock Elasticsearch container without the plugin installed; the drift plugin build-time dependency is removed. The testcontainers version in the flow itests is aligned with the version managed in the root pom.
marshallmassengill
requested review from
cgorantla,
Copilot,
dino2gnt and
indigo423
July 16, 2026 14:40
marshallmassengill
marked this pull request as draft
July 16, 2026 14:41
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes the runtime/test dependency on the Elasticsearch Drift plugin for flow “proportional sum” computations by rendering an equivalent scripted_metric aggregation using inline Painless scripts, while keeping an opt-in fallback to the legacy plugin aggregation via configuration.
Changes:
- Introduces
ProportionalSumQueryto render either legacyproportional_sum(plugin) or newscripted_metric(Painless) aggregations, defaulting to Painless. - Wires a new
proportionalSumStrategyconfig property through OSGi blueprint into the raw and aggregated flow query services and updates query templates to consume the rendered aggregation JSON. - Updates integration tests and build files to run against stock Elasticsearch (no plugin) and adds focused unit tests for rendering/parsing.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pom.xml | Removes unused drift plugin version property from root build properties. |
| features/flows/itests/src/test/java/org/opennms/netmgt/flows/elastic/FlowQueryIT.java | Switches IT to stock Elasticsearch container (no drift plugin install/verification). |
| features/flows/itests/src/test/java/org/opennms/netmgt/flows/elastic/AggregatedFlowQueryIT.java | Switches aggregated IT to stock Elasticsearch container (no drift plugin install/verification). |
| features/flows/itests/pom.xml | Removes drift plugin download/test dependency; aligns Testcontainers versions to ${testcontainers.version}. |
| features/flows/elastic/src/test/java/org/opennms/netmgt/flows/elastic/ProportionalSumQueryTest.java | Adds unit tests for aggregation rendering and result-shape parsing. |
| features/flows/elastic/src/main/resources/OSGI-INF/blueprint/blueprint.xml | Adds proportionalSumStrategy property and injects it into flow query services. |
| features/flows/elastic/src/main/resources/org/opennms/netmgt/flows/elastic/series_for_terms.ftl | Replaces hardcoded proportional_sum with injected ${proportionalSum} JSON. |
| features/flows/elastic/src/main/resources/org/opennms/netmgt/flows/elastic/series_for_others.ftl | Same as above for “others” series query. |
| features/flows/elastic/src/main/resources/org/opennms/netmgt/flows/elastic/series_for_missing.ftl | Same as above for “missing” series query. |
| features/flows/elastic/src/main/resources/org/opennms/netmgt/flows/elastic/agg/series_totals.ftl | Uses injected proportional-sum aggregations for ingress/egress totals. |
| features/flows/elastic/src/main/resources/org/opennms/netmgt/flows/elastic/agg/series_top_n.ftl | Uses injected proportional-sum aggregations for ingress/egress in top-N series. |
| features/flows/elastic/src/main/java/org/opennms/netmgt/flows/elastic/SearchQueryProvider.java | Adds strategy-aware proportional sum aggregation injection into raw-flow templates. |
| features/flows/elastic/src/main/java/org/opennms/netmgt/flows/elastic/RawFlowQueryService.java | Accepts proportional-sum strategy via constructor and passes it into query provider. |
| features/flows/elastic/src/main/java/org/opennms/netmgt/flows/elastic/ProportionalSumQuery.java | New: renders plugin or scripted-metric aggregation with dense/sparse Painless implementations. |
| features/flows/elastic/src/main/java/org/opennms/netmgt/flows/elastic/ProportionalSumAggregation.java | Extends parsing to support scripted-metric “value map” response shape. |
| features/flows/elastic/src/main/java/org/opennms/netmgt/flows/elastic/agg/AggregatedSearchQueryProvider.java | Adds strategy-aware proportional sum aggregation injection into aggregated-flow templates. |
| features/flows/elastic/src/main/java/org/opennms/netmgt/flows/elastic/agg/AggregatedFlowQueryService.java | Accepts proportional-sum strategy via constructor and passes it into query provider. |
| docs/modules/releasenotes/pages/whatsnew.adoc | Release notes describing the move away from drift plugin and the fallback strategy. |
| docs/modules/operation/pages/deep-dive/flows/basic.adoc | Updates operational docs to reflect stock ES/OpenSearch support and the strategy toggle. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+61
to
+74
| private void parseValueMap(JsonObject valueMap) { | ||
| for (Map.Entry<String, JsonElement> entry : valueMap.entrySet()) { | ||
| final long time; | ||
| try { | ||
| time = Long.parseLong(entry.getKey()); | ||
| } catch (NumberFormatException e) { | ||
| continue; // Skip entries without a parseable bucket key | ||
| } | ||
| final double value = entry.getValue().getAsDouble(); | ||
| // Per-bucket document counts are not tracked by the scripted variant | ||
| dateHistograms.add(new DateHistogram(valueMap, time, entry.getKey(), value, 0L)); | ||
| } | ||
| dateHistograms.sort(Comparator.comparing(DateHistogram::getTime)); | ||
| } |
dino2gnt
reviewed
Jul 16, 2026
Comment on lines
+118
to
+120
| // Stock Elasticsearch, on purpose: the proportional sums used by the series/totals | ||
| // queries are computed with inline Painless scripts and must work without the | ||
| // elasticsearch-drift-plugin being installed. |
Contributor
There was a problem hiding this comment.
Drift is the reference implementation. Until these Painless scripts have been battle-tested at scale, I'd want to see the tests run both side by side for the newest supported version, and verify that we produce the same results in both cases.
Open
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replace the proportional_sum aggregation provided by the elasticsearch-drift-plugin with an equivalent scripted_metric aggregation built from inline Painless scripts. This removes the need to install a plugin build that exactly matches the Elasticsearch version, and makes flow queries work against stock Elasticsearch and OpenSearch clusters.
Putting this in as a draft for now. Would love feedback.
Assisted by Anthropic Claude Fable 5.
External References