Combine ready, not ready, terminating, and pending pod counter reporters

pkg/autoscaler/autoscaler_test.go

@@ -320,75 +320,38 @@ func TestAutoscalerUpdateTarget(t *testing.T) {
 
 type mockReporter struct{}
 
-// ReportDesiredPodCount of a mockReporter does nothing and return nil for error.
-func (r *mockReporter) ReportDesiredPodCount(v int64) error {
-	return nil
-}
+// ReportDesiredPodCount of a mockReporter does nothing.
+func (r *mockReporter) ReportDesiredPodCount(v int64) {}
 
-// ReportRequestedPodCount of a mockReporter does nothing and return nil for error.
-func (r *mockReporter) ReportRequestedPodCount(v int64) error {
-	return nil
-}
+// ReportRequestedPodCount of a mockReporter does nothing.
+func (r *mockReporter) ReportRequestedPodCount(v int64) {}
 
-// ReportActualPodCount of a mockReporter does nothing and return nil for error.
-func (r *mockReporter) ReportActualPodCount(v int64) error {
-	return nil
-}
+// ReportActualPodCount of a mockReporter does nothing.
+func (r *mockReporter) ReportActualPodCount(ready, notReady, terminating, pending int64) {}
 
-// ReportNotReadyPodCount of a mockReporter does nothing and return nil for error.
-func (r *mockReporter) ReportNotReadyPodCount(v int64) error {
-	return nil
-}
+// ReportStableRequestConcurrency of a mockReporter does nothing.
+func (r *mockReporter) ReportStableRequestConcurrency(v float64) {}
 
-// ReportPendingPodCount of a mockReporter does nothing and return nil for error.
-func (r *mockReporter) ReportPendingPodCount(v int64) error {
-	return nil
-}
+// ReportPanicRequestConcurrency of a mockReporter does nothing.
+func (r *mockReporter) ReportPanicRequestConcurrency(v float64) {}
 
-// ReportTerminatingPodCount of a mockReporter does nothing and return nil for error.
-func (r *mockReporter) ReportTerminatingPodCount(v int64) error {
-	return nil
-}
+// ReportStableRPS of a mockReporter does nothing.
+func (r *mockReporter) ReportStableRPS(v float64) {}
 
-// ReportStableRequestConcurrency of a mockReporter does nothing and return nil for error.
-func (r *mockReporter) ReportStableRequestConcurrency(v float64) error {
-	return nil
-}
+// ReportPanicRPS of a mockReporter does nothing.
+func (r *mockReporter) ReportPanicRPS(v float64) {}
 
-// ReportPanicRequestConcurrency of a mockReporter does nothing and return nil for error.
-func (r *mockReporter) ReportPanicRequestConcurrency(v float64) error {
-	return nil
-}
+// ReportTargetRPS of a mockReporter does nothing.
+func (r *mockReporter) ReportTargetRPS(v float64) {}
 
-// ReportStableRPS of a mockReporter does nothing and return nil for error.
-func (r *mockReporter) ReportStableRPS(v float64) error {
-	return nil
-}
+// ReportTargetRequestConcurrency of a mockReporter does nothing.
+func (r *mockReporter) ReportTargetRequestConcurrency(v float64) {}
 
-// ReportPanicRPS of a mockReporter does nothing and return nil for error.
-func (r *mockReporter) ReportPanicRPS(v float64) error {
-	return nil
-}
-
-// ReportTargetRPS of a mockReporter does nothing and return nil for error.
-func (r *mockReporter) ReportTargetRPS(v float64) error {
-	return nil
-}
-
-// ReportTargetRequestConcurrency of a mockReporter does nothing and return nil for error.
-func (r *mockReporter) ReportTargetRequestConcurrency(v float64) error {
-	return nil
-}
-
-// ReportPanic of a mockReporter does nothing and return nil for error.
-func (r *mockReporter) ReportPanic(v int64) error {
-	return nil
-}
+// ReportPanic of a mockReporter does nothing.
+func (r *mockReporter) ReportPanic(v int64) {}
 
-// ReportExcessBurstCapacity retports excess burst capacity.
-func (r *mockReporter) ReportExcessBurstCapacity(v float64) error {
-	return nil
-}
+// ReportExcessBurstCapacity of a mockReporter does nothing.
+func (r *mockReporter) ReportExcessBurstCapacity(v float64) {}
 
 func newTestAutoscaler(t *testing.T, targetValue, targetBurstCapacity float64, metrics MetricClient) *Autoscaler {
 	return newTestAutoscalerWithScalingMetric(t, targetValue, targetBurstCapacity, metrics, "concurrency")

pkg/autoscaler/stats_reporter.go

@@ -18,8 +18,6 @@ package autoscaler
 
 import (
 	"context"
-	"errors"
-
 	pkgmetrics "knative.dev/pkg/metrics"
 	"knative.dev/pkg/metrics/metricskey"
 	"knative.dev/serving/pkg/metrics"
@@ -188,26 +186,22 @@ func register() {
 
 // StatsReporter defines the interface for sending autoscaler metrics
 type StatsReporter interface {
-	ReportDesiredPodCount(v int64) error
-	ReportRequestedPodCount(v int64) error
-	ReportActualPodCount(v int64) error
-	ReportNotReadyPodCount(v int64) error
-	ReportTerminatingPodCount(v int64) error
-	ReportPendingPodCount(v int64) error
-	ReportStableRequestConcurrency(v float64) error
-	ReportPanicRequestConcurrency(v float64) error
-	ReportTargetRequestConcurrency(v float64) error
-	ReportStableRPS(v float64) error
-	ReportPanicRPS(v float64) error
-	ReportTargetRPS(v float64) error
-	ReportExcessBurstCapacity(v float64) error
-	ReportPanic(v int64) error
+	ReportDesiredPodCount(v int64)
+	ReportRequestedPodCount(v int64)
+	ReportActualPodCount(ready, notReady, terminating, pending int64)
+	ReportStableRequestConcurrency(v float64)
+	ReportPanicRequestConcurrency(v float64)
+	ReportTargetRequestConcurrency(v float64)
+	ReportStableRPS(v float64)
+	ReportPanicRPS(v float64)
+	ReportTargetRPS(v float64)
+	ReportExcessBurstCapacity(v float64)
+	ReportPanic(v int64)
 }
 
 // Reporter holds cached metric objects to report autoscaler metrics
 type Reporter struct {
-	ctx         context.Context
-	initialized bool
+	ctx context.Context
 }
 
 func valueOrUnknown(v string) string {
@@ -235,86 +229,68 @@ func NewStatsReporter(ns, service, config, revision string) (*Reporter, error) {
 	}
 
 	r.ctx = ctx
-	r.initialized = true
 	return r, nil
 }
 
 // ReportDesiredPodCount captures value v for desired pod count measure.
-func (r *Reporter) ReportDesiredPodCount(v int64) error {
-	return r.report(desiredPodCountM.M(v))
+func (r *Reporter) ReportDesiredPodCount(v int64) {
+	r.report(desiredPodCountM.M(v))
 }
 
 // ReportRequestedPodCount captures value v for requested pod count measure.
-func (r *Reporter) ReportRequestedPodCount(v int64) error {
-	return r.report(requestedPodCountM.M(v))
-}
-
-// ReportActualPodCount captures value v for actual pod count measure.
-func (r *Reporter) ReportActualPodCount(v int64) error {
-	return r.report(actualPodCountM.M(v))
-}
-
-// ReportNotReadyPodCount captures value v for not ready pod count measure.
-func (r *Reporter) ReportNotReadyPodCount(v int64) error {
-	return r.report(notReadyPodCountM.M(v))
+func (r *Reporter) ReportRequestedPodCount(v int64) {
+	r.report(requestedPodCountM.M(v))
 }
 
-// ReportPendingPodCount captures value v for pending pod count measure.
-func (r *Reporter) ReportPendingPodCount(v int64) error {
-	return r.report(pendingPodCountM.M(v))
-}
-
-// ReportTerminatingPodCount captures value v for terminating pod count measure.
-func (r *Reporter) ReportTerminatingPodCount(v int64) error {
-	return r.report(terminatingPodCountM.M(v))
+// ReportActualPodCount captures values for ready, not ready, terminating, and pending pod count measure.
+func (r *Reporter) ReportActualPodCount(ready, notReady, terminating, pending int64) {
+	r.report(actualPodCountM.M(ready))
+	r.report(notReadyPodCountM.M(notReady))
+	r.report(terminatingPodCountM.M(terminating))
+	r.report(pendingPodCountM.M(pending))
 }
 
 // ReportExcessBurstCapacity captures value v for excess target burst capacity.
-func (r *Reporter) ReportExcessBurstCapacity(v float64) error {
-	return r.report(excessBurstCapacityM.M(v))
+func (r *Reporter) ReportExcessBurstCapacity(v float64) {
+	r.report(excessBurstCapacityM.M(v))
 }
 
 // ReportStableRequestConcurrency captures value v for stable request concurrency measure.
-func (r *Reporter) ReportStableRequestConcurrency(v float64) error {
-	return r.report(stableRequestConcurrencyM.M(v))
+func (r *Reporter) ReportStableRequestConcurrency(v float64) {
+	r.report(stableRequestConcurrencyM.M(v))
 }
 
 // ReportPanicRequestConcurrency captures value v for panic request concurrency measure.
-func (r *Reporter) ReportPanicRequestConcurrency(v float64) error {
-	return r.report(panicRequestConcurrencyM.M(v))
+func (r *Reporter) ReportPanicRequestConcurrency(v float64) {
+	r.report(panicRequestConcurrencyM.M(v))
 }
 
 // ReportTargetRequestConcurrency captures value v for target request concurrency measure.
-func (r *Reporter) ReportTargetRequestConcurrency(v float64) error {
-	return r.report(targetRequestConcurrencyM.M(v))
+func (r *Reporter) ReportTargetRequestConcurrency(v float64) {
+	r.report(targetRequestConcurrencyM.M(v))
 }
 
 // ReportStableRPS captures value v for stable RPS measure.
-func (r *Reporter) ReportStableRPS(v float64) error {
-	return r.report(stableRPSM.M(v))
+func (r *Reporter) ReportStableRPS(v float64) {
+	r.report(stableRPSM.M(v))
 }
 
 // ReportPanicRPS captures value v for panic RPS measure.
-func (r *Reporter) ReportPanicRPS(v float64) error {
-	return r.report(panicRPSM.M(v))
+func (r *Reporter) ReportPanicRPS(v float64) {
+	r.report(panicRPSM.M(v))
 }
 
 // ReportTargetRPS captures value v for target requests-per-second measure.
-func (r *Reporter) ReportTargetRPS(v float64) error {
-	return r.report(targetRPSM.M(v))
+func (r *Reporter) ReportTargetRPS(v float64) {
+	r.report(targetRPSM.M(v))
 
 }
 
 // ReportPanic captures value v for panic mode measure.
-func (r *Reporter) ReportPanic(v int64) error {
-	return r.report(panicM.M(v))
+func (r *Reporter) ReportPanic(v int64) {
+	r.report(panicM.M(v))
 }
 
-func (r *Reporter) report(m stats.Measurement) error {
-	if !r.initialized {
-		return errors.New("StatsReporter is not initialized yet")
-	}
-
+func (r *Reporter) report(m stats.Measurement) {
 	pkgmetrics.Record(r.ctx, m)
-	return nil
 }

pkg/autoscaler/stats_reporter_test.go

@@ -42,9 +42,6 @@ func TestNewStatsReporterErrors(t *testing.T) {
 func TestReporterReport(t *testing.T) {
 	resetMetrics()
 	r := &Reporter{}
-	if err := r.ReportDesiredPodCount(10); err == nil {
-		t.Error("Reporter.ReportDesiredPodCount() expected an error for Report call before init. Got success.")
-	}
 
 	r, _ = NewStatsReporter("testns", "testsvc", "testconfig", "testrev")
 	wantTags := map[string]string{
@@ -55,20 +52,17 @@ func TestReporterReport(t *testing.T) {
 	}
 
 	// Send statistics only once and observe the results
-	expectSuccess(t, "ReportDesiredPodCount", func() error { return r.ReportDesiredPodCount(10) })
-	expectSuccess(t, "ReportRequestedPodCount", func() error { return r.ReportRequestedPodCount(7) })
-	expectSuccess(t, "ReportActualPodCount", func() error { return r.ReportActualPodCount(5) })
-	expectSuccess(t, "ReportNotReadyPodCount", func() error { return r.ReportNotReadyPodCount(9) })
-	expectSuccess(t, "ReportPendingPodCount", func() error { return r.ReportPendingPodCount(6) })
-	expectSuccess(t, "ReportTerminatingPodCount", func() error { return r.ReportTerminatingPodCount(8) })
-	expectSuccess(t, "ReportPanic", func() error { return r.ReportPanic(0) })
-	expectSuccess(t, "ReportStableRequestConcurrency", func() error { return r.ReportStableRequestConcurrency(2) })
-	expectSuccess(t, "ReportPanicRequestConcurrency", func() error { return r.ReportPanicRequestConcurrency(3) })
-	expectSuccess(t, "ReportTargetRequestConcurrency", func() error { return r.ReportTargetRequestConcurrency(0.9) })
-	expectSuccess(t, "ReportStableRPS", func() error { return r.ReportStableRPS(5) })
-	expectSuccess(t, "ReportPanicRPS", func() error { return r.ReportPanicRPS(6) })
-	expectSuccess(t, "ReportTargetRPS", func() error { return r.ReportTargetRPS(7) })
-	expectSuccess(t, "ReportExcessBurstCapacity", func() error { return r.ReportExcessBurstCapacity(19.84) })
+	r.ReportDesiredPodCount(10)
+	r.ReportRequestedPodCount(7)
+	r.ReportActualPodCount(5 /* ready */, 9 /* notReady */, 8 /* terminating */, 6 /* pending */)
+	r.ReportPanic(0)
+	r.ReportStableRequestConcurrency(2)
+	r.ReportPanicRequestConcurrency(3)
+	r.ReportTargetRequestConcurrency(0.9)
+	r.ReportStableRPS(5)
+	r.ReportPanicRPS(6)
+	r.ReportTargetRPS(7)
+	r.ReportExcessBurstCapacity(19.84)
 	metricstest.CheckLastValueData(t, "desired_pods", wantTags, 10)
 	metricstest.CheckLastValueData(t, "requested_pods", wantTags, 7)
 	metricstest.CheckLastValueData(t, "actual_pods", wantTags, 5)
@@ -85,42 +79,42 @@ func TestReporterReport(t *testing.T) {
 	metricstest.CheckLastValueData(t, "target_requests_per_second", wantTags, 7)
 
 	// All the stats are gauges - record multiple entries for one stat - last one should stick
-	expectSuccess(t, "ReportDesiredPodCount", func() error { return r.ReportDesiredPodCount(1) })
-	expectSuccess(t, "ReportDesiredPodCount", func() error { return r.ReportDesiredPodCount(2) })
-	expectSuccess(t, "ReportDesiredPodCount", func() error { return r.ReportDesiredPodCount(3) })
+	r.ReportDesiredPodCount(1)
+	r.ReportDesiredPodCount(2)
+	r.ReportDesiredPodCount(3)
 	metricstest.CheckLastValueData(t, "desired_pods", wantTags, 3)
 
-	expectSuccess(t, "ReportRequestedPodCount", func() error { return r.ReportRequestedPodCount(4) })
-	expectSuccess(t, "ReportRequestedPodCount", func() error { return r.ReportRequestedPodCount(5) })
-	expectSuccess(t, "ReportRequestedPodCount", func() error { return r.ReportRequestedPodCount(6) })
+	r.ReportRequestedPodCount(4)
+	r.ReportRequestedPodCount(5)
+	r.ReportRequestedPodCount(6)
 	metricstest.CheckLastValueData(t, "requested_pods", wantTags, 6)
 
-	expectSuccess(t, "ReportActualPodCount", func() error { return r.ReportActualPodCount(7) })
-	expectSuccess(t, "ReportActualPodCount", func() error { return r.ReportActualPodCount(8) })
-	expectSuccess(t, "ReportActualPodCount", func() error { return r.ReportActualPodCount(9) })
+	r.ReportActualPodCount(7 /* ready */, 0 /* notReady */, 0 /* terminating */, 0 /* pending */)
+	r.ReportActualPodCount(8 /* ready */, 0 /* notReady */, 0 /* terminating */, 0 /* pending */)
+	r.ReportActualPodCount(9 /* ready */, 0 /* notReady */, 0 /* terminating */, 0 /* pending */)
 	metricstest.CheckLastValueData(t, "actual_pods", wantTags, 9)
 
-	expectSuccess(t, "ReportNotReadyPodCount", func() error { return r.ReportNotReadyPodCount(6) })
-	expectSuccess(t, "ReportNotReadyPodCount", func() error { return r.ReportNotReadyPodCount(5) })
-	expectSuccess(t, "ReportNotReadyPodCount", func() error { return r.ReportNotReadyPodCount(4) })
+	r.ReportActualPodCount(0 /* ready */, 6 /* notReady */, 0 /* terminating */, 0 /* pending */)
+	r.ReportActualPodCount(0 /* ready */, 5 /* notReady */, 0 /* terminating */, 0 /* pending */)
+	r.ReportActualPodCount(0 /* ready */, 4 /* notReady */, 0 /* terminating */, 0 /* pending */)
 	metricstest.CheckLastValueData(t, "not_ready_pods", wantTags, 4)
 
-	expectSuccess(t, "ReportPendingPodCount", func() error { return r.ReportPendingPodCount(3) })
-	expectSuccess(t, "ReportPendingPodCount", func() error { return r.ReportPendingPodCount(2) })
-	expectSuccess(t, "ReportPendingPodCount", func() error { return r.ReportPendingPodCount(1) })
+	r.ReportActualPodCount(0 /* ready */, 0 /* notReady */, 0 /* terminating */, 3 /* pending */)
+	r.ReportActualPodCount(0 /* ready */, 0 /* notReady */, 0 /* terminating */, 2 /* pending */)
+	r.ReportActualPodCount(0 /* ready */, 0 /* notReady */, 0 /* terminating */, 1 /* pending */)
 	metricstest.CheckLastValueData(t, "pending_pods", wantTags, 1)
 
-	expectSuccess(t, "ReportTerminatingPodCount", func() error { return r.ReportTerminatingPodCount(5) })
-	expectSuccess(t, "ReportTerminatingPodCount", func() error { return r.ReportTerminatingPodCount(3) })
-	expectSuccess(t, "ReportTerminatingPodCount", func() error { return r.ReportTerminatingPodCount(8) })
+	r.ReportActualPodCount(0 /* ready */, 0 /* notReady */, 5 /* terminating */, 0 /* pending */)
+	r.ReportActualPodCount(0 /* ready */, 0 /* notReady */, 3 /* terminating */, 0 /* pending */)
+	r.ReportActualPodCount(0 /* ready */, 0 /* notReady */, 8 /* terminating */, 0 /* pending */)
 	metricstest.CheckLastValueData(t, "terminating_pods", wantTags, 8)
 
-	expectSuccess(t, "ReportPanic", func() error { return r.ReportPanic(1) })
-	expectSuccess(t, "ReportPanic", func() error { return r.ReportPanic(0) })
-	expectSuccess(t, "ReportPanic", func() error { return r.ReportPanic(1) })
+	r.ReportPanic(1)
+	r.ReportPanic(0)
+	r.ReportPanic(1)
 	metricstest.CheckLastValueData(t, "panic_mode", wantTags, 1)
 
-	expectSuccess(t, "ReportPanic", func() error { return r.ReportPanic(0) })
+	r.ReportPanic(0)
 	metricstest.CheckLastValueData(t, "panic_mode", wantTags, 0)
 }
 
@@ -134,16 +128,10 @@ func TestReporterEmptyServiceName(t *testing.T) {
 		metricskey.LabelConfigurationName: "testconfig",
 		metricskey.LabelRevisionName:      "testrev",
 	}
-	expectSuccess(t, "ReportDesiredPodCount", func() error { return r.ReportDesiredPodCount(10) })
+	r.ReportDesiredPodCount(10)
 	metricstest.CheckLastValueData(t, "desired_pods", wantTags, 10)
 }
 
-func expectSuccess(t *testing.T, funcName string, f func() error) {
-	if err := f(); err != nil {
-		t.Errorf("Reporter.%v() expected success but got error %v", funcName, err)
-	}
-}
-
 // Resets global state from the opencensus package
 // Required to run at the beginning of tests that check metrics' values
 // to make the tests idempotent.

pkg/reconciler/autoscaling/kpa/kpa.go

@@ -273,10 +273,7 @@ func reportMetrics(pa *pav1alpha1.PodAutoscaler, pc podCounts) error {
 		return err
 	}
 
-	reporter.ReportActualPodCount(int64(pc.ready))
-	reporter.ReportNotReadyPodCount(int64(pc.notReady))
-	reporter.ReportPendingPodCount(int64(pc.pending))
-	reporter.ReportTerminatingPodCount(int64(pc.terminating))
+	reporter.ReportActualPodCount(int64(pc.ready), int64(pc.notReady), int64(pc.pending), int64(pc.terminating))
 	// Negative "want" values represent an empty metrics pipeline and thus no specific request is being made.
 	if pc.want >= 0 {
 		reporter.ReportRequestedPodCount(int64(pc.want))