fix: treat prometheus counters as rates in autoscaling signals by nXtCyberNet · Pull Request #1042 · volcano-sh/kthena

nXtCyberNet · 2026-05-13T16:34:19Z

What type of PR is this?
/kind bug
/kind enhancement
What this PR does / why we need it:

Prometheus counter metrics are monotonically increasing cumulative values.
The autoscaler was treating these raw cumulative values as instantaneous load
signals, causing two critical failures:

Scale-up runaway: A pod that handled 50 total requests since startup would
trigger scaling to 5 replicas (ceil(50 / target)), regardless of current traffic.
No scale-down: Even after traffic stops, the counter value persists at 50,
preventing the autoscaler from ever scaling back down.

The fix: Track per-pod counter snapshots across scrape cycles and compute the
rate of change (delta/elapsed_seconds) instead of the raw cumulative value.
This correctly reflects instantaneous demand and enables both scale-up and scale-down.

Implementation Details:

Added CounterMap and ScrapeTimestamp fields to HistogramInfo to maintain
per-pod/metric baseline state across scrape cycles.
New rate calculation in metric collection:

rate = (current_value - previous_value) / elapsed_seconds

Counter resets (current < previous) detected and handled by clamping rate to 0.
First scrape returns rate=0 until baseline is established.
Added GetLastUnfreshSnapshotWithTimestamp() to SnapshotSlidingWindow
to expose precise per-pod scrape timestamps (more accurate than window-level timestamps).
Backward compatibility: Nil CounterMap guards protect in-memory snapshots
created before this change.

Which issue(s) this PR fixes:
Fixes #1037

Special notes for your reviewer:

Why per-pod timestamps? The sliding-window-level timestamp is too coarse;
per-pod scrape times give us the actual elapsed duration for each counter,
improving rate precision when scrape intervals vary.
Counter reset handling: If a pod restarts, its counter resets to 0.
Detecting current < previous avoids reporting a massive negative rate;
returning 0 is safe and gives the pod a grace period to re-accumulate load signals.
Backward compatibility: Any in-memory snapshots from before this change
will have CounterMap == nil. These are safely handled with a nil-check guard.
Testing recommendation: Add bench tests for rate calculation under
varying scrape intervals and counter reset scenarios.

Does this PR introduce a user-facing change?:
Yes. Autoscaling behavior for counter-based metrics will change—scaling will now
respond to instantaneous rate of change rather than cumulative totals, enabling
proper scale-down behavior.

Fix autoscaler counter metric handling: Prometheus counter metrics are now 
correctly treated as rates (delta/elapsed_seconds) instead of raw cumulative 
values. This fixes runaway scale-up and enables proper scale-down when traffic stops.

Signed-off-by: nXtCyberNet <rohantech2005@gmail.com>

volcano-sh-bot · 2026-05-13T16:34:27Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign git-malu for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

pkg/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

gemini-code-assist

Code Review

This pull request updates the metric collector to calculate rates for Prometheus counters by comparing current values with previous snapshots, introducing a new GetLastUnfreshSnapshotWithTimestamp method in the sliding window structure to track scrape intervals. The reviewer suggested improving the precision of rate calculations by using a single consistent timestamp per pod scrape rather than calling the timestamp function multiple times, and recommended refactoring GetLastUnfreshSnapshot to utilize the new method to reduce code duplication.

gemini-code-assist · 2026-05-13T16:37:46Z

+			collector.processPrometheusString(result, pastHistogramMap, pastCounterMap, currentHistogramMap, currentCounterMap, pastScrapeTimestamp, instanceInfo.MetricsMap)
 			(*currentHistograms)[pod.Name] = HistogramInfo{
-				PodStartTime: pod.Status.StartTime,
-				HistogramMap: currentHistogramMap,
+				PodStartTime:    pod.Status.StartTime,
+				HistogramMap:    currentHistogramMap,
+				CounterMap:      currentCounterMap,
+				ScrapeTimestamp: util.GetCurrentTimestamp(),
 			}


For improved precision in rate calculation, it's better to determine the current scrape timestamp once per pod and use it consistently. Currently, util.GetCurrentTimestamp() is called inside processPrometheusString for each metric, and again when creating HistogramInfo. This can introduce minor inaccuracies because the timestamp used for the rate calculation (now) will be slightly different from the timestamp stored for the next cycle (ScrapeTimestamp).

To improve this, you can get the timestamp once before processing the metrics and use it in both places. This ensures the elapsed_seconds for the rate calculation is based on the exact interval between the stored scrape timestamps.

Example of the proposed change:

// In fetchMetricsFromPods: ... result := string(bodyStr) now := util.GetCurrentTimestamp() collector.processPrometheusString(result, pastHistogramMap, pastCounterMap, currentHistogramMap, currentCounterMap, pastScrapeTimestamp, now, instanceInfo.MetricsMap) (*currentHistograms)[pod.Name] = HistogramInfo{ PodStartTime: pod.Status.StartTime, HistogramMap: currentHistogramMap, CounterMap: currentCounterMap, ScrapeTimestamp: now, } ... // And update processPrometheusString signature and body: func (c *MetricCollector) processPrometheusString(..., pastScrapeTimestamp int64, now int64, instanceMetricMap algorithm.Metrics) { // ... // inside case ..._COUNTER: // remove: now := util.GetCurrentTimestamp() // ... }

