Skip to content

Commit 5da546e

Browse files
dannykoppingdannykopping
committed
chore: improvements
Signed-off-by: Danny Kopping <dannykopping@gmail.com>
1 parent 562b56d commit 5da546e

File tree

2 files changed

+40
-16
lines changed

2 files changed

+40
-16
lines changed

‎enterprise/coderd/prebuilds/metricscollector.go

+32-15
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@ package prebuilds
22

33
import (
44
"context"
5-
"sync/atomic"
5+
"fmt"
6+
"sync"
67
"time"
78

89
"github.com/prometheus/client_golang/prometheus"
@@ -11,7 +12,6 @@ import (
1112
"cdr.dev/slog"
1213

1314
"github.com/coder/coder/v2/coderd/database"
14-
"github.com/coder/coder/v2/coderd/database/dbauthz"
1515
"github.com/coder/coder/v2/coderd/prebuilds"
1616
)
1717

@@ -57,6 +57,12 @@ var (
5757
labels,
5858
nil,
5959
)
60+
lastUpdateDesc = prometheus.NewDesc(
61+
"coderd_prebuilt_workspaces_metrics_last_updated",
62+
"The unix timestamp when the metrics related to prebuilt workspaces were last updated; these metrics are cached.",
63+
[]string{},
64+
nil,
65+
)
6066
)
6167

