mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
test: fix TestDescCacheTimestampUpdate flake (#20975)
## Problem `TestDescCacheTimestampUpdate` was flaky on Windows CI because `time.Now()` has ~15.6ms resolution, causing consecutive calls to return identical timestamps. ## Solution Inject `quartz.Clock` into `MetricsAggregator` using an options pattern, making the test deterministic by using a mock clock with explicit time advancement. ### Changes - Add `clock quartz.Clock` field to `MetricsAggregator` struct - Add `WithClock()` option for dependency injection - Replace all `time.Now()` calls with `ma.clock.Now()` - Update test to use mock clock with `mClock.Advance(time.Second)` --- This PR was fully generated by [`mux`](https://github.com/coder/mux) using Claude Opus 4.5, and reviewed by me. Closes https://github.com/coder/internal/issues/1146
This commit is contained in:
@@ -16,6 +16,8 @@ import (
|
||||
agentproto "github.com/coder/coder/v2/agent/proto"
|
||||
"github.com/coder/coder/v2/coderd/agentmetrics"
|
||||
"github.com/coder/coder/v2/coderd/pproflabel"
|
||||
|
||||
"github.com/coder/quartz"
|
||||
)
|
||||
|
||||
const (
|
||||
@@ -47,6 +49,7 @@ type MetricsAggregator struct {
|
||||
|
||||
log slog.Logger
|
||||
metricsCleanupInterval time.Duration
|
||||
clock quartz.Clock
|
||||
|
||||
collectCh chan (chan []prometheus.Metric)
|
||||
updateCh chan updateRequest
|
||||
@@ -151,7 +154,7 @@ func (am *annotatedMetric) shallowCopy() annotatedMetric {
|
||||
}
|
||||
}
|
||||
|
||||
func NewMetricsAggregator(logger slog.Logger, registerer prometheus.Registerer, duration time.Duration, aggregateByLabels []string) (*MetricsAggregator, error) {
|
||||
func NewMetricsAggregator(logger slog.Logger, registerer prometheus.Registerer, duration time.Duration, aggregateByLabels []string, options ...func(*MetricsAggregator)) (*MetricsAggregator, error) {
|
||||
metricsCleanupInterval := defaultMetricsCleanupInterval
|
||||
if duration > 0 {
|
||||
metricsCleanupInterval = duration
|
||||
@@ -192,9 +195,10 @@ func NewMetricsAggregator(logger slog.Logger, registerer prometheus.Registerer,
|
||||
return nil, err
|
||||
}
|
||||
|
||||
return &MetricsAggregator{
|
||||
ma := &MetricsAggregator{
|
||||
log: logger.Named(loggerName),
|
||||
metricsCleanupInterval: metricsCleanupInterval,
|
||||
clock: quartz.NewReal(),
|
||||
|
||||
store: map[metricKey]annotatedMetric{},
|
||||
|
||||
@@ -206,7 +210,19 @@ func NewMetricsAggregator(logger slog.Logger, registerer prometheus.Registerer,
|
||||
cleanupHistogram: cleanupHistogram,
|
||||
|
||||
aggregateByLabels: aggregateByLabels,
|
||||
}, nil
|
||||
}
|
||||
|
||||
for _, option := range options {
|
||||
option(ma)
|
||||
}
|
||||
|
||||
return ma, nil
|
||||
}
|
||||
|
||||
func WithClock(clock quartz.Clock) func(*MetricsAggregator) {
|
||||
return func(ma *MetricsAggregator) {
|
||||
ma.clock = clock
|
||||
}
|
||||
}
|
||||
|
||||
// labelAggregator is used to control cardinality of collected Prometheus metrics by pre-aggregating series based on given labels.
|
||||
@@ -349,7 +365,7 @@ func (ma *MetricsAggregator) Run(ctx context.Context) func() {
|
||||
ma.log.Debug(ctx, "clean expired metrics")
|
||||
|
||||
timer := prometheus.NewTimer(ma.cleanupHistogram)
|
||||
now := time.Now()
|
||||
now := ma.clock.Now()
|
||||
|
||||
for key, val := range ma.store {
|
||||
if now.After(val.expiryDate) {
|
||||
@@ -407,7 +423,7 @@ func (ma *MetricsAggregator) getOrCreateDesc(name string, help string, baseLabel
|
||||
}
|
||||
key := cacheKeyForDesc(name, baseLabelNames, extraLabels)
|
||||
if d, ok := ma.descCache[key]; ok {
|
||||
d.lastUsed = time.Now()
|
||||
d.lastUsed = ma.clock.Now()
|
||||
ma.descCache[key] = d
|
||||
return d.desc
|
||||
}
|
||||
@@ -419,7 +435,7 @@ func (ma *MetricsAggregator) getOrCreateDesc(name string, help string, baseLabel
|
||||
labels[nBase+i] = l.Name
|
||||
}
|
||||
d := prometheus.NewDesc(name, help, labels, nil)
|
||||
ma.descCache[key] = descCacheEntry{d, time.Now()}
|
||||
ma.descCache[key] = descCacheEntry{d, ma.clock.Now()}
|
||||
return d
|
||||
}
|
||||
|
||||
@@ -497,7 +513,7 @@ func (ma *MetricsAggregator) Update(ctx context.Context, labels AgentMetricLabel
|
||||
templateName: labels.TemplateName,
|
||||
metrics: metrics,
|
||||
|
||||
timestamp: time.Now(),
|
||||
timestamp: ma.clock.Now(),
|
||||
}:
|
||||
case <-ctx.Done():
|
||||
ma.log.Debug(ctx, "update request is canceled")
|
||||
@@ -508,7 +524,7 @@ func (ma *MetricsAggregator) Update(ctx context.Context, labels AgentMetricLabel
|
||||
|
||||
// Move to a function for testability
|
||||
func (ma *MetricsAggregator) cleanupDescCache() {
|
||||
now := time.Now()
|
||||
now := ma.clock.Now()
|
||||
for key, entry := range ma.descCache {
|
||||
if now.Sub(entry.lastUsed) > ma.metricsCleanupInterval {
|
||||
delete(ma.descCache, key)
|
||||
|
||||
@@ -11,6 +11,7 @@ import (
|
||||
agentproto "github.com/coder/coder/v2/agent/proto"
|
||||
"github.com/coder/coder/v2/coderd/agentmetrics"
|
||||
"github.com/coder/coder/v2/testutil"
|
||||
"github.com/coder/quartz"
|
||||
)
|
||||
|
||||
func TestDescCache_DescExpire(t *testing.T) {
|
||||
@@ -62,8 +63,9 @@ func TestDescCache_DescExpire(t *testing.T) {
|
||||
func TestDescCacheTimestampUpdate(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
mClock := quartz.NewMock(t)
|
||||
registry := prometheus.NewRegistry()
|
||||
ma, err := NewMetricsAggregator(slogtest.Make(t, nil), registry, time.Hour, nil)
|
||||
ma, err := NewMetricsAggregator(slogtest.Make(t, nil), registry, time.Hour, nil, WithClock(mClock))
|
||||
require.NoError(t, err)
|
||||
|
||||
baseLabelNames := []string{"label1", "label2"}
|
||||
@@ -78,6 +80,9 @@ func TestDescCacheTimestampUpdate(t *testing.T) {
|
||||
initialEntry := ma.descCache[key]
|
||||
initialTime := initialEntry.lastUsed
|
||||
|
||||
// Advance the mock clock to ensure a different timestamp
|
||||
mClock.Advance(time.Second)
|
||||
|
||||
desc2 := ma.getOrCreateDesc("test_metric", "help text", baseLabelNames, extraLabels)
|
||||
require.NotNil(t, desc2)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user