Skip to content

Commit 0c1b65e

Browse files
lucaslopezflucaslopezf
authored
fix: Combined cache Get, requestID middleware and cache metrics (#74)
* Fix get - combined cache * minor fix * Fix requestID middleware * Add metrics on zcache * fix combined cache GET * fix mocks * delete global ttl * minor fix * fix mock * minor fix * minor fix * minor fix
1 parent 9492c68 commit 0c1b65e

File tree

12 files changed

+83
-41
lines changed

12 files changed

+83
-41
lines changed

‎pkg/zcache/combined_cache.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ type combinedCache struct {
1818
localCache LocalCache
1919
remoteCache RemoteCache
2020
logger *zap.Logger
21-
ttl time.Duration
2221
isRemoteBestEffort bool
2322
metricsServer *metrics.TaskMetrics
2423
appName string
@@ -27,7 +26,7 @@ type combinedCache struct {
2726
func (c *combinedCache) Set(ctx context.Context, key string, value interface{}, ttl time.Duration) error {
2827
c.logger.Sugar().Debugf("set key on combined cache, key: [%s]", key)
2928

30-
if err := c.remoteCache.Set(ctx, key, value, c.ttl); err != nil {
29+
if err := c.remoteCache.Set(ctx, key, value, ttl); err != nil {
3130
c.logger.Sugar().Errorf("error setting key on combined/remote cache, key: [%s], err: %s", key, err)
3231
if !c.isRemoteBestEffort {
3332
c.logger.Sugar().Debugf("emitting error as remote best effort is false, key: [%s]", key)
@@ -64,10 +63,14 @@ func (c *combinedCache) Get(ctx context.Context, key string, data interface{}) e
6463
}
6564

6665
c.logger.Sugar().Debugf("set value found on remote cache in the local cache, key: [%s]", key)
66+
ttl, ttlErr := c.remoteCache.TTL(ctx, key)
6767

6868
// Refresh data TTL on both caches
69-
_ = c.localCache.Set(ctx, key, data, c.ttl)
70-
_ = c.remoteCache.Set(ctx, key, data, c.ttl)
69+
if ttlErr == nil {
70+
_ = c.localCache.Set(ctx, key, data, ttl)
71+
} else {
72+
c.logger.Sugar().Errorf("error getting TTL for key [%s] from remote cache, err: %s", key, ttlErr)
73+
}
7174
}
7275

7376
return nil

‎pkg/zcache/config.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@ type CombinedConfig struct {
5757
Local *LocalConfig
5858
Remote *RemoteConfig
5959
GlobalLogger *zap.Logger
60-
GlobalTtlSeconds int
6160
GlobalPrefix string
6261
IsRemoteBestEffort bool
6362
}

‎pkg/zcache/redis_cache.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ type RemoteCache interface {
2727
HGet(ctx context.Context, key, field string) (string, error)
2828
FlushAll(ctx context.Context) error
2929
Exists(ctx context.Context, keys ...string) (int64, error)
30+
TTL(ctx context.Context, key string) (time.Duration, error)
3031
}
3132

3233
type redisCache struct {
@@ -142,6 +143,13 @@ func (c *redisCache) HGet(ctx context.Context, key, field string) (string, error
142143
return c.client.HGet(ctx, realKey, field).Result()
143144
}
144145

146+
func (c *redisCache) TTL(ctx context.Context, key string) (time.Duration, error) {
147+
realKey := getKeyWithPrefix(c.prefix, key)
148+
c.logger.Sugar().Debugf("ttl on redis cache, realKey: [%s]", realKey)
149+
150+
return c.client.TTL(ctx, realKey).Result()
151+
}
152+
145153
func (c *redisCache) GetStats() ZCacheStats {
146154
poolStats := c.client.PoolStats()
147155
c.logger.Sugar().Debugf("redis cache pool stats: [%v]", poolStats)

‎pkg/zcache/zcache.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,6 @@ func NewCombinedCache(combinedConfig *CombinedConfig) (CombinedCache, error) {
8181
remoteCache: remoteClient,
8282
localCache: localClient,
8383
isRemoteBestEffort: combinedConfig.IsRemoteBestEffort,
84-
ttl: time.Duration(combinedConfig.GlobalTtlSeconds) * time.Second,
8584
logger: combinedConfig.GlobalLogger,
8685
}, nil
8786
}

‎pkg/zcache/zcache_mock.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,3 +90,8 @@ func (m *MockZCache) HGet(ctx context.Context, key, field string) (string, error
9090
args := m.Called(ctx, key, field)
9191
return args.Get(0).(string), args.Error(1)
9292
}
93+
94+
func (m *MockZCache) TTL(ctx context.Context, key string) (time.Duration, error) {
95+
args := m.Called(ctx, key)
96+
return args.Get(0).(time.Duration), args.Error(1)
97+
}

‎pkg/zrouter/context_mock.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package zrouter
22

33
import (
4+
"context"
45
"github.com/stretchr/testify/mock"
56
"net/http"
67
)
@@ -37,3 +38,8 @@ func (m *MockContext) DefaultQuery(key, defaultValue string) string {
3738
args := m.Called(key, defaultValue)
3839
return args.String(0)
3940
}
41+
42+
func (m *MockContext) Context() context.Context {
43+
args := m.Called()
44+
return args.Get(0).(context.Context)
45+
}

‎pkg/zrouter/zmiddlewares/cache.go

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package zmiddlewares
33
import (
44
"fmt"
55
"github.com/zondax/golem/pkg/logger"
6+
"github.com/zondax/golem/pkg/metrics"
67
"github.com/zondax/golem/pkg/zcache"
78
"github.com/zondax/golem/pkg/zrouter/domain"
89
"net/http"
@@ -12,15 +13,18 @@ import (
1213
)
1314

1415
const (
15-
cacheKeyPrefix = "zrouter_cache"
16+
cacheKeyPrefix = "zrouter_cache"
17+
cacheSetsMetric = "cache_sets"
18+
cacheHitsMetric = "cache_hits"
19+
cacheMissesMetric = "cache_misses"
1620
)
1721

1822
type CacheProcessedPath struct {
1923
Regex *regexp.Regexp
2024
TTL time.Duration
2125
}
2226

23-
func CacheMiddleware(cache zcache.ZCache, config domain.CacheConfig) func(next http.Handler) http.Handler {
27+
func CacheMiddleware(metricServer metrics.TaskMetrics, cache zcache.ZCache, config domain.CacheConfig) func(next http.Handler) http.Handler {
2428
processedPaths := processCachePaths(config.Paths)
2529

2630
return func(next http.Handler) http.Handler {
@@ -32,13 +36,13 @@ func CacheMiddleware(cache zcache.ZCache, config domain.CacheConfig) func(next h
3236
if pPath.Regex.MatchString(path) {
3337
key := constructCacheKey(fullURL)
3438

35-
if tryServeFromCache(w, r, cache, key) {
39+
if tryServeFromCache(w, r, cache, key, metricServer) {
3640
return
3741
}
3842

3943
rw := &responseWriter{ResponseWriter: w}
4044
next.ServeHTTP(rw, r) // Important: this line needs to be BEFORE setting the cache.
41-
cacheResponseIfNeeded(rw, r, cache, key, pPath.TTL)
45+
cacheResponseIfNeeded(rw, r, cache, key, pPath.TTL, metricServer)
4246
return
4347
}
4448
}
@@ -60,32 +64,42 @@ func constructCacheKey(fullURL string) string {
6064
return fmt.Sprintf("%s:%s", cacheKeyPrefix, fullURL)
6165
}
6266

63-
func tryServeFromCache(w http.ResponseWriter, r *http.Request, cache zcache.ZCache, key string) bool {
67+
func tryServeFromCache(w http.ResponseWriter, r *http.Request, cache zcache.ZCache, key string, metricServer metrics.TaskMetrics) bool {
6468
var cachedResponse []byte
6569
err := cache.Get(r.Context(), key, &cachedResponse)
6670
if err == nil && cachedResponse != nil {
6771
w.Header().Set(domain.ContentTypeHeader, domain.ContentTypeApplicationJSON)
6872
_, _ = w.Write(cachedResponse)
69-
requestID := r.Header.Get(RequestIDHeader)
7073

71-
logger.GetLoggerFromContext(r.Context()).Debugf("[Cache] request_id: %s - Method: %s - URL: %s | Status: %d - Response Body: %s",
72-
requestID, r.Method, r.URL.String(), http.StatusOK, string(cachedResponse))
74+
if err = metricServer.IncrementMetric(cacheHitsMetric, r.URL.Path); err != nil {
75+
logger.GetLoggerFromContext(r.Context()).Errorf("Error incrementing cache_hits metric: %v", err)
76+
}
77+
7378
return true
7479
}
80+
81+
if err = metricServer.IncrementMetric(cacheMissesMetric, r.URL.Path); err != nil {
82+
logger.GetLoggerFromContext(r.Context()).Errorf("Error incrementing cache_misses metric: %v", err)
83+
}
84+
7585
return false
7686
}
7787

78-
func cacheResponseIfNeeded(rw *responseWriter, r *http.Request, cache zcache.ZCache, key string, ttl time.Duration) {
88+
func cacheResponseIfNeeded(rw *responseWriter, r *http.Request, cache zcache.ZCache, key string, ttl time.Duration, metricServer metrics.TaskMetrics) {
7989
if rw.status != http.StatusOK {
8090
return
8191
}
8292

8393
responseBody := rw.Body()
8494
if err := cache.Set(r.Context(), key, responseBody, ttl); err != nil {
8595
logger.GetLoggerFromContext(r.Context()).Errorf("Internal error when setting cache response: %v\n%s", err, debug.Stack())
96+
return
8697
}
87-
}
8898

99+
if err := metricServer.IncrementMetric(cacheSetsMetric, r.URL.Path); err != nil {
100+
logger.GetLoggerFromContext(r.Context()).Errorf("Error incrementing cache_sets metric: %v", err)
101+
}
102+
}
89103
func ParseCacheConfigPaths(paths map[string]string) (domain.CacheConfig, error) {
90104
parsedPaths := make(map[string]time.Duration)
91105

‎pkg/zrouter/zmiddlewares/http.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,11 @@ func requestIDMiddleware(next http.Handler) http.Handler {
2424

2525
w.Header().Set(RequestIDHeader, requestID)
2626
rw := &responseWriter{ResponseWriter: w}
27-
ctx := logger.ContextWithLogger(r.Context(), logger.NewLogger(logger.Field{
27+
newLogger := logger.NewLogger(logger.Field{
2828
Key: logger.RequestIDKey,
2929
Value: requestID,
30-
}))
31-
30+
})
31+
ctx := logger.ContextWithLogger(r.Context(), newLogger)
3232
next.ServeHTTP(rw, r.WithContext(ctx))
3333
})
3434
}

‎pkg/zrouter/zmiddlewares/logging.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010

1111
type LoggingMiddlewareOptions struct {
1212
ExcludePaths []string
13+
Enable bool
1314
}
1415

1516
func LoggingMiddleware(options LoggingMiddlewareOptions) func(http.Handler) http.Handler {
@@ -38,10 +39,7 @@ func LoggingMiddleware(options LoggingMiddlewareOptions) func(http.Handler) http
3839
start := time.Now()
3940
next.ServeHTTP(rw, r)
4041
duration := time.Since(start)
41-
ctx := r.Context()
42-
43-
log := logger.GetLoggerFromContext(ctx)
44-
42+
log := logger.GetLoggerFromContext(r.Context())
4543
if log.IsDebugEnabled() {
4644
log.Debugf("Method: %s - URL: %s | Status: %d - Duration: %s - Response Body: %s",
4745
r.Method, r.URL.String(), rw.status, duration, string(rw.Body()))

‎pkg/zrouter/zmiddlewares/metrics.go

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -30,20 +30,23 @@ func RegisterRequestMetrics(appName string, metricsServer metrics.TaskMetrics) [
3030
}
3131
}
3232

33-
totalRequestsMetricName := getMetricName(appName, totalRequestMetricType)
34-
responseSizeMetricName := getMetricName(appName, responseSizeMetricType)
35-
durationMillisecondsMetricName := getMetricName(appName, durationMillisecondsMetricType)
36-
activeConnectionsMetricName := getMetricName(appName, activeConnectionsMetricType)
37-
register(totalRequestsMetricName, "Total number of HTTP requests made.", []string{methodLabel, pathLabel, statusLabel}, &collectors.Counter{})
38-
register(durationMillisecondsMetricName, "Duration of HTTP requests.", []string{methodLabel, pathLabel, statusLabel}, &collectors.Histogram{})
39-
register(responseSizeMetricName, "Size of HTTP response in bytes.", []string{methodLabel, pathLabel, statusLabel}, &collectors.Histogram{})
33+
totalRequestsMetricName := getMetricName(appName, "total_requests")
34+
responseSizeMetricName := getMetricName(appName, "response_size")
35+
durationMillisecondsMetricName := getMetricName(appName, "request_duration_ms")
36+
activeConnectionsMetricName := getMetricName(appName, "active_connections")
37+
register(totalRequestsMetricName, "Total number of HTTP requests made.", []string{"method", "path", "status"}, &collectors.Counter{})
38+
register(durationMillisecondsMetricName, "Duration of HTTP requests in milliseconds.", []string{"method", "path", "status"}, &collectors.Histogram{})
39+
register(responseSizeMetricName, "Size of HTTP response in bytes.", []string{"method", "path", "status"}, &collectors.Histogram{})
4040
register(activeConnectionsMetricName, "Number of active HTTP connections.", nil, &collectors.Gauge{})
4141

42-
if len(errs) > 0 {
43-
return errs
44-
}
42+
cacheHitsMetricName := getMetricName(appName, cacheHitsMetric)
43+
cacheMissesMetricName := getMetricName(appName, cacheMissesMetric)
44+
cacheSetsMetricName := getMetricName(appName, cacheSetsMetric)
45+
register(cacheHitsMetricName, "Number of cache hits.", []string{pathLabel}, &collectors.Counter{})
46+
register(cacheMissesMetricName, "Number of cache misses.", []string{pathLabel}, &collectors.Counter{})
47+
register(cacheSetsMetricName, "Number of responses added to the cache.", []string{pathLabel}, &collectors.Counter{})
4548

46-
return nil
49+
return errs
4750
}
4851

4952
func RequestMetrics(appName string, metricsServer metrics.TaskMetrics) Middleware {

0 commit comments

Comments
 (0)