feat: add v10dev with default-aggregation and per-metric aggregation control

[k-rister]

Summary

• Add CDM v10dev with a default-aggregation field on metric_desc that controls how metrics are aggregated across breakout dimensions — sum, avg, max, or min
• Add --aggregation query-time override to get-metric-data for ad-hoc analysis with a different aggregation method
• Consolidate version validation into shared isValidCdmVersion() and cdmVersionOptionDesc() helpers — adding future CDM versions only requires updating docTypes
• Fix pre-existing regex bugs in getCdmVerFromIndex and getInstancesInfo that would break any multi-digit version ([\d+] matched single digits only)

Aggregation types

Type	Behavior	Use case
sum	Duration-weighted sum (default fallback)	Throughput, IOPS
avg	Duration-weighted average ÷ metric count	Pre-aggregated latency
max	Maximum value across dimensions	RT worst-case latency
min	Minimum value across dimensions	Idle floor

Forward compatibility

The default-aggregation field is accepted on both v9dev and v10dev mappings. Post-processors can start emitting it before users switch to v10dev — the field is stored on v9dev and the aggregation dispatch works regardless of index version.

Jira

PERFNFV-410 (under epic PERFNFV-409)

Test plan

• 4-engine cyclictest on kube endpoint: without default-aggregation, CDM sums latencies (24860); with default-aggregation: "max", CDM reports the max (4431)
• --aggregation override verified for all 4 types against the same indexed data
• Invalid --aggregation values return 400 error with clear message
• v9dev forward compatibility: default-aggregation field accepted and aggregation works on v9dev data
• node -c syntax check passes on all 12 changed files
• prettier --check passes

🤖 Generated with Claude Code

[k-rister]

PR Review: CommonDataModel#202 — feat: add v10dev with default-aggregation and per-metric aggregation control

Summary: Adds CDM v10dev with a default-aggregation field on metric_desc that controls per-metric aggregation (sum/avg/max/min), adds --aggregation query-time override, consolidates version validation, and fixes pre-existing regex bugs.
Changed files: 12
Review dimensions: Correctness, API & Contracts, Build & Deploy, Documentation, Style

Issues

• [cdm.js:3422] sendMetricReq calls calcAvg with 7 args but calcAvg now takes 8 (missing defaultAggregation) — calcAvg's signature was updated to (thisBegin, thisEnd, responses, jsonArrIdx, jsonArrTracker, numMetricIds, defaultAggregation, values) but the call in sendMetricReq still passes valueSets[setIdx][trackerLabel] as the 7th arg, which lands in defaultAggregation instead of values. The actual values parameter is undefined, so values.push(dataSample) would throw a TypeError. However, sendMetricReq is never called — it appears to be dead/legacy code fully superseded by getMetricDataFromIdsSets. If it is truly dead code, consider removing it or adding the missing defaultAggregation argument (e.g., 'sum') for correctness. Not blocking since the function is unreachable.

• [cdm.js:3645] baseFilter variable defined but never used — The var baseFilter = ... on line 3645 constructs a filter string using the initial thisBegin/thisEnd values, but is never referenced — the filter variable inside the while (true) loop does the same construction with updated values. Dead code, likely a leftover from refactoring.

File Coverage

• VERSION — No issues (v8dev→v10dev; v9dev already existed in cdm.js)
• queries/cdmq/add-run-worker.js — No issues
• queries/cdmq/add-run.js — No issues
• queries/cdmq/cdm.js — 2 issues (sendMetricReq stale callsite, unused baseFilter)
• queries/cdmq/create-index.js — No issues
• queries/cdmq/delete-run.js — No issues
• queries/cdmq/get-instances-info.js — No issues
• queries/cdmq/get-metric-data.js — No issues
• queries/cdmq/get-primary-periods.js — No issues
• queries/cdmq/get-result-summary.js — No issues
• queries/cdmq/server.js — No issues
• templates/metric_desc.base — No issues

Limitations

• Cannot verify runtime behavior of max/min aggregation against live OpenSearch data
• Cannot verify that esRequest (sync HTTP) in getDefaultAggregation performs acceptably under load — it blocks the Node.js event loop, but this matches the existing pattern used elsewhere in the codebase
• The prettier reformatting accounts for a large portion of the diff; functional changes are concentrated in the aggregation dispatch logic

Verdict

Approve with comments — The aggregation dispatch logic is correct: query templates properly switch between weighted_avg/max/min aggregations, calcAvg handles partial-document boundary cases for both sum/avg and max/min paths, and the --aggregation override flows cleanly from CLI through server API to the query engine. The two issues (dead code in sendMetricReq, unused baseFilter) are non-blocking cleanup items. The version validation consolidation into isValidCdmVersion()/cdmVersionOptionDesc() is a clean refactor that eliminates the version enumeration duplication across 8 files. The regex fixes ([\d+] → \d+) are correct and address real bugs.

