chore: review 1

prebid · Dec 13, 2023 · b9f0ed2 · b9f0ed2
1 parent 366fd2d
commit b9f0ed2
Show file tree

Hide file tree

Showing 6 changed files with 30 additions and 29 deletions.
diff --git a/README.md b/README.md
@@ -208,7 +208,7 @@ This section does not describe permanent API contracts; it just describes limita
 
 ## Backend Configuration
 
-Prebid Cache requires a backend data store which enforces TTL expiration. The following storage options are supported: Aerospike, Cassandra, Memcache, and Redis/Redis sentinel. You're welcomed to contribute a new backend adapter if needed.
+Prebid Cache requires a backend data store which enforces TTL expiration. The following storage options are supported: Aerospike, Cassandra, Memcache, and Redis/Redis sentinel. You're welcome to contribute a new backend adapter if needed.
 
 There is also an option (enabled by default) for a basic in-memory data store intended only for development. This backend does not support TTL expiration and is not built for production use.
 
@@ -247,8 +247,8 @@ Prebid Cache makes use of a Redis Go client compatible with Redis 6. Full docume
 | --- | --- | --- |
 | host | string | Redis server URI (redis standalone mode) |
 | port | integer | Redis server port (redis standalone mode) |
-| hosts |string array | Redis server sentinel URI (redis sentinel mode)|
-| mastername | string | Redis master sentinel name (redis sentinel mode)|
+| sentinel_hosts_ports |string array | Redis server sentinel host:port list (redis sentinel mode)|
+| sentinel_mastername | string | Redis master sentinel name (redis sentinel mode)|
 | password | string | Redis password |
 | db | integer | Database to be selected after connecting to the server |
 | expiration | integer | Availability in the Redis system in Minutes |

