From fa5d9ccdb51a8c7bba57084df8434714a9768f3b Mon Sep 17 00:00:00 2001
From: Yichao Yang <ycyang@uber.com>
Date: Mon, 6 May 2019 15:27:37 -0700
Subject: [PATCH] Switch to File Based Dynamic Config Client (#1795)

---
 cmd/server/server.go                          |  9 ++++-
 common/service/config/config.go               |  3 ++
 .../service/dynamicconfig/fileBasedClient.go  | 10 ++---
 config/development.yaml                       |  4 ++
 config/dynamicconfig/development.yaml         | 37 +++++++++++++++++++
 5 files changed, 55 insertions(+), 8 deletions(-)
 create mode 100644 config/dynamicconfig/development.yaml

diff --git a/cmd/server/server.go b/cmd/server/server.go
index b4f4e1c7759..1d4966b9b29 100644
--- a/cmd/server/server.go
+++ b/cmd/server/server.go
@@ -30,6 +30,7 @@ import (
 	"github.com/uber/cadence/common/cluster"
 	"github.com/uber/cadence/common/elasticsearch"
 	"github.com/uber/cadence/common/log/loggerimpl"
+	"github.com/uber/cadence/common/log/tag"
 	"github.com/uber/cadence/common/messaging"
 	"github.com/uber/cadence/common/metrics"
 	"github.com/uber/cadence/common/service"
@@ -111,7 +112,11 @@ func (s *server) startService() common.Daemon {
 		log.Fatalf("error creating ringpop factory: %v", err)
 	}
 
-	params.DynamicConfig = dynamicconfig.NewNopClient()
+	params.DynamicConfig, err = dynamicconfig.NewFileBasedClient(&s.cfg.DynamicConfigClient, params.Logger.WithTags(tag.Service(params.Name)), s.doneC)
+	if err != nil {
+		log.Printf("error creating file based dynamic config client, use no-op config client instead. error: %v", err)
+		params.DynamicConfig = dynamicconfig.NewNopClient()
+	}
 	dc := dynamicconfig.NewCollection(params.DynamicConfig, params.Logger)
 
 	svcCfg := s.cfg.Services[s.name]
@@ -199,5 +204,5 @@ func (s *server) startService() common.Daemon {
 // execute runs the daemon in a separate go routine
 func execute(d common.Daemon, doneC chan struct{}) {
 	d.Start()
-	doneC <- struct{}{}
+	close(doneC)
 }
diff --git a/common/service/config/config.go b/common/service/config/config.go
index 97fe2dfb4ff..b9799f637e6 100644
--- a/common/service/config/config.go
+++ b/common/service/config/config.go
@@ -57,6 +57,9 @@ type (
 		ElasticSearch elasticsearch.Config `yaml:"elasticsearch"`
 		// PublicClient is config for connecting to cadence frontend
 		PublicClient PublicClient `yaml:"publicClient"`
+		// DynamicConfigClient is the config for setting up the file based dynamic config client
+		// Filepath should be relative to the root directory
+		DynamicConfigClient dynamicconfig.FileBasedClientConfig `yaml:"dynamicConfigClient"`
 	}
 
 	// Service contains the service specific config items
diff --git a/common/service/dynamicconfig/fileBasedClient.go b/common/service/dynamicconfig/fileBasedClient.go
index 2cc8f6cab11..d075e8c82e5 100644
--- a/common/service/dynamicconfig/fileBasedClient.go
+++ b/common/service/dynamicconfig/fileBasedClient.go
@@ -46,8 +46,8 @@ type constrainedValue struct {
 // It specifies where the config file is stored and how often the config should be
 // updated by checking the config file again.
 type FileBasedClientConfig struct {
-	Filepath     string
-	PollInterval time.Duration
+	Filepath     string        `yaml:"filepath"`
+	PollInterval time.Duration `yaml:"pollInterval"`
 }
 
 type fileBasedClient struct {
@@ -79,9 +79,7 @@ func NewFileBasedClient(config *FileBasedClientConfig, logger log.Logger, doneCh
 			case <-ticker.C:
 				err := client.update()
 				if err != nil {
-					client.logger.Error("Failed to update dynamic config", tag.Timestamp(time.Now()))
-				} else {
-					client.logger.Info("Updated dynamic config", tag.Timestamp(time.Now()))
+					client.logger.Error("Failed to update dynamic config", tag.Error(err))
 				}
 			case <-client.doneCh:
 				ticker.Stop()
@@ -220,7 +218,7 @@ func (fc *fileBasedClient) update() error {
 	}
 
 	fc.values.Store(newValues)
-
+	fc.logger.Info("Updated dynamic config")
 	return nil
 }
 
diff --git a/config/development.yaml b/config/development.yaml
index 2cefe8e1c35..bcdfb8d1575 100644
--- a/config/development.yaml
+++ b/config/development.yaml
@@ -125,3 +125,7 @@ elasticsearch:
 publicClient:
   hostPort: "127.0.0.1:7933"
 
+dynamicConfigClient:
+  filepath: "config/dynamicconfig/development.yaml"
+  pollInterval: "10s"
+
diff --git a/config/dynamicconfig/development.yaml b/config/dynamicconfig/development.yaml
new file mode 100644
index 00000000000..d9427d2b769
--- /dev/null
+++ b/config/dynamicconfig/development.yaml
@@ -0,0 +1,37 @@
+# Use this file to override the default dynamic config value (they are specified
+# when creating the service config).
+
+# Each key can have zero or more values and each value can have zero or more
+# constraints. There are only three types of constraint:
+#     1. domainName: string
+#     2. taskListName: string
+#     3. taskType: int (0:Decision, 1:Activity)
+# A value will be selected and returned if all its has exactly the same constraints
+# as the ones specified in query filters (including the number of constraints).
+
+# Please use the following format:
+# testGetBoolPropertyKey:
+#   - value: false
+#   - value: true
+#     constraints:
+#       domainName: "global-samples-domain"
+#   - value: false
+#     constraints:
+#       domainName: "samples-domain"
+# testGetDurationPropertyKey:
+#   - value: "1m"
+#     constraints:
+#       domainName: "samples-domain"
+#       taskListName: "longIdleTimeTasklist"
+# testGetFloat64PropertyKey:
+#   - value: 12.0
+#     constraints:
+#       domainName: "samples-domain"
+# testGetMapPropertyKey:
+#   - value:
+#       key1: 1
+#       key2: "value 2"
+#       key3:
+#         - false
+#         - key4: true
+#           key5: 2.0