gemini-code-assist · 2026-05-13T16:37:46Z

+func (window *SnapshotSlidingWindow[T]) GetLastUnfreshSnapshotWithTimestamp() (value T, timestamp int64, ok bool) {
+	if window.freshMilliseconds == 0 {
+		return value, 0, false
+	}
+	currentTimestamp := window.getCurrentTimestamp()
+	window.expire(currentTimestamp)
+	if window.pool.Len() == 0 {
+		return value, 0, false
+	}
+	front := window.pool.Front()
+	if isFresh(window.freshMilliseconds, currentTimestamp, front.timestamp) {
+		return value, 0, false
+	}
+	return front.value, front.timestamp, true
+}


The logic inside this new function is nearly identical to GetLastUnfreshSnapshot. To reduce code duplication and improve maintainability, GetLastUnfreshSnapshot could be refactored to call this new function and discard the returned timestamp. This would make the code more aligned with the DRY (Don't Repeat Yourself) principle.

hzxuzhonghu · 2026-05-18T02:20:39Z

Thanks for the analysis, can you add some test coverage

hzxuzhonghu · 2026-05-18T02:21:38Z

From another view, it reflects the current api does not fit all.

// AutoscalingPolicyMetric defines a metric and its target value for scaling decisions.
type AutoscalingPolicyMetric struct {
	// MetricName defines the name of the metric to monitor for scaling decisions.
	MetricName string `json:"metricName"`
	// TargetValue defines the target value for the metric that triggers scaling operations.
	TargetValue resource.Quantity `json:"targetValue"`
}

nXtCyberNet · 2026-05-18T04:24:37Z

Hi @hzxuzhonghu, thanks for the review.
I apologize—I didn't fully consider the design implications. Before I proceed, I want to understand the roadmap better. Do you think adding a MetricType field to the API would be a good long-term solution? If you believe it's worth doing, I'm happy to include it in this PR.
However, if adding MetricType would conflict with the existing design (where histogram is the default), I think the best approach is to close this PR for now. Adding counter support without explicit API-level type declaration could introduce ambiguity and break existing behavior.
What are your thoughts on the right path forward?

hzxuzhonghu · 2026-05-18T07:13:33Z

@nXtCyberNet I donot have a good suggestion now, but will deep dive into other scalers first

nXtCyberNet · 2026-05-18T07:23:28Z

Okay, I'll wait for your response. In the meantime, I'll add the test coverage you requested. Thanks!

nXtCyberNet · 2026-06-15T04:54:26Z

@hzxuzhonghu any updates ?

nXtCyberNet added 2 commits May 13, 2026 20:41

added counter delta

befea95

Signed-off-by: nXtCyberNet <rohantech2005@gmail.com>

added counter delta

17fb007

Signed-off-by: nXtCyberNet <rohantech2005@gmail.com>

Copilot AI review requested due to automatic review settings May 13, 2026 16:34

volcano-sh-bot added kind/bug kind/enhancement New feature or request labels May 13, 2026

volcano-sh-bot requested review from YaoZengzeng and git-malu May 13, 2026 16:34

volcano-sh-bot added the size/M label May 13, 2026

Copilot started reviewing on behalf of nXtCyberNet May 13, 2026 16:34 View session

Copilot AI reviewed May 13, 2026

gemini-code-assist Bot reviewed May 13, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: treat prometheus counters as rates in autoscaling signals#1042

fix: treat prometheus counters as rates in autoscaling signals#1042
nXtCyberNet wants to merge 2 commits into
volcano-sh:mainfrom
nXtCyberNet:issue/counter

nXtCyberNet commented May 13, 2026

Uh oh!

volcano-sh-bot commented May 13, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot May 13, 2026

Uh oh!

gemini-code-assist Bot May 13, 2026

Uh oh!

hzxuzhonghu commented May 18, 2026

Uh oh!

hzxuzhonghu commented May 18, 2026

Uh oh!

nXtCyberNet commented May 18, 2026

Uh oh!

hzxuzhonghu commented May 18, 2026

Uh oh!

nXtCyberNet commented May 18, 2026

Uh oh!

nXtCyberNet commented Jun 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

nXtCyberNet commented May 13, 2026

Uh oh!

volcano-sh-bot commented May 13, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot May 13, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 13, 2026

Choose a reason for hiding this comment

Uh oh!

hzxuzhonghu commented May 18, 2026

Uh oh!

hzxuzhonghu commented May 18, 2026

Uh oh!

nXtCyberNet commented May 18, 2026

Uh oh!

hzxuzhonghu commented May 18, 2026

Uh oh!

nXtCyberNet commented May 18, 2026

Uh oh!

nXtCyberNet commented Jun 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants