From 1ed29f35ff2e58a3fd12d15232769a6638feaefa Mon Sep 17 00:00:00 2001 From: Jirka Kremser Date: Thu, 24 Sep 2026 18:37:40 +0200 Subject: [PATCH 1/4] Improve sizing evidence, rollout fallback, and decision traces Use one hour of measured history and distinct observation timestamps, with coverage guards that tolerate collection gaps. Expose shared sample normalization and versioned decision traces. Fall back to qualifying usage from up to three earlier rollouts while retaining current identity, allocation, and inventory guards. Initialize known-unset settings without material-change thresholds. Update the output schema, fixtures, regression tests, and caller guidance. --- README.md | 76 +++++++-- analysis/analyze.go | 221 ++++++++++++------------- analysis/analyze_test.go | 145 +++++++++++------ analysis/fallback.go | 68 ++++++++ analysis/fallback_test.go | 143 ++++++++++++++++ analysis/samples.go | 56 +++++++ analysis/testdata/default-output.json | 226 +++++++++++++++++++++++++- analysis/trace.go | 47 ++++++ analysis/trace_test.go | 53 ++++++ analysis/types.go | 74 ++++++--- analysis/unset_test.go | 146 +++++++++++++++++ 11 files changed, 1051 insertions(+), 204 deletions(-) create mode 100644 analysis/fallback.go create mode 100644 analysis/fallback_test.go create mode 100644 analysis/samples.go create mode 100644 analysis/trace.go create mode 100644 analysis/trace_test.go create mode 100644 analysis/unset_test.go diff --git a/README.md b/README.md index a3a8559..a0000a4 100644 --- a/README.md +++ b/README.md @@ -15,13 +15,16 @@ output, err := analysis.Analyze(snapshot, policy) ``` Callers fetch raw range vectors and map them into `analysis.Input`; calculations -and CPU counter normalization happen in this package. Schema v2 rejects v1 input. +and CPU counter normalization happen in this package. Input schema v2 rejects v1 +input; results use output schema v3 and resource detector version 7. Consumers +should update generated output bindings and invalidate cached findings when upgrading. All timestamps are original Unix milliseconds, including current settings and identity. Do not stamp carried-forward values with query evaluation times. CPU gauges use millicores, memory uses bytes, and `cpu-counter-seconds` uses cumulative CPU seconds. Adjacent counter observations become millicores using delta / elapsed -seconds × 1000; resets use the post-reset counter value. Intervals longer than the -maximum gap are omitted rather than averaged into deceptively small rates. +seconds × 1000; resets use the post-reset counter value. These are interval-average +rates, including across collection outages; no intermediate observations are +invented, and missing measurements reduce coverage. Group observations by namespace, workload kind/name and container. The target and current identity select one workload UID and release. Keep other releases in raw @@ -41,21 +44,44 @@ from metrics alone. An authoritative activation boundary is needed to resolve th source limitation. Deleted/recreated workload UIDs are never pooled. Each resource independently reports observed start/end, distinct sample count, -series count, inferred median cadence, gap count, maximum gap and coverage. Coverage +series count, inferred median cadence and coverage. Coverage is the union of observed timestamps for the current release across pod lifetimes, divided by the selected release segment duration rather than pre-release lookback, with intervals capped at the median source cadence and one cadence of edge tolerance. Healthy scale-out or pod replacement retains established release history. -Sample count reports all distinct per-series samples; confidence uses unique release +`sampleCount` reports all distinct per-series samples; `observationCount` reports +distinct timestamps across series after normalization (including CPU counter +conversion) and is the count checked against `minimumSamples`. Confidence uses unique release timestamps, measured release history and coverage and is capped at 95. It is also capped by the history, sample count and coverage of the series supplying the sizing value; a newly busy replica can justify growth without borrowing another replica's -confidence. Equal sizing values use the strongest independently supporting series. Internal -series gaps remain visible; only fresh observed pods count against current inventory. Long gaps, -stale samples and insufficient history block sizing. The default minimum history -is seven days; shortening a query does not shorten that safety guard. A caller can -explicitly choose another minimum through the effective policy; automatic seasonal -detection is outside this package. +confidence. Equal sizing values use the strongest independently supporting series. +Only fresh observed pods count against current inventory. Stale samples, +insufficient history, too few samples and insufficient overall coverage block +sizing; an isolated collection outage does not independently veto a recommendation. +The defaults require one hour of observed history, 30 distinct observation times, +90% coverage, and observations no older than five minutes. One hour of minute-level +scrapes can qualify, including modest gaps within the coverage guard. Shortening a +query does not shorten the history guard. A caller can explicitly choose other +minimums through the effective policy; automatic seasonal detection is outside +this package. + +Callers may supply `previousReleases`, ordered newest to oldest, on a container +observation. CPU and memory independently fall back when the current rollout has +missing usage, insufficient history, or insufficient samples. At most the first +three previous rollouts are examined. The first candidate passing all usage +quality checks wins; samples from different rollouts are never pooled. An +eligible current rollout always wins, including when it needs no material change. + +Each previous rollout supplies its release ID, raw CPU/memory series, and the last +historical evaluation time; an optional activation boundary can be provided. +Historical evidence is evaluated at that time, capped at the current rollout's +start, and retains its original timestamps. `rolloutFallback` in each resource +result identifies the historical source and preserves the current rollout's +failed data-quality checks. The result target, identity, allocation signals, +and inventory checks remain current. Historical pod +counts cannot authorize downsizing currently unobserved pods. Existing callers +that omit `previousReleases` keep current-rollout-only behavior. Inventory counts must describe the selected UID/release/container: eligible, observed eligible, and excluded containers. Mark inventory unavailable when the @@ -64,6 +90,19 @@ inventory blocks downsizing, including introducing a limit where zero means no limit. Well-covered usage can still justify growth with partial quality. Inconsistent current settings across replicas must be marked unavailable by the adapter. +For a current request or limit that is known to be absent, supply +`Signal{Available: true, Unset: true, Timestamp: observedAt}`. This is distinct +from an observed numeric zero and from `Available: false` (unknown). The value +must remain zero when `Unset` is true; it is a placeholder, not an allocation. +Since detector version 7, initializing an unset setting bypasses absolute and +relative minimum-change thresholds. Evidence, freshness, bounds, inventory, +and requests-only guards still apply. In particular, introducing an unset +limit still requires safe inventory evidence. Numeric signals that omit +`unset` retain their previous behavior, including explicit zero values. +Recommendations mark such initialization with `currentUnset: true`; their +`currentValue` is then only a placeholder. Decision traces record +`initialize unset setting` instead of a change from zero. + Effective policy contains CPU percentile/max, headroom, request/limit ratios, request-only behavior, resource bounds, material-change thresholds and evidence guards. Defaults use CPU bounds 20–64,000 millicores and memory bounds 10 MiB–1 TiB; @@ -100,3 +139,18 @@ subscribers may privately modify and compile covered source. Include both Kedify texts and applicable third-party notices in distributions. Submitted code requires a signed contribution assignment before acceptance. + +### Decision traces and chart normalization + +Resource analyses include an additive `decisionTrace` (version `1`) from detector +version `6`. It records the winning series/pod, aggregation method and source time, +previous-rollout attempts and rejections, and each request/limit calculation and +executed guard. Setting dispositions distinguish `recommended`, `retained`, +`disabled`, and `unavailable`. Numeric calculation values use the resource's native +units (CPU millicores, memory bytes); coefficients and relative changes are ratios. +Traces do not change sizing behavior or contain raw samples. + +`analysis.NormalizeSamples(series, start, end)` exposes the same sample conversion +used by sizing, including CPU counter resets, duplicate handling, and timestamp +filtering. Chart consumers should use it before display-only downsampling; never +recompute recommendations from reduced chart data. diff --git a/analysis/analyze.go b/analysis/analyze.go index 6340e2c..df24c9b 100644 --- a/analysis/analyze.go +++ b/analysis/analyze.go @@ -18,7 +18,7 @@ func DefaultPolicy() Policy { return Policy{ CPU: CPUPolicy{Strategy: CPUStrategyPercentile, Percentile: 95, HeadroomCoefficient: 3, LimitsToRequestsRatio: 5, Bounds: Bounds{Minimum: 20, Maximum: 64000, MinimumAbsoluteChange: 50, MinimumRelativeChange: .1}}, Memory: MemoryPolicy{Strategy: MemoryStrategyMax, HeadroomCoefficient: 1.2, LimitsToRequestsRatio: 3, Bounds: Bounds{Minimum: 10 * 1024 * 1024, Maximum: 1024 * 1024 * 1024 * 1024, MinimumAbsoluteChange: 8 * 1024 * 1024, MinimumRelativeChange: .1}}, - Evidence: EvidencePolicy{MinimumHistorySeconds: 7 * 24 * 3600, MinimumSamples: 100, MinimumCoverage: .9, MaximumGapSeconds: 300, FreshnessSeconds: 300}, + Evidence: EvidencePolicy{MinimumHistorySeconds: 3600, MinimumSamples: 30, MinimumCoverage: .9, FreshnessSeconds: 300}, } } func NormalizePolicy(p Policy) (Policy, error) { @@ -89,13 +89,10 @@ func NormalizePolicy(p Policy) (Policy, error) { if e.MinimumCoverage == 0 { e.MinimumCoverage = d.Evidence.MinimumCoverage } - if e.MaximumGapSeconds == 0 { - e.MaximumGapSeconds = d.Evidence.MaximumGapSeconds - } if e.FreshnessSeconds == 0 { e.FreshnessSeconds = d.Evidence.FreshnessSeconds } - if e.MinimumHistorySeconds < 1 || e.MinimumSamples < 2 || !positive(e.MinimumCoverage) || e.MinimumCoverage > 1 || e.MaximumGapSeconds < 1 || e.FreshnessSeconds < 1 { + if e.MinimumHistorySeconds < 1 || e.MinimumSamples < 2 || !positive(e.MinimumCoverage) || e.MinimumCoverage > 1 || e.FreshnessSeconds < 1 { return Policy{}, fmt.Errorf("invalid evidence policy") } return p, nil @@ -175,6 +172,11 @@ func analyzeResource(in Input, c ContainerObservation, r Resource, p Policy) (Re headroom, ratio, bounds, requestsOnly = p.CPU.HeadroomCoefficient, p.CPU.LimitsToRequestsRatio, p.CPU.Bounds, p.CPU.RequestsOnly } result := ResourceAnalysis{Target: c.Target, Resource: r, Evidence: ResourceEvidence{CurrentRequest: canonicalSignal(obs.CurrentRequest), CurrentLimit: canonicalSignal(obs.CurrentLimit), Identity: c.Identity, Inventory: c.Inventory}, DataQuality: DataQuality{Status: DataQualityAvailable, Reasons: []Reason{}}} + result.DecisionTrace = &DecisionTrace{Version: "1", Settings: []SettingTrace{ + {Setting: SettingRequests, Disposition: "retained"}, {Setting: SettingLimits, Disposition: "retained"}, + }} + trace := result.DecisionTrace + requestTrace, limitTrace := &trace.Settings[0], &trace.Settings[1] q := &result.DataQuality identity := c.Identity switch { @@ -191,10 +193,21 @@ func analyzeResource(in Input, c ContainerObservation, r Resource, p Policy) (Re addReason(q, ReasonUnknownReleaseStart) } identityBlocked := len(q.Reasons) > 0 - usage, confidence, observed, err := normalizeUsage(in, c, obs, r, p, q) + usage, confidence, observed, err := normalizeUsage(in, c, obs, r, p, q, &trace.Source) if err != nil { return ResourceAnalysis{}, err } + if !identityBlocked && insufficientRolloutUsage(*q) { + fallbackUsage, fallbackConfidence, fallbackQuality, fallback, fallbackErr := previousReleaseUsage(in, c, r, p, *q, trace) + if fallbackErr != nil { + return ResourceAnalysis{}, fallbackErr + } + if fallback != nil { + usage, confidence, *q = fallbackUsage, fallbackConfidence, fallbackQuality + result.RolloutFallback = fallback + result.Notices = append(result.Notices, ReasonPreviousReleaseUsage) + } + } result.Evidence.AggregatedUsage = usage usageBlocked := len(q.Reasons) > 0 requestOK, limitOK := true, true @@ -206,6 +219,9 @@ func analyzeResource(in Input, c ContainerObservation, r Resource, p Policy) (Re if s.value.Available && (!finite(s.value.Value) || s.value.Value < 0) { return ResourceAnalysis{}, fmt.Errorf("current signal must be finite and nonnegative") } + if s.value.Available && s.value.Unset && s.value.Value != 0 { + return ResourceAnalysis{}, fmt.Errorf("unset current signal cannot have a numeric value") + } if !s.value.Available { addReason(q, s.missing) *s.ok = false @@ -235,15 +251,19 @@ func analyzeResource(in Input, c ContainerObservation, r Resource, p Policy) (Re if identityBlocked || usageBlocked || !requestOK { q.Status = DataQualityUnavailable result.NoActionReason = q.Reasons[0] + requestTrace.stop("unavailable", q.Reasons...) + limitTrace.stop("unavailable", q.Reasons...) return result, nil } rawSuggestedRequest := usage.Value * headroom if !finite(rawSuggestedRequest) { return ResourceAnalysis{}, fmt.Errorf("suggested value overflow") } + requestTrace.step("usage × headroom", map[string]float64{"usage": usage.Value, "coefficient": headroom, "candidate": rawSuggestedRequest}) suggestedRequest := math.Max(bounds.Minimum, math.Min(bounds.Maximum, rawSuggestedRequest)) + requestTrace.step("request bounds", map[string]float64{"before": rawSuggestedRequest, "minimum": bounds.Minimum, "maximum": bounds.Maximum, "candidate": suggestedRequest}) requestBounded := suggestedRequest != rawSuggestedRequest - boundsSuppressedAction := requestBounded && isMaterial(obs.CurrentRequest.Value, rawSuggestedRequest, bounds) && !isMaterial(obs.CurrentRequest.Value, suggestedRequest, bounds) + boundsSuppressedAction := requestBounded && isMaterial(obs.CurrentRequest, rawSuggestedRequest, bounds) && !isMaterial(obs.CurrentRequest, suggestedRequest, bounds) if requestBounded { addReason(q, ReasonBounds) } @@ -254,20 +274,34 @@ func analyzeResource(in Input, c ContainerObservation, r Resource, p Policy) (Re return ResourceAnalysis{}, fmt.Errorf("suggested value overflow") } suggestedLimit = math.Max(suggestedRequest, math.Min(bounds.Maximum, rawSuggestedLimit)) + limitTrace.step("request × limit ratio", map[string]float64{"request": suggestedRequest, "ratio": ratio, "candidate": rawSuggestedLimit}) + limitTrace.step("limit bounds", map[string]float64{"before": rawSuggestedLimit, "minimum": suggestedRequest, "maximum": bounds.Maximum, "candidate": suggestedLimit}) limitBounded := suggestedLimit != rawSuggestedLimit - boundsSuppressedAction = boundsSuppressedAction || limitOK && limitBounded && isMaterial(obs.CurrentLimit.Value, rawSuggestedLimit, bounds) && !isMaterial(obs.CurrentLimit.Value, suggestedLimit, bounds) + boundsSuppressedAction = boundsSuppressedAction || limitOK && limitBounded && isMaterial(obs.CurrentLimit, rawSuggestedLimit, bounds) && !isMaterial(obs.CurrentLimit, suggestedLimit, bounds) if limitBounded { addReason(q, ReasonBounds) } } - inventorySuppressedDownsize := false + downsizeSuppressed := false for _, s := range []struct { setting Setting current Signal suggested float64 ok bool }{{SettingRequests, obs.CurrentRequest, suggestedRequest, requestOK}, {SettingLimits, obs.CurrentLimit, suggestedLimit, limitOK && !requestsOnly}} { + settingTrace := requestTrace + if s.setting == SettingLimits { + settingTrace = limitTrace + } if !s.ok { + if requestsOnly && s.setting == SettingLimits { + settingTrace.stop("disabled", ReasonLimitDisabled) + } else { + settingTrace.stop("unavailable", ReasonMissingLimit) + if obs.CurrentLimit.Available { + settingTrace.stop("unavailable", ReasonStaleLimit) + } + } continue } if s.setting == SettingLimits { @@ -277,24 +311,51 @@ func analyzeResource(in Input, c ContainerObservation, r Resource, p Policy) (Re retainedRequest = rec.SuggestedValue } } + settingTrace.guard("limit covers retained request", s.suggested >= retainedRequest, map[string]float64{"candidate": s.suggested, "retainedRequest": retainedRequest}) if s.suggested < retainedRequest { + settingTrace.stop("retained", ReasonBounds) addReason(q, ReasonBounds) - boundsSuppressedAction = boundsSuppressedAction || isMaterial(s.current.Value, s.suggested, bounds) + boundsSuppressedAction = boundsSuppressedAction || isMaterial(s.current, s.suggested, bounds) continue } } + if s.suggested < s.current.Value || (s.setting == SettingLimits && s.current.Value == 0) { + settingTrace.guard("safe reduction or introduction of limit", inventoryOK, nil) + } if (s.suggested < s.current.Value || (s.setting == SettingLimits && s.current.Value == 0)) && !inventoryOK { - inventorySuppressedDownsize = inventorySuppressedDownsize || isMaterial(s.current.Value, s.suggested, bounds) + for _, reason := range q.Reasons { + if reason == ReasonUnknownInventory || reason == ReasonStaleInventory || reason == ReasonIncompleteInventory || reason == ReasonExcludedContainers { + settingTrace.Reasons = append(settingTrace.Reasons, reason) + } + } + downsizeSuppressed = downsizeSuppressed || isMaterial(s.current, s.suggested, bounds) continue } // Do not propose a request above a known retained limit. - if s.setting == SettingRequests && limitOK && obs.CurrentLimit.Value > 0 && s.suggested > obs.CurrentLimit.Value && (requestsOnly || !isMaterial(obs.CurrentLimit.Value, suggestedLimit, bounds)) { + if s.setting == SettingRequests && limitOK && obs.CurrentLimit.Value > 0 { + settingTrace.guard("request fits retained or changing limit", s.suggested <= obs.CurrentLimit.Value || (!requestsOnly && isMaterial(obs.CurrentLimit, suggestedLimit, bounds)), map[string]float64{"candidate": s.suggested, "currentLimit": obs.CurrentLimit.Value}) + } + if s.setting == SettingRequests && limitOK && obs.CurrentLimit.Value > 0 && s.suggested > obs.CurrentLimit.Value && (requestsOnly || !isMaterial(obs.CurrentLimit, suggestedLimit, bounds)) { + settingTrace.stop("retained", ReasonBounds) addReason(q, ReasonBounds) - boundsSuppressedAction = boundsSuppressedAction || isMaterial(s.current.Value, s.suggested, bounds) + boundsSuppressedAction = boundsSuppressedAction || isMaterial(s.current, s.suggested, bounds) continue } - if isMaterial(s.current.Value, s.suggested, bounds) { - result.Recommendations = append(result.Recommendations, Recommendation{Setting: s.setting, CurrentValue: s.current.Value, SuggestedValue: s.suggested, Confidence: confidence}) + material := isMaterial(s.current, s.suggested, bounds) + if s.current.Unset { + settingTrace.step("initialize unset setting", map[string]float64{"candidate": s.suggested}) + } else { + values := map[string]float64{"current": s.current.Value, "candidate": s.suggested, "absoluteChange": math.Abs(s.suggested - s.current.Value), "minimumAbsoluteChange": bounds.MinimumAbsoluteChange, "minimumRelativeChange": bounds.MinimumRelativeChange} + if s.current.Value > 0 { + values["relativeChange"] = math.Abs(s.suggested-s.current.Value) / s.current.Value + } + settingTrace.guard("material change", material, values) + } + if material { + settingTrace.stop("recommended") + result.Recommendations = append(result.Recommendations, Recommendation{Setting: s.setting, CurrentValue: s.current.Value, CurrentUnset: s.current.Unset, SuggestedValue: s.suggested, Confidence: confidence}) + } else { + settingTrace.stop("retained", ReasonNoMaterialChange) } } if requestsOnly { @@ -304,7 +365,7 @@ func analyzeResource(in Input, c ContainerObservation, r Resource, p Policy) (Re q.Status = DataQualityPartial } if len(result.Recommendations) == 0 { - if inventorySuppressedDownsize { + if downsizeSuppressed { for _, reason := range q.Reasons { if reason == ReasonUnknownInventory || reason == ReasonStaleInventory || reason == ReasonIncompleteInventory || reason == ReasonExcludedContainers { result.NoActionReason = reason @@ -328,15 +389,23 @@ func canonicalSignal(s Signal) Signal { } return s } -func isMaterial(current, suggested float64, b Bounds) bool { - delta := math.Abs(current - suggested) - return delta > 0 && delta >= b.MinimumAbsoluteChange && (current == 0 || delta/current >= b.MinimumRelativeChange) +func isMaterial(current Signal, suggested float64, b Bounds) bool { + if current.Unset { + return suggested > 0 + } + delta := math.Abs(current.Value - suggested) + return delta > 0 && delta >= b.MinimumAbsoluteChange && (current.Value == 0 || delta/current.Value >= b.MinimumRelativeChange) } // A replica's percentile is computed separately; the largest replica result is // used for the shared container setting. History spans the current release across // pod lifetimes; new healthy replicas do not erase established release history. -func normalizeUsage(in Input, c ContainerObservation, obs ResourceObservation, r Resource, p Policy, q *DataQuality) (Signal, int, int, error) { +func normalizeUsage(in Input, c ContainerObservation, obs ResourceObservation, r Resource, p Policy, q *DataQuality, source *UsageSource) (Signal, int, int, error) { + *source = UsageSource{Release: c.Target.Release, Method: "maximum"} + if r == ResourceCPU && p.CPU.Strategy == CPUStrategyPercentile { + source.Method = "maximum of per-series nearest-rank percentiles" + source.Percentile = p.CPU.Percentile + } series := append([]Series(nil), obs.Series...) sort.Slice(series, func(i, j int) bool { return series[i].ID < series[j].ID }) ids := map[string]bool{} @@ -346,7 +415,6 @@ func normalizeUsage(in Input, c ContainerObservation, obs ResourceObservation, r aggregateTimestamp := int64(0) aggregateConfidence := 0.0 releaseTimes := []int64{} - releaseSpans := [][2]int64{} cadences := []float64{} start := in.WindowStart if c.Identity.ReleaseStartedAt > start { @@ -370,77 +438,17 @@ func normalizeUsage(in Input, c ContainerObservation, obs ResourceObservation, r if r == ResourceMemory && s.Kind != SampleGauge { return Signal{}, 0, 0, fmt.Errorf("memory samples must be byte gauges") } - raw := append([]Sample(nil), s.Samples...) - sort.Slice(raw, func(i, j int) bool { return raw[i].Timestamp < raw[j].Timestamp }) - clean := make([]Sample, 0, len(raw)) - for _, v := range raw { - if v.Timestamp < start || v.Timestamp > in.EvaluationTime { - continue - } - if !finite(v.Value) || v.Value < 0 { - return Signal{}, 0, 0, fmt.Errorf("sample values must be finite and nonnegative") - } - if len(clean) > 0 && v.Timestamp == clean[len(clean)-1].Timestamp { - if v.Value != clean[len(clean)-1].Value { - return Signal{}, 0, 0, fmt.Errorf("conflicting samples at one source timestamp") - } - continue - } - clean = append(clean, v) - } - if len(clean) == 0 { - continue - } - // Preserve actual source gaps before counter intervals are discarded. - // Derived rate timestamps can omit a terminal gap or combine an internal - // gap with the following valid interval. - sourceGaps := make([]float64, 0, len(clean)-1) - for i := 1; i < len(clean); i++ { - sourceGaps = append(sourceGaps, float64(clean[i].Timestamp-clean[i-1].Timestamp)/1000) - } - if len(sourceGaps) > 0 { - sort.Float64s(sourceGaps) - sourceCadence := sourceGaps[(len(sourceGaps)-1)/2] - for _, gap := range sourceGaps { - q.MaximumGapSeconds = math.Max(q.MaximumGapSeconds, gap) - oversizedGap := gap > float64(p.Evidence.MaximumGapSeconds) - if gap > 1.5*sourceCadence || oversizedGap { - q.GapCount++ - } - if s.Kind == SampleCPUCounterSeconds && oversizedGap { - addReason(q, ReasonInterruptedHistory) - } - } + values, err := NormalizeSamples(s, start, in.EvaluationTime) + if err != nil { + return Signal{}, 0, 0, err } - values := make([]Sample, 0, len(clean)) - timestamps := make([]int64, 0, len(clean)) - if s.Kind == SampleCPUCounterSeconds { - for i := 1; i < len(clean); i++ { - dt := float64(clean[i].Timestamp-clean[i-1].Timestamp) / 1000 - if dt > float64(p.Evidence.MaximumGapSeconds) { - continue - } - delta := clean[i].Value - clean[i-1].Value - if delta < 0 { - delta = clean[i].Value - } - rate := delta / dt * 1000 - if !finite(rate) { - return Signal{}, 0, 0, fmt.Errorf("CPU rate overflow") - } - values = append(values, Sample{Value: rate, Timestamp: clean[i].Timestamp}) - timestamps = append(timestamps, clean[i].Timestamp) - } - } else { - for _, v := range clean { - values = append(values, v) - timestamps = append(timestamps, v.Timestamp) - } + timestamps := make([]int64, len(values)) + for i, sample := range values { + timestamps[i] = sample.Timestamp } if len(values) == 0 { continue } - releaseSpans = append(releaseSpans, [2]int64{clean[0].Timestamp, clean[len(clean)-1].Timestamp}) selected++ pod := s.PodUID if pod == "" { @@ -492,11 +500,11 @@ func normalizeUsage(in Input, c ContainerObservation, obs ResourceObservation, r // The release may have enough history while a new replica alone supplies // its sizing value. Keep eligibility release-wide but do not borrow // confidence from another replica with a lower aggregate. - seriesCovered := math.Min(cadence, float64(p.Evidence.MaximumGapSeconds)) + seriesCovered := cadence for _, gap := range gaps { seriesCovered += math.Min(gap, cadence) } - seriesSpan := float64(last-first)/1000 + math.Min(cadence, float64(p.Evidence.MaximumGapSeconds)) + seriesSpan := float64(last-first)/1000 + cadence seriesCoverage := 0.0 if seriesSpan > 0 { seriesCoverage = math.Min(1, seriesCovered/seriesSpan) @@ -508,6 +516,7 @@ func normalizeUsage(in Input, c ContainerObservation, obs ResourceObservation, r aggregate = value.Value aggregateTimestamp = value.Timestamp aggregateConfidence = seriesConfidence + source.SeriesID, source.PodUID, source.Timestamp = s.ID, s.PodUID, value.Timestamp } } q.SeriesCount = selected @@ -528,54 +537,30 @@ func normalizeUsage(in Input, c ContainerObservation, obs ResourceObservation, r cadence = cadences[len(cadences)/2] q.CadenceSeconds = cadence } - covered := math.Min(cadence, float64(p.Evidence.MaximumGapSeconds)) - releaseMaxGap := 0.0 + covered := cadence for i := 1; i < len(unique); i++ { gap := float64(unique[i]-unique[i-1]) / 1000 covered += math.Min(gap, cadence) - releaseMaxGap = math.Max(releaseMaxGap, gap) - } - // Derived CPU rate endpoints can exaggerate a gap after an interval is - // discarded. Use raw source spans to report gaps between pod lifetimes. - sort.Slice(releaseSpans, func(i, j int) bool { return releaseSpans[i][0] < releaseSpans[j][0] }) - if len(releaseSpans) > 0 { - last := releaseSpans[0][1] - for _, span := range releaseSpans[1:] { - if span[0] > last { - gap := float64(span[0]-last) / 1000 - q.MaximumGapSeconds = math.Max(q.MaximumGapSeconds, gap) - if cadence > 0 && gap > 1.5*cadence || gap > float64(p.Evidence.MaximumGapSeconds) { - q.GapCount++ - } - } - last = max(last, span[1]) - } } if in.EvaluationTime > start { q.Coverage = math.Min(1, covered/(float64(in.EvaluationTime-start)/1000)) } q.ObservedIntervalHours = float64(q.ObservedEnd-q.ObservedStart) / 3600000 - minSpan := float64(q.ObservedEnd-q.ObservedStart)/1000 + math.Min(cadence, float64(p.Evidence.MaximumGapSeconds)) - minCount := len(unique) + minSpan := float64(q.ObservedEnd-q.ObservedStart)/1000 + cadence + q.ObservationCount = len(unique) if stale(q.ObservedEnd, in, p.Evidence) { addReason(q, ReasonStaleUsage) } - if releaseMaxGap > float64(p.Evidence.MaximumGapSeconds) { - addReason(q, ReasonInterruptedHistory) - } if minSpan < float64(p.Evidence.MinimumHistorySeconds) { addReason(q, ReasonInsufficientHistory) } - if minCount < p.Evidence.MinimumSamples { + if q.ObservationCount < p.Evidence.MinimumSamples { addReason(q, ReasonInsufficientSamples) } if q.Coverage < p.Evidence.MinimumCoverage { addReason(q, ReasonSparseCoverage) } - if q.MaximumGapSeconds > float64(p.Evidence.MaximumGapSeconds) { - addReason(q, ReasonInterruptedHistory) - } - confidence := int(math.Floor(95 * math.Min(1, minSpan/float64(p.Evidence.MinimumHistorySeconds)) * q.Coverage * math.Min(1, float64(minCount)/float64(p.Evidence.MinimumSamples)))) + confidence := int(math.Floor(95 * math.Min(1, minSpan/float64(p.Evidence.MinimumHistorySeconds)) * q.Coverage * math.Min(1, float64(q.ObservationCount)/float64(p.Evidence.MinimumSamples)))) confidence = min(confidence, int(math.Floor(aggregateConfidence))) return Signal{Available: true, Value: aggregate, Timestamp: aggregateTimestamp}, confidence, len(pods), nil } diff --git a/analysis/analyze_test.go b/analysis/analyze_test.go index 8c3dda5..d8bd5b6 100644 --- a/analysis/analyze_test.go +++ b/analysis/analyze_test.go @@ -103,12 +103,12 @@ func TestRawOffGridPeakAndCPUPercentile(t *testing.T) { t.Fatal("offline/connected normalized results differ") } } -func TestWeeklyDefaultAndMeasuredCoverage(t *testing.T) { +func TestHourlyDefaultAndMeasuredCoverage(t *testing.T) { tests := []struct { name string in Input reason Reason - }{{"three quiet days", fixture(3*86400, 60), ReasonInsufficientHistory}, {"sparse week", fixture(7*86400, 86400), ReasonInsufficientSamples}, {"stable week", fixture(7*86400, 60), ""}} + }{{"half an hour", fixture(1800, 10), ReasonInsufficientHistory}, {"one hour", fixture(3600, 60), ""}, {"sparse week", fixture(7*86400, 86400), ReasonInsufficientSamples}, {"stable week", fixture(7*86400, 60), ""}} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { out := run(t, tt.in, Policy{}) @@ -119,7 +119,7 @@ func TestWeeklyDefaultAndMeasuredCoverage(t *testing.T) { } } else { if len(r.Recommendations) == 0 || r.Recommendations[0].Confidence > 95 { - t.Fatalf("stable week should size with capped confidence: %+v", r) + t.Fatalf("sufficient history should size with capped confidence: %+v", r) } } } @@ -133,6 +133,63 @@ func TestWeeklyDefaultAndMeasuredCoverage(t *testing.T) { t.Fatal("requested lookback substituted for observed CPU history or memory blocked") } } + +func TestHourlyDefaultWithNativeMinuteScrapes(t *testing.T) { + for _, missing := range []int{0, 3, 10} { + t.Run(fmt.Sprintf("%d missing scrapes", missing), func(t *testing.T) { + in := fixture(3600, 60) + c := &in.Containers[0] + c.CPU.Series[0].Kind = SampleCPUCounterSeconds + for _, s := range []*Series{&c.CPU.Series[0], &c.Memory.Series[0]} { + var samples []Sample + for i, sample := range s.Samples { + if s.Kind == SampleCPUCounterSeconds { + sample.Value = float64(i) * 12 // 200 millicores. + } + if i > 0 && i%5 == 0 && i/5 <= missing { + continue + } + samples = append(samples, sample) + } + s.Samples = samples + } + for _, r := range run(t, in, Policy{}).Results { + wantCount := 61 - missing + if r.Resource == ResourceCPU { + wantCount-- // The first counter sample has no preceding interval. + } + if r.DataQuality.ObservationCount != wantCount || has(r.DataQuality, ReasonInsufficientSamples) || has(r.DataQuality, ReasonInsufficientHistory) { + t.Fatalf("one hour was blocked by history or sample count: %+v", r) + } + if missing == 10 { + if !has(r.DataQuality, ReasonSparseCoverage) || len(r.Recommendations) != 0 { + t.Fatalf("lower sample minimum bypassed coverage guard: %+v", r) + } + } else if r.DataQuality.Status == DataQualityUnavailable || len(r.Recommendations) == 0 { + t.Fatalf("one hour of minute scrapes should qualify: %+v", r) + } + } + }) + } +} + +func TestObservationCountUsesDistinctTimestampsAcrossReplicas(t *testing.T) { + in := fixture(3600, 60) + for _, obs := range []*ResourceObservation{&in.Containers[0].CPU, &in.Containers[0].Memory} { + other := obs.Series[0] + other.ID, other.PodUID = "pod-2", "pod-2" + obs.Series = append(obs.Series, other) + } + in.Containers[0].Inventory.Eligible, in.Containers[0].Inventory.Observed = 2, 2 + p := DefaultPolicy() + p.Evidence.MinimumSamples = 100 + for _, r := range run(t, in, p).Results { + if r.DataQuality.SampleCount != 122 || r.DataQuality.ObservationCount != 61 || !has(r.DataQuality, ReasonInsufficientSamples) || len(r.Recommendations) != 0 { + t.Fatalf("concurrent replica samples bypassed the explicit observation minimum: %+v", r) + } + } +} + func TestReleaseIdentityPartitions(t *testing.T) { for _, mode := range []string{"old-new", "rollback", "recreated", "mixed", "stale-identity", "mismatch", "unknown"} { t.Run(mode, func(t *testing.T) { @@ -252,66 +309,59 @@ func TestCurrentEvidenceMustBelongToSelectedRelease(t *testing.T) { } } -func TestCounterGapSurvivesDiscardedRate(t *testing.T) { +func TestCounterSamplesAcrossCollectionOutage(t *testing.T) { for _, terminal := range []bool{false, true} { t.Run(map[bool]string{false: "internal", true: "terminal"}[terminal], func(t *testing.T) { - in := fixture(1200, 60) + in := fixture(86400, 60) s := &in.Containers[0].CPU.Series[0] s.Kind = SampleCPUCounterSeconds for i := range s.Samples { s.Samples[i].Value = float64(i) * .6 } if terminal { - s.Samples = append(s.Samples[:19], s.Samples[20]) // Last raw interval is 120 seconds. + s.Samples = append(s.Samples[:len(s.Samples)-15], s.Samples[len(s.Samples)-1]) } else { - s.Samples = append(s.Samples[:10], s.Samples[11:]...) + s.Samples = append(s.Samples[:600], s.Samples[614:]...) } p := shortPolicy() - p.Evidence.MaximumGapSeconds = 90 - p.Evidence.FreshnessSeconds = 300 r := run(t, in, p).Results[1] - if r.DataQuality.MaximumGapSeconds != 120 || r.DataQuality.GapCount != 1 || !has(r.DataQuality, ReasonInterruptedHistory) { - t.Fatalf("discarded rate hid or duplicated the raw counter gap: %+v", r.DataQuality) + if len(r.Recommendations) == 0 || len(r.DataQuality.Reasons) != 0 || math.Abs(r.Evidence.AggregatedUsage.Value-10) > 1e-9 { + t.Fatalf("collection outage prevented sizing or changed elapsed-time CPU rates: %+v", r) } - if len(r.Recommendations) != 0 || r.NoActionReason != ReasonInterruptedHistory || has(r.DataQuality, ReasonStaleUsage) || has(r.DataQuality, ReasonSparseCoverage) { - t.Fatalf("raw counter gap must independently block otherwise fresh, covered usage: %+v", r) + if r.DataQuality.Coverage >= 1 || r.DataQuality.Coverage < .98 || r.DataQuality.SampleCount != len(s.Samples)-1 { + t.Fatalf("missing samples were invented or counter endpoints discarded: %+v", r.DataQuality) } }) } } -func TestReleaseGapAcrossPodLifetimesIsReported(t *testing.T) { - in := fixture(660, 60) +func TestReleaseReplacementToleratesCollectionOutage(t *testing.T) { + in := fixture(86400, 60) c := &in.Containers[0] - first := c.CPU.Series[0] - first.ID, first.PodUID = "old-pod", "old-pod" - first.Samples = []Sample{{Timestamp: epoch, Value: 10}, {Timestamp: epoch + 60_000, Value: 10}} - second := first - second.ID, second.PodUID = "new-pod", "new-pod" - second.Samples = []Sample{{Timestamp: epoch + 600_000, Value: 10}, {Timestamp: epoch + 660_000, Value: 10}} - c.CPU.Series = []Series{first, second} - c.Inventory.Eligible, c.Inventory.Observed = 2, 2 - - p := shortPolicy() - p.Evidence.MaximumGapSeconds = 300 - p.Evidence.FreshnessSeconds = 300 - r := run(t, in, p).Results[1] - if r.DataQuality.MaximumGapSeconds != 540 || r.DataQuality.GapCount != 1 || !has(r.DataQuality, ReasonInterruptedHistory) { - t.Fatalf("gap between pod lifetimes was not reported: %+v", r.DataQuality) + for _, observation := range []*ResourceObservation{&c.CPU, &c.Memory} { + first := observation.Series[0] + first.ID, first.PodUID = "old-pod", "old-pod" + second := first + second.ID, second.PodUID = "new-pod", "new-pod" + first.Samples, second.Samples = first.Samples[:720], second.Samples[735:] + observation.Series = []Series{first, second} + } + for _, r := range run(t, in, shortPolicy()).Results { + if len(r.Recommendations) == 0 || len(r.DataQuality.Reasons) != 0 || r.DataQuality.Coverage >= 1 || r.DataQuality.Coverage < .98 { + t.Fatalf("replacement outage should reduce coverage, not prevent sizing: %+v", r) + } } } -func TestGaugeGapBeyondPolicyIsCountedAtRegularCadence(t *testing.T) { - in := fixture(1200, 600) - p := shortPolicy() - p.Evidence.MaximumGapSeconds = 300 - r := run(t, in, p).Results[1] - if r.DataQuality.MaximumGapSeconds != 600 || r.DataQuality.GapCount != 2 || !has(r.DataQuality, ReasonInterruptedHistory) { - t.Fatalf("regular gauge gaps beyond policy were not counted: %+v", r.DataQuality) +func TestRegularSlowScrapesCanSize(t *testing.T) { + for _, r := range run(t, fixture(7*86400, 600), Policy{}).Results { + if len(r.Recommendations) == 0 || len(r.DataQuality.Reasons) != 0 || r.DataQuality.Coverage != 1 { + t.Fatalf("sufficient observations at a slower cadence should size: %+v", r) + } } } -func TestSingleSamplePodChurnBelowGapPolicyIsNotCounted(t *testing.T) { +func TestSingleSamplePodChurnKeepsMeasurements(t *testing.T) { in := fixture(120, 60) c := &in.Containers[0] base := c.CPU.Series[0] @@ -325,10 +375,9 @@ func TestSingleSamplePodChurnBelowGapPolicyIsNotCounted(t *testing.T) { } c.Inventory.Eligible, c.Inventory.Observed = 3, 3 p := shortPolicy() - p.Evidence.MaximumGapSeconds = 300 r := run(t, in, p).Results[1] - if r.DataQuality.MaximumGapSeconds != 60 || r.DataQuality.GapCount != 0 || has(r.DataQuality, ReasonInterruptedHistory) { - t.Fatalf("sub-policy churn gaps were counted without a known cadence: %+v", r.DataQuality) + if r.DataQuality.SampleCount != 3 || r.DataQuality.SeriesCount != 3 || !has(r.DataQuality, ReasonInsufficientSamples) { + t.Fatalf("pod churn lost observations or invented sufficient evidence: %+v", r.DataQuality) } } @@ -499,15 +548,15 @@ func TestStaleLimitClampCannotBecomeNoActionReason(t *testing.T) { t.Fatalf("stale bounded limit was omitted from quality evidence: %+v", r.DataQuality) } } -func TestStaleGapsDuplicatesAndFreshSignals(t *testing.T) { - for _, mode := range []string{"stale", "gap", "duplicates", "request", "limit"} { +func TestStaleSparseDuplicatesAndFreshSignals(t *testing.T) { + for _, mode := range []string{"stale", "sparse", "duplicates", "request", "limit"} { t.Run(mode, func(t *testing.T) { in := fixture(1200, 60) s := &in.Containers[0].CPU.Series[0] switch mode { case "stale": s.Samples = s.Samples[:5] - case "gap": + case "sparse": s.Samples = append(s.Samples[:5], s.Samples[15:]...) case "duplicates": s.Samples = []Sample{s.Samples[0], s.Samples[0], s.Samples[0], s.Samples[0]} @@ -527,8 +576,8 @@ func TestStaleGapsDuplicatesAndFreshSignals(t *testing.T) { if mode == "duplicates" && r.DataQuality.SampleCount != 1 { t.Fatal("duplicate/carried timestamp counted as new observation") } - if mode == "gap" && (!has(r.DataQuality, ReasonInterruptedHistory) || r.DataQuality.GapCount == 0 || r.DataQuality.Coverage >= .9) { - t.Fatalf("gap hidden: %+v", r.DataQuality) + if mode == "sparse" && (!has(r.DataQuality, ReasonSparseCoverage) || r.DataQuality.Coverage >= .9) { + t.Fatalf("missing coverage hidden: %+v", r.DataQuality) } }) } @@ -733,7 +782,7 @@ func TestStableReleaseSurvivesReplicaChurn(t *testing.T) { c.Inventory.Eligible = 2 c.Inventory.Observed = 2 } - out := run(t, in, Policy{}) + out := run(t, in, Policy{Evidence: EvidencePolicy{MinimumHistorySeconds: 7 * 86400}}) for _, r := range out.Results { expectedConfidence := 95 if replacement { @@ -747,7 +796,7 @@ func TestStableReleaseSurvivesReplicaChurn(t *testing.T) { c.CPU.Series[1].Samples[i].Value = 1000 } c.Inventory.Available = false - r := run(t, in, Policy{}).Results[1] + r := run(t, in, Policy{Evidence: EvidencePolicy{MinimumHistorySeconds: 7 * 86400}}).Results[1] if len(r.Recommendations) == 0 || r.Recommendations[0].SuggestedValue != 3000 || r.Recommendations[0].Confidence >= 95 { t.Fatalf("new replica peak cannot grow stable release %+v", r) } diff --git a/analysis/fallback.go b/analysis/fallback.go new file mode 100644 index 0000000..97134ad --- /dev/null +++ b/analysis/fallback.go @@ -0,0 +1,68 @@ +// Copyright Kedify Inc. +// SPDX-License-Identifier: LicenseRef-Kedify-Commercial-1.0 AND LicenseRef-Kedify-Public-Source-1.0 + +package analysis + +const ReasonPreviousReleaseUsage Reason = "previous-release-usage" + +// Fallback replaces only usage evidence. In particular, historical pod counts +// cannot stand in for the current inventory's freshly observed pods. +func insufficientRolloutUsage(q DataQuality) bool { + insufficient := false + for _, reason := range q.Reasons { + if reason == ReasonUnknownSeriesIdentity { + return false + } + if reason == ReasonMissingUsage || reason == ReasonInsufficientHistory || reason == ReasonInsufficientSamples { + insufficient = true + } + } + return insufficient +} + +func previousReleaseUsage(in Input, c ContainerObservation, resource Resource, p Policy, current DataQuality, trace *DecisionTrace) (Signal, int, DataQuality, *RolloutFallback, error) { + seen := map[string]bool{c.Target.Release: true} + for _, previous := range c.PreviousReleases[:min(MaxPreviousReleases, len(c.PreviousReleases))] { + if previous.Release == "" || seen[previous.Release] { + continue + } + seen[previous.Release] = true + pastIn := in + pastIn.EvaluationTime = min(previous.EvaluationTime, in.EvaluationTime, c.Identity.ReleaseStartedAt) + if pastIn.EvaluationTime <= in.WindowStart { + trace.Fallbacks = append(trace.Fallbacks, FallbackAttempt{Release: previous.Release, Reasons: []Reason{ReasonMissingUsage}}) + continue + } + past := c + past.Target.Release = previous.Release + past.Identity.Release = previous.Release + past.Identity.ReleaseStartedAt = previous.ReleaseStartedAt + past.Identity.ReleaseStartInferred = false + past.CPU.Series, past.Memory.Series = previous.CPU, previous.Memory + past = selectReleaseSegment(pastIn, past) + obs := past.Memory + if resource == ResourceCPU { + obs = past.CPU + } + quality := DataQuality{Status: DataQualityAvailable, Reasons: []Reason{}} + var source UsageSource + usage, confidence, _, err := normalizeUsage(pastIn, past, obs, resource, p, &quality, &source) + if err != nil { + return Signal{}, 0, DataQuality{}, nil, err + } + if !usage.Available || len(quality.Reasons) != 0 { + trace.Fallbacks = append(trace.Fallbacks, FallbackAttempt{Release: previous.Release, Reasons: append([]Reason(nil), quality.Reasons...)}) + continue + } + trace.Source = source + trace.Fallbacks = append(trace.Fallbacks, FallbackAttempt{Release: previous.Release, Selected: true}) + current.Status = DataQualityUnavailable + fallback := &RolloutFallback{ + Release: previous.Release, ReleaseStartedAt: past.Identity.ReleaseStartedAt, + ReleaseStartInferred: past.Identity.ReleaseStartInferred, + EvaluationTime: pastIn.EvaluationTime, CurrentDataQuality: current, + } + return usage, confidence, quality, fallback, nil + } + return Signal{}, 0, DataQuality{}, nil, nil +} diff --git a/analysis/fallback_test.go b/analysis/fallback_test.go new file mode 100644 index 0000000..2fa67d3 --- /dev/null +++ b/analysis/fallback_test.go @@ -0,0 +1,143 @@ +// Copyright Kedify Inc. +// SPDX-License-Identifier: LicenseRef-Kedify-Commercial-1.0 AND LicenseRef-Kedify-Public-Source-1.0 + +package analysis + +import ( + "reflect" + "testing" +) + +func previousFixture(name string, end, duration, step int64) PreviousRelease { + base := fixture(duration, step).Containers[0] + start := end - duration*1000 + for _, resource := range []*ResourceObservation{&base.CPU, &base.Memory} { + resource.Series[0].Release = name + resource.Series[0].ID = name + for i := range resource.Series[0].Samples { + resource.Series[0].Samples[i].Timestamp += start - epoch + } + } + return PreviousRelease{Release: name, ReleaseStartedAt: start, EvaluationTime: end, CPU: base.CPU.Series, Memory: base.Memory.Series} +} + +func fallbackFixture() Input { + in := fixture(600, 30) + in.WindowStart = epoch - 24*3600000 + return in +} + +func TestPreviousRolloutsAreTriedInOrderAndNeverPooled(t *testing.T) { + for _, eligible := range []int{0, 1, 2, 3, -1} { + in := fallbackFixture() + for i, name := range []string{"previous", "second", "third", "fourth"} { + duration, step := int64(1200), int64(10) // Enough samples, insufficient history. + if i == eligible { + duration, step = 3600, 30 + } + in.Containers[0].PreviousReleases = append(in.Containers[0].PreviousReleases, previousFixture(name, epoch-int64(i+1)*3*3600000, duration, step)) + } + out := run(t, in, Policy{}) + for _, result := range out.Results { + if eligible < 0 || eligible >= MaxPreviousReleases { + if result.RolloutFallback != nil || result.DataQuality.Status != DataQualityUnavailable || len(result.Recommendations) != 0 { + t.Fatalf("pooled short histories or searched a fourth rollout: %+v", result) + } + continue + } + want := in.Containers[0].PreviousReleases[eligible] + fallback := result.RolloutFallback + if fallback == nil || fallback.Release != want.Release || len(result.Recommendations) == 0 { + t.Fatalf("did not choose newest eligible previous rollout: %+v", result) + } + if fallback.EvaluationTime != want.EvaluationTime || result.Evidence.AggregatedUsage.Timestamp > want.EvaluationTime || result.DataQuality.ObservedEnd > want.EvaluationTime { + t.Fatal("historical observations were stamped with today's time") + } + if result.Target.Release != "A" || result.Evidence.Identity != in.Containers[0].Identity || result.Evidence.CurrentRequest.Timestamp != in.EvaluationTime { + t.Fatal("fallback replaced the current target or allocation snapshot") + } + if !has(fallback.CurrentDataQuality, ReasonInsufficientHistory) || fallback.CurrentDataQuality.Status != DataQualityUnavailable { + t.Fatal("current rollout's failed evidence guard was lost") + } + } + } +} + +func TestFallbackIsIndependentPerResource(t *testing.T) { + in := fallbackFixture() + previous := previousFixture("previous", epoch-60000, 7200, 30) + previous.CPU[0].Samples = previous.CPU[0].Samples[:1] + in.Containers[0].PreviousReleases = []PreviousRelease{previous, previousFixture("second", epoch-4*3600000, 3600, 30)} + out := run(t, in, Policy{}) + if out.Results[0].RolloutFallback.Release != "previous" || out.Results[1].RolloutFallback.Release != "second" { + t.Fatalf("resource histories were coupled: %+v", out.Results) + } +} + +func TestEligibleCurrentRolloutAlwaysWins(t *testing.T) { + in := fixture(7200, 30) + in.WindowStart = epoch - 24*3600000 + want := run(t, in, Policy{}) + in.Containers[0].PreviousReleases = []PreviousRelease{previousFixture("previous", epoch-60000, 7200, 30)} + got := run(t, in, Policy{}) + if !reflect.DeepEqual(got, want) { + t.Fatal("previous rollout affected an eligible current rollout") + } +} + +func TestFallbackRetainsIdentityAllocationAndInventoryGuards(t *testing.T) { + for _, mode := range []string{"ambiguous", "missing identity", "stale identity", "missing request", "stale request", "unknown inventory", "foreign usage"} { + t.Run(mode, func(t *testing.T) { + in := fallbackFixture() + c := &in.Containers[0] + c.PreviousReleases = []PreviousRelease{previousFixture("previous", epoch-60000, 7200, 30)} + switch mode { + case "ambiguous": + c.Identity.Ambiguous = true + case "missing identity": + c.Identity.Available = false + case "stale identity": + c.Identity.Timestamp = epoch + case "missing request": + c.CPU.CurrentRequest.Available, c.Memory.CurrentRequest.Available = false, false + case "stale request": + c.CPU.CurrentRequest.Timestamp, c.Memory.CurrentRequest.Timestamp = epoch, epoch + case "unknown inventory": + c.Inventory.Available = false + case "foreign usage": + c.PreviousReleases[0].CPU[0].WorkloadUID = "another-workload" + c.PreviousReleases[0].Memory[0].WorkloadUID = "another-workload" + } + for _, result := range run(t, in, Policy{}).Results { + if len(result.Recommendations) != 0 { + t.Fatalf("fallback bypassed %s guard: %+v", mode, result) + } + } + }) + } +} + +func TestFallbackRequiresHistoricalCoverage(t *testing.T) { + in := fallbackFixture() + previous := previousFixture("previous", epoch-60000, 7200, 30) + for _, rows := range [][]Series{previous.CPU, previous.Memory} { + rows[0].Samples = append(rows[0].Samples[:60], rows[0].Samples[140:]...) + } + in.Containers[0].PreviousReleases = []PreviousRelease{previous, previousFixture("second", epoch-4*3600000, 3600, 30)} + for _, result := range run(t, in, Policy{}).Results { + if result.RolloutFallback == nil || result.RolloutFallback.Release != "second" { + t.Fatalf("sparse historical usage authorized sizing: %+v", result) + } + } +} + +func TestFallbackHandlesMissingCurrentSamplesWithoutBorrowingInventory(t *testing.T) { + in := fallbackFixture() + c := &in.Containers[0] + c.CPU.Series = nil + c.PreviousReleases = []PreviousRelease{previousFixture("previous", epoch-60000, 7200, 30)} + result := run(t, in, Policy{}).Results[1] + if result.RolloutFallback == nil || !has(result.RolloutFallback.CurrentDataQuality, ReasonMissingUsage) || !has(result.DataQuality, ReasonIncompleteInventory) || len(result.Recommendations) != 0 { + t.Fatalf("historical pod counts authorized downsizing a currently unobserved pod: %+v", result) + } +} diff --git a/analysis/samples.go b/analysis/samples.go new file mode 100644 index 0000000..f1aedd2 --- /dev/null +++ b/analysis/samples.go @@ -0,0 +1,56 @@ +// Copyright Kedify Inc. +// SPDX-License-Identifier: LicenseRef-Kedify-Commercial-1.0 AND LicenseRef-Kedify-Public-Source-1.0 +package analysis + +import ( + "fmt" + "sort" +) + +// NormalizeSamples uses the analyzer's native sample conversion. It never mutates +// input. CPU counters become millicores; gauge values retain their units. +func NormalizeSamples(s Series, start, end int64) ([]Sample, error) { + if s.Kind != SampleGauge && s.Kind != SampleCPUCounterSeconds { + return nil, fmt.Errorf("unsupported sample kind %q", s.Kind) + } + raw := append([]Sample(nil), s.Samples...) + sort.Slice(raw, func(i, j int) bool { return raw[i].Timestamp < raw[j].Timestamp }) + clean := make([]Sample, 0, len(raw)) + for _, v := range raw { + if v.Timestamp < start || v.Timestamp > end { + continue + } + if !finite(v.Value) || v.Value < 0 { + return nil, fmt.Errorf("sample values must be finite and nonnegative") + } + if len(clean) > 0 && v.Timestamp == clean[len(clean)-1].Timestamp { + if v.Value != clean[len(clean)-1].Value { + return nil, fmt.Errorf("conflicting samples at one source timestamp") + } + continue + } + clean = append(clean, v) + } + if len(clean) == 0 { + return nil, nil + } + values := make([]Sample, 0, len(clean)) + if s.Kind == SampleCPUCounterSeconds { + for i := 1; i < len(clean); i++ { + dt := float64(clean[i].Timestamp-clean[i-1].Timestamp) / 1000 + delta := clean[i].Value - clean[i-1].Value + if delta < 0 { + delta = clean[i].Value + } + rate := delta / dt * 1000 + if !finite(rate) { + return nil, fmt.Errorf("CPU rate overflow") + } + values = append(values, Sample{Value: rate, Timestamp: clean[i].Timestamp}) + } + } else { + values = append(values, clean...) + } + + return values, nil +} diff --git a/analysis/testdata/default-output.json b/analysis/testdata/default-output.json index cfd5716..38f5527 100644 --- a/analysis/testdata/default-output.json +++ b/analysis/testdata/default-output.json @@ -1,7 +1,7 @@ { - "schemaVersion": "resource-analysis-output/v2", - "detectorVersion": "2", - "policyVersion": "v2:4ee0c63f2f57b6a5be6984cb78619879194d3200e9288b28c4a4bd247f4029d7", + "schemaVersion": "resource-analysis-output/v3", + "detectorVersion": "7", + "policyVersion": "v2:5509b23e9c4314e2c5455ab47a985a8ba022812cb6904b593407df37908ae377", "effectivePolicy": { "cpu": { "strategy": "percentile", @@ -32,12 +32,117 @@ "minimumHistorySeconds": 600, "minimumSamples": 10, "minimumCoverage": 0.9, - "maximumGapSeconds": 300, "freshnessSeconds": 300 } }, "results": [ { + "decisionTrace": { + "version": "1", + "source": { + "release": "A", + "seriesID": "pod-1", + "podUID": "pod-1", + "timestamp": 1800000233127, + "method": "maximum" + }, + "settings": [ + { + "setting": "requests", + "disposition": "recommended", + "steps": [ + { + "rule": "usage × headroom", + "values": { + "candidate": 125829120, + "coefficient": 1.2, + "usage": 104857600 + } + }, + { + "rule": "request bounds", + "values": { + "before": 125829120, + "candidate": 125829120, + "maximum": 1099511627776, + "minimum": 10485760 + } + }, + { + "rule": "safe reduction or introduction of limit", + "passed": true + }, + { + "rule": "request fits retained or changing limit", + "values": { + "candidate": 125829120, + "currentLimit": 536870912 + }, + "passed": true + }, + { + "rule": "material change", + "values": { + "absoluteChange": 142606336, + "candidate": 125829120, + "current": 268435456, + "minimumAbsoluteChange": 8388608, + "minimumRelativeChange": 0.1, + "relativeChange": 0.53125 + }, + "passed": true + } + ] + }, + { + "setting": "limits", + "disposition": "recommended", + "steps": [ + { + "rule": "request × limit ratio", + "values": { + "candidate": 377487360, + "ratio": 3, + "request": 125829120 + } + }, + { + "rule": "limit bounds", + "values": { + "before": 377487360, + "candidate": 377487360, + "maximum": 1099511627776, + "minimum": 125829120 + } + }, + { + "rule": "limit covers retained request", + "values": { + "candidate": 377487360, + "retainedRequest": 125829120 + }, + "passed": true + }, + { + "rule": "safe reduction or introduction of limit", + "passed": true + }, + { + "rule": "material change", + "values": { + "absoluteChange": 159383552, + "candidate": 377487360, + "current": 536870912, + "minimumAbsoluteChange": 8388608, + "minimumRelativeChange": 0.1, + "relativeChange": 0.296875 + }, + "passed": true + } + ] + } + ] + }, "target": { "namespace": "shop", "kind": "Deployment", @@ -99,16 +204,122 @@ "observedStart": 1800000000000, "observedEnd": 1800000600000, "sampleCount": 12, + "observationCount": 12, "seriesCount": 1, "cadenceSeconds": 60, - "gapCount": 0, - "maximumGapSeconds": 60, "coverage": 1, "observedIntervalHours": 0.16666666666666666, "reasons": [] } }, { + "decisionTrace": { + "version": "1", + "source": { + "release": "A", + "seriesID": "pod-1", + "podUID": "pod-1", + "timestamp": 1800000300000, + "method": "maximum of per-series nearest-rank percentiles", + "percentile": 50 + }, + "settings": [ + { + "setting": "requests", + "disposition": "recommended", + "steps": [ + { + "rule": "usage × headroom", + "values": { + "candidate": 150, + "coefficient": 3, + "usage": 50 + } + }, + { + "rule": "request bounds", + "values": { + "before": 150, + "candidate": 150, + "maximum": 64000, + "minimum": 20 + } + }, + { + "rule": "safe reduction or introduction of limit", + "passed": true + }, + { + "rule": "request fits retained or changing limit", + "values": { + "candidate": 150, + "currentLimit": 4000 + }, + "passed": true + }, + { + "rule": "material change", + "values": { + "absoluteChange": 850, + "candidate": 150, + "current": 1000, + "minimumAbsoluteChange": 50, + "minimumRelativeChange": 0.1, + "relativeChange": 0.85 + }, + "passed": true + } + ] + }, + { + "setting": "limits", + "disposition": "recommended", + "steps": [ + { + "rule": "request × limit ratio", + "values": { + "candidate": 750, + "ratio": 5, + "request": 150 + } + }, + { + "rule": "limit bounds", + "values": { + "before": 750, + "candidate": 750, + "maximum": 64000, + "minimum": 150 + } + }, + { + "rule": "limit covers retained request", + "values": { + "candidate": 750, + "retainedRequest": 150 + }, + "passed": true + }, + { + "rule": "safe reduction or introduction of limit", + "passed": true + }, + { + "rule": "material change", + "values": { + "absoluteChange": 3250, + "candidate": 750, + "current": 4000, + "minimumAbsoluteChange": 50, + "minimumRelativeChange": 0.1, + "relativeChange": 0.8125 + }, + "passed": true + } + ] + } + ] + }, "target": { "namespace": "shop", "kind": "Deployment", @@ -170,10 +381,9 @@ "observedStart": 1800000000000, "observedEnd": 1800000600000, "sampleCount": 11, + "observationCount": 11, "seriesCount": 1, "cadenceSeconds": 60, - "gapCount": 0, - "maximumGapSeconds": 60, "coverage": 1, "observedIntervalHours": 0.16666666666666666, "reasons": [] diff --git a/analysis/trace.go b/analysis/trace.go new file mode 100644 index 0000000..9f40556 --- /dev/null +++ b/analysis/trace.go @@ -0,0 +1,47 @@ +// Copyright Kedify Inc. +// SPDX-License-Identifier: LicenseRef-Kedify-Commercial-1.0 AND LicenseRef-Kedify-Public-Source-1.0 + +package analysis + +// DecisionTrace describes executed sizing branches, in native units (CPU +// millicores, memory bytes). It is evidence, never an alternative sizing engine. +type DecisionTrace struct { + Version string `json:"version"` + Source UsageSource `json:"source"` + Fallbacks []FallbackAttempt `json:"fallbacks,omitempty"` + Settings []SettingTrace `json:"settings"` +} +type UsageSource struct { + Release string `json:"release"` + SeriesID string `json:"seriesID,omitempty"` + PodUID string `json:"podUID,omitempty"` + Timestamp int64 `json:"timestamp,omitempty"` + Method string `json:"method"` + Percentile float64 `json:"percentile,omitempty"` +} +type FallbackAttempt struct { + Release string `json:"release"` + Selected bool `json:"selected"` + Reasons []Reason `json:"reasons,omitempty"` +} +type SettingTrace struct { + Setting Setting `json:"setting"` + Disposition string `json:"disposition"` + Reasons []Reason `json:"reasons,omitempty"` + Steps []DecisionStep `json:"steps,omitempty"` +} +type DecisionStep struct { + Rule string `json:"rule"` + Values map[string]float64 `json:"values,omitempty"` + Passed *bool `json:"passed,omitempty"` +} + +func (t *SettingTrace) step(rule string, values map[string]float64) { + t.Steps = append(t.Steps, DecisionStep{Rule: rule, Values: values}) +} +func (t *SettingTrace) guard(rule string, passed bool, values map[string]float64) { + t.Steps = append(t.Steps, DecisionStep{Rule: rule, Passed: &passed, Values: values}) +} +func (t *SettingTrace) stop(disposition string, reasons ...Reason) { + t.Disposition, t.Reasons = disposition, append([]Reason(nil), reasons...) +} diff --git a/analysis/trace_test.go b/analysis/trace_test.go new file mode 100644 index 0000000..f24fc8b --- /dev/null +++ b/analysis/trace_test.go @@ -0,0 +1,53 @@ +package analysis + +import ( + "testing" +) + +func TestDecisionTraceDispositions(t *testing.T) { + for _, tt := range []struct { + name string + change func(*Input, *Policy) + expected string + setting int + }{ + {"recommended", func(*Input, *Policy) {}, "recommended", 0}, + {"request-only limit", func(_ *Input, p *Policy) { p.CPU.RequestsOnly = true }, "disabled", 1}, + {"missing usage", func(in *Input, _ *Policy) { in.Containers[0].CPU.Series = nil }, "unavailable", 0}, + {"no material change", func(in *Input, _ *Policy) { in.Containers[0].CPU.CurrentRequest.Value = 30 }, "retained", 0}, + {"inventory guard", func(in *Input, _ *Policy) { in.Containers[0].Inventory.Available = false }, "retained", 0}, + {"missing limit", func(in *Input, _ *Policy) { in.Containers[0].CPU.CurrentLimit.Available = false }, "unavailable", 1}, + {"bounds", func(in *Input, p *Policy) { p.CPU.RequestsOnly = true; in.Containers[0].CPU.CurrentLimit.Value = 20 }, "retained", 0}, + } { + t.Run(tt.name, func(t *testing.T) { + in, p := fixture(600, 60), shortPolicy() + tt.change(&in, &p) + out := run(t, in, p) + trace := out.Results[1].DecisionTrace + if trace == nil || len(trace.Settings) != 2 || trace.Settings[tt.setting].Disposition != tt.expected { + t.Fatalf("trace: %+v", trace) + } + if tt.name == "recommended" && (trace.Source.SeriesID != "pod-1" || trace.Source.Timestamp == 0 || len(trace.Settings[0].Steps) < 3) { + t.Fatalf("missing calculation/source: %+v", trace) + } + }) + } +} +func TestSharedSampleNormalization(t *testing.T) { + values, err := NormalizeSamples(Series{Kind: SampleCPUCounterSeconds, Samples: []Sample{{Timestamp: 3000, Value: 1}, {Timestamp: 1000, Value: 10}, {Timestamp: 2000, Value: 12}, {Timestamp: 2000, Value: 12}}}, 1000, 3000) + if err != nil || len(values) != 2 || values[0].Value != 2000 || values[1].Value != 1000 { + t.Fatalf("counter/reset normalization: %v %v", values, err) + } +} + +func TestTraceExplainsFallbackRejections(t *testing.T) { + in := fallbackFixture() + in.Containers[0].PreviousReleases = []PreviousRelease{previousFixture("too-short", epoch-60000, 300, 30), previousFixture("selected", epoch-4*3600000, 3600, 30)} + out := run(t, in, Policy{}) + for _, r := range out.Results { + trace := r.DecisionTrace + if trace.Source.Release != "selected" || len(trace.Fallbacks) != 2 || trace.Fallbacks[0].Selected || len(trace.Fallbacks[0].Reasons) == 0 || !trace.Fallbacks[1].Selected { + t.Fatalf("trace lost fallback reasoning: %+v", trace) + } + } +} diff --git a/analysis/types.go b/analysis/types.go index 2ba4cdb..41a8d56 100644 --- a/analysis/types.go +++ b/analysis/types.go @@ -7,8 +7,9 @@ package analysis const ( InputSchemaVersion = "resource-analysis-input/v2" - OutputSchemaVersion = "resource-analysis-output/v2" - ResourceRightSizeDetectorVersion = "2" + OutputSchemaVersion = "resource-analysis-output/v3" + ResourceRightSizeDetectorVersion = "7" + MaxPreviousReleases = 3 ) // All timestamps are Unix milliseconds. Observation timestamps retain the source @@ -33,6 +34,20 @@ type ContainerObservation struct { Inventory Inventory `json:"inventory"` CPU ResourceObservation `json:"cpu"` Memory ResourceObservation `json:"memory"` + // Newest first. Only the first three entries are eligible as fallback usage. + PreviousReleases []PreviousRelease `json:"previousReleases,omitempty"` +} + +// PreviousRelease supplies raw usage from an earlier rollout of this workload. +// EvaluationTime is the last historical observation, not today's timestamp. +// ReleaseStartedAt may be zero to infer the observed segment. Current settings +// and inventory always refer to the current rollout. +type PreviousRelease struct { + Release string `json:"release"` + ReleaseStartedAt int64 `json:"releaseStartedAt,omitempty"` + EvaluationTime int64 `json:"evaluationTime"` + CPU []Series `json:"cpu"` + Memory []Series `json:"memory"` } // ReleaseStartedAt is an activation boundary or a conservative observed segment @@ -84,12 +99,17 @@ type Sample struct { Value float64 `json:"value"` } -// Signal distinguishes observed zero from missing. The timestamp is the source -// observation time; adapters mark inconsistent replica settings unavailable. +// Signal distinguishes observed zero, known unset allocations, and missing data. +// For current requests/limits, Available=true with Unset=true means the setting +// was observed to be absent; Value must be zero and is not a numeric allocation. +// Unset defaults to false so existing numeric signals keep their meaning. +// The timestamp is the source observation time; adapters mark inconsistent +// replica settings unavailable. // Aggregated usage retains the selected sample's timestamp (the interval end for // a CPU rate). DataQuality.ObservedEnd separately reports the latest usable sample. type Signal struct { Available bool `json:"available"` + Unset bool `json:"unset,omitempty"` Value float64 `json:"value"` Timestamp int64 `json:"timestamp"` } @@ -113,7 +133,6 @@ type EvidencePolicy struct { MinimumHistorySeconds int64 `json:"minimumHistorySeconds"` MinimumSamples int `json:"minimumSamples"` MinimumCoverage float64 `json:"minimumCoverage"` - MaximumGapSeconds int64 `json:"maximumGapSeconds"` FreshnessSeconds int64 `json:"freshnessSeconds"` } type Bounds struct { @@ -152,13 +171,28 @@ const ( ) type ResourceAnalysis struct { + DecisionTrace *DecisionTrace `json:"decisionTrace,omitempty"` Target Target `json:"target"` Resource Resource `json:"resource"` Recommendations []Recommendation `json:"recommendations,omitempty"` Evidence ResourceEvidence `json:"evidence"` DataQuality DataQuality `json:"dataQuality"` NoActionReason Reason `json:"noActionReason,omitempty"` + Notices []Reason `json:"notices,omitempty"` + RolloutFallback *RolloutFallback `json:"rolloutFallback,omitempty"` } + +// RolloutFallback identifies historical usage used to size the current target. +// DataQuality and AggregatedUsage describe this source; CurrentDataQuality +// preserves the reason current-rollout usage was insufficient. +type RolloutFallback struct { + Release string `json:"release"` + ReleaseStartedAt int64 `json:"releaseStartedAt"` + ReleaseStartInferred bool `json:"releaseStartInferred"` + EvaluationTime int64 `json:"evaluationTime"` + CurrentDataQuality DataQuality `json:"currentDataQuality"` +} + type ResourceEvidence struct { AggregatedUsage Signal `json:"aggregatedUsage"` CurrentRequest Signal `json:"currentRequest"` @@ -174,8 +208,10 @@ const ( ) type Recommendation struct { - Setting Setting `json:"setting"` - CurrentValue float64 `json:"currentValue"` + Setting Setting `json:"setting"` + CurrentValue float64 `json:"currentValue"` + // CurrentUnset means CurrentValue is a placeholder, not an observed zero. + CurrentUnset bool `json:"currentUnset,omitempty"` SuggestedValue float64 `json:"suggestedValue"` Confidence int `json:"confidence"` } @@ -200,7 +236,6 @@ const ( ReasonInsufficientHistory Reason = "insufficient-history" ReasonInsufficientSamples Reason = "insufficient-samples" ReasonSparseCoverage Reason = "sparse-coverage" - ReasonInterruptedHistory Reason = "interrupted-history" ReasonStaleUsage Reason = "stale-usage" ReasonMissingRequest Reason = "missing-current-request" ReasonStaleRequest Reason = "stale-current-request" @@ -216,15 +251,16 @@ const ( ) type DataQuality struct { - Status DataQualityStatus `json:"status"` - ObservedStart int64 `json:"observedStart"` - ObservedEnd int64 `json:"observedEnd"` - SampleCount int `json:"sampleCount"` - SeriesCount int `json:"seriesCount"` - CadenceSeconds float64 `json:"cadenceSeconds"` - GapCount int `json:"gapCount"` - MaximumGapSeconds float64 `json:"maximumGapSeconds"` - Coverage float64 `json:"coverage"` - ObservedIntervalHours float64 `json:"observedIntervalHours"` - Reasons []Reason `json:"reasons"` + Status DataQualityStatus `json:"status"` + ObservedStart int64 `json:"observedStart"` + ObservedEnd int64 `json:"observedEnd"` + SampleCount int `json:"sampleCount"` + // ObservationCount counts distinct timestamps across series after normalization, + // including CPU counter conversion. MinimumSamples applies to this count. + ObservationCount int `json:"observationCount"` + SeriesCount int `json:"seriesCount"` + CadenceSeconds float64 `json:"cadenceSeconds"` + Coverage float64 `json:"coverage"` + ObservedIntervalHours float64 `json:"observedIntervalHours"` + Reasons []Reason `json:"reasons"` } diff --git a/analysis/unset_test.go b/analysis/unset_test.go new file mode 100644 index 0000000..b05d2a6 --- /dev/null +++ b/analysis/unset_test.go @@ -0,0 +1,146 @@ +package analysis + +import ( + "encoding/json" + "testing" +) + +func unsetFixture(resource Resource) (Input, Policy, int, float64) { + in, p := fixture(600, 60), shortPolicy() + index, candidate := 1, float64(24) + obs := &in.Containers[0].CPU + p.CPU.HeadroomCoefficient, p.CPU.LimitsToRequestsRatio = 1, 1 + if resource == ResourceMemory { + index, candidate = 0, 2*1024*1024 + obs = &in.Containers[0].Memory + p.Memory.HeadroomCoefficient, p.Memory.LimitsToRequestsRatio = 1, 1 + p.Memory.Bounds.Minimum = 1024 * 1024 + } + for i := range obs.Series[0].Samples { + obs.Series[0].Samples[i].Value = candidate + } + obs.CurrentRequest = Signal{Available: true, Unset: true, Timestamp: in.EvaluationTime} + obs.CurrentLimit = obs.CurrentRequest + return in, p, index, candidate +} + +func TestUnsetSettingsBypassMinimumChange(t *testing.T) { + for _, resource := range []Resource{ResourceCPU, ResourceMemory} { + t.Run(string(resource), func(t *testing.T) { + in, p, index, candidate := unsetFixture(resource) + // Exercise the public JSON boundary, preserving known absence. + data, err := json.Marshal(in) + if err != nil { + t.Fatal(err) + } + var decoded Input + if err := json.Unmarshal(data, &decoded); err != nil { + t.Fatal(err) + } + r := run(t, decoded, p).Results[index] + if len(r.Recommendations) != 2 || !r.Evidence.CurrentRequest.Unset || !r.Evidence.CurrentLimit.Unset { + t.Fatalf("unset settings were not initialized: %+v", r) + } + for _, rec := range r.Recommendations { + if rec.SuggestedValue != candidate || !rec.CurrentUnset { + t.Fatalf("incorrect initialization: %+v", rec) + } + } + for _, trace := range r.DecisionTrace.Settings { + initialized := false + for _, step := range trace.Steps { + if step.Rule == "material change" { + t.Fatal("unset setting was compared with numeric zero") + } + initialized = initialized || step.Rule == "initialize unset setting" + } + if !initialized || trace.Disposition != "recommended" { + t.Fatalf("missing initialization trace: %+v", trace) + } + } + }) + } +} + +func TestNumericSettingsKeepMinimumChange(t *testing.T) { + for _, resource := range []Resource{ResourceCPU, ResourceMemory} { + for _, existing := range []float64{0, 1} { + in, p, index, candidate := unsetFixture(resource) + obs := &in.Containers[0].CPU + if resource == ResourceMemory { + obs = &in.Containers[0].Memory + } + obs.CurrentRequest.Unset, obs.CurrentLimit.Unset = false, false + obs.CurrentRequest.Value, obs.CurrentLimit.Value = existing*candidate/2, existing*candidate/2 + r := run(t, in, p).Results[index] + if len(r.Recommendations) != 0 || r.NoActionReason != ReasonNoMaterialChange { + t.Fatalf("%s numeric setting bypassed thresholds: %+v", resource, r) + } + } + } +} + +func TestUnsetSettingsPreserveGuards(t *testing.T) { + for _, resource := range []Resource{ResourceCPU, ResourceMemory} { + for _, tc := range []struct { + name string + change func(*Input, *ResourceObservation, *Policy) + want []Setting + reason Reason + }{ + {"unknown request", func(_ *Input, o *ResourceObservation, _ *Policy) { o.CurrentRequest = Signal{} }, nil, ReasonMissingRequest}, + {"stale unset request", func(_ *Input, o *ResourceObservation, _ *Policy) { o.CurrentRequest.Timestamp = epoch }, nil, ReasonStaleRequest}, + {"unknown limit", func(_ *Input, o *ResourceObservation, _ *Policy) { o.CurrentLimit = Signal{} }, []Setting{SettingRequests}, ""}, + {"stale unset limit", func(_ *Input, o *ResourceObservation, _ *Policy) { o.CurrentLimit.Timestamp = epoch }, []Setting{SettingRequests}, ""}, + {"missing usage", func(_ *Input, o *ResourceObservation, _ *Policy) { o.Series = nil }, nil, ReasonMissingUsage}, + {"short history", func(_ *Input, _ *ResourceObservation, p *Policy) { p.Evidence.MinimumHistorySeconds = 1200 }, nil, ReasonInsufficientHistory}, + {"missing inventory", func(in *Input, _ *ResourceObservation, _ *Policy) { in.Containers[0].Inventory.Available = false }, []Setting{SettingRequests}, ""}, + {"requests only", func(_ *Input, _ *ResourceObservation, p *Policy) { + p.CPU.RequestsOnly, p.Memory.RequestsOnly = true, true + }, []Setting{SettingRequests}, ""}, + {"retained lower limit", func(in *Input, o *ResourceObservation, p *Policy) { + o.CurrentLimit = Signal{Available: true, Timestamp: in.EvaluationTime, Value: o.Series[0].Samples[0].Value / 2} + p.CPU.RequestsOnly, p.Memory.RequestsOnly = true, true + }, nil, ReasonBounds}, + {"limit below retained request", func(_ *Input, o *ResourceObservation, _ *Policy) { + o.CurrentRequest.Unset = false + o.CurrentRequest.Value = o.Series[0].Samples[0].Value + 1 + }, nil, ReasonBounds}, + } { + t.Run(string(resource)+"/"+tc.name, func(t *testing.T) { + in, p, index, _ := unsetFixture(resource) + obs := &in.Containers[0].CPU + if resource == ResourceMemory { + obs = &in.Containers[0].Memory + } + tc.change(&in, obs, &p) + r := run(t, in, p).Results[index] + if len(r.Recommendations) != len(tc.want) { + t.Fatalf("guard not preserved: %+v", r) + } + for i, setting := range tc.want { + if r.Recommendations[i].Setting != setting { + t.Fatalf("wrong setting initialized: %+v", r) + } + } + if tc.reason != "" && r.NoActionReason != tc.reason { + t.Fatalf("no-action reason = %s, want %s", r.NoActionReason, tc.reason) + } + }) + } + } +} + +func TestUnsetSignalRejectsNumericValue(t *testing.T) { + for _, limit := range []bool{false, true} { + in, p, _, _ := unsetFixture(ResourceCPU) + signal := &in.Containers[0].CPU.CurrentRequest + if limit { + signal = &in.Containers[0].CPU.CurrentLimit + } + signal.Value = 1 + if _, err := Analyze(in, p); err == nil { + t.Fatal("contradictory unset and numeric allocation accepted") + } + } +} From d551f1970ceda8414a3c076ec5d66b31c86e3a0b Mon Sep 17 00:00:00 2001 From: Jirka Kremser Date: Thu, 24 Sep 2026 18:37:50 +0200 Subject: [PATCH 2/4] Account for OOM kills in memory sizing Normalize and deduplicate release-scoped OOM events, and use failed limits or explicit fallback allocations to establish a memory request floor. Allow justified growth without usage history while retaining identity, freshness, bounds, and allocation consistency guards. Report OOM evidence and sizing adjustments in results and decision traces. Cover interactions with rollout fallback and unset settings, update the policy fixture, and document event normalization for adapters. --- README.md | 108 ++++++- analysis/analyze.go | 89 +++++- analysis/fallback_test.go | 14 + analysis/oom.go | 77 +++++ analysis/oom_test.go | 403 ++++++++++++++++++++++++++ analysis/testdata/default-output.json | 5 +- analysis/types.go | 42 ++- analysis/unset_test.go | 13 + 8 files changed, 732 insertions(+), 19 deletions(-) create mode 100644 analysis/oom.go create mode 100644 analysis/oom_test.go diff --git a/README.md b/README.md index a0000a4..6b63af9 100644 --- a/README.md +++ b/README.md @@ -16,7 +16,7 @@ output, err := analysis.Analyze(snapshot, policy) Callers fetch raw range vectors and map them into `analysis.Input`; calculations and CPU counter normalization happen in this package. Input schema v2 rejects v1 -input; results use output schema v3 and resource detector version 7. Consumers +input; results use output schema v3 and resource detector version 8. Consumers should update generated output bindings and invalidate cached findings when upgrading. All timestamps are original Unix milliseconds, including current settings and identity. Do not stamp carried-forward values with query evaluation times. CPU @@ -79,7 +79,7 @@ Historical evidence is evaluated at that time, capped at the current rollout's start, and retains its original timestamps. `rolloutFallback` in each resource result identifies the historical source and preserves the current rollout's failed data-quality checks. The result target, identity, allocation signals, -and inventory checks remain current. Historical pod +inventory checks and OOM events remain current. Historical pod counts cannot authorize downsizing currently unobserved pods. Existing callers that omit `previousReleases` keep current-rollout-only behavior. @@ -96,8 +96,8 @@ from an observed numeric zero and from `Available: false` (unknown). The value must remain zero when `Unset` is true; it is a placeholder, not an allocation. Since detector version 7, initializing an unset setting bypasses absolute and relative minimum-change thresholds. Evidence, freshness, bounds, inventory, -and requests-only guards still apply. In particular, introducing an unset -limit still requires safe inventory evidence. Numeric signals that omit +OOM and requests-only guards still apply. In particular, introducing an unset +limit still requires safe inventory and OOM evidence. Numeric signals that omit `unset` retain their previous behavior, including explicit zero values. Recommendations mark such initialization with `currentUnset: true`; their `currentValue` is then only a placeholder. Decision traces record @@ -112,6 +112,106 @@ reasons; an empty recommendation list always includes a no-action reason. Detect and normalized-policy identities are independent from schema versions. Consumers should pin a released module version and invalidate/recompute old findings. +## OOM observations + +Callers may supply `ContainerObservation.OOMKills` as normalized positive OOM +termination events. The engine has no metric names, PromQL, Kubernetes clients, +or source-specific adapters. Each event has a stable `ID`, the original termination +`Timestamp` in Unix milliseconds, `WorkloadUID`, `Release`, optional `PodUID`, and +optional `MemoryLimitBytes` for the finite limit in effect at termination: + +```go +container.OOMKills = []analysis.OOMKill{{ + ID: "pod-uid/app/1800000060000", + PodUID: "pod-uid", + WorkloadUID: container.Target.WorkloadUID, + Release: container.Target.Release, + Timestamp: 1800000060000, + MemoryLimitBytes: 512 * 1024 * 1024, +}} +policy := analysis.DefaultPolicy() +policy.Memory.OOMKilledCoefficient = 1.5 // Default: 50% increase after an OOM kill. +``` + +The adapter must establish the workload/release identity at the event time before +assigning it to this container. Use zero or omit `MemoryLimitBytes` when the +event-time limit is unknown or unlimited; do not substitute today's limit. +Repeated scrapes of one termination retain the same event ID and timestamp. +Identical events are deduplicated; conflicting observations sharing an ID are +rejected. Events are sorted deterministically and input is not mutated. + +Only events within the requested window and selected workload UID/release segment +affect memory. Events from before a rollback boundary or from another workload UID +are excluded, as are future events. Event age is not current-signal freshness: +a kill earlier in the selected history still matters even if its pod has gone. +Events do not establish usage coverage. A verified current-release event can +establish an inferred activation boundary when usage samples are absent; known +activation boundaries and observed rollback boundaries still restrict events. + +For events with a known limit, the calculation follows KRR's memory rule: + +```text +baseline = peak observed memory × memory.headroomCoefficient +OOM floor = maximum event-time memory limit × memory.oomKilledCoefficient +request candidate = max(baseline, OOM floor) +``` + +For example, a 512 MiB limit exceeded by an OOM produces a 768 MiB floor with the +default coefficient. The normal memory limit/request ratio then applies; set +`memory.limitsToRequestsRatio` to 1 for equal requests and limits, as in KRR. + +A timestamp-only event uses the current finite memory limit as its sizing base, +then the current memory request if no finite limit is known, then qualifying +usage as a final fallback. This is recorded as a fallback, never as the historical +failed limit. The coefficient applies once; repeated events do not compound it. +Unknown failed limits still block reductions and introduction of a finite limit, +and produce `unknown-oom-memory-limit`. The largest event contribution wins. +The coefficient must be finite and at least 1; zero selects the default 1.5. + +A current-release OOM with a positive sizing base authorizes memory increases +without usage samples, minimum history, coverage, or fresh usage. Missing usage +remains visible as partial data quality, and usage confidence is zero for this +path. No reduction is authorized by insufficient usage. Identity, current-setting +freshness, material-change, bounds, and request/limit consistency guards still +apply. CPU sizing remains usage-based. Omitting the optional events preserves +usage-based sizing; it does not assert that no kills occurred. + +Memory output includes: + +- `notices: ["oom-kill-detected"]` and `evidence.oomKills`, even if a guard blocks + recommendations. Detection by itself does not degrade data quality. +- `oomAdjustment` when sizing is eligible: `baselineRequestBytes`, + `oomRequestFloorBytes`, `usedCurrentFallback`, and `usedUsageFallback` explain the calculation before + bounds and action filtering. The floor may already be below the baseline, or + bounds/material-change guards may prevent a resulting recommendation. +- `effectivePolicy.memory.oomKilledCoefficient` records the effective multiplier. + +The OOM coefficient is included in the normalized policy hash. Consumers using +this feature must pin a module version containing it and invalidate cached +findings from older detectors/policies. + +### Adapter guidance + +- **Kedify agent:** `container_last_oom_kill_timestamp` stores the termination + time in its **value**, in Unix seconds. Convert that value to milliseconds; + the metric sample timestamp is the collection time. Use the `podUID`, + `workloadUID`, `workloadRevision`, and container labels to establish identity. + Construct a stable event ID from pod UID, container, and termination time. + The timestamp metric itself supplies no memory limit. Attach a limit only if + historical evidence establishes it at termination. Respect + `container_oom_collection_status`; absence of series under denied, unavailable, + or over-budget collection is not proof of zero kills. +- **kube-state-metrics:** join a positive + `kube_pod_container_status_last_terminated_reason{reason="OOMKilled"}` with + `kube_pod_container_status_last_terminated_timestamp` for the same pod UID and + container, then convert its timestamp value to milliseconds. Resolve historical + owner/release identity and, if available, the memory resource limit at the event + time. A reason gauge alone does not establish the event time; do not stamp it + with each scrape time. See the [KSM pod metrics reference](https://github.com/kubernetes/kube-state-metrics/blob/main/docs/metrics/workload/pod-metrics.md). +- **Other sources:** Kubernetes termination status, event stores, or offline JSON + may populate the same structure directly. Do not infer an OOM solely from a + restart count or exit code. Source collection and conversion belong to callers. + ## Consumers - [`dashboard-api-service`](https://github.com/kedify/dashboard-api-service) diff --git a/analysis/analyze.go b/analysis/analyze.go index df24c9b..cdb1c08 100644 --- a/analysis/analyze.go +++ b/analysis/analyze.go @@ -17,7 +17,7 @@ import ( func DefaultPolicy() Policy { return Policy{ CPU: CPUPolicy{Strategy: CPUStrategyPercentile, Percentile: 95, HeadroomCoefficient: 3, LimitsToRequestsRatio: 5, Bounds: Bounds{Minimum: 20, Maximum: 64000, MinimumAbsoluteChange: 50, MinimumRelativeChange: .1}}, - Memory: MemoryPolicy{Strategy: MemoryStrategyMax, HeadroomCoefficient: 1.2, LimitsToRequestsRatio: 3, Bounds: Bounds{Minimum: 10 * 1024 * 1024, Maximum: 1024 * 1024 * 1024 * 1024, MinimumAbsoluteChange: 8 * 1024 * 1024, MinimumRelativeChange: .1}}, + Memory: MemoryPolicy{Strategy: MemoryStrategyMax, HeadroomCoefficient: 1.2, OOMKilledCoefficient: 1.5, LimitsToRequestsRatio: 3, Bounds: Bounds{Minimum: 10 * 1024 * 1024, Maximum: 1024 * 1024 * 1024 * 1024, MinimumAbsoluteChange: 8 * 1024 * 1024, MinimumRelativeChange: .1}}, Evidence: EvidencePolicy{MinimumHistorySeconds: 3600, MinimumSamples: 30, MinimumCoverage: .9, FreshnessSeconds: 300}, } } @@ -37,6 +37,12 @@ func NormalizePolicy(p Policy) (Policy, error) { if p.Memory.Strategy != MemoryStrategyMax { return Policy{}, fmt.Errorf("unsupported memory.strategy %q", p.Memory.Strategy) } + if p.Memory.OOMKilledCoefficient == 0 { + p.Memory.OOMKilledCoefficient = d.Memory.OOMKilledCoefficient + } + if !finite(p.Memory.OOMKilledCoefficient) || p.Memory.OOMKilledCoefficient < 1 { + return Policy{}, fmt.Errorf("memory.oomKilledCoefficient must be finite and at least one") + } if p.CPU.Percentile == 0 { p.CPU.Percentile = d.CPU.Percentile } @@ -210,6 +216,22 @@ func analyzeResource(in Input, c ContainerObservation, r Resource, p Policy) (Re } result.Evidence.AggregatedUsage = usage usageBlocked := len(q.Reasons) > 0 + oomLimitUnknown := false + if r == ResourceMemory { + kills, oomErr := selectOOMKills(in, c) + if oomErr != nil { + return ResourceAnalysis{}, oomErr + } + result.Evidence.OOMKills = kills + if len(kills) > 0 { + result.Notices = append(result.Notices, ReasonOOMKillDetected) + } + for _, kill := range kills { + if kill.MemoryLimitBytes == 0 { + oomLimitUnknown = true + } + } + } requestOK, limitOK := true, true for _, s := range []struct { value Signal @@ -248,19 +270,56 @@ func analyzeResource(in Input, c ContainerObservation, r Resource, p Policy) (Re addReason(q, ReasonIncompleteInventory) inventoryOK = false } - if identityBlocked || usageBlocked || !requestOK { + if oomLimitUnknown { + addReason(q, ReasonOOMLimitUnknown) + } + // OOM evidence can size memory independently of usage history. Keep the + // missing-usage reasons as partial evidence, rather than inventing samples. + rawSuggestedRequest := 0.0 + if !usageBlocked { + rawSuggestedRequest = usage.Value * headroom + } + if !finite(rawSuggestedRequest) { + return ResourceAnalysis{}, fmt.Errorf("suggested value overflow") + } + var adjustment *OOMAdjustment + if !identityBlocked && requestOK && len(result.Evidence.OOMKills) > 0 { + currentBase := obs.CurrentRequest.Value + if limitOK && obs.CurrentLimit.Value > 0 { + currentBase = obs.CurrentLimit.Value + } + value, oomErr := oomAdjustment(result.Evidence.OOMKills, rawSuggestedRequest, currentBase, p.Memory.OOMKilledCoefficient) + if oomErr != nil { + return ResourceAnalysis{}, oomErr + } + if value.OOMRequestFloorBytes > 0 { + adjustment = &value + } + } + if identityBlocked || !requestOK || (usageBlocked && adjustment == nil) { q.Status = DataQualityUnavailable result.NoActionReason = q.Reasons[0] requestTrace.stop("unavailable", q.Reasons...) limitTrace.stop("unavailable", q.Reasons...) return result, nil } - rawSuggestedRequest := usage.Value * headroom - if !finite(rawSuggestedRequest) { - return ResourceAnalysis{}, fmt.Errorf("suggested value overflow") + if !usageBlocked { + requestTrace.step("usage × headroom", map[string]float64{"usage": usage.Value, "coefficient": headroom, "candidate": rawSuggestedRequest}) + } else { + confidence = 0 // There is no qualifying usage history to score. + latest := result.Evidence.OOMKills[len(result.Evidence.OOMKills)-1] + trace.Source = UsageSource{Release: c.Target.Release, PodUID: latest.PodUID, Timestamp: latest.Timestamp, Method: "OOM kill"} + requestTrace.step("OOM sizing without usage history", nil) + } + if adjustment != nil { + result.OOMAdjustment = adjustment + rawSuggestedRequest = math.Max(rawSuggestedRequest, adjustment.OOMRequestFloorBytes) + requestTrace.step("OOM floor", map[string]float64{"baseline": adjustment.BaselineRequestBytes, "floor": adjustment.OOMRequestFloorBytes, "candidate": rawSuggestedRequest, "coefficient": p.Memory.OOMKilledCoefficient}) } - requestTrace.step("usage × headroom", map[string]float64{"usage": usage.Value, "coefficient": headroom, "candidate": rawSuggestedRequest}) suggestedRequest := math.Max(bounds.Minimum, math.Min(bounds.Maximum, rawSuggestedRequest)) + if usageBlocked { + suggestedRequest = math.Max(suggestedRequest, obs.CurrentRequest.Value) + } requestTrace.step("request bounds", map[string]float64{"before": rawSuggestedRequest, "minimum": bounds.Minimum, "maximum": bounds.Maximum, "candidate": suggestedRequest}) requestBounded := suggestedRequest != rawSuggestedRequest boundsSuppressedAction := requestBounded && isMaterial(obs.CurrentRequest, rawSuggestedRequest, bounds) && !isMaterial(obs.CurrentRequest, suggestedRequest, bounds) @@ -274,6 +333,9 @@ func analyzeResource(in Input, c ContainerObservation, r Resource, p Policy) (Re return ResourceAnalysis{}, fmt.Errorf("suggested value overflow") } suggestedLimit = math.Max(suggestedRequest, math.Min(bounds.Maximum, rawSuggestedLimit)) + if usageBlocked && limitOK { + suggestedLimit = math.Max(suggestedLimit, obs.CurrentLimit.Value) + } limitTrace.step("request × limit ratio", map[string]float64{"request": suggestedRequest, "ratio": ratio, "candidate": rawSuggestedLimit}) limitTrace.step("limit bounds", map[string]float64{"before": rawSuggestedLimit, "minimum": suggestedRequest, "maximum": bounds.Maximum, "candidate": suggestedLimit}) limitBounded := suggestedLimit != rawSuggestedLimit @@ -320,11 +382,11 @@ func analyzeResource(in Input, c ContainerObservation, r Resource, p Policy) (Re } } if s.suggested < s.current.Value || (s.setting == SettingLimits && s.current.Value == 0) { - settingTrace.guard("safe reduction or introduction of limit", inventoryOK, nil) + settingTrace.guard("safe reduction or introduction of limit", inventoryOK && !oomLimitUnknown, nil) } - if (s.suggested < s.current.Value || (s.setting == SettingLimits && s.current.Value == 0)) && !inventoryOK { + if (s.suggested < s.current.Value || (s.setting == SettingLimits && s.current.Value == 0)) && (!inventoryOK || oomLimitUnknown) { for _, reason := range q.Reasons { - if reason == ReasonUnknownInventory || reason == ReasonStaleInventory || reason == ReasonIncompleteInventory || reason == ReasonExcludedContainers { + if reason == ReasonUnknownInventory || reason == ReasonStaleInventory || reason == ReasonIncompleteInventory || reason == ReasonExcludedContainers || reason == ReasonOOMLimitUnknown { settingTrace.Reasons = append(settingTrace.Reasons, reason) } } @@ -367,7 +429,7 @@ func analyzeResource(in Input, c ContainerObservation, r Resource, p Policy) (Re if len(result.Recommendations) == 0 { if downsizeSuppressed { for _, reason := range q.Reasons { - if reason == ReasonUnknownInventory || reason == ReasonStaleInventory || reason == ReasonIncompleteInventory || reason == ReasonExcludedContainers { + if reason == ReasonUnknownInventory || reason == ReasonStaleInventory || reason == ReasonIncompleteInventory || reason == ReasonExcludedContainers || reason == ReasonOOMLimitUnknown { result.NoActionReason = reason break } @@ -594,6 +656,13 @@ func selectReleaseSegment(in Input, c ContainerObservation) ContainerObservation } } } + // A verified current-rollout OOM also proves the rollout was active at + // that instant, including containers that died before their first scrape. + for _, kill := range c.OOMKills { + if kill.WorkloadUID == c.Target.WorkloadUID && kill.Release == c.Target.Release && kill.Timestamp > cutoff && kill.Timestamp <= in.EvaluationTime && (boundary == 0 || kill.Timestamp < boundary) { + boundary = kill.Timestamp + } + } if boundary == 0 && cutoff >= in.WindowStart { boundary = cutoff + 1 } diff --git a/analysis/fallback_test.go b/analysis/fallback_test.go index 2fa67d3..cfa85ff 100644 --- a/analysis/fallback_test.go +++ b/analysis/fallback_test.go @@ -141,3 +141,17 @@ func TestFallbackHandlesMissingCurrentSamplesWithoutBorrowingInventory(t *testin t.Fatalf("historical pod counts authorized downsizing a currently unobserved pod: %+v", result) } } + +func TestFallbackKeepsCurrentOOMProtection(t *testing.T) { + in := fallbackFixture() + c := &in.Containers[0] + c.PreviousReleases = []PreviousRelease{previousFixture("previous", epoch-60000, 7200, 30)} + c.OOMKills = []OOMKill{ + {ID: "current-oom", WorkloadUID: "uid", Release: "A", Timestamp: in.EvaluationTime - 60000, MemoryLimitBytes: 1024 * 1024 * 1024}, + {ID: "old-oom", WorkloadUID: "uid", Release: "previous", Timestamp: epoch - 120000, MemoryLimitBytes: 4 * 1024 * 1024 * 1024}, + } + result := run(t, in, Policy{}).Results[0] + if result.RolloutFallback == nil || result.OOMAdjustment == nil || result.OOMAdjustment.OOMRequestFloorBytes != 1.5*1024*1024*1024 || len(result.Evidence.OOMKills) != 1 || result.Evidence.OOMKills[0].ID != "current-oom" { + t.Fatalf("fallback changed current OOM protection: %+v", result) + } +} diff --git a/analysis/oom.go b/analysis/oom.go new file mode 100644 index 0000000..a4db3f0 --- /dev/null +++ b/analysis/oom.go @@ -0,0 +1,77 @@ +// Copyright Kedify Inc. +// SPDX-License-Identifier: LicenseRef-Kedify-Commercial-1.0 AND LicenseRef-Kedify-Public-Source-1.0 +// See LICENSE and PUBLIC_SOURCE_LICENSE. + +package analysis + +import ( + "fmt" + "math" + "sort" +) + +// Select by event time, not observation freshness: a historical kill remains +// relevant throughout the selected release segment, even after its pod is gone. +func selectOOMKills(in Input, c ContainerObservation) ([]OOMKill, error) { + start := max(in.WindowStart, c.Identity.ReleaseStartedAt) + var kills []OOMKill + seen := make(map[string]OOMKill) + for _, kill := range c.OOMKills { + if kill.Timestamp <= 0 { + return nil, fmt.Errorf("OOM kill timestamp must be positive Unix milliseconds") + } + if kill.Timestamp < start || kill.Timestamp > in.EvaluationTime { + continue + } + if kill.WorkloadUID == "" || kill.Release == "" { + return nil, fmt.Errorf("OOM kill workload UID and release are required") + } + if kill.WorkloadUID != c.Target.WorkloadUID || kill.Release != c.Target.Release { + continue + } + if kill.ID == "" { + return nil, fmt.Errorf("OOM kill ID is required") + } + if !finite(kill.MemoryLimitBytes) || kill.MemoryLimitBytes < 0 { + return nil, fmt.Errorf("OOM kill memory limit must be finite and nonnegative") + } + if previous, ok := seen[kill.ID]; ok { + if previous != kill { + return nil, fmt.Errorf("conflicting observations for OOM kill %q", kill.ID) + } + continue + } + seen[kill.ID] = kill + kills = append(kills, kill) + } + sort.Slice(kills, func(i, j int) bool { + if kills[i].Timestamp != kills[j].Timestamp { + return kills[i].Timestamp < kills[j].Timestamp + } + return kills[i].ID < kills[j].ID + }) + return kills, nil +} + +func oomAdjustment(kills []OOMKill, baseline, currentBase, coefficient float64) (OOMAdjustment, error) { + adjustment := OOMAdjustment{BaselineRequestBytes: baseline} + for _, kill := range kills { + base := kill.MemoryLimitBytes + if base == 0 { + base = currentBase + if base > 0 { + adjustment.UsedCurrentFallback = true + } else { + base = baseline + adjustment.UsedUsageFallback = true + } + } + floor := base * coefficient + if !finite(floor) { + return OOMAdjustment{}, fmt.Errorf("OOM suggested value overflow") + } + // Repeated kills do not compound the increase. The strongest event wins. + adjustment.OOMRequestFloorBytes = math.Max(adjustment.OOMRequestFloorBytes, floor) + } + return adjustment, nil +} diff --git a/analysis/oom_test.go b/analysis/oom_test.go new file mode 100644 index 0000000..4a7632e --- /dev/null +++ b/analysis/oom_test.go @@ -0,0 +1,403 @@ +// Copyright Kedify Inc. +// SPDX-License-Identifier: LicenseRef-Kedify-Commercial-1.0 AND LicenseRef-Kedify-Public-Source-1.0 +// See LICENSE and PUBLIC_SOURCE_LICENSE. + +package analysis + +import ( + "encoding/json" + "math" + "reflect" + "strings" + "testing" +) + +const mib = 1024 * 1024 + +func oomKill(id string, timestamp int64, limit float64) OOMKill { + return OOMKill{ID: id, PodUID: "pod-1", WorkloadUID: "uid", Release: "A", Timestamp: timestamp, MemoryLimitBytes: limit} +} + +func TestOOMUsesFailedLimitAndReportsEvidence(t *testing.T) { + in := fixture(600, 60) + p := shortPolicy() + p.Memory.LimitsToRequestsRatio = 1 // KRR's equal memory request and limit. + baseline := run(t, in, p) + // Historical event time, outside the current-signal freshness interval. + kill := oomKill("pod-1/oom-1", epoch+60000, 512*mib) + in.Containers[0].OOMKills = []OOMKill{kill} + out := run(t, in, p) + r := out.Results[0] + if len(r.Recommendations) != 2 { + t.Fatalf("expected memory request and limit: %+v", r) + } + for _, rec := range r.Recommendations { + if rec.SuggestedValue != 768*mib || rec.Confidence != 95 { + t.Fatalf("OOM sizing must use failed limit plus 50%%: %+v", rec) + } + } + if r.DataQuality.Status != DataQualityAvailable || !reflect.DeepEqual(r.Notices, []Reason{ReasonOOMKillDetected}) { + t.Fatalf("OOM notice must not degrade otherwise complete evidence: %+v", r) + } + if !reflect.DeepEqual(r.Evidence.OOMKills, []OOMKill{kill}) || r.Evidence.AggregatedUsage != baseline.Results[0].Evidence.AggregatedUsage { + t.Fatalf("OOM evidence replaced or contaminated usage evidence: %+v", r.Evidence) + } + wantAdjustment := &OOMAdjustment{BaselineRequestBytes: 24 * mib, OOMRequestFloorBytes: 768 * mib} + if !reflect.DeepEqual(r.OOMAdjustment, wantAdjustment) { + t.Fatalf("wrong sizing explanation: %+v", r.OOMAdjustment) + } + if !reflect.DeepEqual(out.Results[1], baseline.Results[1]) { + t.Fatal("OOM changed CPU analysis") + } + encoded, err := json.Marshal(out) + if err != nil { + t.Fatal(err) + } + for _, field := range []string{`"oom-kill-detected"`, `"oomKills"`, `"memoryLimitBytes":536870912`, `"oomRequestFloorBytes":805306368`} { + if !strings.Contains(string(encoded), field) { + t.Fatalf("output omits OOM evidence %s: %s", field, encoded) + } + } + inputJSON, err := json.Marshal(in) + if err != nil { + t.Fatal(err) + } + var decoded Input + if err = json.Unmarshal(inputJSON, &decoded); err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(run(t, decoded, p), out) { + t.Fatal("JSON adapters do not produce equivalent OOM analysis") + } +} + +func TestOOMKilledCoefficientDoesNotCompoundOrReduceUsageSizing(t *testing.T) { + for _, peak := range []float64{20 * mib, 800 * mib} { + in := fixture(600, 60) + for i := range in.Containers[0].Memory.Series[0].Samples { + in.Containers[0].Memory.Series[0].Samples[i].Value = peak + } + in.Containers[0].OOMKills = []OOMKill{ + oomKill("oom-1", epoch+60000, 256*mib), + oomKill("oom-2", epoch+120000, 512*mib), + oomKill("oom-3", epoch+180000, 128*mib), + } + p := shortPolicy() + p.Memory.OOMKilledCoefficient = 1.5 + p.Memory.LimitsToRequestsRatio = 2 + r := run(t, in, p).Results[0] + wantRequest := math.Max(peak*1.2, 768*mib) + if len(r.Recommendations) != 2 || r.Recommendations[0].SuggestedValue != wantRequest || r.Recommendations[1].SuggestedValue != wantRequest*2 { + t.Fatalf("incorrect combination of usage, OOMs, or limit ratio: %+v", r) + } + if r.OOMAdjustment.OOMRequestFloorBytes != 768*mib { + t.Fatalf("OOM adjustment compounded across events: %+v", r.OOMAdjustment) + } + } +} + +func TestOOMTimestampOnlyFallback(t *testing.T) { + tests := []struct { + name string + request float64 + limit float64 + wantSettings []Setting + floor float64 + }{ + {"growth", 20 * mib, 24 * mib, []Setting{SettingRequests, SettingLimits}, 36 * mib}, + {"current limit fallback", 256 * mib, 512 * mib, []Setting{SettingRequests, SettingLimits}, 768 * mib}, + {"no new finite limit", 20 * mib, 0, []Setting{SettingRequests}, 30 * mib}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + in := fixture(600, 60) + c := &in.Containers[0] + c.Memory.CurrentRequest.Value = tt.request + c.Memory.CurrentLimit.Value = tt.limit + c.OOMKills = []OOMKill{oomKill("oom-1", epoch+60000, 0), oomKill("oom-2", epoch+120000, 0)} + r := run(t, in, shortPolicy()).Results[0] + if r.OOMAdjustment == nil || !r.OOMAdjustment.UsedCurrentFallback || r.OOMAdjustment.UsedUsageFallback || r.OOMAdjustment.OOMRequestFloorBytes != tt.floor { + t.Fatalf("timestamp-only kills must apply one 50%% buffer to current settings: %+v", r) + } + if r.DataQuality.Status != DataQualityPartial || !has(r.DataQuality, ReasonOOMLimitUnknown) { + t.Fatalf("unknown failed limit was hidden: %+v", r) + } + if len(r.Recommendations) != len(tt.wantSettings) { + t.Fatalf("unexpected actions: %+v", r) + } + for i, rec := range r.Recommendations { + want := tt.floor + if rec.Setting == SettingLimits { + want *= 3 + } + if rec.Setting != tt.wantSettings[i] || rec.SuggestedValue != want { + t.Fatalf("incorrect fallback recommendation: %+v", rec) + } + } + if len(tt.wantSettings) == 0 && r.NoActionReason != ReasonOOMLimitUnknown { + t.Fatalf("missing reason for suppressed downsize: %+v", r) + } + }) + } + + in := fixture(600, 60) + in.Containers[0].OOMKills = []OOMKill{oomKill("known", epoch+60000, 512*mib), oomKill("unknown", epoch+120000, 0)} + r := run(t, in, shortPolicy()).Results[0] + if len(r.Recommendations) != 2 || r.Recommendations[0].SuggestedValue != 768*mib || !r.OOMAdjustment.UsedCurrentFallback { + t.Fatalf("known event must still justify growth alongside incomplete evidence: %+v", r) + } +} + +func TestOOMFiltersIdentityWindowAndReleaseSegment(t *testing.T) { + for _, mode := range []string{"other UID", "other release", "before window", "future", "before release", "before inferred rollback"} { + t.Run(mode, func(t *testing.T) { + in := fixture(1200, 60) + c := &in.Containers[0] + kill := oomKill("oom", epoch+300000, 512*mib) + switch mode { + case "other UID": + kill.WorkloadUID = "deleted-workload" + case "other release": + kill.Release = "old" + case "before window": + kill.Timestamp = in.WindowStart - 1 + case "future": + kill.Timestamp = in.EvaluationTime + 1 + case "before release": + c.Identity.ReleaseStartedAt = epoch + 600000 + case "before inferred rollback": + c.Identity.ReleaseStartedAt = 0 + c.Memory.Series = append(c.Memory.Series, Series{ID: "release-B", WorkloadUID: "uid", Release: "B", Kind: SampleGauge, Samples: []Sample{{Timestamp: epoch + 599999, Value: 1}}}) + } + baseline := run(t, in, shortPolicy()) + c.OOMKills = []OOMKill{kill} + if got := run(t, in, shortPolicy()); !reflect.DeepEqual(got, baseline) { + t.Fatalf("irrelevant OOM altered analysis: %+v", got.Results[0]) + } + }) + } + for _, ts := range []int64{epoch, epoch + 600000} { + in := fixture(600, 60) + in.Containers[0].OOMKills = []OOMKill{oomKill("boundary", ts, 512*mib)} + if r := run(t, in, shortPolicy()).Results[0]; len(r.Evidence.OOMKills) != 1 { + t.Fatalf("OOM at inclusive boundary was lost: %+v", r) + } + } +} + +func TestOOMDoesNotBypassIdentityAndCurrentSettingGuards(t *testing.T) { + tests := []struct { + name string + mutate func(*Input, *Policy) + }{ + {"stale identity", func(in *Input, _ *Policy) { in.Containers[0].Identity.Timestamp = epoch }}, + {"ambiguous identity", func(in *Input, _ *Policy) { in.Containers[0].Identity.Ambiguous = true }}, + {"missing request", func(in *Input, _ *Policy) { in.Containers[0].Memory.CurrentRequest = Signal{} }}, + {"stale request", func(in *Input, _ *Policy) { in.Containers[0].Memory.CurrentRequest.Timestamp = epoch }}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + in := fixture(600, 60) + in.Containers[0].OOMKills = []OOMKill{oomKill("oom", epoch+60000, 512*mib)} + p := shortPolicy() + tt.mutate(&in, &p) + r := run(t, in, p).Results[0] + if len(r.Recommendations) != 0 || r.NoActionReason == "" || r.OOMAdjustment != nil { + t.Fatalf("OOM bypassed evidence guard: %+v", r) + } + if len(r.Evidence.OOMKills) != 1 || !reflect.DeepEqual(r.Notices, []Reason{ReasonOOMKillDetected}) { + t.Fatalf("blocked analysis must still report OOM evidence: %+v", r) + } + }) + } +} + +func TestOOMRespectsBoundsInventoryAndRequestOnlyPolicy(t *testing.T) { + in := fixture(600, 60) + c := &in.Containers[0] + c.OOMKills = []OOMKill{oomKill("oom", epoch+60000, 512*mib)} + c.Inventory.Available = false + p := shortPolicy() + p.Memory.Bounds.Maximum = 600 * mib + r := run(t, in, p).Results[0] + if len(r.Recommendations) != 2 || !has(r.DataQuality, ReasonBounds) || !has(r.DataQuality, ReasonUnknownInventory) { + t.Fatalf("bounded growth with partial inventory failed: %+v", r) + } + for _, rec := range r.Recommendations { + if rec.SuggestedValue != 600*mib { + t.Fatalf("OOM escaped resource maximum: %+v", rec) + } + } + p.Memory.RequestsOnly = true + r = run(t, in, p).Results[0] + if len(r.Recommendations) != 0 || r.NoActionReason != ReasonBounds { + t.Fatalf("request-only advice exceeded retained limit: %+v", r) + } + c.Memory.CurrentLimit.Value = 1024 * mib + r = run(t, in, p).Results[0] + if len(r.Recommendations) != 1 || r.Recommendations[0].Setting != SettingRequests { + t.Fatalf("request-only policy changed limit: %+v", r) + } + + in = fixture(600, 60) + c = &in.Containers[0] + c.OOMKills = []OOMKill{oomKill("oom", epoch+60000, 512*mib)} + c.Memory.CurrentRequest.Value = 630 * mib + c.Memory.CurrentLimit.Value = 1900 * mib + p = shortPolicy() + p.Memory.OOMKilledCoefficient = 1.25 + r = run(t, in, p).Results[0] + if len(r.Recommendations) != 0 || r.NoActionReason != ReasonNoMaterialChange || len(r.Notices) != 1 { + t.Fatalf("OOM bypassed material-change thresholds or lost its notice: %+v", r) + } +} + +func TestOOMDeterminismDeduplicationAndValidation(t *testing.T) { + in := fixture(600, 60) + in.Containers[0].OOMKills = []OOMKill{oomKill("b", epoch+120000, 512*mib), oomKill("a", epoch+60000, 256*mib), oomKill("c", epoch+120000, 128*mib)} + before, _ := json.Marshal(in) + want := run(t, in, shortPolicy()) + after, _ := json.Marshal(in) + if string(before) != string(after) { + t.Fatal("OOM input was mutated") + } + kills := in.Containers[0].OOMKills + in.Containers[0].OOMKills = []OOMKill{kills[2], kills[1], kills[0], kills[0]} + if got := run(t, in, shortPolicy()); !reflect.DeepEqual(got, want) { + t.Fatal("event ordering or repeated observations changed output") + } + for _, mode := range []string{"missing ID", "missing UID", "missing release", "zero timestamp", "negative timestamp", "negative limit", "NaN limit", "infinite limit", "conflict", "overflow"} { + t.Run(mode, func(t *testing.T) { + in := fixture(600, 60) + kill := oomKill("oom", epoch+60000, 512*mib) + switch mode { + case "missing ID": + kill.ID = "" + case "missing UID": + kill.WorkloadUID = "" + case "missing release": + kill.Release = "" + case "zero timestamp": + kill.Timestamp = 0 + case "negative timestamp": + kill.Timestamp = -1 + case "negative limit": + kill.MemoryLimitBytes = -1 + case "NaN limit": + kill.MemoryLimitBytes = math.NaN() + case "infinite limit": + kill.MemoryLimitBytes = math.Inf(1) + case "overflow": + kill.MemoryLimitBytes = math.MaxFloat64 + } + in.Containers[0].OOMKills = []OOMKill{kill} + if mode == "conflict" { + kill.MemoryLimitBytes++ + in.Containers[0].OOMKills = append(in.Containers[0].OOMKills, kill) + } + if _, err := Analyze(in, shortPolicy()); err == nil || !strings.Contains(err.Error(), "OOM") { + t.Fatalf("invalid OOM input was accepted: %v", err) + } + }) + } +} + +func TestOOMPolicyNormalizationAndIdentity(t *testing.T) { + for _, coefficient := range []float64{0, 1, 1.25, 2} { + p, err := NormalizePolicy(Policy{Memory: MemoryPolicy{OOMKilledCoefficient: coefficient}}) + if err != nil { + t.Fatal(err) + } + want := coefficient + if coefficient == 0 { + want = 1.5 + } + if p.Memory.OOMKilledCoefficient != want { + t.Fatalf("normalized OOM coefficient = %v, want %v", p.Memory.OOMKilledCoefficient, want) + } + } + for _, coefficient := range []float64{-1, .99, math.NaN(), math.Inf(1)} { + if _, err := NormalizePolicy(Policy{Memory: MemoryPolicy{OOMKilledCoefficient: coefficient}}); err == nil { + t.Fatalf("invalid OOM coefficient accepted: %v", coefficient) + } + } + defaultVersion, err := PolicyVersion(Policy{}) + if err != nil { + t.Fatal(err) + } + customVersion, err := PolicyVersion(Policy{Memory: MemoryPolicy{OOMKilledCoefficient: 1.25}}) + if err != nil || customVersion == defaultVersion { + t.Fatalf("OOM policy not included in policy identity: %s, %v", customVersion, err) + } +} + +func TestOOMBypassesUsageRequirements(t *testing.T) { + for _, tc := range []struct { + name string + mutate func(*Input, *Policy) + }{ + {"no metrics or live pods", func(in *Input, _ *Policy) { + in.Containers[0].Memory.Series = nil + in.Containers[0].CPU.Series = nil + in.Containers[0].Inventory.Eligible = 0 + in.Containers[0].Inventory.Observed = 0 + }}, + {"one observation", func(in *Input, _ *Policy) { + in.Containers[0].Memory.Series[0].Samples = in.Containers[0].Memory.Series[0].Samples[:1] + }}, + {"insufficient history", func(_ *Input, p *Policy) { p.Evidence.MinimumHistorySeconds = 3600 }}, + {"insufficient observations", func(_ *Input, p *Policy) { p.Evidence.MinimumSamples = 1000 }}, + {"stale usage", func(in *Input, _ *Policy) { + in.Containers[0].Memory.Series[0].Samples = in.Containers[0].Memory.Series[0].Samples[:2] + }}, + {"sparse coverage", func(in *Input, _ *Policy) { + s := &in.Containers[0].Memory.Series[0] + s.Samples = append(s.Samples[:2], s.Samples[len(s.Samples)-2:]...) + }}, + {"infer rollout from OOM", func(in *Input, _ *Policy) { + in.Containers[0].Identity.ReleaseStartedAt = 0 + in.Containers[0].CPU.Series = nil + in.Containers[0].Memory.Series = nil + }}, + } { + t.Run(tc.name, func(t *testing.T) { + for _, failedLimit := range []float64{0, 50 * mib} { + in, p := fixture(600, 60), shortPolicy() + p.Memory.LimitsToRequestsRatio = 1 + c := &in.Containers[0] + c.Memory.CurrentRequest.Value, c.Memory.CurrentLimit.Value = 50*mib, 50*mib + tc.mutate(&in, &p) + c.OOMKills = []OOMKill{oomKill("oom", epoch+60000, failedLimit)} + r := run(t, in, p).Results[0] + if r.DataQuality.Status != DataQualityPartial || len(r.Recommendations) != 2 || r.OOMAdjustment == nil { + t.Fatalf("OOM growth blocked by usage quality: %+v", r) + } + for _, rec := range r.Recommendations { + if rec.SuggestedValue != 75*mib || rec.Confidence != 0 { + t.Fatalf("expected 50Mi × 1.5 without invented usage confidence: %+v", rec) + } + } + if r.DecisionTrace.Source.Method != "OOM kill" { + t.Fatal("missing OOM decision source") + } + } + }) + } +} + +func TestOOMWithoutUsageCannotDownsizeOrInventBaseline(t *testing.T) { + in, p := fixture(600, 60), shortPolicy() + c := &in.Containers[0] + c.Memory.Series = nil + c.OOMKills = []OOMKill{oomKill("oom", epoch+60000, 50*mib)} + c.Memory.CurrentRequest.Value, c.Memory.CurrentLimit.Value = 512*mib, 2048*mib + if r := run(t, in, p).Results[0]; len(r.Recommendations) != 0 { + t.Fatalf("old low limit justified a reduction without usage: %+v", r) + } + c.OOMKills[0].MemoryLimitBytes = 0 + c.Memory.CurrentRequest = Signal{Available: true, Unset: true, Timestamp: in.EvaluationTime} + c.Memory.CurrentLimit = c.Memory.CurrentRequest + if r := run(t, in, p).Results[0]; r.DataQuality.Status != DataQualityUnavailable || len(r.Recommendations) != 0 { + t.Fatalf("invented a baseline without usage or allocations: %+v", r) + } +} diff --git a/analysis/testdata/default-output.json b/analysis/testdata/default-output.json index 38f5527..bb54cba 100644 --- a/analysis/testdata/default-output.json +++ b/analysis/testdata/default-output.json @@ -1,7 +1,7 @@ { "schemaVersion": "resource-analysis-output/v3", - "detectorVersion": "7", - "policyVersion": "v2:5509b23e9c4314e2c5455ab47a985a8ba022812cb6904b593407df37908ae377", + "detectorVersion": "8", + "policyVersion": "v2:05fcc132cfc77a9510dabdd15412df2f86e5ee8b8c8c703ea3f3738423463a51", "effectivePolicy": { "cpu": { "strategy": "percentile", @@ -19,6 +19,7 @@ "memory": { "strategy": "max", "headroomCoefficient": 1.2, + "oomKilledCoefficient": 1.5, "limitsToRequestsRatio": 3, "bounds": { "minimum": 10485760, diff --git a/analysis/types.go b/analysis/types.go index 41a8d56..ef57ebf 100644 --- a/analysis/types.go +++ b/analysis/types.go @@ -8,7 +8,7 @@ package analysis const ( InputSchemaVersion = "resource-analysis-input/v2" OutputSchemaVersion = "resource-analysis-output/v3" - ResourceRightSizeDetectorVersion = "7" + ResourceRightSizeDetectorVersion = "8" MaxPreviousReleases = 3 ) @@ -34,14 +34,15 @@ type ContainerObservation struct { Inventory Inventory `json:"inventory"` CPU ResourceObservation `json:"cpu"` Memory ResourceObservation `json:"memory"` + OOMKills []OOMKill `json:"oomKills,omitempty"` // Newest first. Only the first three entries are eligible as fallback usage. PreviousReleases []PreviousRelease `json:"previousReleases,omitempty"` } // PreviousRelease supplies raw usage from an earlier rollout of this workload. // EvaluationTime is the last historical observation, not today's timestamp. -// ReleaseStartedAt may be zero to infer the observed segment. Current settings -// and inventory always refer to the current rollout. +// ReleaseStartedAt may be zero to infer the observed segment. Current settings, +// inventory and OOM events always refer to the current rollout. type PreviousRelease struct { Release string `json:"release"` ReleaseStartedAt int64 `json:"releaseStartedAt,omitempty"` @@ -50,6 +51,23 @@ type PreviousRelease struct { Memory []Series `json:"memory"` } +// OOMKill is a positive out-of-memory termination observation for this container. +// ID identifies the event, independently of its source (for example, pod UID plus +// termination time). Repeated observations of the same event must retain its ID +// and original Unix-millisecond Timestamp, not a scrape or query evaluation time. +// WorkloadUID and Release describe the container at termination. PodUID is optional. +// MemoryLimitBytes is the positive memory limit in effect at termination, when +// known; zero means no known finite limit. Do not substitute a later/current limit. +// Omitted OOMKills means no supplied evidence, not proof that no OOMs occurred. +type OOMKill struct { + ID string `json:"id"` + PodUID string `json:"podUID,omitempty"` + WorkloadUID string `json:"workloadUID"` + Release string `json:"release"` + Timestamp int64 `json:"timestamp"` + MemoryLimitBytes float64 `json:"memoryLimitBytes,omitempty"` +} + // ReleaseStartedAt is an activation boundary or a conservative observed segment // start, including rollbacks. Zero asks the engine to infer the observed segment. // Ambiguous is required when current sources disagree or mixed replicas cannot be isolated. @@ -152,6 +170,7 @@ type CPUPolicy struct { type MemoryPolicy struct { Strategy MemoryStrategy `json:"strategy"` HeadroomCoefficient float64 `json:"headroomCoefficient"` + OOMKilledCoefficient float64 `json:"oomKilledCoefficient"` LimitsToRequestsRatio float64 `json:"limitsToRequestsRatio"` Bounds Bounds `json:"bounds"` RequestsOnly bool `json:"requestsOnly"` @@ -179,6 +198,7 @@ type ResourceAnalysis struct { DataQuality DataQuality `json:"dataQuality"` NoActionReason Reason `json:"noActionReason,omitempty"` Notices []Reason `json:"notices,omitempty"` + OOMAdjustment *OOMAdjustment `json:"oomAdjustment,omitempty"` RolloutFallback *RolloutFallback `json:"rolloutFallback,omitempty"` } @@ -193,12 +213,26 @@ type RolloutFallback struct { CurrentDataQuality DataQuality `json:"currentDataQuality"` } +// OOMAdjustment records memory sizing before bounds and material-change guards. +// The request candidate is max(BaselineRequestBytes, OOMRequestFloorBytes). +// Events without a known failed limit use the current finite memory limit, +// then the current request, then qualifying usage. The fallback flags identify +// which sources were used without presenting current settings as historical facts. +// Presence does not promise an emitted recommendation: the remaining guards apply. +type OOMAdjustment struct { + BaselineRequestBytes float64 `json:"baselineRequestBytes"` + OOMRequestFloorBytes float64 `json:"oomRequestFloorBytes"` + UsedUsageFallback bool `json:"usedUsageFallback"` + UsedCurrentFallback bool `json:"usedCurrentFallback,omitempty"` +} + type ResourceEvidence struct { AggregatedUsage Signal `json:"aggregatedUsage"` CurrentRequest Signal `json:"currentRequest"` CurrentLimit Signal `json:"currentLimit"` Identity CurrentIdentity `json:"identity"` Inventory Inventory `json:"inventory"` + OOMKills []OOMKill `json:"oomKills,omitempty"` } type Setting string @@ -248,6 +282,8 @@ const ( ReasonNoMaterialChange Reason = "no-material-change" ReasonBounds Reason = "bound-applied" ReasonLimitDisabled Reason = "limit-changes-disabled" + ReasonOOMKillDetected Reason = "oom-kill-detected" + ReasonOOMLimitUnknown Reason = "unknown-oom-memory-limit" ) type DataQuality struct { diff --git a/analysis/unset_test.go b/analysis/unset_test.go index b05d2a6..a36fe2c 100644 --- a/analysis/unset_test.go +++ b/analysis/unset_test.go @@ -131,6 +131,19 @@ func TestUnsetSettingsPreserveGuards(t *testing.T) { } } +func TestUnsetMemoryLimitPreservesOOMGuard(t *testing.T) { + in, p, index, _ := unsetFixture(ResourceMemory) + in.Containers[0].OOMKills = []OOMKill{oomKill("unknown-limit", epoch+60000, 0)} + r := run(t, in, p).Results[index] + if len(r.Recommendations) != 1 || r.Recommendations[0].Setting != SettingRequests { + t.Fatalf("unsafe unset memory limit initialized after OOM: %+v", r) + } + trace := r.DecisionTrace.Settings[1] + if trace.Disposition != "retained" || len(trace.Reasons) != 1 || trace.Reasons[0] != ReasonOOMLimitUnknown { + t.Fatalf("missing OOM guard trace: %+v", trace) + } +} + func TestUnsetSignalRejectsNumericValue(t *testing.T) { for _, limit := range []bool{false, true} { in, p, _, _ := unsetFixture(ResourceCPU) From 0acdad55dfcc06fbf940021cdc066cf16b6da899 Mon Sep 17 00:00:00 2001 From: Jirka Kremser Date: Thu, 24 Sep 2026 18:37:59 +0200 Subject: [PATCH 3/4] Add opt-in memory leak detection Detect sustained growth in bucketed memory baselines using bounded robust trend analysis, with warmup, coverage, freshness, and recent-growth guards. Keep replicas and container lifetimes separate and correlate OOM events with individual episodes. Expose normalized policy and advisory findings without changing resource recommendations. Add regression coverage for growth, plateaus, recovery, insufficient evidence, identity boundaries, and deterministic output. Document configuration, evidence, and interpretation limits. --- README.md | 96 ++++++- analysis/analyze.go | 13 + analysis/memory_leak.go | 320 +++++++++++++++++++++++ analysis/memory_leak_test.go | 469 ++++++++++++++++++++++++++++++++++ analysis/memory_leak_types.go | 85 ++++++ analysis/types.go | 27 +- 6 files changed, 997 insertions(+), 13 deletions(-) create mode 100644 analysis/memory_leak.go create mode 100644 analysis/memory_leak_test.go create mode 100644 analysis/memory_leak_types.go diff --git a/README.md b/README.md index 6b63af9..b72f4fe 100644 --- a/README.md +++ b/README.md @@ -79,7 +79,7 @@ Historical evidence is evaluated at that time, capped at the current rollout's start, and retains its original timestamps. `rolloutFallback` in each resource result identifies the historical source and preserves the current rollout's failed data-quality checks. The result target, identity, allocation signals, -inventory checks and OOM events remain current. Historical pod +inventory checks, OOM events, and leak detection remain current. Historical pod counts cannot authorize downsizing currently unobserved pods. Existing callers that omit `previousReleases` keep current-rollout-only behavior. @@ -212,6 +212,100 @@ findings from older detectors/policies. may populate the same structure directly. Do not infer an OOM solely from a restart count or exit code. Source collection and conversion belong to callers. +## Optional potential memory-leak detection + +Enable the advisory detector when calling the same `Analyze` entry point: + +```go +policy := analysis.DefaultPolicy() +policy.Memory.LeakDetection = &analysis.MemoryLeakPolicy{} // Enable defaults. +output, err := analysis.Analyze(snapshot, policy) +``` + +`nil` (the default) disables detection and omits its policy/output fields. In JSON, +use `"memory": {"leakDetection": {}}` within the policy to enable defaults; omit +`leakDetection` or use `null` to disable it. Nonzero policy fields override defaults. +The detector consumes the existing byte-valued memory gauges and normalized OOM +events; it has no metric names, runtime dependencies, or source-specific adapters. +Keep the memory measurement semantics consistent within a series. + +The algorithm looks for a persistently rising **lower baseline**, rather than +ever-higher peaks that may be reclaimed by GC: + +1. Select the current workload UID/release segment and the latest 24 hours. Analyze + each replica/container lifetime separately; do not stitch releases or replicas + together. Skip the first 30 minutes of each observed lifetime as warmup. A known + OOM termination also splits an episode when the adapter has reused a series ID. + Adapters must still give all container lifetimes distinct IDs: non-OOM restarts + cannot reliably be inferred from a drop in memory usage. +2. Divide each episode into complete 30-minute buckets. Sort the memory values in + each bucket and take P10: the value at the one-based rank `ceil(0.10 * count)`. + This estimates the lower baseline and reduces the effect of short-lived peaks; + it is not a measurement of live heap or a detected GC event. +3. Require at least six hours after warmup and at least 12 usable complete buckets, + each with five distinct source samples. Skip empty or undersampled buckets; + they need not be adjacent and are never replaced with zero-valued baselines. + Apply `evidence.minimumCoverage` to the **whole episode** (default 90%), + including empty buckets in its denominator. The usable buckets must also + supply the configured minimum history. Apply `freshnessSeconds` (default five + minutes) to the episode's final sample. One replica cannot supply another's + missing coverage. A matching pod's OOM may explain why an episode ended before + evaluation time if its final sample was fresh at termination. An OOM with + unknown pod identity cannot establish that link. +4. Fit a robust [Theil–Sen trend](https://docs.scipy.org/doc/scipy/reference/generated/scipy.stats.theilslopes.html): + the median slope between every pair of usable bucket baselines, using their + actual elapsed time so missing buckets do not compress time, implemented locally + without dependencies. Require a positive slope and at least 80% of earlier/later + bucket pairs to increase. The median baseline of the last quarter of buckets + must exceed that of the first quarter by both 64 MiB and 20%. When the starting + baseline is zero, only the absolute growth threshold applies. +5. Require the trend to continue in the latest quarter (at least six buckets): + the same 80% consistency threshold and a slope at least 25% of the whole-episode + slope. This suppresses startup steps, caches that have plateaued, and sustained + recovery. Correlated OOMs support the finding but never establish a leak alone. + +Tune the exposed lookback, bucket duration, warmup, minimum history/samples, +absolute/relative growth, and trend consistency through `MemoryLeakPolicy`. +Durations use seconds; zero fields select defaults. Policies must retain at least +12 buckets of minimum history and at most 256 buckets in the lookback, bounding the +pairwise computation independently of the number of raw samples. P10 and the recent +trend check are fixed parts of detector version `2`. + +Each memory result gains `memoryLeak`, including: + +- `status`: `potential-leak` if any episode passes the heuristic; + `no-leak-pattern` if all considered episodes were evaluable and none passes; + `insufficient-data` when none passes and evidence is incomplete or unavailable. +- The detector version, selected window, evaluated/suspected/skipped episode counts, + and explicit reasons. A positive finding may coexist with skipped episodes. +- Per-episode series/pod identity, observed interval, sample/bucket counts, coverage, + starting/ending baselines, growth in bytes and as a fraction, overall/recent slopes + in bytes/hour, and trend consistency. Consistency is a measured fraction of + increasing pairs, **not a probability of a leak**. `relativeGrowth` is omitted + when the starting baseline is zero. +- `oomKillIDs` on corroborated episodes, referencing the existing `evidence.oomKills`. + A positive finding also adds `potential-memory-leak` to the resource's `notices`. + +This diagnostic does not modify request/limit recommendations, sizing confidence, +or existing OOM adjustments. Its default six-hour history requirement is +independent of the default one-hour sizing guard. A caller can configure a longer +sizing minimum, allowing leak detection to report a pattern while sizing remains +blocked. Missing resource settings or incomplete inventory do not prevent this +diagnostic; ambiguous/stale workload identity and insufficient usage evidence do. +The normalized policy hash includes enabled detector settings. Enabling the leak +heuristic does not alter resource sizing; it has its own +`memoryLeak.detectorVersion` for cache invalidation. + +Treat this as a **potential leak, not a diagnosis**. Growing useful allocations, +unbounded caches, increasing traffic, and allocator retention can look identical +in container memory metrics. Conversely, slow leaks, short OOM loops, long GC cycles, +or growth that started only recently may not pass these conservative thresholds. +No-pattern means no qualifying pattern in the supplied window, not proof of safety +or complete replica/OOM collection. Confirmation needs application/load context and +runtime diagnostics such as [heap profiles](https://go.dev/doc/diagnostics). +Comparing leak incidence across historical releases is left to callers; one result +only evaluates its selected release segment. + ## Consumers - [`dashboard-api-service`](https://github.com/kedify/dashboard-api-service) diff --git a/analysis/analyze.go b/analysis/analyze.go index cdb1c08..1b75bf7 100644 --- a/analysis/analyze.go +++ b/analysis/analyze.go @@ -101,6 +101,13 @@ func NormalizePolicy(p Policy) (Policy, error) { if e.MinimumHistorySeconds < 1 || e.MinimumSamples < 2 || !positive(e.MinimumCoverage) || e.MinimumCoverage > 1 || e.FreshnessSeconds < 1 { return Policy{}, fmt.Errorf("invalid evidence policy") } + if p.Memory.LeakDetection != nil { + leak, err := normalizeMemoryLeakPolicy(*p.Memory.LeakDetection) + if err != nil { + return Policy{}, err + } + p.Memory.LeakDetection = &leak + } return p, nil } func PolicyVersion(p Policy) (string, error) { @@ -150,6 +157,12 @@ func Analyze(in Input, p Policy) (Output, error) { if err != nil { return Output{}, fmt.Errorf("%s %s: %w", c.Target.Name, r, err) } + if r == ResourceMemory && p.Memory.LeakDetection != nil { + result.MemoryLeak = analyzeMemoryLeak(in, c, p, result) + if result.MemoryLeak.Status == MemoryLeakPotential { + result.Notices = append(result.Notices, ReasonPotentialMemoryLeak) + } + } out.Results = append(out.Results, result) } } diff --git a/analysis/memory_leak.go b/analysis/memory_leak.go new file mode 100644 index 0000000..8d287d2 --- /dev/null +++ b/analysis/memory_leak.go @@ -0,0 +1,320 @@ +// Copyright Kedify Inc. +// SPDX-License-Identifier: LicenseRef-Kedify-Commercial-1.0 AND LicenseRef-Kedify-Public-Source-1.0 +// See LICENSE and PUBLIC_SOURCE_LICENSE. + +package analysis + +import ( + "fmt" + "math" + "sort" +) + +const maxMemoryLeakBuckets = 256 + +func normalizeMemoryLeakPolicy(p MemoryLeakPolicy) (MemoryLeakPolicy, error) { + for _, field := range []struct { + value *int64 + fallback int64 + }{ + {&p.LookbackSeconds, 24 * 3600}, + {&p.BucketDurationSeconds, 30 * 60}, + {&p.MinimumHistorySeconds, 6 * 3600}, + {&p.WarmupSeconds, 30 * 60}, + } { + if *field.value == 0 { + *field.value = field.fallback + } + if *field.value < 1 || *field.value > math.MaxInt64/1000 { + return MemoryLeakPolicy{}, fmt.Errorf("memory.leakDetection durations must be positive and fit Unix milliseconds") + } + } + if p.MinimumSamplesPerBucket == 0 { + p.MinimumSamplesPerBucket = 5 + } + if p.MinimumGrowthBytes == 0 { + p.MinimumGrowthBytes = 64 * 1024 * 1024 + } + if p.MinimumRelativeGrowth == 0 { + p.MinimumRelativeGrowth = .2 + } + if p.MinimumTrendConsistency == 0 { + p.MinimumTrendConsistency = .8 + } + if p.MinimumSamplesPerBucket < 2 || !positive(p.MinimumGrowthBytes) || !positive(p.MinimumRelativeGrowth) || !finite(p.MinimumTrendConsistency) || p.MinimumTrendConsistency <= .5 || p.MinimumTrendConsistency > 1 { + return MemoryLeakPolicy{}, fmt.Errorf("invalid memory.leakDetection samples, growth thresholds or trend consistency (must be in (0.5,1])") + } + if p.MinimumHistorySeconds > p.LookbackSeconds || p.WarmupSeconds >= p.LookbackSeconds || p.MinimumHistorySeconds/p.BucketDurationSeconds < 12 { + return MemoryLeakPolicy{}, fmt.Errorf("memory.leakDetection requires at least 12 buckets of history within the lookback and a shorter warmup") + } + // Bound quadratic pairwise regression independently of raw sample count. + if p.LookbackSeconds/p.BucketDurationSeconds > maxMemoryLeakBuckets || p.LookbackSeconds/p.BucketDurationSeconds == maxMemoryLeakBuckets && p.LookbackSeconds%p.BucketDurationSeconds != 0 { + return MemoryLeakPolicy{}, fmt.Errorf("memory.leakDetection lookback must span at most %d buckets", maxMemoryLeakBuckets) + } + return p, nil +} + +func analyzeMemoryLeak(in Input, c ContainerObservation, p Policy, resource ResourceAnalysis) *MemoryLeakAnalysis { + lp := *p.Memory.LeakDetection + start := max(in.WindowStart, c.Identity.ReleaseStartedAt, in.EvaluationTime-lp.LookbackSeconds*1000) + out := &MemoryLeakAnalysis{ + DetectorVersion: MemoryLeakDetectorVersion, Status: MemoryLeakInsufficientData, + WindowStart: start, WindowEnd: in.EvaluationTime, Reasons: []Reason{}, Episodes: []MemoryLeakEvidence{}, + } + for _, reason := range resource.DataQuality.Reasons { + switch reason { + case ReasonMissingIdentity, ReasonAmbiguousIdentity, ReasonStaleIdentity, ReasonUnknownReleaseStart, ReasonUnknownSeriesIdentity: + out.Reasons = append(out.Reasons, reason) + } + } + if len(out.Reasons) > 0 { + return out + } + series := append([]Series(nil), c.Memory.Series...) + sort.Slice(series, func(i, j int) bool { return series[i].ID < series[j].ID }) + for _, s := range series { + if s.WorkloadUID != c.Target.WorkloadUID || s.Release != c.Target.Release { + continue + } + // analyzeResource already validated selected series and conflicting samples. + // Keep the first observed time before the leak lookback for warmup handling. + firstObserved := int64(0) + var samples []Sample + for _, sample := range s.Samples { + if sample.Timestamp < max(in.WindowStart, c.Identity.ReleaseStartedAt) || sample.Timestamp > in.EvaluationTime { + continue + } + if firstObserved == 0 || sample.Timestamp < firstObserved { + firstObserved = sample.Timestamp + } + if sample.Timestamp >= start { + samples = append(samples, sample) + } + } + if len(samples) == 0 { + continue + } + sort.Slice(samples, func(i, j int) bool { return samples[i].Timestamp < samples[j].Timestamp }) + clean := samples[:0] + for _, sample := range samples { + if len(clean) == 0 || clean[len(clean)-1].Timestamp != sample.Timestamp { + clean = append(clean, sample) + } + } + for _, episode := range memoryLeakEpisodes(clean, s.PodUID, firstObserved, resource.Evidence.OOMKills, p.Evidence.FreshnessSeconds) { + evidence := analyzeMemoryEpisode(in, s, episode, start, p) + out.Episodes = append(out.Episodes, evidence) + switch evidence.Status { + case MemoryLeakPotential: + out.SuspectedEpisodes++ + out.EvaluatedEpisodes++ + case MemoryLeakNoPattern: + out.EvaluatedEpisodes++ + case MemoryLeakInsufficientData: + out.SkippedEpisodes++ + } + } + } + switch { + case out.SuspectedEpisodes > 0: + out.Status = MemoryLeakPotential + out.Reasons = append(out.Reasons, ReasonMemoryBaselineGrowth) + case out.EvaluatedEpisodes > 0 && out.SkippedEpisodes == 0: + out.Status = MemoryLeakNoPattern + out.Reasons = append(out.Reasons, ReasonMemoryBaselineStable) + case len(out.Episodes) == 0: + out.Reasons = append(out.Reasons, ReasonMissingUsage) + default: + // Details (history, freshness, coverage) stay with their episode. + for _, e := range out.Episodes { + if e.Status == MemoryLeakInsufficientData { + for _, reason := range e.Reasons { + out.Reasons = appendUniqueReason(out.Reasons, reason) + } + } + } + } + return out +} + +type memoryEpisode struct { + samples []Sample + firstObserved int64 + oomIDs []string +} + +func memoryLeakEpisodes(samples []Sample, podUID string, firstObserved int64, kills []OOMKill, freshnessSeconds int64) []memoryEpisode { + var episodes []memoryEpisode + position := 0 + for _, kill := range kills { + if podUID == "" || kill.PodUID != podUID || position == len(samples) || kill.Timestamp < samples[position].Timestamp { + continue + } + end := position + for end < len(samples) && samples[end].Timestamp <= kill.Timestamp { + end++ + } + if end == position { + continue + } + episode := memoryEpisode{samples: samples[position:end], firstObserved: firstObserved} + // A distant event must not make an otherwise stale series look complete. + if float64(kill.Timestamp-samples[end-1].Timestamp)/1000 <= float64(freshnessSeconds) { + episode.oomIDs = []string{kill.ID} + } + episodes = append(episodes, episode) + position = end + if position < len(samples) { + firstObserved = samples[position].Timestamp + } + } + if position < len(samples) { + episodes = append(episodes, memoryEpisode{samples: samples[position:], firstObserved: firstObserved}) + } + return episodes +} + +func analyzeMemoryEpisode(in Input, s Series, episode memoryEpisode, windowStart int64, p Policy) MemoryLeakEvidence { + lp := *p.Memory.LeakDetection + e := MemoryLeakEvidence{SeriesID: s.ID, PodUID: s.PodUID, Status: MemoryLeakInsufficientData, OOMKillIDs: episode.oomIDs, Reasons: []Reason{}} + if lp.WarmupSeconds*1000 > in.EvaluationTime-episode.firstObserved { + e.Reasons = append(e.Reasons, ReasonInsufficientHistory) + return e + } + start := max(windowStart, episode.firstObserved+lp.WarmupSeconds*1000) + first := sort.Search(len(episode.samples), func(i int) bool { return episode.samples[i].Timestamp >= start }) + samples := episode.samples[first:] + e.SampleCount = len(samples) + if len(samples) < 2 { + e.Reasons = append(e.Reasons, ReasonInsufficientSamples) + return e + } + e.ObservedStart, e.ObservedEnd = samples[0].Timestamp, samples[len(samples)-1].Timestamp + if stale(e.ObservedEnd, in, p.Evidence) && len(episode.oomIDs) == 0 { + e.Reasons = append(e.Reasons, ReasonStaleUsage) + } + gaps := make([]float64, 0, len(samples)-1) + for i := 1; i < len(samples); i++ { + gap := float64(samples[i].Timestamp - samples[i-1].Timestamp) + gaps = append(gaps, gap) + } + cadence := medianMemoryValues(gaps) + // At most one observed cadence of edge tolerance; never extrapolate to now. + end := e.ObservedEnd + int64(math.Min(cadence, float64(in.EvaluationTime-e.ObservedEnd))) + bucketMillis := lp.BucketDurationSeconds * 1000 + e.BucketCount = int((end - start) / bucketMillis) + if e.BucketCount < 12 || int64(e.BucketCount)*lp.BucketDurationSeconds < lp.MinimumHistorySeconds { + e.Reasons = append(e.Reasons, ReasonMemoryLeakBuckets) + return e + } + buckets := make([][]Sample, e.BucketCount) + for _, sample := range samples { + index := int((sample.Timestamp - start) / bucketMillis) + if index < len(buckets) { + buckets[index] = append(buckets[index], sample) + } + } + baselines := make([]float64, 0, len(buckets)) + baselineHours := make([]float64, 0, len(buckets)) + for i, bucket := range buckets { + if len(bucket) == 0 { + continue + } + covered := math.Min(cadence, float64(bucketMillis)) + values := make([]float64, len(bucket)) + for j, sample := range bucket { + values[j] = sample.Value + if j > 0 { + covered += math.Min(cadence, float64(sample.Timestamp-bucket[j-1].Timestamp)) + } + } + coverage := math.Min(1, covered/float64(bucketMillis)) + e.Coverage += coverage / float64(len(buckets)) + if len(bucket) < lp.MinimumSamplesPerBucket { + continue + } + sort.Float64s(values) + baselines = append(baselines, values[(len(values)+9)/10-1]) // Nearest-rank P10. + baselineHours = append(baselineHours, float64(i)*float64(lp.BucketDurationSeconds)/3600) + } + e.Coverage = math.Min(1, e.Coverage) + if e.Coverage < p.Evidence.MinimumCoverage { + e.Reasons = appendUniqueReason(e.Reasons, ReasonSparseCoverage) + } + if len(baselines) < 12 || int64(len(baselines))*lp.BucketDurationSeconds < lp.MinimumHistorySeconds { + e.Reasons = appendUniqueReason(e.Reasons, ReasonMemoryLeakBuckets) + } + if len(e.Reasons) > 0 { + return e + } + quarter := len(baselines) / 4 + e.BaselineStartBytes = medianMemoryValues(append([]float64(nil), baselines[:quarter]...)) + e.BaselineEndBytes = medianMemoryValues(append([]float64(nil), baselines[len(baselines)-quarter:]...)) + e.GrowthBytes = e.BaselineEndBytes - e.BaselineStartBytes + if e.BaselineStartBytes > 0 { + relative := e.GrowthBytes / e.BaselineStartBytes + if !finite(relative) { + e.Reasons = append(e.Reasons, ReasonMemoryLeakNumericRange) + return e + } + e.RelativeGrowth = &relative + } + e.SlopeBytesPerHour, e.TrendConsistency = memoryBaselineTrend(baselines, baselineHours) + // Require growth to continue near the end, not just earlier in the lookback. + // Six buckets keep this check meaningful even with the minimum history. + recentBuckets := max(6, len(baselines)/4) + e.RecentSlopeBytesPerHour, e.RecentTrendConsistency = memoryBaselineTrend(baselines[len(baselines)-recentBuckets:], baselineHours[len(baselines)-recentBuckets:]) + if !finite(e.SlopeBytesPerHour) || !finite(e.RecentSlopeBytesPerHour) { + e.SlopeBytesPerHour, e.RecentSlopeBytesPerHour = 0, 0 + e.Reasons = append(e.Reasons, ReasonMemoryLeakNumericRange) + return e + } + e.Status = MemoryLeakNoPattern + if e.GrowthBytes < lp.MinimumGrowthBytes || e.RelativeGrowth != nil && *e.RelativeGrowth < lp.MinimumRelativeGrowth || e.SlopeBytesPerHour <= 0 || e.TrendConsistency < lp.MinimumTrendConsistency { + e.Reasons = append(e.Reasons, ReasonMemoryBaselineStable) + return e + } + if e.RecentSlopeBytesPerHour <= 0 || e.RecentSlopeBytesPerHour < .25*e.SlopeBytesPerHour || e.RecentTrendConsistency < lp.MinimumTrendConsistency { + e.Reasons = append(e.Reasons, ReasonMemoryBaselineRecovery) + return e + } + e.Status = MemoryLeakPotential + e.Reasons = append(e.Reasons, ReasonMemoryBaselineGrowth) + if len(e.OOMKillIDs) > 0 { + e.Reasons = append(e.Reasons, ReasonOOMKillDetected) + } + return e +} + +func memoryBaselineTrend(values, hours []float64) (float64, float64) { + slopes := make([]float64, 0, len(values)*(len(values)-1)/2) + increasing := 0 + for i, first := range values { + for j := i + 1; j < len(values); j++ { + slope := (values[j] - first) / (hours[j] - hours[i]) + slopes = append(slopes, slope) + if slope > 0 { + increasing++ + } + } + } + return medianMemoryValues(slopes), float64(increasing) / float64(len(slopes)) +} + +func medianMemoryValues(values []float64) float64 { + sort.Float64s(values) + middle := len(values) / 2 + if len(values)%2 != 0 { + return values[middle] + } + return values[middle-1]/2 + values[middle]/2 +} + +func appendUniqueReason(reasons []Reason, reason Reason) []Reason { + for _, existing := range reasons { + if existing == reason { + return reasons + } + } + return append(reasons, reason) +} diff --git a/analysis/memory_leak_test.go b/analysis/memory_leak_test.go new file mode 100644 index 0000000..f24f04d --- /dev/null +++ b/analysis/memory_leak_test.go @@ -0,0 +1,469 @@ +// Copyright Kedify Inc. +// SPDX-License-Identifier: LicenseRef-Kedify-Commercial-1.0 AND LicenseRef-Kedify-Public-Source-1.0 +// See LICENSE and PUBLIC_SOURCE_LICENSE. + +package analysis + +import ( + "encoding/json" + "math" + "reflect" + "strings" + "testing" +) + +func leakFixture(hours int64, profile func(float64) float64) Input { + in := fixture(hours*3600, 60) + for i := range in.Containers[0].Memory.Series[0].Samples { + s := &in.Containers[0].Memory.Series[0].Samples[i] + s.Value = profile(float64(s.Timestamp-epoch)/3600000) * mib + } + return in +} + +func leakPolicy() Policy { + p := DefaultPolicy() + p.Memory.LeakDetection = &MemoryLeakPolicy{} + return p +} + +func TestMemoryLeakProfiles(t *testing.T) { + profiles := []struct { + name string + profile func(float64) float64 + want MemoryLeakStatus + }{ + {"linear", func(h float64) float64 { return 100 + 12*h }, MemoryLeakPotential}, + {"rising GC floors", func(h float64) float64 { return 100 + 12*h + 48*math.Mod(h*6, 1) }, MemoryLeakPotential}, + {"staircase with GC", func(h float64) float64 { return 100 + 24*math.Floor(h) + 32*math.Mod(h*6, 1) }, MemoryLeakPotential}, + {"flat", func(float64) float64 { return 128 }, MemoryLeakNoPattern}, + {"bounded GC", func(h float64) float64 { return 128 + 64*math.Mod(h*6, 1) }, MemoryLeakNoPattern}, + {"long bounded GC cycles", func(h float64) float64 { return 128 + 128*math.Mod(h, 2) }, MemoryLeakNoPattern}, + {"startup allocation", func(h float64) float64 { return 100 + 300*math.Min(h, 1) }, MemoryLeakNoPattern}, + {"one permanent step", func(h float64) float64 { + if h < 12 { + return 100 + } + return 500 + }, MemoryLeakNoPattern}, + {"cache plateau", func(h float64) float64 { return 100 + 12*math.Min(h, 16) }, MemoryLeakNoPattern}, + {"recent cache plateau", func(h float64) float64 { return 100 + 12*math.Min(h, 20) }, MemoryLeakNoPattern}, + {"recovery", func(h float64) float64 { + if h < 18 { + return 100 + 12*h + } + return 100 + }, MemoryLeakNoPattern}, + {"falling", func(h float64) float64 { return 500 - 12*h }, MemoryLeakNoPattern}, + {"small drift", func(h float64) float64 { return 100 + .5*h }, MemoryLeakNoPattern}, + {"small relative growth", func(h float64) float64 { return 10000 + 12*h }, MemoryLeakNoPattern}, + {"increasing transient peaks", func(h float64) float64 { + if int(math.Round(h*60))%30 == 0 { + return 500 + 100*h + } + return 100 + }, MemoryLeakNoPattern}, + {"bounded noisy baseline", func(h float64) float64 { return 200 + 10*math.Sin(h*37) + 5*math.Cos(h*11) }, MemoryLeakNoPattern}, + } + for _, tt := range profiles { + t.Run(tt.name, func(t *testing.T) { + in := leakFixture(24, tt.profile) + r := run(t, in, leakPolicy()).Results[0] + leak := r.MemoryLeak + if leak == nil || leak.Status != tt.want || leak.EvaluatedEpisodes != 1 || len(leak.Episodes) != 1 { + t.Fatalf("unexpected leak analysis: %+v", leak) + } + if leak.DetectorVersion != MemoryLeakDetectorVersion || leak.Episodes[0].BucketCount != 47 || leak.Episodes[0].Coverage < .99 { + t.Fatalf("missing measured evidence: %+v", leak) + } + if tt.want == MemoryLeakPotential { + if !reflect.DeepEqual(r.Notices, []Reason{ReasonPotentialMemoryLeak}) || leak.SuspectedEpisodes != 1 || leak.Episodes[0].SlopeBytesPerHour <= 0 { + t.Fatalf("missing potential leak notice/evidence: %+v", r) + } + } else if len(r.Notices) != 0 { + t.Fatalf("non-leak profile generated a notice: %+v", r) + } + if _, err := json.Marshal(r); err != nil { + t.Fatal(err) + } + }) + } +} + +func TestMemoryLeakIsOptionalAdvisoryAndIndependentOfSizingHistory(t *testing.T) { + in := leakFixture(24, func(h float64) float64 { return 100 + 12*h }) + p := leakPolicy() + p.Evidence.MinimumHistorySeconds = 7 * 86400 + baselinePolicy := p + baselinePolicy.Memory.LeakDetection = nil + baseline := run(t, in, baselinePolicy) + encoded, err := json.Marshal(baseline) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(encoded), `"memoryLeak"`) || strings.Contains(string(encoded), `"leakDetection"`) { + t.Fatal("disabled detection leaked fields into existing output") + } + out := run(t, in, p) + if out.Results[0].MemoryLeak.Status != MemoryLeakPotential || !has(out.Results[0].DataQuality, ReasonInsufficientHistory) { + t.Fatal("leak detection must use its own shorter history without bypassing sizing guards") + } + for i := range out.Results { + actual, want := out.Results[i], baseline.Results[i] + actual.MemoryLeak, actual.Notices = nil, nil + if !reflect.DeepEqual(actual, want) { + t.Fatalf("leak detection changed resource sizing: %+v", actual) + } + } + if out.Results[1].MemoryLeak != nil { + t.Fatal("CPU received memory leak analysis") + } + in.Containers[0].Memory.CurrentRequest = Signal{} + if run(t, in, leakPolicy()).Results[0].MemoryLeak.Status != MemoryLeakPotential { + t.Fatal("leak detection unnecessarily depends on configured resource requests") + } +} + +func TestMemoryLeakUsesEachReplicaAndLifetimeSeparately(t *testing.T) { + in := leakFixture(24, func(float64) float64 { return 100 }) + c := &in.Containers[0] + busy := c.Memory.Series[0] + busy.ID, busy.PodUID = "busy", "busy-pod" + busy.Samples = append([]Sample(nil), busy.Samples...) + for i := range busy.Samples { + busy.Samples[i].Value = (100 + 12*float64(i)/60) * mib + } + c.Memory.Series = append(c.Memory.Series, busy) + r := run(t, in, leakPolicy()).Results[0].MemoryLeak + if r.Status != MemoryLeakPotential || r.EvaluatedEpisodes != 2 || r.SuspectedEpisodes != 1 || r.Episodes[0].SeriesID != "busy" { + t.Fatalf("busy replica was pooled or hidden: %+v", r) + } + // Replacement raises the absolute allocation level, but neither lifetime grows. + in = leakFixture(24, func(float64) float64 { return 100 }) + c = &in.Containers[0] + old := c.Memory.Series[0] + old.Samples = append([]Sample(nil), old.Samples[:12*60]...) + replacement := c.Memory.Series[0] + replacement.ID, replacement.PodUID = "replacement", "new-pod" + replacement.Samples = append([]Sample(nil), replacement.Samples[12*60:]...) + for i := range replacement.Samples { + replacement.Samples[i].Value = 500 * mib + } + c.Memory.Series = []Series{old, replacement} + c.OOMKills = []OOMKill{oomKill("old-pod-oom", epoch+12*3600000-1, 128*mib)} + r = run(t, in, leakPolicy()).Results[0].MemoryLeak + if r.Status != MemoryLeakNoPattern || r.EvaluatedEpisodes != 2 { + t.Fatalf("replica replacement manufactured a trend: %+v", r) + } + // The same pod can have separate container lifetimes, too. + c.Memory.Series[1].PodUID = old.PodUID + if r = run(t, in, leakPolicy()).Results[0].MemoryLeak; r.Status != MemoryLeakNoPattern { + t.Fatalf("container lifetime change manufactured a trend: %+v", r) + } +} + +func TestMemoryLeakPreservesEligibleRecommendationsAndOOMAdjustment(t *testing.T) { + for _, withOOM := range []bool{false, true} { + in := leakFixture(24, func(h float64) float64 { return 100 + 12*h }) + if withOOM { + in.Containers[0].OOMKills = []OOMKill{oomKill("terminal", in.EvaluationTime, 512*mib)} + } + p := DefaultPolicy() + p.Evidence.MinimumHistorySeconds = 6 * 3600 + baseline := run(t, in, p) + if len(baseline.Results[0].Recommendations) == 0 { + t.Fatal("fixture must exercise eligible memory sizing") + } + p.Memory.LeakDetection = &MemoryLeakPolicy{} + out := run(t, in, p) + if out.Results[0].MemoryLeak.Status != MemoryLeakPotential { + t.Fatal("fixture must produce a leak finding") + } + for i := range out.Results { + actual, want := out.Results[i], baseline.Results[i] + actual.MemoryLeak, actual.Notices = nil, want.Notices + if !reflect.DeepEqual(actual, want) { + t.Fatalf("advisory diagnostic changed recommendations or OOM adjustment: %+v", actual) + } + } + } +} + +func TestMemoryLeakCustomPolicy(t *testing.T) { + in := leakFixture(24, func(h float64) float64 { return 100 + 12*h }) + p := leakPolicy() + p.Memory.LeakDetection = &MemoryLeakPolicy{ + LookbackSeconds: 12 * 3600, BucketDurationSeconds: 15 * 60, + MinimumHistorySeconds: 3 * 3600, WarmupSeconds: 15 * 60, + MinimumSamplesPerBucket: 10, MinimumGrowthBytes: 32 * mib, + MinimumRelativeGrowth: .1, MinimumTrendConsistency: .9, + } + leak := run(t, in, p).Results[0].MemoryLeak + if leak.Status != MemoryLeakPotential || leak.WindowStart != epoch+12*3600000 || leak.Episodes[0].BucketCount != 48 { + t.Fatalf("custom policy not applied: %+v", leak) + } + p.Memory.LeakDetection.MinimumGrowthBytes = 1024 * mib + if leak = run(t, in, p).Results[0].MemoryLeak; leak.Status != MemoryLeakNoPattern { + t.Fatalf("custom growth threshold not applied: %+v", leak) + } +} + +func TestMemoryLeakPartitionsReleasesAndLookback(t *testing.T) { + in := leakFixture(24, func(h float64) float64 { + if h < 12 { + return 100 + 40*h + } + return 100 + }) + c := &in.Containers[0] + c.Identity.ReleaseStartedAt = epoch + 12*3600000 + old := c.Memory.Series[0] + old.ID, old.Release = "old-release", "B" + old.Samples = append([]Sample(nil), old.Samples[:12*60]...) + c.Memory.Series = append(c.Memory.Series, old) + for _, inferred := range []bool{false, true} { + if inferred { + c.Identity.ReleaseStartedAt = 0 + } + r := run(t, in, leakPolicy()).Results[0].MemoryLeak + if r.Status != MemoryLeakNoPattern || r.WindowStart < epoch+12*3600000 || len(r.Episodes) != 1 { + t.Fatalf("old release or pre-rollback observations affected diagnosis: %+v", r) + } + } + // A recreated workload with the same name cannot contribute an episode. + c.Memory.Series[1].WorkloadUID = "deleted-workload" + c.Identity.ReleaseStartedAt = epoch + 12*3600000 + if r := run(t, in, leakPolicy()).Results[0].MemoryLeak; len(r.Episodes) != 1 || r.Status != MemoryLeakNoPattern { + t.Fatalf("deleted workload leaked into analysis: %+v", r) + } + in = leakFixture(72, func(h float64) float64 { + if h < 48 { + return 100 + 40*h + } + return 100 + }) + r := run(t, in, leakPolicy()).Results[0].MemoryLeak + if r.Status != MemoryLeakNoPattern || r.WindowStart != epoch+48*3600000 { + t.Fatalf("data outside the leak lookback changed analysis: %+v", r) + } +} + +func TestMemoryLeakOOMCorrelationAndEpisodeSplitting(t *testing.T) { + in := leakFixture(24, func(h float64) float64 { return 100 + 20*math.Mod(h, 12) }) + c := &in.Containers[0] + c.OOMKills = []OOMKill{ + oomKill("first", epoch+12*3600000-1, 512*mib), + oomKill("second", in.EvaluationTime, 512*mib), + } + r := run(t, in, leakPolicy()).Results[0] + leak := r.MemoryLeak + if leak.Status != MemoryLeakPotential || leak.SuspectedEpisodes != 2 || leak.SkippedEpisodes != 0 { + t.Fatalf("OOM resets obscured repeated leak episodes: %+v", leak) + } + for i, e := range leak.Episodes { + if !reflect.DeepEqual(e.OOMKillIDs, []string{c.OOMKills[i].ID}) || len(e.Reasons) != 2 || e.Reasons[1] != ReasonOOMKillDetected { + t.Fatalf("missing episode-specific OOM corroboration: %+v", e) + } + } + if !reflect.DeepEqual(r.Notices, []Reason{ReasonOOMKillDetected, ReasonPotentialMemoryLeak}) { + t.Fatalf("OOM and leak notices should coexist: %+v", r.Notices) + } + // An OOM on a flat profile does not prove a leak. + in = leakFixture(24, func(float64) float64 { return 200 }) + in.Containers[0].OOMKills = []OOMKill{oomKill("flat-oom", in.EvaluationTime, 256*mib)} + if r = run(t, in, leakPolicy()).Results[0]; r.MemoryLeak.Status != MemoryLeakNoPattern || len(r.Notices) != 1 { + t.Fatalf("OOM alone was treated as a leak: %+v", r) + } + for _, mode := range []string{"different pod", "unknown pod", "distant event"} { + t.Run(mode, func(t *testing.T) { + in := leakFixture(24, func(h float64) float64 { return 100 + 20*h }) + in.Containers[0].Memory.Series[0].Samples = in.Containers[0].Memory.Series[0].Samples[:12*60] + kill := oomKill("oom", epoch+12*3600000-1, 512*mib) + switch mode { + case "different pod": + kill.PodUID = "unrelated" + case "unknown pod": + kill.PodUID = "" + case "distant event": + kill.Timestamp = in.EvaluationTime + } + in.Containers[0].OOMKills = []OOMKill{kill} + leak := run(t, in, leakPolicy()).Results[0].MemoryLeak + if leak.Status != MemoryLeakInsufficientData || len(leak.Episodes[0].OOMKillIDs) != 0 { + t.Fatalf("unrelated OOM made stale evidence eligible: %+v", leak) + } + }) + } +} + +func TestMemoryLeakInsufficientEvidence(t *testing.T) { + tests := []struct { + name string + mutate func(*Input) + reason Reason + }{ + {"short history", func(in *Input) { *in = leakFixture(3, func(h float64) float64 { return 100 + 100*h }) }, ReasonMemoryLeakBuckets}, + {"no usage", func(in *Input) { in.Containers[0].Memory.Series = nil }, ReasonMissingUsage}, + {"stale identity", func(in *Input) { in.Containers[0].Identity.Timestamp = epoch }, ReasonStaleIdentity}, + {"ambiguous identity", func(in *Input) { in.Containers[0].Identity.Ambiguous = true }, ReasonAmbiguousIdentity}, + {"unknown series identity", func(in *Input) { in.Containers[0].Memory.Series[0].Release = "" }, ReasonUnknownSeriesIdentity}, + {"stale samples", func(in *Input) { + s := &in.Containers[0].Memory.Series[0] + s.Samples = s.Samples[:len(s.Samples)-10] + }, ReasonStaleUsage}, + {"insufficient episode coverage", func(in *Input) { + s := &in.Containers[0].Memory.Series[0] + s.Samples = append(s.Samples[:12*60], s.Samples[16*60:]...) + }, ReasonSparseCoverage}, + {"sparse coverage", func(in *Input) { + s := &in.Containers[0].Memory.Series[0] + var kept []Sample + for i, v := range s.Samples { + if i%5 != 0 { + kept = append(kept, v) + } + } + s.Samples = kept + }, ReasonSparseCoverage}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + in := leakFixture(24, func(h float64) float64 { return 100 + 12*h }) + tt.mutate(&in) + leak := run(t, in, leakPolicy()).Results[0].MemoryLeak + if leak.Status != MemoryLeakInsufficientData || !has(DataQuality{Reasons: leak.Reasons}, tt.reason) { + t.Fatalf("invalid evidence was classified: %+v", leak) + } + }) + } + // Another replica must not supply the first replica's missing coverage. + in := leakFixture(24, func(h float64) float64 { return 100 + 12*h }) + c := &in.Containers[0] + other := c.Memory.Series[0] + other.ID, other.PodUID = "healthy", "healthy-pod" + other.Samples = append([]Sample(nil), other.Samples...) + for i := range other.Samples { + other.Samples[i].Value = 100 * mib + } + c.Memory.Series[0].Samples = append(c.Memory.Series[0].Samples[:12*60], c.Memory.Series[0].Samples[16*60:]...) + c.Memory.Series = append(c.Memory.Series, other) + if leak := run(t, in, leakPolicy()).Results[0].MemoryLeak; leak.Status != MemoryLeakInsufficientData || leak.EvaluatedEpisodes != 1 || leak.SkippedEpisodes != 1 { + t.Fatalf("replicas pooled coverage or incomplete evidence reported no pattern: %+v", leak) + } +} + +func TestMemoryLeakToleratesCollectionOutages(t *testing.T) { + for _, minutes := range []int{15, 60} { + in := leakFixture(24, func(h float64) float64 { return 100 + 12*h }) + s := &in.Containers[0].Memory.Series[0] + s.Samples = append(s.Samples[:12*60], s.Samples[12*60+minutes:]...) + leak := run(t, in, leakPolicy()).Results[0].MemoryLeak + if leak.Status != MemoryLeakPotential || leak.SuspectedEpisodes != 1 || leak.Episodes[0].Coverage >= 1 || leak.Episodes[0].Coverage < .9 { + t.Fatalf("%d-minute collection outage rejected usable trend evidence: %+v", minutes, leak) + } + if math.Abs(leak.Episodes[0].SlopeBytesPerHour-12*mib) > 1e-6 { + t.Fatalf("missing buckets compressed elapsed time: %+v", leak.Episodes[0]) + } + } + slope, consistency := memoryBaselineTrend([]float64{100, 112, 148}, []float64{0, 1, 4}) + if slope != 12 || consistency != 1 { + t.Fatalf("trend must use actual bucket timestamps: slope=%g, consistency=%g", slope, consistency) + } +} + +func TestMemoryLeakDeterminismAndInputImmutability(t *testing.T) { + in := leakFixture(24, func(h float64) float64 { return 100 + 12*h }) + p := leakPolicy() + before, _ := json.Marshal(in) + policyBefore, _ := json.Marshal(p) + want := run(t, in, p) + after, _ := json.Marshal(in) + policyAfter, _ := json.Marshal(p) + if string(before) != string(after) || string(policyBefore) != string(policyAfter) { + t.Fatal("analysis mutated input or the caller's optional policy") + } + s := &in.Containers[0].Memory.Series[0] + for i, j := 0, len(s.Samples)-1; i < j; i, j = i+1, j-1 { + s.Samples[i], s.Samples[j] = s.Samples[j], s.Samples[i] + } + s.Samples = append(s.Samples, s.Samples[0]) + if got := run(t, in, p); !reflect.DeepEqual(got, want) { + t.Fatal("sample ordering or duplicate timestamps changed diagnosis") + } + var decoded Input + if err := json.Unmarshal(before, &decoded); err != nil { + t.Fatal(err) + } + if got := run(t, decoded, p); !reflect.DeepEqual(got, want) { + t.Fatal("offline and connected inputs differ") + } + for _, base := range []float64{0, 1e-320} { + in = leakFixture(24, func(h float64) float64 { + if h < 6 { + return base + } + return (h - 6) * 24 + }) + r := run(t, in, p).Results[0] + if _, err := json.Marshal(r); err != nil { + t.Fatalf("non-finite leak output: %v", err) + } + if base == 0 && (r.MemoryLeak.Status != MemoryLeakPotential || r.MemoryLeak.Episodes[0].RelativeGrowth != nil) { + t.Fatalf("zero starting baseline mishandled: %+v", r.MemoryLeak) + } + } +} + +func TestMemoryLeakPolicyValidationAndIdentity(t *testing.T) { + p := leakPolicy() + q, err := NormalizePolicy(p) + if err != nil { + t.Fatal(err) + } + if *p.Memory.LeakDetection != (MemoryLeakPolicy{}) { + t.Fatal("normalization mutated caller's pointer") + } + if q.Memory.LeakDetection.LookbackSeconds != 86400 || q.Memory.LeakDetection.MinimumGrowthBytes != 64*mib { + t.Fatalf("wrong defaults: %+v", q.Memory.LeakDetection) + } + defaultVersion, _ := PolicyVersion(Policy{}) + enabledVersion, _ := PolicyVersion(p) + normalizedVersion, _ := PolicyVersion(q) + if defaultVersion == enabledVersion || enabledVersion != normalizedVersion { + t.Fatal("optional detector policy identity is not normalized") + } + for _, change := range []func(*MemoryLeakPolicy){ + func(p *MemoryLeakPolicy) { p.LookbackSeconds = -1 }, + func(p *MemoryLeakPolicy) { p.LookbackSeconds = math.MaxInt64 }, + func(p *MemoryLeakPolicy) { p.BucketDurationSeconds = 1 }, + func(p *MemoryLeakPolicy) { p.BucketDurationSeconds = 24 * 3600 }, + func(p *MemoryLeakPolicy) { p.MinimumHistorySeconds = 48 * 3600 }, + func(p *MemoryLeakPolicy) { p.WarmupSeconds = 48 * 3600 }, + func(p *MemoryLeakPolicy) { p.MinimumSamplesPerBucket = 1 }, + func(p *MemoryLeakPolicy) { p.MinimumGrowthBytes = math.NaN() }, + func(p *MemoryLeakPolicy) { p.MinimumRelativeGrowth = math.Inf(1) }, + func(p *MemoryLeakPolicy) { p.MinimumTrendConsistency = .5 }, + func(p *MemoryLeakPolicy) { p.MinimumTrendConsistency = 1.1 }, + } { + p := leakPolicy() + change(p.Memory.LeakDetection) + if _, err := NormalizePolicy(p); err == nil || !strings.Contains(err.Error(), "memory.leakDetection") { + t.Fatalf("invalid leak policy accepted: %+v, %v", p, err) + } + } + q.Memory.LeakDetection.MinimumGrowthBytes *= 10 + if changedVersion, _ := PolicyVersion(q); changedVersion == enabledVersion { + t.Fatal("threshold not included in policy identity") + } +} + +func TestMemoryBaselineRobustSlope(t *testing.T) { + slope, consistency := memoryBaselineTrend([]float64{10, 12, 14, 16, 18}, []float64{0, 1, 2, 3, 4}) + if slope != 2 || consistency != 1 { + t.Fatalf("wrong slope %v or consistency %v", slope, consistency) + } + slope, _ = memoryBaselineTrend([]float64{10, 12, 14, 1000, 18, 20, 22}, []float64{0, 1, 2, 3, 4, 5, 6}) + if slope != 2 { + t.Fatalf("single outlier distorted robust slope: %v", slope) + } +} diff --git a/analysis/memory_leak_types.go b/analysis/memory_leak_types.go new file mode 100644 index 0000000..74cf43b --- /dev/null +++ b/analysis/memory_leak_types.go @@ -0,0 +1,85 @@ +// Copyright Kedify Inc. +// SPDX-License-Identifier: LicenseRef-Kedify-Commercial-1.0 AND LicenseRef-Kedify-Public-Source-1.0 +// See LICENSE and PUBLIC_SOURCE_LICENSE. + +package analysis + +const MemoryLeakDetectorVersion = "2" + +// MemoryLeakPolicy enables an advisory heuristic, not a diagnosis. Zero fields +// select defaults. Setting MemoryPolicy.LeakDetection to nil disables detection. +// Coverage and freshness come from Policy.Evidence. Leak history is +// independent of the longer history required for resource sizing. +type MemoryLeakPolicy struct { + LookbackSeconds int64 `json:"lookbackSeconds"` + BucketDurationSeconds int64 `json:"bucketDurationSeconds"` + MinimumHistorySeconds int64 `json:"minimumHistorySeconds"` + WarmupSeconds int64 `json:"warmupSeconds"` + MinimumSamplesPerBucket int `json:"minimumSamplesPerBucket"` + MinimumGrowthBytes float64 `json:"minimumGrowthBytes"` + MinimumRelativeGrowth float64 `json:"minimumRelativeGrowth"` + MinimumTrendConsistency float64 `json:"minimumTrendConsistency"` +} + +type MemoryLeakStatus string + +const ( + MemoryLeakPotential MemoryLeakStatus = "potential-leak" + MemoryLeakNoPattern MemoryLeakStatus = "no-leak-pattern" + MemoryLeakInsufficientData MemoryLeakStatus = "insufficient-data" +) + +const ( + ReasonPotentialMemoryLeak Reason = "potential-memory-leak" + ReasonMemoryBaselineGrowth Reason = "sustained-memory-baseline-growth" + ReasonMemoryBaselineStable Reason = "no-sustained-memory-baseline-growth" + ReasonMemoryBaselineRecovery Reason = "memory-baseline-plateau-or-recovery" + ReasonMemoryLeakBuckets Reason = "insufficient-memory-trend-buckets" + ReasonMemoryLeakNumericRange Reason = "memory-trend-outside-numeric-range" +) + +// MemoryLeakAnalysis summarizes only the selected release and supplied series. +// A potential leak means at least one episode passed the heuristic. NoPattern is +// reported only when every considered episode could be evaluated. Neither status +// proves whether allocations are useful: load, caches and runtime heaps are not +// part of this input. The independent detector version identifies this heuristic. +type MemoryLeakAnalysis struct { + DetectorVersion string `json:"detectorVersion"` + Status MemoryLeakStatus `json:"status"` + WindowStart int64 `json:"windowStart"` + WindowEnd int64 `json:"windowEnd"` + EvaluatedEpisodes int `json:"evaluatedEpisodes"` + SuspectedEpisodes int `json:"suspectedEpisodes"` + SkippedEpisodes int `json:"skippedEpisodes"` + Reasons []Reason `json:"reasons"` + Episodes []MemoryLeakEvidence `json:"episodes"` +} + +// MemoryLeakEvidence describes one container episode. Known OOM +// terminations split episodes even if an adapter reused a series ID. Baselines +// are medians of the first/last quarters of per-bucket P10 values. Slopes use +// Theil-Sen (median pairwise slope); consistency is the fraction of increasing +// pairs, not a probability or statistical confidence. RelativeGrowth is absent +// when the starting baseline is zero. Recent statistics use the latest quarter +// of buckets, with a minimum of six buckets. OOMKillIDs refer to evidence.oomKills +// and only corroborate an episode when its last sample is close to that termination. +type MemoryLeakEvidence struct { + SeriesID string `json:"seriesID"` + PodUID string `json:"podUID,omitempty"` + Status MemoryLeakStatus `json:"status"` + ObservedStart int64 `json:"observedStart"` + ObservedEnd int64 `json:"observedEnd"` + SampleCount int `json:"sampleCount"` + BucketCount int `json:"bucketCount"` + Coverage float64 `json:"coverage"` + BaselineStartBytes float64 `json:"baselineStartBytes"` + BaselineEndBytes float64 `json:"baselineEndBytes"` + GrowthBytes float64 `json:"growthBytes"` + RelativeGrowth *float64 `json:"relativeGrowth,omitempty"` + SlopeBytesPerHour float64 `json:"slopeBytesPerHour"` + RecentSlopeBytesPerHour float64 `json:"recentSlopeBytesPerHour"` + TrendConsistency float64 `json:"trendConsistency"` + RecentTrendConsistency float64 `json:"recentTrendConsistency"` + OOMKillIDs []string `json:"oomKillIDs,omitempty"` + Reasons []Reason `json:"reasons"` +} diff --git a/analysis/types.go b/analysis/types.go index ef57ebf..629233e 100644 --- a/analysis/types.go +++ b/analysis/types.go @@ -42,7 +42,7 @@ type ContainerObservation struct { // PreviousRelease supplies raw usage from an earlier rollout of this workload. // EvaluationTime is the last historical observation, not today's timestamp. // ReleaseStartedAt may be zero to infer the observed segment. Current settings, -// inventory and OOM events always refer to the current rollout. +// inventory, OOM events and leak findings always refer to the current rollout. type PreviousRelease struct { Release string `json:"release"` ReleaseStartedAt int64 `json:"releaseStartedAt,omitempty"` @@ -102,7 +102,7 @@ const ( SampleCPUCounterSeconds SampleKind = "cpu-counter-seconds" ) -// ID uniquely identifies an uninterrupted time series. PodUID is preferred when +// ID uniquely identifies a container lifetime's time series. PodUID is preferred when // available; ID is the fallback. Never merge replicas or counter lifetimes. type Series struct { ID string `json:"id"` @@ -174,6 +174,8 @@ type MemoryPolicy struct { LimitsToRequestsRatio float64 `json:"limitsToRequestsRatio"` Bounds Bounds `json:"bounds"` RequestsOnly bool `json:"requestsOnly"` + // Nil disables leak detection; an empty policy enables its defaults. + LeakDetection *MemoryLeakPolicy `json:"leakDetection,omitempty"` } type Output struct { SchemaVersion string `json:"schemaVersion"` @@ -190,16 +192,17 @@ const ( ) type ResourceAnalysis struct { - DecisionTrace *DecisionTrace `json:"decisionTrace,omitempty"` - Target Target `json:"target"` - Resource Resource `json:"resource"` - Recommendations []Recommendation `json:"recommendations,omitempty"` - Evidence ResourceEvidence `json:"evidence"` - DataQuality DataQuality `json:"dataQuality"` - NoActionReason Reason `json:"noActionReason,omitempty"` - Notices []Reason `json:"notices,omitempty"` - OOMAdjustment *OOMAdjustment `json:"oomAdjustment,omitempty"` - RolloutFallback *RolloutFallback `json:"rolloutFallback,omitempty"` + DecisionTrace *DecisionTrace `json:"decisionTrace,omitempty"` + Target Target `json:"target"` + Resource Resource `json:"resource"` + Recommendations []Recommendation `json:"recommendations,omitempty"` + Evidence ResourceEvidence `json:"evidence"` + DataQuality DataQuality `json:"dataQuality"` + NoActionReason Reason `json:"noActionReason,omitempty"` + Notices []Reason `json:"notices,omitempty"` + OOMAdjustment *OOMAdjustment `json:"oomAdjustment,omitempty"` + MemoryLeak *MemoryLeakAnalysis `json:"memoryLeak,omitempty"` + RolloutFallback *RolloutFallback `json:"rolloutFallback,omitempty"` } // RolloutFallback identifies historical usage used to size the current target. From 1f6aae611ff5ae290bfa70af3988aea4ad22cb21 Mon Sep 17 00:00:00 2001 From: Jirka Kremser Date: Fri, 25 Sep 2026 11:59:46 +0200 Subject: [PATCH 4/4] Address review comments Signed-off-by: Jirka Kremser --- README.md | 7 +++-- analysis/analyze.go | 20 ++++++++----- analysis/fallback.go | 1 + analysis/fallback_test.go | 1 + analysis/memory_leak.go | 11 +++++++ analysis/memory_leak_test.go | 47 +++++++++++++++++++++++++++++ analysis/memory_leak_types.go | 2 +- analysis/oom.go | 11 ++++--- analysis/oom_test.go | 56 +++++++++++++++++++++++++++++++++++ analysis/samples.go | 1 + analysis/trace.go | 1 + analysis/trace_test.go | 37 +++++++++++++++++++++++ analysis/unset_test.go | 4 +++ 13 files changed, 182 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index b72f4fe..ad2641c 100644 --- a/README.md +++ b/README.md @@ -138,7 +138,8 @@ assigning it to this container. Use zero or omit `MemoryLimitBytes` when the event-time limit is unknown or unlimited; do not substitute today's limit. Repeated scrapes of one termination retain the same event ID and timestamp. Identical events are deduplicated; conflicting observations sharing an ID are -rejected. Events are sorted deterministically and input is not mutated. +rejected. All supplied observations are validated before filtering by time or +workload/release identity. Events are sorted deterministically and input is not mutated. Only events within the requested window and selected workload UID/release segment affect memory. Events from before a rollback boundary or from another workload UID @@ -269,7 +270,7 @@ absolute/relative growth, and trend consistency through `MemoryLeakPolicy`. Durations use seconds; zero fields select defaults. Policies must retain at least 12 buckets of minimum history and at most 256 buckets in the lookback, bounding the pairwise computation independently of the number of raw samples. P10 and the recent -trend check are fixed parts of detector version `2`. +trend check are fixed parts of detector version `3`. Each memory result gains `memoryLeak`, including: @@ -278,6 +279,8 @@ Each memory result gains `memoryLeak`, including: `insufficient-data` when none passes and evidence is incomplete or unavailable. - The detector version, selected window, evaluated/suspected/skipped episode counts, and explicit reasons. A positive finding may coexist with skipped episodes. + Selected series with no samples in the lookback count as skipped episodes, with + missing or stale usage reasons; they cannot support a `no-leak-pattern` result. - Per-episode series/pod identity, observed interval, sample/bucket counts, coverage, starting/ending baselines, growth in bytes and as a fraction, overall/recent slopes in bytes/hour, and trend consistency. Consistency is a measured fraction of diff --git a/analysis/analyze.go b/analysis/analyze.go index 1b75bf7..7e0a797 100644 --- a/analysis/analyze.go +++ b/analysis/analyze.go @@ -359,11 +359,15 @@ func analyzeResource(in Input, c ContainerObservation, r Resource, p Policy) (Re } downsizeSuppressed := false for _, s := range []struct { - setting Setting - current Signal - suggested float64 - ok bool - }{{SettingRequests, obs.CurrentRequest, suggestedRequest, requestOK}, {SettingLimits, obs.CurrentLimit, suggestedLimit, limitOK && !requestsOnly}} { + setting Setting + current Signal + suggested float64 + ok bool + missing, old Reason + }{ + {SettingRequests, obs.CurrentRequest, suggestedRequest, requestOK, ReasonMissingRequest, ReasonStaleRequest}, + {SettingLimits, obs.CurrentLimit, suggestedLimit, limitOK && !requestsOnly, ReasonMissingLimit, ReasonStaleLimit}, + } { settingTrace := requestTrace if s.setting == SettingLimits { settingTrace = limitTrace @@ -372,9 +376,9 @@ func analyzeResource(in Input, c ContainerObservation, r Resource, p Policy) (Re if requestsOnly && s.setting == SettingLimits { settingTrace.stop("disabled", ReasonLimitDisabled) } else { - settingTrace.stop("unavailable", ReasonMissingLimit) - if obs.CurrentLimit.Available { - settingTrace.stop("unavailable", ReasonStaleLimit) + settingTrace.stop("unavailable", s.missing) + if s.current.Available { + settingTrace.stop("unavailable", s.old) } } continue diff --git a/analysis/fallback.go b/analysis/fallback.go index 97134ad..d0ace9e 100644 --- a/analysis/fallback.go +++ b/analysis/fallback.go @@ -1,5 +1,6 @@ // Copyright Kedify Inc. // SPDX-License-Identifier: LicenseRef-Kedify-Commercial-1.0 AND LicenseRef-Kedify-Public-Source-1.0 +// See LICENSE and PUBLIC_SOURCE_LICENSE. package analysis diff --git a/analysis/fallback_test.go b/analysis/fallback_test.go index cfa85ff..546e295 100644 --- a/analysis/fallback_test.go +++ b/analysis/fallback_test.go @@ -1,5 +1,6 @@ // Copyright Kedify Inc. // SPDX-License-Identifier: LicenseRef-Kedify-Commercial-1.0 AND LicenseRef-Kedify-Public-Source-1.0 +// See LICENSE and PUBLIC_SOURCE_LICENSE. package analysis diff --git a/analysis/memory_leak.go b/analysis/memory_leak.go index 8d287d2..b1d6312 100644 --- a/analysis/memory_leak.go +++ b/analysis/memory_leak.go @@ -92,6 +92,17 @@ func analyzeMemoryLeak(in Input, c ContainerObservation, p Policy, resource Reso } } if len(samples) == 0 { + // Keep missing series in the coverage accounting: another replica's + // stable baseline cannot establish that this lifetime has no leak. + reason := ReasonMissingUsage + if firstObserved > 0 { + reason = ReasonStaleUsage + } + out.Episodes = append(out.Episodes, MemoryLeakEvidence{ + SeriesID: s.ID, PodUID: s.PodUID, Status: MemoryLeakInsufficientData, + Reasons: []Reason{reason}, + }) + out.SkippedEpisodes++ continue } sort.Slice(samples, func(i, j int) bool { return samples[i].Timestamp < samples[j].Timestamp }) diff --git a/analysis/memory_leak_test.go b/analysis/memory_leak_test.go index f24f04d..59bdf1b 100644 --- a/analysis/memory_leak_test.go +++ b/analysis/memory_leak_test.go @@ -352,6 +352,53 @@ func TestMemoryLeakInsufficientEvidence(t *testing.T) { } } +func TestMemoryLeakAccountsForUnusableSeries(t *testing.T) { + for _, growing := range []bool{false, true} { + for _, tt := range []struct { + name string + mutate func(*Series) + reason Reason + }{ + {"empty", func(s *Series) { s.Samples = nil }, ReasonMissingUsage}, + {"outside lookback", func(s *Series) { s.Samples = s.Samples[:24*60] }, ReasonStaleUsage}, + {"stale within lookback", func(s *Series) { s.Samples = s.Samples[:len(s.Samples)-10] }, ReasonStaleUsage}, + } { + name := tt.name + "/flat companion" + if growing { + name = tt.name + "/growing companion" + } + t.Run(name, func(t *testing.T) { + in := leakFixture(48, func(h float64) float64 { + if growing { + return 100 + 12*h + } + return 100 + }) + c := &in.Containers[0] + other := c.Memory.Series[0] + other.ID, other.PodUID = "unusable", "unusable-pod" + tt.mutate(&other) + c.Memory.Series = append(c.Memory.Series, other) + leak := run(t, in, leakPolicy()).Results[0].MemoryLeak + want := MemoryLeakInsufficientData + if growing { + want = MemoryLeakPotential + } + if leak.Status != want || leak.EvaluatedEpisodes != 1 || leak.SkippedEpisodes != 1 || len(leak.Episodes) != 2 { + t.Fatalf("unusable series was lost: %+v", leak) + } + skipped := leak.Episodes[1] + if skipped.SeriesID != other.ID || skipped.PodUID != other.PodUID || skipped.Status != MemoryLeakInsufficientData || !has(DataQuality{Reasons: skipped.Reasons}, tt.reason) { + t.Fatalf("missing skipped-series evidence: %+v", skipped) + } + if !growing && !has(DataQuality{Reasons: leak.Reasons}, tt.reason) { + t.Fatalf("missing aggregate reason: %+v", leak) + } + }) + } + } +} + func TestMemoryLeakToleratesCollectionOutages(t *testing.T) { for _, minutes := range []int{15, 60} { in := leakFixture(24, func(h float64) float64 { return 100 + 12*h }) diff --git a/analysis/memory_leak_types.go b/analysis/memory_leak_types.go index 74cf43b..928a93a 100644 --- a/analysis/memory_leak_types.go +++ b/analysis/memory_leak_types.go @@ -4,7 +4,7 @@ package analysis -const MemoryLeakDetectorVersion = "2" +const MemoryLeakDetectorVersion = "3" // MemoryLeakPolicy enables an advisory heuristic, not a diagnosis. Zero fields // select defaults. Setting MemoryPolicy.LeakDetection to nil disables detection. diff --git a/analysis/oom.go b/analysis/oom.go index a4db3f0..e823588 100644 --- a/analysis/oom.go +++ b/analysis/oom.go @@ -20,15 +20,9 @@ func selectOOMKills(in Input, c ContainerObservation) ([]OOMKill, error) { if kill.Timestamp <= 0 { return nil, fmt.Errorf("OOM kill timestamp must be positive Unix milliseconds") } - if kill.Timestamp < start || kill.Timestamp > in.EvaluationTime { - continue - } if kill.WorkloadUID == "" || kill.Release == "" { return nil, fmt.Errorf("OOM kill workload UID and release are required") } - if kill.WorkloadUID != c.Target.WorkloadUID || kill.Release != c.Target.Release { - continue - } if kill.ID == "" { return nil, fmt.Errorf("OOM kill ID is required") } @@ -42,6 +36,11 @@ func selectOOMKills(in Input, c ContainerObservation) ([]OOMKill, error) { continue } seen[kill.ID] = kill + // Validate every supplied observation, including conflicts across the + // selection boundary, before deciding whether it affects this analysis. + if kill.Timestamp < start || kill.Timestamp > in.EvaluationTime || kill.WorkloadUID != c.Target.WorkloadUID || kill.Release != c.Target.Release { + continue + } kills = append(kills, kill) } sort.Slice(kills, func(i, j int) bool { diff --git a/analysis/oom_test.go b/analysis/oom_test.go index 4a7632e..436e0c7 100644 --- a/analysis/oom_test.go +++ b/analysis/oom_test.go @@ -302,6 +302,62 @@ func TestOOMDeterminismDeduplicationAndValidation(t *testing.T) { } } +func TestOOMValidatesObservationsBeforeFiltering(t *testing.T) { + for _, scope := range []struct { + name string + mutate func(*OOMKill) + }{ + {"before window", func(k *OOMKill) { k.Timestamp = epoch - 1 }}, + {"before release", func(k *OOMKill) { k.Timestamp = epoch + 1 }}, + {"future", func(k *OOMKill) { k.Timestamp = epoch + 600001 }}, + {"other workload", func(k *OOMKill) { k.WorkloadUID = "other" }}, + {"other release", func(k *OOMKill) { k.Release = "B" }}, + } { + for _, invalid := range []struct { + name string + mutate func(*OOMKill) + }{ + {"missing ID", func(k *OOMKill) { k.ID = "" }}, + {"missing UID", func(k *OOMKill) { k.WorkloadUID = "" }}, + {"missing release", func(k *OOMKill) { k.Release = "" }}, + {"negative limit", func(k *OOMKill) { k.MemoryLimitBytes = -1 }}, + {"NaN limit", func(k *OOMKill) { k.MemoryLimitBytes = math.NaN() }}, + {"infinite limit", func(k *OOMKill) { k.MemoryLimitBytes = math.Inf(1) }}, + } { + t.Run(scope.name+"/"+invalid.name, func(t *testing.T) { + in := fixture(600, 60) + in.Containers[0].Identity.ReleaseStartedAt = epoch + 60000 + kill := oomKill("oom", epoch+120000, 512*mib) + scope.mutate(&kill) + invalid.mutate(&kill) + in.Containers[0].OOMKills = []OOMKill{kill} + if _, err := Analyze(in, shortPolicy()); err == nil || !strings.Contains(err.Error(), "OOM") { + t.Fatalf("invalid filtered OOM was accepted: %v", err) + } + }) + } + t.Run(scope.name+"/conflicting ID", func(t *testing.T) { + in := fixture(600, 60) + in.Containers[0].Identity.ReleaseStartedAt = epoch + 60000 + selected := oomKill("oom", epoch+120000, 512*mib) + filtered := selected + scope.mutate(&filtered) + conflicting := filtered + conflicting.MemoryLimitBytes++ + for _, kills := range [][]OOMKill{{selected, filtered}, {filtered, selected}, {filtered, conflicting}} { + in.Containers[0].OOMKills = kills + if _, err := Analyze(in, shortPolicy()); err == nil || !strings.Contains(err.Error(), "conflicting observations for OOM kill") { + t.Fatalf("conflicting filtered OOMs were accepted: %v", err) + } + } + in.Containers[0].OOMKills = []OOMKill{filtered, filtered} + if r := run(t, in, shortPolicy()).Results[0]; len(r.Evidence.OOMKills) != 0 { + t.Fatalf("valid filtered duplicates affected sizing: %+v", r) + } + }) + } +} + func TestOOMPolicyNormalizationAndIdentity(t *testing.T) { for _, coefficient := range []float64{0, 1, 1.25, 2} { p, err := NormalizePolicy(Policy{Memory: MemoryPolicy{OOMKilledCoefficient: coefficient}}) diff --git a/analysis/samples.go b/analysis/samples.go index f1aedd2..4d60def 100644 --- a/analysis/samples.go +++ b/analysis/samples.go @@ -1,5 +1,6 @@ // Copyright Kedify Inc. // SPDX-License-Identifier: LicenseRef-Kedify-Commercial-1.0 AND LicenseRef-Kedify-Public-Source-1.0 +// See LICENSE and PUBLIC_SOURCE_LICENSE. package analysis import ( diff --git a/analysis/trace.go b/analysis/trace.go index 9f40556..5b3468c 100644 --- a/analysis/trace.go +++ b/analysis/trace.go @@ -1,5 +1,6 @@ // Copyright Kedify Inc. // SPDX-License-Identifier: LicenseRef-Kedify-Commercial-1.0 AND LicenseRef-Kedify-Public-Source-1.0 +// See LICENSE and PUBLIC_SOURCE_LICENSE. package analysis diff --git a/analysis/trace_test.go b/analysis/trace_test.go index f24fc8b..c130a22 100644 --- a/analysis/trace_test.go +++ b/analysis/trace_test.go @@ -1,9 +1,46 @@ +// Copyright Kedify Inc. +// SPDX-License-Identifier: LicenseRef-Kedify-Commercial-1.0 AND LicenseRef-Kedify-Public-Source-1.0 +// See LICENSE and PUBLIC_SOURCE_LICENSE. + package analysis import ( + "reflect" "testing" ) +func TestDecisionTraceCurrentSettingReasons(t *testing.T) { + for _, tt := range []struct { + name string + setting Setting + available bool + reason Reason + }{ + {"missing request", SettingRequests, false, ReasonMissingRequest}, + {"stale request", SettingRequests, true, ReasonStaleRequest}, + {"missing limit", SettingLimits, false, ReasonMissingLimit}, + {"stale limit", SettingLimits, true, ReasonStaleLimit}, + } { + t.Run(tt.name, func(t *testing.T) { + in := fixture(600, 60) + for _, obs := range []*ResourceObservation{&in.Containers[0].CPU, &in.Containers[0].Memory} { + signal := &obs.CurrentRequest + if tt.setting == SettingLimits { + signal = &obs.CurrentLimit + } + signal.Available, signal.Timestamp = tt.available, epoch + } + for _, result := range run(t, in, shortPolicy()).Results { + for _, trace := range result.DecisionTrace.Settings { + if trace.Setting == tt.setting && (trace.Disposition != "unavailable" || !reflect.DeepEqual(trace.Reasons, []Reason{tt.reason})) { + t.Fatalf("%s %s trace: %+v", result.Resource, tt.setting, trace) + } + } + } + }) + } +} + func TestDecisionTraceDispositions(t *testing.T) { for _, tt := range []struct { name string diff --git a/analysis/unset_test.go b/analysis/unset_test.go index a36fe2c..3bef889 100644 --- a/analysis/unset_test.go +++ b/analysis/unset_test.go @@ -1,3 +1,7 @@ +// Copyright Kedify Inc. +// SPDX-License-Identifier: LicenseRef-Kedify-Commercial-1.0 AND LicenseRef-Kedify-Public-Source-1.0 +// See LICENSE and PUBLIC_SOURCE_LICENSE. + package analysis import (