[k-rister]

Addressed

• sendMetricReq stale callsite (aeab274): Added 'sum' as the defaultAggregation argument to match the updated calcAvg signature. This is dead code (superseded by getMetricDataFromIdsSets) but now correct if ever called.
• Unused baseFilter (aeab274): Removed the dead variable — leftover from query template refactoring.

[k-rister]

PR Review: CommonDataModel#202 — re-review after aeab274

Both prior findings addressed in commit aeab274:

1. ✅ sendMetricReq now passes 'sum' as defaultAggregation to calcAvg — signature match is correct
2. ✅ Unused baseFilter variable removed

No new issues introduced.

Verdict

Approve — All findings resolved; no new issues.

[atheurer]

PR Review: CommonDataModel #202

feat: add v10dev with default-aggregation and per-metric aggregation control

Overview

Solid PR. The core aggregation dispatch in calcAvg is correct — it properly switches OpenSearch query templates between weighted_avg, max, and min aggregations, and handles partial-document boundary cases appropriately for each mode. The version validation consolidation eliminates real maintenance burden across 8 files, and the regex fixes ([\d+] → \d+) fix genuine bugs that would have broken any future multi-digit CDM version.

~60% of the diff is prettier reformatting — the functional changes are concentrated in calcAvg, getMetricDataFromIdsSets, getMetricDataSets, and the v10dev index definitions.

Aggregation math verification

Traced the sum/avg/max/min logic:

• sum: result = (aggAvgTimesWeight + sumValueTimesWeight) / totalWeightTimesMetrics * numMetricIds = total weighted value / timeWindowDuration. Correct — reconstructs the sum from the weighted average.
• avg: same formula without the * numMetricIds multiplication. Correct — gives the mean, not the total.
• max/min: collects raw values from both aggregated and partial documents, applies Math.max/Math.min. Correct — duration weighting is irrelevant for extrema.

Minor items (non-blocking)

1. Redundant default-aggregation on v10dev — v9dev gets the field added, then v10dev is deep-cloned from v9dev (which now has it), then the same field is set again on v10dev. The second assignment is a no-op. Not a bug, but reads as though v10dev is adding something v9dev doesn't have.

2. getDefaultAggregation doesn't validate the stored value — garbage values in OpenSearch fall through to the weighted-avg path (safe default), but a log warning on unexpected values would catch misconfigured data early.

3. q2 (total_weight) sent for max/min — the total_weight query is always sent but unused for max/min. One extra OpenSearch request per time window per label. Probably not worth the complexity to skip conditionally.

4. Hardcoded version dispatches — getDocType and getIndexBaseName still use || cdmVer == 'v10dev' checks. When v11dev arrives these will need manual updates. Consider a helper or numeric comparison for future-proofing.

5. Undocumented fix: var cdmVer in getCdmVerFromIndex — previously an implicit global. Good catch, worth mentioning in the PR description.

What's good

• Forward compatibility is well thought out — v9dev gets the field too
• API validation is solid — aggregation checked against a whitelist
• Version validation is DRY — adding a new version is a one-line change
• Regex fixes prevent real breakage
• --aggregation override flows cleanly end-to-end

Approved ✅

[k-rister]

Addressed (94dcb00)

1. Redundant v10dev field assignment — removed. v10dev now inherits default-aggregation from v9dev via deep clone, with a comment noting this.
2. Stored value validation — getDefaultAggregation now validates the stored value against [sum, avg, max, min] and logs a warning if unrecognized, falling back to sum.
3. q2 for max/min — left as-is per reviewer's recommendation (not worth the complexity to skip conditionally).
4. Hardcoded version dispatches — getDocType and getIndexBaseName now use isValidCdmVersion(cdmVer) instead of cdmVer == 'v9dev' || cdmVer == 'v10dev'. Adding v11dev won't require updating these functions.
5. var cdmVer fix — noted; this was a pre-existing implicit global in getCdmVerFromIndex, fixed as part of the regex correction.

Skipped

• q2 (total_weight) for max/min — reviewer agreed not worth the complexity. One extra lightweight OpenSearch agg per time window, result is ignored.

[atheurer]

Re-review: CommonDataModel #202 — after 94dcb00

All three items from my prior review are addressed:

1. ✅ Redundant default-aggregation on v10dev — removed; comment updated to note inheritance via deep clone
2. ✅ Unvalidated stored values — getDefaultAggregation now validates against ['sum', 'avg', 'max', 'min'] and logs a warning before falling back to sum
3. ✅ Hardcoded version dispatches — getDocType and getIndexBaseName now use isValidCdmVersion() instead of || cdmVer == 'v10dev'. The v7dev/v8dev early-return guards correctly prevent double-matching.

No new issues introduced. Approved.