feat: add explicit default-aggregation to all metrics

[k-rister]

Summary

• Add default-aggregation to every numeric metric in trafficgen-post-process.py so aggregation semantics are self-documenting
• Change latency metrics from class: count to class: latency with appropriate aggregation (max for worst-case, avg for pre-aggregated averages, min for best-case)
• Throughput/rate metrics: sum
• Percentage/utilization metrics: avg
• Accumulative count metrics: sum
• Remove placeholder comment that referenced waiting for PERFNFV-409

Covers all 5 metric groups: STL device stats (9), ASTF stats (41), ptp-latency stats (7), ptp-latency percentiles (7), TRex profiler time-series (28).

Depends on CDM PR#202 (merged) — the default-aggregation field is accepted by both v9dev and v10dev, so this change is forward-compatible.

Validated with STL run on nutanix-b:

• max-roundtrip-usec: returns 4119 (max of port pairs), not 8235 (old sum behavior)
• mean-roundtrip-usec: returns 8.52 (avg)
• rx-pps: returns 29.7M (sum, unchanged)
• latency-max-usec: forward=7.75, reverse=8.00, aggregated=8.00 (max)
• latency-min-usec: forward=5.75, reverse=6.00, aggregated=5.75 (min)

Test plan

• Run STL trafficgen test, verify indexing into v10dev succeeds
• Query latency metrics and verify max/avg/min aggregation across breakout dimensions
• Query throughput metrics and verify sum aggregation is unchanged
• Run ASTF test to validate ASTF-specific latency metrics (optional — same code path)

🤖 Generated with Claude Code

[pradiptapks]

Run Review: ASTF Test with feat-default-aggregation

Run ID: c0caf33b-b5fd-4af5-b8b7-03a82c876c07
Branch: feat-default-aggregation
Status: Successful

Test Configuration

• Backend: trex-astf, TCP, 4 port pairs via VM
• DUT: grout v0.16.0, IPv4 forwarding
• Client: nfv-intel-5.perf.eng.bos2.dc.redhat.com (alma9, chroot)
• Server: 192.168.0.103 (alma10, chroot)
• Rate: 10,000 CPS, 30s search/validation, 1 sample
• Primary metric result: connections-per-second = 18,604.66 CPS

default-aggregation Validation

All 21 numeric ASTF metrics have default-aggregation set correctly. The only metric without it is trial-result (class pass/fail, non-numeric status — expected).

Category	Metrics	class	default-aggregation
Latency (worst-case)	tcp-rtt-max-usec	latency	max
Latency (best-case)	tcp-rtt-min-usec	latency	min
Latency (average)	tcp-rtt-avg-usec, tcp-rto-avg-usec	latency	avg
Throughput/rate	connections-per-second, connections-attempted-per-second, l7-tx-bps, l7-rx-bps, tx-bps, rx-bps, tx-pps, rx-pps	throughput	sum
Accumulative counts	tx-packets, rx-packets, server-accepts, server-connects, tcp-snd-bytes, tcp-rcv-bytes, active-flows, established-flows	count	sum
Percentage	tcp-overhead-pct	count	avg

OpenSearch Indexing

Indexing succeeded (2,806,946 docs). The indexed metric_desc documents carry the correct default-aggregation values — all 21 unique metric type/class/aggregation combinations are present and match the post-processed data.

Observations

1. All ASTF metrics have correct aggregation — latency metrics properly use class: latency (changed from count) with semantically correct max/min/avg.

2. No errors — get-result-summary-stderr.txt is empty, no indexing failures.

3. One minor note for PR review: tcp-overhead-pct has class: count with default-aggregation: avg. The avg aggregation is correct for a percentage metric, but the class could arguably be something other than count. This appears to be a pre-existing classification that the PR didn't change — worth noting but not a blocker.

4. STL-specific metrics not exercised — This ASTF run doesn't exercise the STL device stats (9 metrics), ptp-latency stats (7), ptp-latency percentiles (7), or TRex profiler time-series (28). Those code paths would require an STL run to validate. The PR author already validated STL separately on nutanix-b.

