Skip to content

Commit fd8b5a0

Browse files
MustafaSaberMustafaSaber
authored
cache: use mockMetrics instead of the no-op implementation (#4239)
see subject follow up on #4033 Signed-off-by: Mustafa Abdelrahman <mustafa.abdelrahman@zalando.de>
1 parent 0d64d85 commit fd8b5a0

4 files changed

Lines changed: 74 additions & 103 deletions

File tree

‎filters/cache/filter_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4127,7 +4127,7 @@ func TestProxy_L2Cache_HitAfterL1Eviction(t *testing.T) {
41274127
defer backend.Close()
41284128

41294129
stub := newStubValkeyClient()
4130-
m := &testMetrics{}
4130+
m := &metricstest.MockMetrics{}
41314131
lru := NewLRUStorage(1<<20, nil, m)
41324132

41334133
spec := NewCacheFilter(Options{
@@ -4205,7 +4205,7 @@ func TestProxy_L2Cache_FallsBackToL1OnL2Failure(t *testing.T) {
42054205
defer backend.Close()
42064206

42074207
stub := newBrokenStubValkeyClient()
4208-
m := &testMetrics{}
4208+
m := &metricstest.MockMetrics{}
42094209
lru := NewLRUStorage(1<<20, nil, m)
42104210

42114211
isNoErr := func(err error) bool { return false } // broken stub always returns real errors
@@ -4246,7 +4246,7 @@ func TestProxy_L2Cache_FallsBackToL1OnL2Failure(t *testing.T) {
42464246
}
42474247

42484248
// L2 Set fallback must have written to L1 — check l2_set_fallback counter
4249-
if m.counter("cache.l2_set_fallback") == 0 {
4249+
if v, ok := m.Counter("cache.l2_set_fallback"); ok && v == 0 {
42504250
t.Error("expected l2_set_fallback to be incremented when L2 Set fails")
42514251
}
42524252
}
@@ -4261,7 +4261,7 @@ func TestProxy_L2Cache_MissWhenBothMiss(t *testing.T) {
42614261
defer backend.Close()
42624262

42634263
stub := newStubValkeyClient() // empty
4264-
m := &testMetrics{}
4264+
m := &metricstest.MockMetrics{}
42654265
lru := NewLRUStorage(1<<20, nil, m)
42664266
isNoErr := func(err error) bool {
42674267
return err != nil && strings.Contains(err.Error(), "valkey: nil")

‎filters/cache/l2_storage_valkey_test.go‎

Lines changed: 60 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,13 @@ import (
44
"context"
55
"encoding/json"
66
"errors"
7-
"net/http"
87
"sync"
98
"testing"
109
"time"
1110

1211
"github.com/valkey-io/valkey-go"
1312
"github.com/zalando/skipper/metrics"
13+
"github.com/zalando/skipper/metrics/metricstest"
1414
skpnet "github.com/zalando/skipper/net"
1515
"github.com/zalando/skipper/net/valkeytest"
1616
)
@@ -88,58 +88,6 @@ func (s *stubValkeyClient) Del(_ context.Context, key string) (int64, error) {
8888
return 1, nil
8989
}
9090

91-
// testMetrics is a minimal metrics.Metrics stub for testing.
92-
// Only IncCounter does real work; all other methods are no-ops.
93-
type testMetrics struct {
94-
mu sync.Mutex
95-
counters map[string]int
96-
}
97-
98-
var _ metrics.Metrics = (*testMetrics)(nil)
99-
100-
func (m *testMetrics) IncCounter(key string) {
101-
m.mu.Lock()
102-
defer m.mu.Unlock()
103-
if m.counters == nil {
104-
m.counters = make(map[string]int)
105-
}
106-
m.counters[key]++
107-
}
108-
109-
func (m *testMetrics) counter(key string) int {
110-
m.mu.Lock()
111-
defer m.mu.Unlock()
112-
return m.counters[key]
113-
}
114-
115-
// metrics.Metrics no-op implementations
116-
func (m *testMetrics) MeasureSince(key string, start time.Time) {}
117-
func (m *testMetrics) IncCounterBy(key string, value int64) {}
118-
func (m *testMetrics) IncFloatCounterBy(key string, value float64) {}
119-
func (m *testMetrics) MeasureRouteLookup(start time.Time) {}
120-
func (m *testMetrics) MeasureFilterCreate(filterName string, start time.Time) {}
121-
func (m *testMetrics) MeasureFilterRequest(filterName string, start time.Time) {}
122-
func (m *testMetrics) MeasureAllFiltersRequest(routeId string, start time.Time) {}
123-
func (m *testMetrics) MeasureBackendRequestHeader(host string, size int) {}
124-
func (m *testMetrics) MeasureBackend(routeId string, start time.Time) {}
125-
func (m *testMetrics) MeasureBackendHost(routeBackendHost string, start time.Time) {}
126-
func (m *testMetrics) MeasureBackendZone(zone string, start time.Time) {}
127-
func (m *testMetrics) MeasureFilterResponse(filterName string, start time.Time) {}
128-
func (m *testMetrics) MeasureAllFiltersResponse(routeId string, start time.Time) {}
129-
func (m *testMetrics) MeasureResponse(code int, method string, routeId string, start time.Time) {}
130-
func (m *testMetrics) MeasureResponseSize(host string, size int64) {}
131-
func (m *testMetrics) MeasureProxy(requestDuration, responseDuration time.Duration) {}
132-
func (m *testMetrics) MeasureServe(routeId, host, method string, code int, start time.Time) {}
133-
func (m *testMetrics) IncRoutingFailures() {}
134-
func (m *testMetrics) IncErrorsBackend(routeId string) {}
135-
func (m *testMetrics) MeasureBackend5xx(t time.Time) {}
136-
func (m *testMetrics) IncErrorsStreaming(routeId string) {}
137-
func (m *testMetrics) RegisterHandler(path string, handler *http.ServeMux) {}
138-
func (m *testMetrics) UpdateGauge(key string, value float64) {}
139-
func (m *testMetrics) SetInvalidRoute(routeId, reason string) {}
140-
func (m *testMetrics) Close() {}
141-
func (m *testMetrics) String() string { return "testMetrics" }
142-
14391
func TestValkeyStorage_GetSetDelete(t *testing.T) {
14492
addr, done := valkeytest.NewTestValkey(t)
14593
defer done()
@@ -153,7 +101,7 @@ func TestValkeyStorage_GetSetDelete(t *testing.T) {
153101
defer ring.Close()
154102

155103
lru := NewLRUStorage(64<<20, nil, metrics.Default)
156-
s := NewL2Storage(ring, lru, &testMetrics{}, 0, valkey.IsValkeyNil)
104+
s := NewL2Storage(ring, lru, &metricstest.MockMetrics{}, 0, valkey.IsValkeyNil)
157105

158106
ctx := context.Background()
159107
key := "test-key"
@@ -205,7 +153,7 @@ func TestValkeyStorage_FallsBackToL1OnValkeyUnavailable(t *testing.T) {
205153
defer ring.Close()
206154

207155
lru := NewLRUStorage(64<<20, nil, metrics.Default)
208-
m := &testMetrics{}
156+
m := &metricstest.MockMetrics{}
209157
s := NewL2Storage(ring, lru, m, 0, valkey.IsValkeyNil)
210158

211159
// Stop valkey before exercising fallback paths.
@@ -231,11 +179,13 @@ func TestValkeyStorage_FallsBackToL1OnValkeyUnavailable(t *testing.T) {
231179
if got == nil {
232180
t.Fatal("expected L1 fallback hit, got nil")
233181
}
234-
if m.counter("cache.l1_hit") == 0 {
182+
if v, _ := m.Counter("cache.l1_hit"); v == 0 {
235183
t.Error("expected l1_hit to be incremented: Set fallback warmed L1, Get should serve from it")
236184
}
237-
if m.counter("cache.l2_get_error") != 0 {
238-
t.Errorf("expected l2_get_error=0 (L1 served before Valkey was contacted), got %d", m.counter("cache.l2_get_error"))
185+
186+
l2Error, _ := m.Counter("cache.l2_get_error")
187+
if l2Error != 0 {
188+
t.Errorf("expected l2_get_error=0 (L1 served before Valkey was contacted), got %d", l2Error)
239189
}
240190

241191
// Confirm the entry was physically written to L1 — not just returned via some
@@ -252,7 +202,7 @@ func TestValkeyStorage_FallsBackToL1OnValkeyUnavailable(t *testing.T) {
252202
func TestValkeyStorage_RecordsValkeyMiss(t *testing.T) {
253203
// Uses a stub client — no Docker or live Valkey needed.
254204
stub := newStubValkeyClient()
255-
m := &testMetrics{}
205+
m := &metricstest.MockMetrics{}
256206
lru := NewLRUStorage(64<<20, nil, metrics.Default)
257207
s := NewL2Storage(stub, lru, m, 0, valkey.IsValkeyNil)
258208

@@ -263,17 +213,19 @@ func TestValkeyStorage_RecordsValkeyMiss(t *testing.T) {
263213
if got != nil {
264214
t.Fatalf("expected nil on miss, got %+v", got)
265215
}
266-
if m.counter("cache.l2_miss") != 1 {
267-
t.Errorf("expected l2_miss=1, got %d", m.counter("cache.l2_miss"))
216+
l2Miss, _ := m.Counter("cache.l2_miss")
217+
if l2Miss != 1 {
218+
t.Errorf("expected l2_miss=1, got %d", l2Miss)
268219
}
269-
if m.counter("cache.l2_get_error") != 0 {
270-
t.Errorf("expected l2_get_error=0 on clean miss, got %d", m.counter("cache.l2_get_error"))
220+
l2Error, _ := m.Counter("cache.l2_get_error")
221+
if l2Error != 0 {
222+
t.Errorf("expected l2_get_error=0 on clean miss, got %d", l2Error)
271223
}
272224
}
273225

274226
func TestValkeyStorage_WriteThroughWarmsL1(t *testing.T) {
275227
stub := newStubValkeyClient()
276-
m := &testMetrics{}
228+
m := &metricstest.MockMetrics{}
277229
lru := NewLRUStorage(64<<20, nil, metrics.Default)
278230
s := NewL2Storage(stub, lru, m, 60*time.Second, valkey.IsValkeyNil)
279231

@@ -303,15 +255,16 @@ func TestValkeyStorage_WriteThroughWarmsL1(t *testing.T) {
303255
if string(got.Payload) != "warm" {
304256
t.Errorf("payload: got %q, want %q", string(got.Payload), "warm")
305257
}
306-
if m.counter("cache.l1_hit") != 1 {
307-
t.Errorf("expected l1_hit=1, got %d", m.counter("cache.l1_hit"))
258+
l1hit, _ := m.Counter("cache.l1_hit")
259+
if l1hit != 1 {
260+
t.Errorf("expected l1_hit=1, got %d", l1hit)
308261
}
309262
}
310263

311264
func TestValkeyStorage_L1TTLBoundedToEntryTTL(t *testing.T) {
312265
stub := newStubValkeyClient()
313266
lru := NewLRUStorage(64<<20, nil, metrics.Default)
314-
s := NewL2Storage(stub, lru, &testMetrics{}, 60*time.Second, valkey.IsValkeyNil)
267+
s := NewL2Storage(stub, lru, &metricstest.MockMetrics{}, 60*time.Second, valkey.IsValkeyNil)
315268

316269
ctx := context.Background()
317270
key := "bounded-key"
@@ -341,7 +294,7 @@ func TestValkeyStorage_L1TTLBoundedToEntryTTL(t *testing.T) {
341294

342295
func TestValkeyStorage_L1TTL_Zero_DisablesWarming(t *testing.T) {
343296
stub := newStubValkeyClient()
344-
m := &testMetrics{}
297+
m := &metricstest.MockMetrics{}
345298
lru := NewLRUStorage(64<<20, nil, metrics.Default)
346299
s := NewL2Storage(stub, lru, m, 0, valkey.IsValkeyNil)
347300

@@ -372,7 +325,7 @@ func TestValkeyStorage_L1TTL_Zero_DisablesWarming(t *testing.T) {
372325

373326
func TestValkeyStorage_RecordsL2Hit(t *testing.T) {
374327
stub := newStubValkeyClient()
375-
m := &testMetrics{}
328+
m := &metricstest.MockMetrics{}
376329
lru := NewLRUStorage(64<<20, nil, metrics.Default)
377330
s := NewL2Storage(stub, lru, m, 0, valkey.IsValkeyNil)
378331

@@ -391,11 +344,13 @@ func TestValkeyStorage_RecordsL2Hit(t *testing.T) {
391344
if got == nil {
392345
t.Fatal("expected entry from Valkey, got nil")
393346
}
394-
if m.counter("cache.l2_hit") != 1 {
395-
t.Errorf("expected l2_hit=1, got %d", m.counter("cache.l2_hit"))
347+
l2Hit, _ := m.Counter("cache.l2_hit")
348+
if l2Hit != 1 {
349+
t.Errorf("expected l2_hit=1, got %d", l2Hit)
396350
}
397-
if m.counter("cache.l1_hit") != 0 {
398-
t.Errorf("expected l1_hit=0 (write-around), got %d", m.counter("cache.l1_hit"))
351+
l1Hit, _ := m.Counter("cache.l1_hit")
352+
if l1Hit != 0 {
353+
t.Errorf("expected l1_hit=0 (write-around), got %d", l1Hit)
399354
}
400355
}
401356

@@ -405,32 +360,38 @@ func TestValkeyStorage_SplitFallbackCounters(t *testing.T) {
405360
// Get checks L1 first (L1-first reads) and finds the entry — incrementing l1_hit,
406361
// not l2_get_error.
407362
stub := newBrokenStubValkeyClient()
408-
m := &testMetrics{}
363+
m := &metricstest.MockMetrics{}
409364
lru := NewLRUStorage(64<<20, nil, metrics.Default)
410365
s := NewL2Storage(stub, lru, m, 0, valkey.IsValkeyNil)
411366

412367
ctx := context.Background()
413368
entry := &Entry{StatusCode: 200, Payload: []byte("x"), TTL: time.Minute, CreatedAt: time.Now()}
414369

415370
_ = s.Set(ctx, "k", entry)
416-
if m.counter("cache.l2_set_fallback") != 1 {
417-
t.Errorf("expected l2_set_fallback=1, got %d", m.counter("cache.l2_set_fallback"))
371+
l2SetFallback, _ := m.Counter("cache.l2_set_fallback")
372+
if l2SetFallback != 1 {
373+
t.Errorf("expected l2_set_fallback=1, got %d", l2SetFallback)
418374
}
419-
if m.counter("cache.l2_get_error") != 0 {
420-
t.Errorf("expected l2_get_error=0 after Set, got %d", m.counter("cache.l2_get_error"))
375+
l2GetError, _ := m.Counter("cache.l2_get_error")
376+
if l2GetError != 0 {
377+
t.Errorf("expected l2_get_error=0 after Set, got %d", l2GetError)
421378
}
422379

423380
// L1-first: the entry was written to L1 by the Set fallback path, so Get returns
424381
// it from L1 without ever touching (broken) Valkey.
425382
_, _ = s.Get(ctx, "k")
426-
if m.counter("cache.l1_hit") != 1 {
427-
t.Errorf("expected l1_hit=1, got %d", m.counter("cache.l1_hit"))
383+
l1Hit, _ := m.Counter("cache.l1_hit")
384+
if l1Hit != 1 {
385+
t.Errorf("expected l1_hit=1, got %d", l1Hit)
428386
}
429-
if m.counter("cache.l2_get_error") != 0 {
430-
t.Errorf("expected l2_get_error=0 (L1 served before Valkey check), got %d", m.counter("cache.l2_get_error"))
387+
388+
l2GetError, _ = m.Counter("cache.l2_get_error")
389+
if l2GetError != 0 {
390+
t.Errorf("expected l2_get_error=0 (L1 served before Valkey check), got %d", l2GetError)
431391
}
432-
if m.counter("cache.l2_set_fallback") != 1 {
433-
t.Errorf("l2_set_fallback should still be 1, got %d", m.counter("cache.l2_set_fallback"))
392+
l2SetFallback, _ = m.Counter("cache.l2_set_fallback")
393+
if l2SetFallback != 1 {
394+
t.Errorf("l2_set_fallback should still be 1, got %d", l2SetFallback)
434395
}
435396
}
436397

@@ -439,7 +400,7 @@ func TestValkeyStorage_DeleteCleansL1EvenOnValkeyError(t *testing.T) {
439400
// regardless of the Expire error from Valkey.
440401
stub := newBrokenStubValkeyClient()
441402
lru := NewLRUStorage(64<<20, nil, metrics.Default)
442-
s := NewL2Storage(stub, lru, &testMetrics{}, 0, valkey.IsValkeyNil)
403+
s := NewL2Storage(stub, lru, &metricstest.MockMetrics{}, 0, valkey.IsValkeyNil)
443404

444405
ctx := context.Background()
445406
entry := &Entry{StatusCode: 200, Payload: []byte("body"), TTL: time.Minute, CreatedAt: time.Now()}
@@ -461,17 +422,17 @@ func TestValkeyStorage_DeleteCleansL1EvenOnValkeyError(t *testing.T) {
461422

462423
func TestL2Storage_NewL2Storage_NegativeL1TTL_ClampsToDefault(t *testing.T) {
463424
stub := newStubValkeyClient()
464-
lru := NewLRUStorage(64<<20, nil, &testMetrics{})
465-
s := NewL2Storage(stub, lru, &testMetrics{}, -time.Second, valkey.IsValkeyNil)
425+
lru := NewLRUStorage(64<<20, nil, &metricstest.MockMetrics{})
426+
s := NewL2Storage(stub, lru, &metricstest.MockMetrics{}, -time.Second, valkey.IsValkeyNil)
466427
if s.l1TTL != defaultMinTTL {
467428
t.Errorf("expected l1TTL clamped to %v, got %v", defaultMinTTL, s.l1TTL)
468429
}
469430
}
470431

471432
func TestL2Storage_Get_CorruptJSON_ReturnsError(t *testing.T) {
472433
stub := newStubValkeyClient()
473-
lru := NewLRUStorage(64<<20, nil, &testMetrics{})
474-
s := NewL2Storage(stub, lru, &testMetrics{}, 0, valkey.IsValkeyNil)
434+
lru := NewLRUStorage(64<<20, nil, &metricstest.MockMetrics{})
435+
s := NewL2Storage(stub, lru, &metricstest.MockMetrics{}, 0, valkey.IsValkeyNil)
475436

476437
// Write corrupt JSON directly into the stub, bypassing Set.
477438
stub.mu.Lock()
@@ -486,7 +447,7 @@ func TestL2Storage_Get_CorruptJSON_ReturnsError(t *testing.T) {
486447

487448
func TestL2Storage_Get_L2Hit_WarmsL1(t *testing.T) {
488449
stub := newStubValkeyClient()
489-
m := &testMetrics{}
450+
m := &metricstest.MockMetrics{}
490451
lru := NewLRUStorage(64<<20, nil, m)
491452
s := NewL2Storage(stub, lru, m, 60*time.Second, valkey.IsValkeyNil)
492453

@@ -505,8 +466,9 @@ func TestL2Storage_Get_L2Hit_WarmsL1(t *testing.T) {
505466
if err != nil || got == nil {
506467
t.Fatalf("expected L2 hit: err=%v, got=%v", err, got)
507468
}
508-
if m.counter("cache.l2_hit") != 1 {
509-
t.Errorf("expected l2_hit=1, got %d", m.counter("cache.l2_hit"))
469+
l2Hit, _ := m.Counter("cache.l2_hit")
470+
if l2Hit != 1 {
471+
t.Errorf("expected l2_hit=1, got %d", l2Hit)
510472
}
511473

512474
// Break L2 — second Get must come from L1 (write-through warmed it).
@@ -515,14 +477,15 @@ func TestL2Storage_Get_L2Hit_WarmsL1(t *testing.T) {
515477
if err != nil || got2 == nil {
516478
t.Fatalf("expected L1 hit after warming: err=%v, got=%v", err, got2)
517479
}
518-
if m.counter("cache.l1_hit") != 1 {
519-
t.Errorf("expected l1_hit=1 after L1 warming, got %d", m.counter("cache.l1_hit"))
480+
l1Hit, _ := m.Counter("cache.l1_hit")
481+
if l1Hit != 1 {
482+
t.Errorf("expected l1_hit=1 after L1 warming, got %d", l1Hit)
520483
}
521484
}
522485

523486
func TestL2Storage_Get_L2Hit_ExpiredEntry_SkipsL1Warming(t *testing.T) {
524487
stub := newStubValkeyClient()
525-
m := &testMetrics{}
488+
m := &metricstest.MockMetrics{}
526489
lru := NewLRUStorage(64<<20, nil, m)
527490
s := NewL2Storage(stub, lru, m, 60*time.Second, valkey.IsValkeyNil)
528491

@@ -552,8 +515,8 @@ func TestL2Storage_Get_L2Hit_ExpiredEntry_SkipsL1Warming(t *testing.T) {
552515

553516
func TestL2Storage_Set_ZeroTTL_UsesDefaultMinTTL(t *testing.T) {
554517
stub := newStubValkeyClient()
555-
lru := NewLRUStorage(64<<20, nil, &testMetrics{})
556-
s := NewL2Storage(stub, lru, &testMetrics{}, 0, valkey.IsValkeyNil)
518+
lru := NewLRUStorage(64<<20, nil, &metricstest.MockMetrics{})
519+
s := NewL2Storage(stub, lru, &metricstest.MockMetrics{}, 0, valkey.IsValkeyNil)
557520

558521
ctx := context.Background()
559522
// TTL=0, StaleIfError=0, StaleWhileRevalidate=0 → l2TTL=0 → must use defaultMinTTL.

‎filters/cache/lru_test.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"time"
1010

1111
"github.com/zalando/skipper/metrics"
12+
"github.com/zalando/skipper/metrics/metricstest"
1213
)
1314

1415
func makeEntry(payload string, ttl time.Duration) *Entry {
@@ -179,7 +180,7 @@ func TestLRUStorage_OversizedEntry(t *testing.T) {
179180
// With 256 shards and 1 KB total capacity, each shard holds 4 bytes.
180181
// A payload larger than 4 bytes exceeds every shard's maxBytes.
181182
const totalBytes = 1024 // 1 KB → 4 bytes per shard
182-
m := &testMetrics{}
183+
m := &metricstest.MockMetrics{}
183184
s := NewLRUStorage(totalBytes, nil, m)
184185

185186
ctx := context.Background()
@@ -191,7 +192,7 @@ func TestLRUStorage_OversizedEntry(t *testing.T) {
191192
}
192193

193194
// The lru_oversized counter must have been incremented exactly once.
194-
if got := m.counter("cache.lru_oversized"); got != 1 {
195+
if got, _ := m.Counter("cache.lru_oversized"); got != 1 {
195196
t.Errorf("lru_oversized counter: got %d, want 1", got)
196197
}
197198

0 commit comments

Comments
 (0)