Skip to content

Commit 364d6ab

Browse files
committed
drop noop marker in favor of comma-ok
Signed-off-by: Siavash Safi <siavash@cloudflare.com>
1 parent f6f5fd2 commit 364d6ab

10 files changed

Lines changed: 74 additions & 80 deletions

File tree

api/v2/api.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -487,7 +487,7 @@ func (api *API) alertFilter(parent context.Context, matchers []*labels.Matcher,
487487

488488
// Set alert's current status based on its label set.
489489
// The inhibitor and silencer write to m via the context.
490-
ctx = marker.WithAlertMarker(ctx, m)
490+
ctx = marker.WithContext(ctx, m)
491491
api.setAlertStatus(ctx, a.Labels)
492492

493493
// Get alert's current status after seeing if it is suppressed.

dispatch/dispatch.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -687,7 +687,7 @@ func (ag *aggrGroup) run(nf notifyFunc) {
687687
ctx = notify.WithRouteID(ctx, ag.routeID)
688688
ctx = notify.WithFlushID(ctx, ag.flushIdx)
689689
ctx = notify.WithGroupMatchers(ctx, ag.matchers)
690-
ctx = marker.WithAlertMarker(ctx, ag.marker)
690+
ctx = marker.WithContext(ctx, ag.marker)
691691

692692
ag.flushIdx++
693693

inhibit/inhibit.go

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,15 @@ func (ih *Inhibitor) Mutes(ctx context.Context, lset model.LabelSet) bool {
193193
)
194194
defer span.End()
195195

196-
m := marker.GetAlertMarker(ctx)
196+
var inhibitedBy []string
197+
defer func() {
198+
// Get the marker from context and set the inhibited alerts on it if any.
199+
m, ok := marker.FromContext(ctx)
200+
if ok {
201+
m.SetInhibited(fp, inhibitedBy)
202+
}
203+
}()
204+
197205
now := time.Now()
198206
for _, r := range ih.rules {
199207
if !r.TargetMatchers.Matches(lset) {
@@ -208,7 +216,7 @@ func (ih *Inhibitor) Mutes(ctx context.Context, lset model.LabelSet) bool {
208216
// If we are here, the target side matches. If the source side matches, too, we
209217
// need to exclude inhibiting alerts for which the same is true.
210218
if inhibitedByFP, eq := r.hasEqual(lset, r.SourceMatchers.Matches(lset), now); eq {
211-
m.SetInhibited(fp, []string{inhibitedByFP.String()})
219+
inhibitedBy = append(inhibitedBy, inhibitedByFP.String())
212220
span.AddEvent("alert inhibited",
213221
trace.WithAttributes(
214222
attribute.String("alerting.inhibit_rule.source.fingerprint", inhibitedByFP.String()),
@@ -223,7 +231,6 @@ func (ih *Inhibitor) Mutes(ctx context.Context, lset model.LabelSet) bool {
223231
return true
224232
}
225233
}
226-
m.SetInhibited(fp, nil)
227234
span.AddEvent("alert not inhibited")
228235

229236
return false

inhibit/inhibit_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ var nopLogger = promslog.NewNopLogger()
3939
func checkMutes(t *testing.T, ih *Inhibitor, target model.LabelSet, wantMuted bool, msgAndArgs ...any) {
4040
t.Helper()
4141
m := marker.NewAlertMarker()
42-
ctx := marker.WithAlertMarker(context.Background(), m)
42+
ctx := marker.WithContext(context.Background(), m)
4343
got := ih.Mutes(ctx, target)
4444
require.Equal(t, wantMuted, got, msgAndArgs...)
4545
fp := target.Fingerprint()

marker/alert_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -116,15 +116,15 @@ func TestAlertMarkerContext(t *testing.T) {
116116
ctx := t.Context()
117117

118118
// No marker in context — should return no-op marker.
119-
got := GetAlertMarker(ctx)
120-
require.NotNil(t, got)
121-
// No-op marker returns unknown for any fingerprint.
122-
require.Equal(t, alert.AlertStateUnprocessed, got.Status(model.Fingerprint(1)).State)
119+
got, ok := FromContext(ctx)
120+
require.False(t, ok)
121+
require.Nil(t, got)
123122

124123
// Set marker in context.
125124
gm := NewAlertMarker()
126-
ctx = WithAlertMarker(ctx, gm)
127-
got = GetAlertMarker(ctx)
125+
ctx = WithContext(ctx, gm)
126+
got, ok = FromContext(ctx)
127+
require.True(t, ok)
128128
require.NotNil(t, got)
129129

130130
// Writing through the context-extracted marker should be visible.

marker/context.go

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
// Copyright The Prometheus Authors
2+
// Licensed under the Apache License, Version 2.0 (the "License");
3+
// you may not use this file except in compliance with the License.
4+
// You may obtain a copy of the License at
5+
//
6+
// http://www.apache.org/licenses/LICENSE-2.0
7+
//
8+
// Unless required by applicable law or agreed to in writing, software
9+
// distributed under the License is distributed on an "AS IS" BASIS,
10+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
11+
// See the License for the specific language governing permissions and
12+
// limitations under the License.
13+
14+
package marker
15+
16+
import (
17+
"context"
18+
)
19+
20+
// markerKey defines a custom type with which a context is populated to
21+
// avoid accidental collisions.
22+
type markerKey int
23+
24+
const (
25+
keyAlertMarker markerKey = iota
26+
)
27+
28+
// WithContext returns a copy of ctx carrying the given
29+
// AlertMarker. Inhibitor and Silencer extract it from the context to
30+
// write per-group alert status.
31+
func WithContext(ctx context.Context, m AlertMarker) context.Context {
32+
return context.WithValue(ctx, keyAlertMarker, m)
33+
}
34+
35+
// FromContext extracts the AlertMarker from the context if present,
36+
// otherwise returns a no-op marker.
37+
func FromContext(ctx context.Context) (AlertMarker, bool) {
38+
m, ok := ctx.Value(keyAlertMarker).(AlertMarker)
39+
return m, ok
40+
}

marker/util.go

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

notify/mute_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ func TestMuteStageWithSilences(t *testing.T) {
108108

109109
reg := prometheus.NewRegistry()
110110
alertMarker := marker.NewAlertMarker()
111-
ctx := marker.WithAlertMarker(context.Background(), alertMarker)
111+
ctx := marker.WithContext(context.Background(), alertMarker)
112112
silencer := silence.NewSilencer(silences, promslog.NewNopLogger(), eventrecorder.NopRecorder())
113113
metrics := NewMetrics(reg, featurecontrol.NoopFlags{})
114114
stage := NewMuteStage(silencer, metrics)

silence/silence.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,10 @@ func (s *Silencer) Mutes(ctx context.Context, lset model.LabelSet) bool {
176176
var markedSilences []string
177177
defer func() {
178178
// Get the marker from context and set the silences on it if any.
179-
marker.GetAlertMarker(ctx).SetSilenced(fp, markedSilences)
179+
m, ok := marker.FromContext(ctx)
180+
if ok {
181+
m.SetSilenced(fp, markedSilences)
182+
}
180183
}()
181184

182185
// Get the cached entry for this fingerprint.

silence/silence_test.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2287,7 +2287,7 @@ func TestSilencer(t *testing.T) {
22872287
s := NewSilencer(ss, promslog.NewNopLogger(), eventrecorder.NopRecorder())
22882288

22892289
m := marker.NewAlertMarker()
2290-
ctx := marker.WithAlertMarker(t.Context(), m)
2290+
ctx := marker.WithContext(t.Context(), m)
22912291

22922292
require.False(t, s.Mutes(ctx, model.LabelSet{"foo": "bar"}), "expected alert not silenced without any silences")
22932293
checkMutes(t, m, model.LabelSet{"foo": "bar"}, false, "expected marker not silenced without any silences")
@@ -2302,7 +2302,7 @@ func TestSilencer(t *testing.T) {
23022302
require.NoError(t, ss.Set(t.Context(), sil1))
23032303

23042304
m = marker.NewAlertMarker()
2305-
ctx = marker.WithAlertMarker(t.Context(), m)
2305+
ctx = marker.WithContext(t.Context(), m)
23062306
require.False(t, s.Mutes(ctx, model.LabelSet{"foo": "bar"}), "expected alert not silenced by non-matching silence")
23072307
checkMutes(t, m, model.LabelSet{"foo": "bar"}, false, "expected marker not silenced by non-matching silence")
23082308

@@ -2317,7 +2317,7 @@ func TestSilencer(t *testing.T) {
23172317
require.NotEmpty(t, sil2.Id)
23182318

23192319
m = marker.NewAlertMarker()
2320-
ctx = marker.WithAlertMarker(t.Context(), m)
2320+
ctx = marker.WithContext(t.Context(), m)
23212321
require.True(t, s.Mutes(ctx, model.LabelSet{"foo": "bar"}), "expected alert silenced by matching silence")
23222322
checkMutes(t, m, model.LabelSet{"foo": "bar"}, true, "expected marker silenced by matching silence")
23232323

@@ -2326,7 +2326,7 @@ func TestSilencer(t *testing.T) {
23262326
now = ss.nowUTC()
23272327

23282328
m = marker.NewAlertMarker()
2329-
ctx = marker.WithAlertMarker(t.Context(), m)
2329+
ctx = marker.WithContext(t.Context(), m)
23302330
require.False(t, s.Mutes(ctx, model.LabelSet{"foo": "bar"}), "expected alert not silenced by expired silence")
23312331
checkMutes(t, m, model.LabelSet{"foo": "bar"}, false, "expected marker not silenced by expired silence")
23322332

@@ -2342,7 +2342,7 @@ func TestSilencer(t *testing.T) {
23422342
require.NoError(t, err)
23432343

23442344
m = marker.NewAlertMarker()
2345-
ctx = marker.WithAlertMarker(t.Context(), m)
2345+
ctx = marker.WithContext(t.Context(), m)
23462346
require.False(t, s.Mutes(ctx, model.LabelSet{"foo": "bar"}), "expected alert not silenced by future silence")
23472347
checkMutes(t, m, model.LabelSet{"foo": "bar"}, false, "expected marker not silenced by future silence")
23482348

@@ -2352,7 +2352,7 @@ func TestSilencer(t *testing.T) {
23522352

23532353
// Exposes issue #2426.
23542354
m = marker.NewAlertMarker()
2355-
ctx = marker.WithAlertMarker(t.Context(), m)
2355+
ctx = marker.WithContext(t.Context(), m)
23562356
require.True(t, s.Mutes(ctx, model.LabelSet{"foo": "bar"}), "expected alert silenced by activated silence")
23572357
checkMutes(t, m, model.LabelSet{"foo": "bar"}, true, "expected marker silenced by activated silence")
23582358

@@ -2367,7 +2367,7 @@ func TestSilencer(t *testing.T) {
23672367

23682368
// Note that issue #2426 doesn't apply anymore because we added a new silence.
23692369
m = marker.NewAlertMarker()
2370-
ctx = marker.WithAlertMarker(t.Context(), m)
2370+
ctx = marker.WithContext(t.Context(), m)
23712371
require.True(t, s.Mutes(ctx, model.LabelSet{"foo": "bar"}), "expected alert still silenced by activated silence")
23722372
checkMutes(t, m, model.LabelSet{"foo": "bar"}, true, "expected marker still silenced by activated silence")
23732373

@@ -2376,7 +2376,7 @@ func TestSilencer(t *testing.T) {
23762376

23772377
// Another variant of issue #2426 (overlapping silences).
23782378
m = marker.NewAlertMarker()
2379-
ctx = marker.WithAlertMarker(t.Context(), m)
2379+
ctx = marker.WithContext(t.Context(), m)
23802380
require.True(t, s.Mutes(ctx, model.LabelSet{"foo": "bar"}), "expected alert silenced by activated second silence")
23812381
checkMutes(t, m, model.LabelSet{"foo": "bar"}, true, "expected marker silenced by activated second silence")
23822382
}
@@ -2406,7 +2406,7 @@ func TestSilencerPostDeleteEvictsCache(t *testing.T) {
24062406

24072407
// Mutes populates the cache.
24082408
m := marker.NewAlertMarker()
2409-
ctx := marker.WithAlertMarker(t.Context(), m)
2409+
ctx := marker.WithContext(t.Context(), m)
24102410
require.True(t, s.Mutes(ctx, lset))
24112411
checkMutes(t, m, lset, true, "expected marker silenced after initial Mutes")
24122412
entry := s.cache.get(fp)
@@ -2420,7 +2420,7 @@ func TestSilencerPostDeleteEvictsCache(t *testing.T) {
24202420

24212421
// Mutes re-evaluates from scratch (cache miss) and still finds the silence.
24222422
m = marker.NewAlertMarker()
2423-
ctx = marker.WithAlertMarker(t.Context(), m)
2423+
ctx = marker.WithContext(t.Context(), m)
24242424
require.True(t, s.Mutes(ctx, lset), "expected alert still silenced after cache eviction")
24252425
checkMutes(t, m, lset, true, "expected marker silenced after cache eviction")
24262426
entry = s.cache.get(fp)

0 commit comments

Comments
 (0)