Verdict

The ASTF path validates cleanly. The PR correctly adds default-aggregation to all ASTF metrics with semantically appropriate values, and the data flows through post-processing into OpenSearch without issues.

[pradiptapks]

PR Code Review: feat-default-aggregation

Reviewed the full diff (99 additions, 101 deletions in trafficgen-post-process.py) and validated with an ASTF test run. Here are the review findings:

---

Positive observations

1. Consistent pattern — Every metric definition table (TRIAL_STATS_DEVICE_METRICS, TRIAL_STATS_ASTF_METRICS, TRIAL_LATENCY_METRICS, TRIAL_LATENCY_PERCENTILE_METRICS, TRIAL_PROFILER_METRICS) now carries default-aggregation, and every desc dict construction propagates it. No metric table was missed.

2. Correct aggregation semantics — The aggregation choices are well-reasoned:

   • Latency max/min/avg → max/min/avg (preserves statistical meaning)
   • Throughput/rate → sum (additive across port pairs/streams)
   • Percentiles → max (worst-case across dimensions — correct for tail latency)
   • Percentage/utilization → avg (non-additive ratios)
   • Jitter → max (worst-case — reasonable choice)
   • Std dev → max (debatable, see below)

3. New ptp-latency processing — The new code block for trial["stats"]["latency"] is cleanly structured with proper .get() fallbacks and uses a distinct source (trafficgen-ptp-latency) to separate it from the main trafficgen metrics.

4. Class reclassification — Latency metrics correctly moved from class: count to class: latency for roundtrip, RTT, jitter, percentiles, and syn-ack/request-response latency metrics. This makes CDM queries by class semantically correct.

---

Items worth discussing

1. TRIAL_METRICS (pass/fail) intentionally skipped — The TRIAL_METRICS list (just trial-result with class: pass/fail) doesn't have default-aggregation and its desc construction doesn't propagate it. This is correct since it's a non-numeric status metric, but worth confirming this is intentional rather than an oversight.

2. latency-stddev-usec with default-aggregation: max — Standard deviation isn't a min/max/sum metric in the traditional sense. Using max gives the worst-case variability, which is a reasonable conservative choice, but avg could also be argued. Worth a brief comment in the code or PR description explaining the rationale.

3. tcp-overhead-pct, connection-error-pct, retransmit-pct, out-of-order-pct keep class: count — These percentage metrics get default-aggregation: avg (correct), but their class remains count rather than something like percentage. This is pre-existing and the PR scope is specifically about default-aggregation, so not a blocker — just noting for future cleanup consideration.

4. tsdelta profiler metric with default-aggregation: avg — This is the timestamp delta between profiler samples. Averaging timestamp deltas across dimensions is fine for reporting sampling intervals, but it's worth noting this is more of a diagnostic metric than a performance metric.

5. ptp-latency fwd_iface/rev_iface fallback logic — The interface name resolution uses cross-fallback:

   fwd_iface = fwd_stats.get("tx_device", rev_stats.get("rx_device", ""))
   rev_iface = fwd_stats.get("rx_device", rev_stats.get("tx_device", ""))

   This is clever but worth verifying: if only Reverse data exists (no Forward), fwd_iface falls back to rev_stats["rx_device"]. Is that the correct semantic — that the reverse direction's rx device is the forward direction's interface?

6. latency_stats iteration includes non-direction keys — The loop for direction, dir_stats in latency_stats.items() iterates over all keys in the latency dict. If the latency stats ever include non-direction keys (metadata, timestamps, etc.), they'd be processed as directions. A safeguard like filtering for known direction names (Forward, Reverse) might be worth considering.

---

Verdict

Approve. The change is well-structured, consistent, and the aggregation semantics are correct throughout. The items above are minor discussion points, not blockers. Validated with ASTF test run — all metrics indexed successfully with correct default-aggregation values in OpenSearch.