Skip to content

Commit 18dbabe

Browse files
committed
Small fixes
Signed-off-by: Danny Kopping <danny@coder.com>
1 parent 093e86b commit 18dbabe

File tree

4 files changed

+16
-30
lines changed

4 files changed

+16
-30
lines changed

coderd/notifications/manager.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,10 @@ type Manager struct {
6060
// helpers is a map of template helpers which are used to customize notification messages to use global settings like
6161
// access URL etc.
6262
func NewManager(cfg codersdk.NotificationsConfig, store Store, metrics *Metrics, log slog.Logger) (*Manager, error) {
63-
method, err := dispatchMethodFromCfg(cfg)
64-
if err != nil {
65-
return nil, err
63+
// TODO(dannyk): add the ability to use multiple notification methods.
64+
var method database.NotificationMethod
65+
if err := method.Scan(cfg.Method.String()); err != nil {
66+
return nil, xerrors.Errorf("given notification method %q is invalid", cfg.Method)
6667
}
6768

6869
// If dispatch timeout exceeds lease period, it is possible that messages can be delivered in duplicate because the

coderd/notifications/method.go

Lines changed: 0 additions & 16 deletions
This file was deleted.

coderd/notifications/metrics_test.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ func TestPendingUpdatesMetrics(t *testing.T) {
210210
cfg.StoreSyncInterval = serpent.Duration(time.Millisecond * 100)
211211

212212
syncer := &syncInterceptor{Store: store}
213-
interceptor := newUpdateSignallingInterceptor(store, syncer)
213+
interceptor := newUpdateSignallingInterceptor(syncer)
214214
mgr, err := notifications.NewManager(cfg, interceptor, metrics, logger.Named("manager"))
215215
require.NoError(t, err)
216216
t.Cleanup(func() {
@@ -270,12 +270,12 @@ func hasMatchingFingerprint(metric *dto.Metric, fp model.Fingerprint) bool {
270270

271271
// fingerprintLabelPairs produces a fingerprint unique to the given combination of label pairs.
272272
func fingerprintLabelPairs(lbs []*dto.LabelPair) model.Fingerprint {
273-
lbsSet := model.LabelSet{}
273+
pairs := make([]string, 0, len(lbs)*2)
274274
for _, lp := range lbs {
275-
lbsSet[model.LabelName(lp.GetName())] = model.LabelValue(lp.GetValue())
275+
pairs = append(pairs, lp.GetName(), lp.GetValue())
276276
}
277277

278-
return lbsSet.FastFingerprint()
278+
return fingerprintLabels(pairs...)
279279
}
280280

281281
// fingerprintLabels produces a fingerprint unique to the given pairs of label values.
@@ -295,20 +295,20 @@ func fingerprintLabels(lbs ...string) model.Fingerprint {
295295
return lbsSet.FastFingerprint()
296296
}
297297

298+
// updateSignallingInterceptor intercepts bulk update calls to the store, and waits on the "proceed" condition to be
299+
// signaled by the caller so it can continue.
298300
type updateSignallingInterceptor struct {
299301
notifications.Store
300-
*syncInterceptor
301302

302303
proceed *sync.Cond
303304

304305
updateSuccess chan int
305306
updateFailure chan int
306307
}
307308

308-
func newUpdateSignallingInterceptor(store notifications.Store, interceptor *syncInterceptor) *updateSignallingInterceptor {
309+
func newUpdateSignallingInterceptor(interceptor notifications.Store) *updateSignallingInterceptor {
309310
return &updateSignallingInterceptor{
310-
Store: store,
311-
syncInterceptor: interceptor,
311+
Store: interceptor,
312312

313313
proceed: sync.NewCond(&sync.Mutex{}),
314314

@@ -326,7 +326,7 @@ func (u *updateSignallingInterceptor) BulkMarkNotificationMessagesSent(ctx conte
326326
// Wait until signaled so we have a chance to read the number of pending updates.
327327
u.proceed.Wait()
328328

329-
return u.syncInterceptor.BulkMarkNotificationMessagesSent(ctx, arg)
329+
return u.Store.BulkMarkNotificationMessagesSent(ctx, arg)
330330
}
331331

332332
func (u *updateSignallingInterceptor) BulkMarkNotificationMessagesFailed(ctx context.Context, arg database.BulkMarkNotificationMessagesFailedParams) (int64, error) {
@@ -338,5 +338,5 @@ func (u *updateSignallingInterceptor) BulkMarkNotificationMessagesFailed(ctx con
338338
// Wait until signaled so we have a chance to read the number of pending updates.
339339
u.proceed.Wait()
340340

341-
return u.syncInterceptor.BulkMarkNotificationMessagesFailed(ctx, arg)
341+
return u.Store.BulkMarkNotificationMessagesFailed(ctx, arg)
342342
}

coderd/notifications/notifier.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,8 @@ func (n *notifier) deliver(ctx context.Context, msg database.AcquireNotification
210210

211211
start := time.Now()
212212
retryable, err := deliver(ctx, msg.ID)
213+
n.metrics.DispatcherSendSeconds.WithLabelValues(string(n.method)).Observe(time.Since(start).Seconds())
214+
213215
if err != nil {
214216
// Don't try to accumulate message responses if the context has been canceled.
215217
//
@@ -241,7 +243,6 @@ func (n *notifier) deliver(ctx context.Context, msg database.AcquireNotification
241243
success <- n.newSuccessfulDispatch(msg)
242244
}
243245
}
244-
n.metrics.DispatcherSendSeconds.WithLabelValues(string(n.method)).Observe(time.Since(start).Seconds())
245246
n.metrics.PendingUpdates.Set(float64(len(success) + len(failure)))
246247

247248
return nil

0 commit comments

Comments
 (0)