6268
const (
@@ -69,7 +75,9 @@ type MetricsCollector struct {
6975
logger slog.Logger
7076
snapshotter prebuilds.StateSnapshotter
7177

72-
latestState atomic.Pointer[state]
78+
latestState *state
79+
latestStateUpdatedAt time.Time
80+
latestStateMu sync.Mutex
7381
}
7482

7583
var _ prometheus.Collector = new(MetricsCollector)
@@ -91,35 +99,36 @@ func (*MetricsCollector) Describe(descCh chan<- *prometheus.Desc) {
9199
descCh <- desiredPrebuildsDesc
92100
descCh <- runningPrebuildsDesc
93101
descCh <- eligiblePrebuildsDesc
102+
descCh <- lastUpdateDesc
94103
}
95104

96105
// Collect uses the cached state to set configured metrics.
97106
// The state is cached because this function can be called multiple times per second and retrieving the current state
98107
// is an expensive operation.
99108
func (mc *MetricsCollector) Collect(metricsCh chan<- prometheus.Metric) {
100-
// nolint:gocritic // We need to set an authz context to read metrics from the db.
101-
ctx := dbauthz.AsPrebuildsOrchestrator(context.Background())
109+
// Prevent the state from changing while a collection is occurring.
110+
mc.latestStateMu.Lock()
111+
defer mc.latestStateMu.Unlock()
102112

103-
currentState := mc.latestState.Load()
104-
if currentState == nil {
105-
mc.logger.Warn(ctx, "failed to set prebuilds metrics; state not set")
113+
if mc.latestState == nil {
114+
mc.logger.Warn(context.Background(), "failed to set prebuilds metrics; state not set")
106115
return
107116
}
108117

109-
for _, metric := range currentState.prebuildMetrics {
118+
for _, metric := range mc.latestState.prebuildMetrics {
110119
metricsCh <- prometheus.MustNewConstMetric(createdPrebuildsDesc, prometheus.CounterValue, float64(metric.CreatedCount), metric.TemplateName, metric.PresetName, metric.OrganizationName)
111120
metricsCh <- prometheus.MustNewConstMetric(failedPrebuildsDesc, prometheus.CounterValue, float64(metric.FailedCount), metric.TemplateName, metric.PresetName, metric.OrganizationName)
112121
metricsCh <- prometheus.MustNewConstMetric(claimedPrebuildsDesc, prometheus.CounterValue, float64(metric.ClaimedCount), metric.TemplateName, metric.PresetName, metric.OrganizationName)
113122
}
114123

115-
for _, preset := range currentState.snapshot.Presets {
124+
for _, preset := range mc.latestState.snapshot.Presets {
116125
if !preset.UsingActiveVersion {
117126
continue
118127
}
119128

120-
presetSnapshot, err := currentState.snapshot.FilterByPreset(preset.ID)
129+
presetSnapshot, err := mc.latestState.snapshot.FilterByPreset(preset.ID)
121130
if err != nil {
122-
mc.logger.Error(ctx, "failed to filter by preset", slog.Error(err))
131+
mc.logger.Error(context.Background(), "failed to filter by preset", slog.Error(err))
123132
continue
124133
}
125134
state := presetSnapshot.CalculateState()
@@ -128,6 +137,8 @@ func (mc *MetricsCollector) Collect(metricsCh chan<- prometheus.Metric) {
128137
metricsCh <- prometheus.MustNewConstMetric(runningPrebuildsDesc, prometheus.GaugeValue, float64(state.Actual), preset.TemplateName, preset.Name, preset.OrganizationName)
129138
metricsCh <- prometheus.MustNewConstMetric(eligiblePrebuildsDesc, prometheus.GaugeValue, float64(state.Eligible), preset.TemplateName, preset.Name, preset.OrganizationName)
130139
}
140+
141+
metricsCh <- prometheus.MustNewConstMetric(lastUpdateDesc, prometheus.GaugeValue, float64(mc.latestStateUpdatedAt.Unix()))
131142
}
132143

133144
type state struct {
@@ -157,6 +168,11 @@ func (mc *MetricsCollector) BackgroundFetch(ctx context.Context, updateInterval,
157168

158169
// UpdateState builds the current metrics state.
159170
func (mc *MetricsCollector) UpdateState(ctx context.Context, timeout time.Duration) error {
171+
// Prevent collection from occurring while state is updating.
172+
mc.latestStateMu.Lock()
173+
defer mc.latestStateMu.Unlock()
174+
175+
start := time.Now()
160176
mc.logger.Debug(ctx, "fetching prebuilds metrics state")
161177
fetchCtx, fetchCancel := context.WithTimeout(ctx, timeout)
162178
defer fetchCancel()
@@ -170,11 +186,12 @@ func (mc *MetricsCollector) UpdateState(ctx context.Context, timeout time.Durati
170186
if err != nil {
171187
return xerrors.Errorf("snapshot state: %w", err)
172188
}
173-
mc.logger.Debug(ctx, "fetched prebuilds metrics state")
189+
mc.logger.Debug(ctx, "fetched prebuilds metrics state", slog.F("duration_secs", fmt.Sprintf("%.2f", time.Since(start).Seconds())))
174190

175-
mc.latestState.Store(&state{
191+
mc.latestState = &state{
176192
prebuildMetrics: prebuildMetrics,
177193
snapshot: snapshot,
178-
})
194+
}
195+
mc.latestStateUpdatedAt = time.Now()
179196
return nil
180197
}

‎enterprise/coderd/prebuilds/reconcile.go

+8-1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"database/sql"
66
"fmt"
77
"math"
8+
"sync"
89
"sync/atomic"
910
"time"
1011

@@ -87,10 +88,12 @@ func (c *StoreReconciler) Run(ctx context.Context) {
8788
slog.F("backoff_interval", c.cfg.ReconciliationBackoffInterval.String()),
8889
slog.F("backoff_lookback", c.cfg.ReconciliationBackoffLookback.String()))
8990

91+
var wg sync.WaitGroup
9092
ticker := c.clock.NewTicker(reconciliationInterval)
9193
defer ticker.Stop()
9294
defer func() {
9395
c.done <- struct{}{}
96+
wg.Wait()
9497
}()
9598

9699
// nolint:gocritic // Reconciliation Loop needs Prebuilds Orchestrator permissions.
@@ -99,7 +102,11 @@ func (c *StoreReconciler) Run(ctx context.Context) {
99102

100103
// Start updating metrics in the background.
101104
if c.metrics != nil {
102-
go c.metrics.BackgroundFetch(ctx, metricsUpdateInterval, metricsUpdateTimeout)
105+
wg.Add(1)
106+
go func() {
107+
defer wg.Done()
108+
c.metrics.BackgroundFetch(ctx, metricsUpdateInterval, metricsUpdateTimeout)
109+
}()
103110
}
104111

105112
// Everything is in place, reconciler can now be considered as running.

0 commit comments

Comments
 (0)