Remove time from stat struct as it's never sent anyway. (#8410)

cmd/autoscaler/main.go

@@ -165,7 +165,7 @@ func main() {
 
 	go func() {
 		for sm := range statsCh {
-			collector.Record(sm.Key, sm.Stat)
+			collector.Record(sm.Key, time.Now(), sm.Stat)
 			multiScaler.Poke(sm.Key, sm.Stat)
 		}
 	}()

pkg/autoscaler/metrics/collector.go

@@ -48,9 +48,6 @@ type StatsScraperFactory func(*av1alpha1.Metric, *zap.SugaredLogger) (StatsScrap
 
 // Stat defines a single measurement at a point in time
 type Stat struct {
-	// The time the data point was received by autoscaler.
-	Time time.Time
-
 	// The unique identity of this pod.  Used to count how many pods
 	// are contributing to the metrics.
 	PodName string
@@ -86,7 +83,7 @@ type Collector interface {
 	// it already exist.
 	CreateOrUpdate(*av1alpha1.Metric) error
 	// Record allows stats to be captured that came from outside the Collector.
-	Record(key types.NamespacedName, stat Stat)
+	Record(key types.NamespacedName, now time.Time, stat Stat)
 	// Delete deletes a Metric and halts collection.
 	Delete(string, string) error
 	// Watch registers a singleton function to call when a specific collector's status changes.
@@ -169,12 +166,12 @@ func (c *MetricCollector) Delete(namespace, name string) error {
 }
 
 // Record records a stat that's been generated outside of the metric collector.
-func (c *MetricCollector) Record(key types.NamespacedName, stat Stat) {
+func (c *MetricCollector) Record(key types.NamespacedName, now time.Time, stat Stat) {
 	c.collectionsMutex.RLock()
 	defer c.collectionsMutex.RUnlock()
 
 	if collection, exists := c.collections[key]; exists {
-		collection.record(stat)
+		collection.record(now, stat)
 	}
 }
 
@@ -319,7 +316,7 @@ func newCollection(metric *av1alpha1.Metric, scraper StatsScraper, tickFactory f
 					callback(key)
 				}
 				if stat != emptyStat {
-					c.record(stat)
+					c.record(time.Now(), stat)
 				}
 			}
 		}
@@ -375,15 +372,15 @@ func (c *collection) lastError() error {
 }
 
 // record adds a stat to the current collection.
-func (c *collection) record(stat Stat) {
+func (c *collection) record(now time.Time, stat Stat) {
 	// Proxied requests have been counted at the activator. Subtract
 	// them to avoid double counting.
 	concurr := stat.AverageConcurrentRequests - stat.AverageProxiedConcurrentRequests
-	c.concurrencyBuckets.Record(stat.Time, concurr)
-	c.concurrencyPanicBuckets.Record(stat.Time, concurr)
+	c.concurrencyBuckets.Record(now, concurr)
+	c.concurrencyPanicBuckets.Record(now, concurr)
 	rps := stat.RequestCount - stat.ProxiedRequestCount
-	c.rpsBuckets.Record(stat.Time, rps)
-	c.rpsPanicBuckets.Record(stat.Time, rps)
+	c.rpsBuckets.Record(now, rps)
+	c.rpsPanicBuckets.Record(now, rps)
 }
 
 // add adds the stats from `src` to `dst`.

pkg/autoscaler/metrics/collector_test.go

@@ -133,7 +133,6 @@ func TestMetricCollectorScraper(t *testing.T) {
 		wantPRPS          = 3 * 20
 	)
 	stat := Stat{
-		Time:                      now,
 		PodName:                   "testPod",
 		AverageConcurrentRequests: reportConcurrency,
 		RequestCount:              reportRPS,
@@ -221,7 +220,6 @@ func TestMetricCollectorNoScraper(t *testing.T) {
 	metricKey := types.NamespacedName{Namespace: defaultNamespace, Name: defaultName}
 	const wantStat = 0.
 	stat := Stat{
-		Time:                      now,
 		PodName:                   "testPod",
 		AverageConcurrentRequests: wantStat,
 		RequestCount:              wantStat,
@@ -268,7 +266,7 @@ func TestMetricCollectorNoScraper(t *testing.T) {
 	stat.RequestCount = wantRC
 	stat.AverageConcurrentRequests = wantAverageRC
 
-	coll.Record(metricKey, stat)
+	coll.Record(metricKey, now, stat)
 
 	gotConcurrency, _, _ = coll.StableAndPanicConcurrency(metricKey, now)
 	gotRPS, _, err := coll.StableAndPanicRPS(metricKey, now)
@@ -290,7 +288,6 @@ func TestMetricCollectorNoDataError(t *testing.T) {
 	metricKey := types.NamespacedName{Namespace: defaultNamespace, Name: defaultName}
 	const wantStat = 0.
 	stat := Stat{
-		Time:                      now,
 		PodName:                   "testPod",
 		AverageConcurrentRequests: wantStat,
 		RequestCount:              wantStat,
@@ -323,13 +320,11 @@ func TestMetricCollectorRecord(t *testing.T) {
 	metricKey := types.NamespacedName{Namespace: defaultNamespace, Name: defaultName}
 	const want = 10.0
 	outdatedStat := Stat{
-		Time:                      oldTime,
 		PodName:                   "testPod",
 		AverageConcurrentRequests: 100,
 		RequestCount:              100,
 	}
 	stat := Stat{
-		Time:                             now,
 		PodName:                          "testPod",
 		AverageConcurrentRequests:        want + 10,
 		AverageProxiedConcurrentRequests: 10, // this should be subtracted from the above.
@@ -360,8 +355,8 @@ func TestMetricCollectorRecord(t *testing.T) {
 
 	// Add two stats. The second record operation will remove the first outdated one.
 	// After this the concurrencies are calculated correctly.
-	coll.Record(metricKey, outdatedStat)
-	coll.Record(metricKey, stat)
+	coll.Record(metricKey, oldTime, outdatedStat)
+	coll.Record(metricKey, now, stat)
 	stable, panic, err := coll.StableAndPanicConcurrency(metricKey, now)
 	if err != nil {
 		t.Fatal("StableAndPanicConcurrency:", err)
@@ -508,12 +503,11 @@ func TestMetricCollectorAggregate(t *testing.T) {
 	now := time.Now()
 	for i := time.Duration(0); i < 10; i++ {
 		stat := Stat{
-			Time:                      now.Add(i * time.Second),
 			PodName:                   "testPod",
 			AverageConcurrentRequests: float64(i + 5),
 			RequestCount:              float64(i + 5),
 		}
-		c.record(stat)
+		c.record(now.Add(i*time.Second), stat)
 	}
 
 	now = now.Add(9 * time.Second)

pkg/autoscaler/metrics/stats_scraper.go

@@ -297,7 +297,6 @@ func (s *serviceScraper) scrapePods(readyPods int) (Stat, error) {
 
 func computeAverages(results <-chan Stat, sample, total float64) Stat {
 	ret := Stat{
-		Time:    time.Now(),
 		PodName: scraperPodName,
 	}
 
@@ -379,7 +378,6 @@ func (s *serviceScraper) scrapeService(window time.Duration, readyPods int) (Sta
 	close(youngStatCh)
 
 	ret := Stat{
-		Time:    time.Now(),
 		PodName: scraperPodName,
 	}
 

pkg/autoscaler/metrics/stats_scraper_test.go

@@ -189,16 +189,11 @@ func TestPodDirectScrapeSuccess(t *testing.T) {
 	}
 	fake.Endpoints(3, fake.TestService)
 	makePods(3)
-	now := time.Now()
 
-	got, err := scraper.Scrape(defaultMetric.Spec.StableWindow)
-	if err != nil {
+	if _, err := scraper.Scrape(defaultMetric.Spec.StableWindow); err != nil {
 		t.Fatal("Unexpected error from scraper.Scrape():", err)
 	}
 
-	if got.Time.Before(now) {
-		t.Errorf("stat.Time=%v, want greater than %v", got.Time, now)
-	}
 	if !scraper.podsAddressable {
 		t.Error("PodAddressable switched to false")
 	}
@@ -216,16 +211,12 @@ func TestPodDirectScrapeSomeFailButSuccess(t *testing.T) {
 	}
 	fake.Endpoints(5, fake.TestService)
 	makePods(5)
-	now := time.Now()
 
 	got, err := scraper.Scrape(defaultMetric.Spec.StableWindow)
 	if err != nil {
 		t.Fatal("Unexpected error from scraper.Scrape():", err)
 	}
 
-	if got.Time.Before(now) {
-		t.Errorf("stat.Time=%v, want greater than %v", got.Time, now)
-	}
 	// Checking one of the metrics is enough here.
 	if got.AverageConcurrentRequests != 20.0 {
 		t.Errorf("stat.AverageConcurrentRequests=%v, want %v",
@@ -256,16 +247,12 @@ func TestPodDirectScrapeNoneSucceed(t *testing.T) {
 	}
 	fake.Endpoints(4, fake.TestService)
 	makePods(4)
-	now := time.Now()
 
 	got, err := scraper.Scrape(defaultMetric.Spec.StableWindow)
 	if err != nil {
 		t.Fatal("Unexpected error from scraper.Scrape():", err)
 	}
 
-	if got.Time.Before(now) {
-		t.Errorf("stat.Time=%v, want greater than %v", got.Time, now)
-	}
 	// Checking one of the metrics is enough here.
 	if got.AverageConcurrentRequests != 20.0 {
 		t.Errorf("stat.AverageConcurrentRequests=%v, want %v",
@@ -309,16 +296,11 @@ func TestScrapeReportStatWhenAllCallsSucceed(t *testing.T) {
 	// Make an Endpoints with 3 pods.
 	fake.Endpoints(3, fake.TestService)
 
-	// Scrape will set a timestamp bigger than this.
-	now := time.Now()
 	got, err := scraper.Scrape(defaultMetric.Spec.StableWindow)
 	if err != nil {
 		t.Fatal("Unexpected error from scraper.Scrape():", err)
 	}
 
-	if got.Time.Before(now) {
-		t.Errorf("stat.Time=%v, want greater than %v", got.Time, now)
-	}
 	checkBaseStat(t, got)
 }
 
@@ -341,16 +323,11 @@ func TestScrapeAllPodsYoungPods(t *testing.T) {
 
 	fake.Endpoints(numP, fake.TestService)
 
-	// Scrape will set a timestamp bigger than this.
-	now := time.Now()
 	got, err := scraper.Scrape(defaultMetric.Spec.StableWindow)
 	if err != nil {
 		t.Fatal("Unexpected error from scraper.Scrape():", err)
 	}
 
-	if got.Time.Before(now) {
-		t.Errorf("stat.Time=%v, want bigger than %v", got.Time, now)
-	}
 	if got.PodName != scraperPodName {
 		t.Errorf("stat.PodName=%v, want %v", got.PodName, scraperPodName)
 	}
@@ -374,16 +351,11 @@ func TestScrapeAllPodsOldPods(t *testing.T) {
 
 	fake.Endpoints(numP, fake.TestService)
 
-	// Scrape will set a timestamp bigger than this.
-	now := time.Now()
 	got, err := scraper.Scrape(defaultMetric.Spec.StableWindow)
 	if err != nil {
 		t.Fatal("Unexpected error from scraper.Scrape():", err)
 	}
 
-	if got.Time.Before(now) {
-		t.Errorf("stat.Time=%v, want bigger than %v", got.Time, now)
-	}
 	if got.PodName != scraperPodName {
 		t.Errorf("stat.PodName=%v, want %v", got.PodName, scraperPodName)
 	}
@@ -408,16 +380,11 @@ func TestScrapeSomePodsOldPods(t *testing.T) {
 
 	fake.Endpoints(numP, fake.TestService)
 
-	// Scrape will set a timestamp bigger than this.
-	now := time.Now()
 	got, err := scraper.Scrape(defaultMetric.Spec.StableWindow)
 	if err != nil {
 		t.Fatal("Unexpected error from scraper.Scrape():", err)
 	}
 
-	if got.Time.Before(now) {
-		t.Errorf("stat.Time=%v, want bigger than %v", got.Time, now)
-	}
 	if got.PodName != scraperPodName {
 		t.Errorf("stat.PodName=%v, want %v", got.PodName, scraperPodName)
 	}

pkg/autoscaler/scaling/multiscaler_test.go

@@ -354,7 +354,6 @@ func TestMultiScalerScaleFromZero(t *testing.T) {
 	}
 
 	testStat := metrics.Stat{
-		Time:                      time.Now(),
 		PodName:                   "test-pod",
 		AverageConcurrentRequests: 1,
 		RequestCount:              1,

pkg/autoscaler/statserver/server.go

@@ -175,8 +175,6 @@ func (s *Server) Handler(w http.ResponseWriter, r *http.Request) {
 			continue
 		}
 
-		sm.Stat.Time = time.Now()
-
 		s.logger.Debugf("Received stat message: %+v", sm)
 		s.statsCh <- sm
 	}

pkg/autoscaler/statserver/server_test.go

@@ -29,7 +29,6 @@ import (
 	"time"
 
 	"github.com/google/go-cmp/cmp"
-	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/gorilla/websocket"
 	"go.uber.org/zap"
 	"golang.org/x/sync/errgroup"
@@ -220,12 +219,8 @@ func assertReceivedOK(t *testing.T, sm metrics.StatMessage, statSink *websocket.
 	if !ok {
 		t.Fatal("Statistic not received")
 	}
-	if recv.Stat.Time == (time.Time{}) {
-		t.Fatal("Stat time is nil")
-	}
-	ignoreTimeField := cmpopts.IgnoreFields(metrics.StatMessage{}, "Stat.Time")
-	if !cmp.Equal(sm, recv, ignoreTimeField) {
-		t.Fatalf("StatMessage mismatch: diff (-got, +want) %s", cmp.Diff(recv, sm, ignoreTimeField))
+	if !cmp.Equal(sm, recv) {
+		t.Fatalf("StatMessage mismatch: diff (-got, +want) %s", cmp.Diff(recv, sm))
 	}
 	return true
 }

pkg/reconciler/metric/metric_test.go

@@ -276,7 +276,7 @@ func (c *testCollector) CreateOrUpdate(metric *av1alpha1.Metric) error {
 	return c.createOrUpdateError
 }
 
-func (c *testCollector) Record(key types.NamespacedName, stat metrics.Stat) {
+func (c *testCollector) Record(key types.NamespacedName, now time.Time, stat metrics.Stat) {
 	if c.recordCalls != nil {
 		c.recordCalls <- struct{}{}
 	}