Move BucketSize constant back in to autoscaler config (knative#6838)

This avoids a circular dependency importing autoscaler/config in
certain places.

pkg/autoscaler/config/config.go

@@ -23,7 +23,6 @@ import (
 	"time"
 
 	"knative.dev/serving/pkg/apis/autoscaling"
-	"knative.dev/serving/pkg/autoscaler/metrics"
 
 	corev1 "k8s.io/api/core/v1"
 )
@@ -32,6 +31,11 @@ const (
 	// ConfigName is the name of the config map of the autoscaler.
 	ConfigName = "config-autoscaler"
 
+	// BucketSize is the size of the buckets of stats we create.
+	// NB: if this is more than 1s, we need to average values in the
+	// metrics buckets.
+	BucketSize = 1 * time.Second
+
 	defaultTargetUtilization = 0.7
 )
 
@@ -222,8 +226,8 @@ func validate(lc *Config) (*Config, error) {
 	}
 
 	effPW := time.Duration(lc.PanicWindowPercentage / 100 * float64(lc.StableWindow))
-	if effPW < metrics.BucketSize || effPW > lc.StableWindow {
-		return nil, fmt.Errorf("panic-window-percentage = %v, must be in [%v, 100] interval", lc.PanicWindowPercentage, 100*float64(metrics.BucketSize)/float64(lc.StableWindow))
+	if effPW < BucketSize || effPW > lc.StableWindow {
+		return nil, fmt.Errorf("panic-window-percentage = %v, must be in [%v, 100] interval", lc.PanicWindowPercentage, 100*float64(BucketSize)/float64(lc.StableWindow))
 	}
 
 	return lc, nil

pkg/autoscaler/metrics/collector.go

@@ -27,17 +27,13 @@ import (
 	"knative.dev/pkg/logging/logkey"
 	av1alpha1 "knative.dev/serving/pkg/apis/autoscaling/v1alpha1"
 	"knative.dev/serving/pkg/autoscaler/aggregation"
+	"knative.dev/serving/pkg/autoscaler/config"
 )
 
 const (
 	// scrapeTickInterval is the interval of time between triggering StatsScraper.Scrape()
 	// to get metrics across all pods of a revision.
 	scrapeTickInterval = time.Second
-
-	// BucketSize is the size of the buckets of stats we create.
-	// NB: if this is more than 1s, we need to average values in the
-	// metrics buckets.
-	BucketSize = scrapeTickInterval
 )
 
 var (
@@ -257,13 +253,13 @@ func newCollection(metric *av1alpha1.Metric, scraper StatsScraper, tickFactory f
 	c := &collection{
 		metric: metric,
 		concurrencyBuckets: aggregation.NewTimedFloat64Buckets(
-			metric.Spec.StableWindow, BucketSize),
+			metric.Spec.StableWindow, config.BucketSize),
 		concurrencyPanicBuckets: aggregation.NewTimedFloat64Buckets(
-			metric.Spec.PanicWindow, BucketSize),
+			metric.Spec.PanicWindow, config.BucketSize),
 		rpsBuckets: aggregation.NewTimedFloat64Buckets(
-			metric.Spec.StableWindow, BucketSize),
+			metric.Spec.StableWindow, config.BucketSize),
 		rpsPanicBuckets: aggregation.NewTimedFloat64Buckets(
-			metric.Spec.PanicWindow, BucketSize),
+			metric.Spec.PanicWindow, config.BucketSize),
 		scraper: scraper,
 
 		stopCh: make(chan struct{}),

pkg/autoscaler/metrics/collector_test.go

@@ -33,6 +33,7 @@ import (
 	av1alpha1 "knative.dev/serving/pkg/apis/autoscaling/v1alpha1"
 	"knative.dev/serving/pkg/apis/serving"
 	"knative.dev/serving/pkg/autoscaler/aggregation"
+	"knative.dev/serving/pkg/autoscaler/config"
 	"knative.dev/serving/pkg/autoscaler/fake"
 )
 
@@ -411,10 +412,10 @@ func TestMetricCollectorAggregate(t *testing.T) {
 	m.Spec.PanicWindow = 2 * time.Second
 	c := &collection{
 		metric:                  &m,
-		concurrencyBuckets:      aggregation.NewTimedFloat64Buckets(m.Spec.StableWindow, BucketSize),
-		concurrencyPanicBuckets: aggregation.NewTimedFloat64Buckets(m.Spec.PanicWindow, BucketSize),
-		rpsBuckets:              aggregation.NewTimedFloat64Buckets(m.Spec.StableWindow, BucketSize),
-		rpsPanicBuckets:         aggregation.NewTimedFloat64Buckets(m.Spec.PanicWindow, BucketSize),
+		concurrencyBuckets:      aggregation.NewTimedFloat64Buckets(m.Spec.StableWindow, config.BucketSize),
+		concurrencyPanicBuckets: aggregation.NewTimedFloat64Buckets(m.Spec.PanicWindow, config.BucketSize),
+		rpsBuckets:              aggregation.NewTimedFloat64Buckets(m.Spec.StableWindow, config.BucketSize),
+		rpsPanicBuckets:         aggregation.NewTimedFloat64Buckets(m.Spec.PanicWindow, config.BucketSize),
 	}
 	now := time.Now()
 	for i := 0; i < 10; i++ {

pkg/reconciler/autoscaling/resources/metric.go

@@ -24,7 +24,6 @@ import (
 	"knative.dev/pkg/kmeta"
 	"knative.dev/serving/pkg/apis/autoscaling/v1alpha1"
 	autoscalerconfig "knative.dev/serving/pkg/autoscaler/config"
-	"knative.dev/serving/pkg/autoscaler/metrics"
 	"knative.dev/serving/pkg/resources"
 )
 
@@ -50,8 +49,8 @@ func MakeMetric(ctx context.Context, pa *v1alpha1.PodAutoscaler, metricSvc strin
 		panicWindowPercentage = config.PanicWindowPercentage
 	}
 	panicWindow := time.Duration(float64(stableWindow) * panicWindowPercentage / 100.0).Round(time.Second)
-	if panicWindow < metrics.BucketSize {
-		panicWindow = metrics.BucketSize
+	if panicWindow < autoscalerconfig.BucketSize {
+		panicWindow = autoscalerconfig.BucketSize
 	}
 	return &v1alpha1.Metric{
 		ObjectMeta: metav1.ObjectMeta{

pkg/reconciler/autoscaling/resources/metric_test.go

@@ -27,7 +27,6 @@ import (
 	"knative.dev/serving/pkg/apis/autoscaling"
 	"knative.dev/serving/pkg/apis/autoscaling/v1alpha1"
 	autoscalerconfig "knative.dev/serving/pkg/autoscaler/config"
-	"knative.dev/serving/pkg/autoscaler/metrics"
 	. "knative.dev/serving/pkg/testing"
 )
 
@@ -47,7 +46,7 @@ func TestMakeMetric(t *testing.T) {
 		pa:   pa(WithWindowAnnotation("10s"), WithPanicWindowPercentageAnnotation("10")),
 		msn:  "wil",
 		want: metric(withScrapeTarget("wil"), withWindowAnnotation("10s"),
-			withStableWindow(10*time.Second), withPanicWindow(metrics.BucketSize),
+			withStableWindow(10*time.Second), withPanicWindow(autoscalerconfig.BucketSize),
 			withPanicWindowPercentageAnnotation("10")),
 	}, {
 		name: "with longer stable window, no panic window percentage, defaults to 10%",