Skip to content

Commit

Permalink
Enrich config adjustments (#101)
Browse files Browse the repository at this point in the history 
* Change cache key

* More logs
  • Loading branch information
@miguelreiswildlife
miguelreiswildlife authored Sep 27, 2023
1 parent 4efa817 commit a18fe55
Show file tree
Hide file tree
Showing 14 changed files with 68 additions and 81 deletions.
4 changes: 4 additions & 0 deletions api/app.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,11 @@ func (app *App) configureEnrichment() {
enrichercache.WithLogger(app.Logger),
enrichercache.WithTTL(app.ParsedConfig.Enrichment.Cache.TTL),
)

app.Logger.Info("Enrichment cache configured successfully.")
}

app.Logger.Info("Enrichment configured successfully.")
}

// OnErrorHandler handles panics
Expand Down
2 changes: 1 addition & 1 deletion config/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ type (

CloudSaveConfig struct {
// Enabled indicates whether the Cloud Save service should be used for enrichment for each tenant.
Enabled map[string]bool `mapstructure:"disabled"`
Enabled map[string]bool `mapstructure:"enabled"`

// URL is the URL to call the Cloud Save service.
Url string `mapstructure:"url"`
Expand Down
4 changes: 2 additions & 2 deletions config/default.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ enrichment:
webhook_urls:
cache:
ttl: 24h
addrs: ""
username: ""
addr: ""
password: ""
webhook_timeout: 500ms
cloud_save:
url:
Expand Down
4 changes: 2 additions & 2 deletions config/local.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ enrichment:
webhook_timeout: 500ms
cache:
ttl: 24h
addrs: "localhost:6739"
username: ""
addr: "localhost:6739"
password: ""
cloud_save:
url: "localhost:8888/"
enabled:
18 changes: 8 additions & 10 deletions leaderboard/enriching/cache/cache.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ import (
"time"
)

// cacheKeyFormat is {tenantID}:{leaderboardID}:{memberID}
const cacheKeyFormat = "leaderboards-enrich-caching:%s:%s:%s"
// cacheKeyFormat is {tenantID}:{memberID}
const cacheKeyFormat = "leaderboards-enrich-caching:%s:%s"

type enricherRedisCache struct {
redis *redis.Client
Expand All @@ -29,11 +29,10 @@ func NewEnricherRedisCache(

func (e *enricherRedisCache) Get(
ctx context.Context,
tenantID,
leaderboardID string,
tenantID string,
members []*model.Member,
) (map[string]map[string]string, bool, error) {
keys := getKeysFromMemberArray(tenantID, leaderboardID, members)
keys := getKeysFromMemberArray(tenantID, members)
dataArray, err := e.redis.MGet(ctx, keys...).Result()
if err != nil {
return nil, false, fmt.Errorf("failed to get data from cacheConfig: %w", err)
Expand All @@ -60,12 +59,11 @@ func (e *enricherRedisCache) Get(

func (e *enricherRedisCache) Set(
ctx context.Context,
tenantID,
leaderboardID string,
tenantID string,
members []*model.Member,
ttl time.Duration,
) error {
keys := getKeysFromMemberArray(tenantID, leaderboardID, members)
keys := getKeysFromMemberArray(tenantID, members)
pipe := e.redis.TxPipeline()
for i, member := range members {
if member.Metadata != nil {
Expand All @@ -85,10 +83,10 @@ func (e *enricherRedisCache) Set(
return nil
}

func getKeysFromMemberArray(tenantID, leaderboardID string, members []*model.Member) []string {
func getKeysFromMemberArray(tenantID string, members []*model.Member) []string {
keys := make([]string, len(members))
for i, member := range members {
keys[i] = fmt.Sprintf(cacheKeyFormat, tenantID, leaderboardID, member.PublicID)
keys[i] = fmt.Sprintf(cacheKeyFormat, tenantID, member.PublicID)
}
return keys
}
18 changes: 7 additions & 11 deletions leaderboard/enriching/cache/cache_instrumented.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,19 +38,17 @@ func NewInstrumentedCache(

func (c *instrumentedCache) Get(
ctx context.Context,
tenantID,
leaderboardID string,
tenantID string,
members []*model.Member,
) (map[string]map[string]string, bool, error) {
start := time.Now()

span, ctx := opentracing.StartSpanFromContext(ctx, "podium.enriching_cache", opentracing.Tags{
"tenant_id": tenantID,
"leaderboard_id": leaderboardID,
"tenant_id": tenantID,
})
defer span.Finish()

metadata, hit, err := c.impl.Get(ctx, tenantID, leaderboardID, members)
metadata, hit, err := c.impl.Get(ctx, tenantID, members)

c.metricsReporter.Increment(enrichmentCacheGets)
c.metricsReporter.Timing(enrichmentCacheGetTimingMilli, time.Since(start))
Expand All @@ -71,21 +69,19 @@ func (c *instrumentedCache) Get(

func (c *instrumentedCache) Set(
ctx context.Context,
tenantID,
leaderboardID string,
tenantID string,
members []*model.Member,
ttl time.Duration,
) error {
start := time.Now()

span, ctx := opentracing.StartSpanFromContext(ctx, "podium.enriching_cache", opentracing.Tags{
"tenant_id": tenantID,
"leaderboard_id": leaderboardID,
"ttl": ttl,
"tenant_id": tenantID,
"ttl": ttl,
})
defer span.Finish()

err := c.impl.Set(ctx, tenantID, leaderboardID, members, ttl)
err := c.impl.Set(ctx, tenantID, members, ttl)

c.metricsReporter.Increment(enrichmentCacheSets)
c.metricsReporter.Timing(enrichmentCacheSetTimingMilli, time.Since(start))
Expand Down
22 changes: 10 additions & 12 deletions leaderboard/enriching/cache/cache_instrumented_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (

var _ = Describe("Instrumented enrich cache Get tests", func() {
tenantID := "tenant-id"
leaderboardID := "leaderboard-id"
members := []*model.Member{
{
PublicID: "member1",
Expand All @@ -31,13 +30,13 @@ var _ = Describe("Instrumented enrich cache Get tests", func() {
impl := mock_enriching.NewMockEnricherCache(ctrl)
metricsReporter := mock_extensions.NewMockMetricsReporter(ctrl)

impl.EXPECT().Get(gomock.Any(), tenantID, leaderboardID, members).Return(result, true, nil)
impl.EXPECT().Get(gomock.Any(), tenantID, members).Return(result, true, nil)
metricsReporter.EXPECT().Increment(enrichmentCacheGets).Return(nil)
metricsReporter.EXPECT().Increment(enrichmentCacheHits).Return(nil)
metricsReporter.EXPECT().Timing(enrichmentCacheGetTimingMilli, gomock.Any()).Return(nil)

instrumentedCache := NewInstrumentedCache(impl, metricsReporter)
res, hit, err := instrumentedCache.Get(context.Background(), tenantID, leaderboardID, members)
res, hit, err := instrumentedCache.Get(context.Background(), tenantID, members)

Expect(res).To(Equal(result))
Expect(hit).To(BeTrue())
Expand All @@ -49,12 +48,12 @@ var _ = Describe("Instrumented enrich cache Get tests", func() {
impl := mock_enriching.NewMockEnricherCache(ctrl)
metricsReporter := mock_extensions.NewMockMetricsReporter(ctrl)

impl.EXPECT().Get(gomock.Any(), tenantID, leaderboardID, members).Return(nil, false, nil)
impl.EXPECT().Get(gomock.Any(), tenantID, members).Return(nil, false, nil)
metricsReporter.EXPECT().Increment(enrichmentCacheGets).Return(nil)
metricsReporter.EXPECT().Timing(enrichmentCacheGetTimingMilli, gomock.Any()).Return(nil)

instrumentedCache := NewInstrumentedCache(impl, metricsReporter)
res, hit, err := instrumentedCache.Get(context.Background(), tenantID, leaderboardID, members)
res, hit, err := instrumentedCache.Get(context.Background(), tenantID, members)
Expect(res).To(BeNil())
Expect(hit).To(BeFalse())
Expect(err).To(BeNil())
Expand All @@ -65,13 +64,13 @@ var _ = Describe("Instrumented enrich cache Get tests", func() {
impl := mock_enriching.NewMockEnricherCache(ctrl)
metricsReporter := mock_extensions.NewMockMetricsReporter(ctrl)

impl.EXPECT().Get(gomock.Any(), tenantID, leaderboardID, members).Return(nil, false, errors.New("error"))
impl.EXPECT().Get(gomock.Any(), tenantID, members).Return(nil, false, errors.New("error"))
metricsReporter.EXPECT().Increment(enrichmentCacheGets).Return(nil)
metricsReporter.EXPECT().Increment(enrichmentCacheGetErrors).Return(nil)
metricsReporter.EXPECT().Timing(enrichmentCacheGetTimingMilli, gomock.Any()).Return(nil)

instrumentedCache := NewInstrumentedCache(impl, metricsReporter)
res, hit, err := instrumentedCache.Get(context.Background(), tenantID, leaderboardID, members)
res, hit, err := instrumentedCache.Get(context.Background(), tenantID, members)

Expect(res).To(BeNil())
Expect(hit).To(BeFalse())
Expand All @@ -81,7 +80,6 @@ var _ = Describe("Instrumented enrich cache Get tests", func() {

var _ = Describe("Instrumented enrich cache Set tests", func() {
tenantID := "tenant-id"
leaderboardID := "leaderboard-id"
members := []*model.Member{
{
PublicID: "member1",
Expand All @@ -96,12 +94,12 @@ var _ = Describe("Instrumented enrich cache Set tests", func() {
impl := mock_enriching.NewMockEnricherCache(ctrl)
metricsReporter := mock_extensions.NewMockMetricsReporter(ctrl)

impl.EXPECT().Set(gomock.Any(), tenantID, leaderboardID, members, gomock.Any()).Return(nil)
impl.EXPECT().Set(gomock.Any(), tenantID, members, gomock.Any()).Return(nil)
metricsReporter.EXPECT().Increment(enrichmentCacheSets).Return(nil)
metricsReporter.EXPECT().Timing(enrichmentCacheSetTimingMilli, gomock.Any()).Return(nil)

instrumentedCache := NewInstrumentedCache(impl, metricsReporter)
err := instrumentedCache.Set(context.Background(), tenantID, leaderboardID, members, 0)
err := instrumentedCache.Set(context.Background(), tenantID, members, 0)

Expect(err).To(BeNil())
})
Expand All @@ -111,13 +109,13 @@ var _ = Describe("Instrumented enrich cache Set tests", func() {
impl := mock_enriching.NewMockEnricherCache(ctrl)
metricsReporter := mock_extensions.NewMockMetricsReporter(ctrl)

impl.EXPECT().Set(gomock.Any(), tenantID, leaderboardID, members, gomock.Any()).Return(errors.New("error"))
impl.EXPECT().Set(gomock.Any(), tenantID, members, gomock.Any()).Return(errors.New("error"))
metricsReporter.EXPECT().Increment(enrichmentCacheSets).Return(nil)
metricsReporter.EXPECT().Increment(enrichmentCacheSetErrors).Return(nil)
metricsReporter.EXPECT().Timing(enrichmentCacheSetTimingMilli, gomock.Any()).Return(nil)

instrumentedCache := NewInstrumentedCache(impl, metricsReporter)
err := instrumentedCache.Set(context.Background(), tenantID, leaderboardID, members, 0)
err := instrumentedCache.Set(context.Background(), tenantID, members, 0)

Expect(err).To(HaveOccurred())
})
Expand Down
35 changes: 16 additions & 19 deletions leaderboard/enriching/cache/cache_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (

var _ = Describe("Members array to keys array test", func() {
tenantID := "tenantID"
leaderboardID := "leaderboardID"

It("should return keys correctly", func() {
members := []*model.Member{
Expand All @@ -25,17 +24,16 @@ var _ = Describe("Members array to keys array test", func() {
},
}

keys := getKeysFromMemberArray(tenantID, leaderboardID, members)
keys := getKeysFromMemberArray(tenantID, members)

Expect(keys).To(HaveLen(2))
Expect(keys[0]).To(Equal("leaderboards-enrich-caching:tenantID:leaderboardID:member1"))
Expect(keys[1]).To(Equal("leaderboards-enrich-caching:tenantID:leaderboardID:member2"))
Expect(keys[0]).To(Equal("leaderboards-enrich-caching:tenantID:member1"))
Expect(keys[1]).To(Equal("leaderboards-enrich-caching:tenantID:member2"))
})
})

var _ = Describe("Enricher cacheConfig Get tests", func() {
tenantID := "tenantID"
leaderboardID := "leaderboardID"

It("should return false and error if redis fails", func() {
redis, redisMock := redismock.NewClientMock()
Expand All @@ -47,11 +45,11 @@ var _ = Describe("Enricher cacheConfig Get tests", func() {
}

redisMock.ExpectMGet(
getKeysFromMemberArray(tenantID, leaderboardID, members)...,
getKeysFromMemberArray(tenantID, members)...,
).SetErr(errors.New("some error"))

cache := NewEnricherRedisCache(redis)
res, hit, err := cache.Get(context.Background(), tenantID, leaderboardID, members)
res, hit, err := cache.Get(context.Background(), tenantID, members)

Expect(res).To(BeNil())
Expect(hit).To(BeFalse())
Expand All @@ -72,11 +70,11 @@ var _ = Describe("Enricher cacheConfig Get tests", func() {
}

redisMock.ExpectMGet(
getKeysFromMemberArray(tenantID, leaderboardID, members)...,
getKeysFromMemberArray(tenantID, members)...,
).SetVal([]interface{}{nil, nil})

cache := NewEnricherRedisCache(redis)
res, hit, err := cache.Get(context.Background(), tenantID, leaderboardID, members)
res, hit, err := cache.Get(context.Background(), tenantID, members)

Expect(res).To(BeNil())
Expect(hit).To(BeFalse())
Expand All @@ -101,11 +99,11 @@ var _ = Describe("Enricher cacheConfig Get tests", func() {
}

redisMock.ExpectMGet(
getKeysFromMemberArray(tenantID, leaderboardID, members)...,
getKeysFromMemberArray(tenantID, members)...,
).SetVal(mgetExpectedResult)

cache := NewEnricherRedisCache(redis)
res, hit, err := cache.Get(context.Background(), tenantID, leaderboardID, members)
res, hit, err := cache.Get(context.Background(), tenantID, members)

Expect(res).To(BeNil())
Expect(hit).To(BeFalse())
Expand All @@ -130,11 +128,11 @@ var _ = Describe("Enricher cacheConfig Get tests", func() {
}

redisMock.ExpectMGet(
getKeysFromMemberArray(tenantID, leaderboardID, members)...,
getKeysFromMemberArray(tenantID, members)...,
).SetVal(mgetExpectedResult)

cache := NewEnricherRedisCache(redis)
res, hit, err := cache.Get(context.Background(), tenantID, leaderboardID, members)
res, hit, err := cache.Get(context.Background(), tenantID, members)

expectedResult := map[string]map[string]string{
"member1": {
Expand All @@ -153,7 +151,6 @@ var _ = Describe("Enricher cacheConfig Get tests", func() {

var _ = Describe("Ericher cacheConfig Set tests", func() {
tenantID := "tenantID"
leaderboardID := "leaderboardID"

It("should set the data in redis", func() {
redis := redis.NewClient(&redis.Options{Addr: "localhost:6379"})
Expand All @@ -174,20 +171,20 @@ var _ = Describe("Ericher cacheConfig Set tests", func() {
},
}

err := cache.Set(context.Background(), tenantID, leaderboardID, members, 0)
err := cache.Set(context.Background(), tenantID, members, 0)

res, err := redis.Get(context.Background(), fmt.Sprintf(cacheKeyFormat, tenantID, leaderboardID, "member1")).Result()
res, err := redis.Get(context.Background(), fmt.Sprintf(cacheKeyFormat, tenantID, "member1")).Result()
Expect(res).To(Equal("{\"key1\":\"value1\"}"))
Expect(err).To(BeNil())

res, err = redis.Get(context.Background(), fmt.Sprintf(cacheKeyFormat, tenantID, leaderboardID, "member2")).Result()
res, err = redis.Get(context.Background(), fmt.Sprintf(cacheKeyFormat, tenantID, "member2")).Result()
Expect(res).To(Equal("{\"key2\":\"value2\"}"))
Expect(err).To(BeNil())
})

AfterEach(func() {
redis := redis.NewClient(&redis.Options{Addr: "localhost:6379"})
redis.Del(context.Background(), fmt.Sprintf(cacheKeyFormat, tenantID, leaderboardID, "member1"))
redis.Del(context.Background(), fmt.Sprintf(cacheKeyFormat, tenantID, leaderboardID, "member2"))
redis.Del(context.Background(), fmt.Sprintf(cacheKeyFormat, tenantID, "member1"))
redis.Del(context.Background(), fmt.Sprintf(cacheKeyFormat, tenantID, "member2"))
})
})
4 changes: 2 additions & 2 deletions leaderboard/enriching/cache/cached_enricher.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func (e *cachedEnricher) Enrich(
zap.String("leaderboardID", leaderboardID),
)

cached, hit, err := e.cache.Get(ctx, tenantID, leaderboardID, members)
cached, hit, err := e.cache.Get(ctx, tenantID, members)
if err != nil {
l.Error("could not get cached enrichment data", zap.Error(err))
}
Expand All @@ -72,7 +72,7 @@ func (e *cachedEnricher) Enrich(
return nil, err
}

err = e.cache.Set(ctx, tenantID, leaderboardID, members, e.config.ttl)
err = e.cache.Set(ctx, tenantID, members, e.config.ttl)
if err != nil {
l.Error("could not set cached enrichment data", zap.Error(err))
}
Expand Down
Loading

0 comments on commit a18fe55

Please sign in to comment.