diff --git a/backends/config/config_test.go b/backends/config/config_test.go
@@ -150,8 +150,8 @@ func TestNewBaseBackend(t *testing.T) {
 			inConfig: config.Backend{Type: config.BackendRedis},
 			expectedLogEntries: []logEntry{
 				{msg: "Error creating Redis backend: At least one Host[s] is required.", lvl: logrus.FatalLevel},
-				{msg: "Creating Redis backend", lvl: logrus.InfoLevel},
 				{msg: "Error creating Redis backend: ", lvl: logrus.FatalLevel},
+				{msg: "Creating Redis backend", lvl: logrus.InfoLevel},
 			},
 		},
 		{

diff --git a/backends/redis.go b/backends/redis.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"crypto/tls"
 	"strconv"
+	"strings"
 	"time"
 
 	"github.com/prebid/prebid-cache/config"
@@ -48,21 +49,21 @@ type RedisBackend struct {
 // NewRedisBackend initializes the redis client and pings to make sure connection was successful
 func NewRedisBackend(cfg config.Redis, ctx context.Context) *RedisBackend {
 
-	if len(cfg.Hosts) < 1 || cfg.Host == "" {
+	if len(cfg.SentinelHostsPorts) < 1 || cfg.Host == "" {
 		log.Fatalf("Error creating Redis backend: At least one Host[s] is required.")
 	}
 
 	var redisClient RedisDBClient
 
 	sentinel := false
 
-	if cfg.MasterName != "" {
+	if cfg.SentinelMasterName != "" {
 		sentinel = true
 		// Mode failover (sentinel)
 		log.Info("Creating Redis sentinel backend")
 		options := &redis.FailoverOptions{
-			MasterName:       cfg.MasterName,
-			SentinelAddrs:    cfg.Hosts,
+			MasterName:       cfg.SentinelMasterName,
+			SentinelAddrs:    cfg.SentinelHostsPorts,
 			DB:               cfg.Db,
 			Password:         cfg.Password,
 			SentinelPassword: cfg.Password,
@@ -99,7 +100,7 @@ func NewRedisBackend(cfg config.Redis, ctx context.Context) *RedisBackend {
 	}
 
 	if sentinel {
-		log.Infof("Connected to Redis: %v", cfg.Hosts)
+		log.Infof("Connected to Redis: %s", strings.Join(cfg.SentinelHostsPorts, ","))
 	} else {
 		log.Infof("Connected to Redis: %s:%v", cfg.Host, cfg.Port)
 	}

diff --git a/config/backends.go b/config/backends.go
@@ -151,14 +151,14 @@ func (cfg *Memcache) validateAndLog() error {
 }
 
 type Redis struct {
-	Host              string   `mapstructure:"host"`
-	Port              int      `mapstructure:"port"`
-	Password          string   `mapstructure:"password"`
-	Db                int      `mapstructure:"db"`
-	ExpirationMinutes int      `mapstructure:"expiration"`
-	TLS               RedisTLS `mapstructure:"tls"`
-	Hosts             []string `mapstructure:"hosts"`
-	MasterName        string   `mapstructure:"mastername"`
+	Host               string   `mapstructure:"host"`
+	Port               int      `mapstructure:"port"`
+	Password           string   `mapstructure:"password"`
+	Db                 int      `mapstructure:"db"`
+	ExpirationMinutes  int      `mapstructure:"expiration"`
+	TLS                RedisTLS `mapstructure:"tls"`
+	SentinelHostsPorts []string `mapstructure:"sentinel_hosts_ports"`
+	SentinelMasterName string   `mapstructure:"sentinel_mastername"`
 }
 
 type RedisTLS struct {
@@ -167,9 +167,9 @@ type RedisTLS struct {
 }
 
 func (cfg *Redis) validateAndLog() error {
-	if cfg.Host != "" && len(cfg.Hosts) > 0 {
-		log.Infof("config.backend.redis.hosts: %s. Note that redis host will be ignore if 'hosts' is define", cfg.Hosts)
-		log.Infof("config.backend.redis.mastername: %s.", cfg.MasterName)
+	if cfg.Host != "" && len(cfg.SentinelHostsPorts) > 0 {
+		log.Infof("config.backend.redis.sentinel_hosts_ports: %s. Note that redis 'host' will be ignore if 'sentinel_hosts_ports' is define", cfg.SentinelHostsPorts)
+		log.Infof("config.backend.redis.sentinel_mastername: %s.", cfg.SentinelMasterName)
 	} else {
 		log.Infof("config.backend.redis.host: %s", cfg.Host)
 		log.Infof("config.backend.redis.port: %d", cfg.Port)

diff --git a/config/config.go b/config/config.go
@@ -63,8 +63,8 @@ func setConfigDefaults(v *viper.Viper) {
 	v.SetDefault("backend.cassandra.default_ttl_seconds", utils.CASSANDRA_DEFAULT_TTL_SECONDS)
 	v.SetDefault("backend.memcache.hosts", []string{})
 	v.SetDefault("backend.redis.host", "")
-	v.SetDefault("backend.redis.hosts", []string{})
-	v.SetDefault("backend.redis.mastername", "")
+	v.SetDefault("backend.redis.sentinel_hosts_ports", []string{})
+	v.SetDefault("backend.redis.sentinel_mastername", "")
 	v.SetDefault("backend.redis.port", 0)
 	v.SetDefault("backend.redis.password", "")
 	v.SetDefault("backend.redis.db", 0)

diff --git a/config/config_test.go b/config/config_test.go
@@ -1202,8 +1202,8 @@ func getExpectedDefaultConfig() Configuration {
 				DefaultTTL: utils.CASSANDRA_DEFAULT_TTL_SECONDS,
 			},
 			Redis: Redis{
-				Hosts:             []string{},
-				ExpirationMinutes: utils.REDIS_DEFAULT_EXPIRATION_MINUTES,
+				SentinelHostsPorts: []string{},
+				ExpirationMinutes:  utils.REDIS_DEFAULT_EXPIRATION_MINUTES,
 			},
 			Ignite: Ignite{
 				Headers: map[string]string{},
@@ -1270,13 +1270,13 @@ func getExpectedFullConfigForTestFile() Configuration {
 			Redis: Redis{
 				Host: "127.0.0.1",
 				Port: 6379,
-				Hosts: []string{
+				SentinelHostsPorts: []string{
 					"10.0.0.1:26379", "127.0.0.1",
 				},
-				MasterName:        "mymaster",
-				Password:          "redis-password",
-				Db:                1,
-				ExpirationMinutes: 1,
+				SentinelMasterName: "mymaster",
+				Password:           "redis-password",
+				Db:                 1,
+				ExpirationMinutes:  1,
 				TLS: RedisTLS{
 					Enabled:            false,
 					InsecureSkipVerify: false,