From 5b9214567cf5799dfce0a7126c8a130a1b749c7f Mon Sep 17 00:00:00 2001 From: Edward Mack Date: Thu, 24 Jun 2021 17:10:56 -0400 Subject: [PATCH 01/11] implement no op telemetry instance --- dot/node.go | 2 +- dot/telemetry/telemetry.go | 29 ++++++++++++++++++++++++-- dot/telemetry/telemetry_test.go | 36 +++++++++++++++++++++++++++++++++ 3 files changed, 64 insertions(+), 3 deletions(-) diff --git a/dot/node.go b/dot/node.go index 768999bb40..c1d52b5503 100644 --- a/dot/node.go +++ b/dot/node.go @@ -341,7 +341,7 @@ func NewNode(cfg *Config, ks *keystore.GlobalKeystore, stopFunc func()) (*Node, } if cfg.Global.NoTelemetry { - return node, nil + telemetry.GetInstance().KillInstance() } telemetry.GetInstance().AddConnections(gd.TelemetryEndpoints) diff --git a/dot/telemetry/telemetry.go b/dot/telemetry/telemetry.go index cd11d65207..c9ef8eab0f 100644 --- a/dot/telemetry/telemetry.go +++ b/dot/telemetry/telemetry.go @@ -45,6 +45,13 @@ type Handler struct { log log.Logger } +type Instance interface { + AddConnections(conns []*genesis.TelemetryEndpoint) + SendMessage(msg *Message) error + startListening() + KillInstance() +} + // KeyValue object to hold key value pairs used in telemetry messages type KeyValue struct { key string @@ -53,11 +60,11 @@ type KeyValue struct { var ( once sync.Once - handlerInstance *Handler + handlerInstance Instance ) // GetInstance singleton pattern to for accessing TelemetryHandler -func GetInstance() *Handler { //nolint +func GetInstance() Instance { //nolint if handlerInstance == nil { once.Do( func() { @@ -134,6 +141,10 @@ func (h *Handler) startListening() { } } +func (h *Handler) KillInstance() { + handlerInstance = &NoopHandler{} +} + type response struct { ID int `json:"id"` Payload map[string]interface{} `json:"payload"` @@ -152,3 +163,17 @@ func msgToBytes(message Message) []byte { } return resB } + +// NoopHandler struct no op handling (ignoring) telemetry messages +type NoopHandler struct { +} + +func (h *NoopHandler) startListening() {} + +func (h *NoopHandler) SendMessage(msg *Message) error { + return nil +} + +func (h *NoopHandler) AddConnections(conns []*genesis.TelemetryEndpoint) {} + +func (h *NoopHandler) KillInstance() {} diff --git a/dot/telemetry/telemetry_test.go b/dot/telemetry/telemetry_test.go index e2b7b9c05b..e27e1ce1ce 100644 --- a/dot/telemetry/telemetry_test.go +++ b/dot/telemetry/telemetry_test.go @@ -139,6 +139,42 @@ func TestListenerConcurrency(t *testing.T) { } } +func TestKillInstance(t *testing.T) { + const qty = 1000 + var wg sync.WaitGroup + wg.Add(qty) + + resultCh = make(chan []byte) + for i := 0; i < qty; i++ { + if i == qty/2 { + GetInstance().KillInstance() + } + go func() { + GetInstance().SendMessage(NewTelemetryMessage( + NewKeyValue("best", "hash"), + NewKeyValue("height", big.NewInt(2)), + NewKeyValue("msg", "block.import"), + NewKeyValue("origin", "NetworkInitialSync"))) + wg.Done() + }() + } + wg.Wait() + counter := 0 + tk := time.NewTicker(time.Second * 2) +main: + for { + select { + case <-tk.C: + break main + case <-resultCh: + counter++ + } + } + tk.Stop() + + require.LessOrEqual(t, counter, qty/2) +} + func listen(w http.ResponseWriter, r *http.Request) { c, err := upgrader.Upgrade(w, r, nil) if err != nil { From 6c08d7c104b28f7f8c7ac6f4fbaddc31fb389abe Mon Sep 17 00:00:00 2001 From: Edward Mack Date: Thu, 24 Jun 2021 17:27:02 -0400 Subject: [PATCH 02/11] add comments --- dot/telemetry/telemetry.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/dot/telemetry/telemetry.go b/dot/telemetry/telemetry.go index c9ef8eab0f..1aaeac0474 100644 --- a/dot/telemetry/telemetry.go +++ b/dot/telemetry/telemetry.go @@ -45,6 +45,7 @@ type Handler struct { log log.Logger } +// Instance functions that telemetry handler instance needs to implement type Instance interface { AddConnections(conns []*genesis.TelemetryEndpoint) SendMessage(msg *Message) error @@ -141,6 +142,7 @@ func (h *Handler) startListening() { } } +// KillInstance replaces current telemetry handler instance with no op telemetry handler instance func (h *Handler) KillInstance() { handlerInstance = &NoopHandler{} } @@ -170,10 +172,13 @@ type NoopHandler struct { func (h *NoopHandler) startListening() {} +// SendMessage no op for telemetry send message function func (h *NoopHandler) SendMessage(msg *Message) error { return nil } +// AddConnections no op for telemetry add connections function func (h *NoopHandler) AddConnections(conns []*genesis.TelemetryEndpoint) {} +// KillInstance no op for telemetry kill instance function func (h *NoopHandler) KillInstance() {} From 38b5d315601256879463c88db9bd70a13345d6eb Mon Sep 17 00:00:00 2001 From: Edward Mack Date: Tue, 29 Jun 2021 14:14:46 -0400 Subject: [PATCH 03/11] create telemetry.Enabled global var --- dot/node.go | 4 +--- dot/telemetry/telemetry.go | 16 ++++++---------- dot/telemetry/telemetry_test.go | 2 +- 3 files changed, 8 insertions(+), 14 deletions(-) diff --git a/dot/node.go b/dot/node.go index c1d52b5503..e41cb7c4af 100644 --- a/dot/node.go +++ b/dot/node.go @@ -340,9 +340,7 @@ func NewNode(cfg *Config, ks *keystore.GlobalKeystore, stopFunc func()) (*Node, return nil, err } - if cfg.Global.NoTelemetry { - telemetry.GetInstance().KillInstance() - } + telemetry.Enabled = !cfg.Global.NoTelemetry telemetry.GetInstance().AddConnections(gd.TelemetryEndpoints) diff --git a/dot/telemetry/telemetry.go b/dot/telemetry/telemetry.go index 1aaeac0474..41e0877e4f 100644 --- a/dot/telemetry/telemetry.go +++ b/dot/telemetry/telemetry.go @@ -50,7 +50,6 @@ type Instance interface { AddConnections(conns []*genesis.TelemetryEndpoint) SendMessage(msg *Message) error startListening() - KillInstance() } // KeyValue object to hold key value pairs used in telemetry messages @@ -62,10 +61,11 @@ type KeyValue struct { var ( once sync.Once handlerInstance Instance + Enabled = true // enabled by default ) // GetInstance singleton pattern to for accessing TelemetryHandler -func GetInstance() Instance { //nolint +func GetInstance() Instance { if handlerInstance == nil { once.Do( func() { @@ -76,6 +76,10 @@ func GetInstance() Instance { //nolint go handlerInstance.startListening() }) } + if !Enabled { + return &NoopHandler{} + } + return handlerInstance } @@ -142,11 +146,6 @@ func (h *Handler) startListening() { } } -// KillInstance replaces current telemetry handler instance with no op telemetry handler instance -func (h *Handler) KillInstance() { - handlerInstance = &NoopHandler{} -} - type response struct { ID int `json:"id"` Payload map[string]interface{} `json:"payload"` @@ -179,6 +178,3 @@ func (h *NoopHandler) SendMessage(msg *Message) error { // AddConnections no op for telemetry add connections function func (h *NoopHandler) AddConnections(conns []*genesis.TelemetryEndpoint) {} - -// KillInstance no op for telemetry kill instance function -func (h *NoopHandler) KillInstance() {} diff --git a/dot/telemetry/telemetry_test.go b/dot/telemetry/telemetry_test.go index e27e1ce1ce..6876863483 100644 --- a/dot/telemetry/telemetry_test.go +++ b/dot/telemetry/telemetry_test.go @@ -147,7 +147,7 @@ func TestKillInstance(t *testing.T) { resultCh = make(chan []byte) for i := 0; i < qty; i++ { if i == qty/2 { - GetInstance().KillInstance() + Enabled = false } go func() { GetInstance().SendMessage(NewTelemetryMessage( From 94dabefbc48c45790375fd49008654f18a449082 Mon Sep 17 00:00:00 2001 From: Edward Mack Date: Tue, 29 Jun 2021 14:21:49 -0400 Subject: [PATCH 04/11] lint --- dot/telemetry/telemetry.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dot/telemetry/telemetry.go b/dot/telemetry/telemetry.go index 41e0877e4f..1aab00a020 100644 --- a/dot/telemetry/telemetry.go +++ b/dot/telemetry/telemetry.go @@ -61,7 +61,9 @@ type KeyValue struct { var ( once sync.Once handlerInstance Instance - Enabled = true // enabled by default + + // Enabled flag to determine if telemetry is enabled + Enabled = true // enabled by default ) // GetInstance singleton pattern to for accessing TelemetryHandler From cd6914890cf631e54c5584fb0faea99d1ea2f828 Mon Sep 17 00:00:00 2001 From: Edward Mack Date: Wed, 30 Jun 2021 11:04:18 -0400 Subject: [PATCH 05/11] Update dot/telemetry/telemetry.go Co-authored-by: Timothy Wu --- dot/telemetry/telemetry.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dot/telemetry/telemetry.go b/dot/telemetry/telemetry.go index 61e4d265cc..580691f9fe 100644 --- a/dot/telemetry/telemetry.go +++ b/dot/telemetry/telemetry.go @@ -46,7 +46,7 @@ type Handler struct { sendMessageTimeout time.Duration } -// Instance functions that telemetry handler instance needs to implement +// Instance interface that telemetry handler instance needs to implement type Instance interface { AddConnections(conns []*genesis.TelemetryEndpoint) SendMessage(msg *Message) error From adf20bb5e59db45b631815f6ed3e4ab94b5c76d0 Mon Sep 17 00:00:00 2001 From: Edward Mack Date: Thu, 1 Jul 2021 10:28:59 -0400 Subject: [PATCH 06/11] fix merge conflicts --- dot/telemetry/telemetry.go | 10 ++-------- dot/telemetry/telemetry_test.go | 9 +++------ 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/dot/telemetry/telemetry.go b/dot/telemetry/telemetry.go index 6a466261bc..00cecfd366 100644 --- a/dot/telemetry/telemetry.go +++ b/dot/telemetry/telemetry.go @@ -48,16 +48,10 @@ type Handler struct { // Instance interface that telemetry handler instance needs to implement type Instance interface { AddConnections(conns []*genesis.TelemetryEndpoint) - SendMessage(msg *Message) error + SendMessage(msg Message) error startListening() } -// KeyValue object to hold key value pairs used in telemetry messages -type KeyValue struct { - key string - value interface{} -} - var ( once sync.Once handlerInstance Instance @@ -318,7 +312,7 @@ type NoopHandler struct { func (h *NoopHandler) startListening() {} // SendMessage no op for telemetry send message function -func (h *NoopHandler) SendMessage(msg *Message) error { +func (h *NoopHandler) SendMessage(msg Message) error { return nil } diff --git a/dot/telemetry/telemetry_test.go b/dot/telemetry/telemetry_test.go index 265bc90eba..9c22790c3d 100644 --- a/dot/telemetry/telemetry_test.go +++ b/dot/telemetry/telemetry_test.go @@ -128,7 +128,7 @@ func TestListenerConcurrency(t *testing.T) { } } -func TestKillInstance(t *testing.T) { +func TestDisableInstance(t *testing.T) { const qty = 1000 var wg sync.WaitGroup wg.Add(qty) @@ -139,11 +139,8 @@ func TestKillInstance(t *testing.T) { Enabled = false } go func() { - GetInstance().SendMessage(NewTelemetryMessage( - NewKeyValue("best", "hash"), - NewKeyValue("height", big.NewInt(2)), - NewKeyValue("msg", "block.import"), - NewKeyValue("origin", "NetworkInitialSync"))) + bh := common.MustHexToHash("0x07b749b6e20fd5f1159153a2e790235018621dd06072a62bcd25e8576f6ff5e6") + GetInstance().SendMessage(NewBlockImportTM(&bh, big.NewInt(2), "NetworkInitialSync")) wg.Done() }() } From c1bfb1178d690b9b51c62c5557cf15714cdd1c94 Mon Sep 17 00:00:00 2001 From: Edward Mack Date: Thu, 1 Jul 2021 10:56:48 -0400 Subject: [PATCH 07/11] implement Initialise function --- dot/node.go | 2 +- dot/telemetry/telemetry.go | 16 +++++++++++++--- dot/telemetry/telemetry_test.go | 3 ++- 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/dot/node.go b/dot/node.go index bab9747de4..cbead65b6d 100644 --- a/dot/node.go +++ b/dot/node.go @@ -340,7 +340,7 @@ func NewNode(cfg *Config, ks *keystore.GlobalKeystore, stopFunc func()) (*Node, return nil, err } - telemetry.Enabled = !cfg.Global.NoTelemetry + telemetry.GetInstance().Initialise(!cfg.Global.NoTelemetry) telemetry.GetInstance().AddConnections(gd.TelemetryEndpoints) genesisHash := stateSrvc.Block.GenesisHash() diff --git a/dot/telemetry/telemetry.go b/dot/telemetry/telemetry.go index 00cecfd366..7a8b6d9fa1 100644 --- a/dot/telemetry/telemetry.go +++ b/dot/telemetry/telemetry.go @@ -50,14 +50,15 @@ type Instance interface { AddConnections(conns []*genesis.TelemetryEndpoint) SendMessage(msg Message) error startListening() + Initialise(enabled bool) } var ( once sync.Once handlerInstance Instance - // Enabled flag to determine if telemetry is enabled - Enabled = true // enabled by default + enabled = true // enabled by default + initilised sync.Once ) const defaultMessageTimeout = time.Second @@ -75,13 +76,20 @@ func GetInstance() Instance { go handlerInstance.startListening() }) } - if !Enabled { + if !enabled { return &NoopHandler{} } return handlerInstance } +func (h *Handler) Initialise(_enabled bool) { + initilised.Do( + func() { + enabled = _enabled + }) +} + // AddConnections adds the given telemetry endpoint as listeners that will receive telemetry data func (h *Handler) AddConnections(conns []*genesis.TelemetryEndpoint) { for _, v := range conns { @@ -309,6 +317,8 @@ func (tm *NetworkStateTM) messageType() string { type NoopHandler struct { } +func (h *NoopHandler) Initialise(enabled bool) {} + func (h *NoopHandler) startListening() {} // SendMessage no op for telemetry send message function diff --git a/dot/telemetry/telemetry_test.go b/dot/telemetry/telemetry_test.go index 9c22790c3d..f6031246e9 100644 --- a/dot/telemetry/telemetry_test.go +++ b/dot/telemetry/telemetry_test.go @@ -136,7 +136,7 @@ func TestDisableInstance(t *testing.T) { resultCh = make(chan []byte) for i := 0; i < qty; i++ { if i == qty/2 { - Enabled = false + GetInstance().Initialise(false) } go func() { bh := common.MustHexToHash("0x07b749b6e20fd5f1159153a2e790235018621dd06072a62bcd25e8576f6ff5e6") @@ -158,6 +158,7 @@ main: } tk.Stop() + require.Greater(t, counter, 0) require.LessOrEqual(t, counter, qty/2) } From 2839806b4f74553e05c011c3ee607bdbef30ab0f Mon Sep 17 00:00:00 2001 From: Edward Mack Date: Thu, 1 Jul 2021 11:00:33 -0400 Subject: [PATCH 08/11] add comments --- dot/telemetry/telemetry.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dot/telemetry/telemetry.go b/dot/telemetry/telemetry.go index 7a8b6d9fa1..bf8eb17993 100644 --- a/dot/telemetry/telemetry.go +++ b/dot/telemetry/telemetry.go @@ -83,6 +83,7 @@ func GetInstance() Instance { return handlerInstance } +// Initialise function to set if telemetry is enabled func (h *Handler) Initialise(_enabled bool) { initilised.Do( func() { @@ -317,6 +318,7 @@ func (tm *NetworkStateTM) messageType() string { type NoopHandler struct { } +// Initialise function to set if telemetry is enabled func (h *NoopHandler) Initialise(enabled bool) {} func (h *NoopHandler) startListening() {} From 6aa440001d4409d4f19b85c12dd96d2cb04c069a Mon Sep 17 00:00:00 2001 From: Edward Mack Date: Thu, 1 Jul 2021 11:58:13 -0400 Subject: [PATCH 09/11] fix tests --- dot/telemetry/telemetry_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/dot/telemetry/telemetry_test.go b/dot/telemetry/telemetry_test.go index f6031246e9..7e89665149 100644 --- a/dot/telemetry/telemetry_test.go +++ b/dot/telemetry/telemetry_test.go @@ -158,7 +158,6 @@ main: } tk.Stop() - require.Greater(t, counter, 0) require.LessOrEqual(t, counter, qty/2) } From 160bb40912b4568a335482426215f9a4c6a8fc83 Mon Sep 17 00:00:00 2001 From: Edward Mack Date: Wed, 7 Jul 2021 10:19:56 -0400 Subject: [PATCH 10/11] remove leading underscore from function signature Co-authored-by: Timothy Wu --- dot/telemetry/telemetry.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dot/telemetry/telemetry.go b/dot/telemetry/telemetry.go index 8ba594cdf0..387bc77cdd 100644 --- a/dot/telemetry/telemetry.go +++ b/dot/telemetry/telemetry.go @@ -84,7 +84,7 @@ func GetInstance() Instance { } // Initialise function to set if telemetry is enabled -func (h *Handler) Initialise(_enabled bool) { +func (h *Handler) Initialise(e bool) { initilised.Do( func() { enabled = _enabled From 204dfe94eb329d4afbaab76d8b72669c004d457e Mon Sep 17 00:00:00 2001 From: Edward Mack Date: Wed, 7 Jul 2021 10:26:19 -0400 Subject: [PATCH 11/11] update variable name --- dot/telemetry/telemetry.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dot/telemetry/telemetry.go b/dot/telemetry/telemetry.go index 387bc77cdd..873ed4273a 100644 --- a/dot/telemetry/telemetry.go +++ b/dot/telemetry/telemetry.go @@ -87,7 +87,7 @@ func GetInstance() Instance { func (h *Handler) Initialise(e bool) { initilised.Do( func() { - enabled = _enabled + enabled = e }) }