feat(analysis): Add Measurements Length Option for Dry-Run Metrics

[agrawroh]

Description

It could be very useful to retain the maximum data points while running an analysis in the dry-run mode to understand what caused a particular metric to error out or why the final result came out as inconclusive.

Currently, the controller only retains the ten latest measurements and it's not possible to configure/change this value as its hard-coded in the code.

Changes

This PR adds a new option called measurementsLength in the dryRun which can be used to retain other than the latest ten results for the metrics running in the dry-run mode. Setting this option to 0 would disable it and, the controller will revert back to the existing behavior of retaining the latest ten measurements.

Checklist

• Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this is a chore.
• The title of the PR is (a) conventional, (b) states what changed, and (c) suffixes the related issues number. E.g. "fix(controller): Updates such and such. Fixes #1234".
• I've signed my commits with DCO
• I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
• My builds are green. Try syncing with master if they are not.
• My organization is added to USERS.md.

---

Signed-off-by: Rohit Agrawal [email protected]

[codecov]

Codecov Report

Merging #1715 (e273eac) into master (061efd4) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1715      +/-   ##
==========================================
+ Coverage   82.04%   82.05%   +0.01%     
==========================================
  Files         116      116              
  Lines       16065    16074       +9     
==========================================
+ Hits        13181    13190       +9     
  Misses       2212     2212              
  Partials      672      672              

Impacted Files	Coverage Δ
analysis/analysis.go	85.86% <100.00%> (+0.12%)	⬆️
controller/metrics/analysis.go	88.31% <100.00%> (+0.64%)	⬆️
utils/analysis/helpers.go	77.93% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 061efd4...e273eac. Read the comment docs.

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

No Coverage information
7.1% Duplication

[jessesuen]

I agree the measurement retention limit should be modifiable. But I also don't think we should limit it to dryRun. So I suggest the following changes:

1. suggest the field name to be: measurementRetention (which is more clear than length)
2. add this to AnalysisRun spec instead of spec.dryRun.
3. Have a similar UX for configuring the retention for different metrics. e.g.:

Retain 20 measurements for each metric:

spec:
  measurementRetention:
  - metricName: .*
    limit: 20

retain 20 success-rate measurements:

spec:
  measurementRetention:
  - metricName: success-rate
    limit: 20

These would apply to either dry runs or wet runs.

[jessesuen]

Closing since superseded by #1729