From d3fc9a50f624cfcdf7c5bb3e7051ac693d036592 Mon Sep 17 00:00:00 2001 From: Romain LE JEUNE Date: Fri, 21 Apr 2023 10:06:01 +0200 Subject: [PATCH] feat(GODT-2556): Add functional test for Heartbeat Init and telemetry availability. --- internal/app/app.go | 1 + internal/bridge/heartbeat.go | 30 +++++---- internal/bridge/settings.go | 9 ++- internal/telemetry/heartbeat.go | 8 ++- tests/bdd_test.go | 11 +++- tests/bridge_test.go | 23 ++++++- tests/ctx_bridge_test.go | 4 ++ tests/features/bridge/heartbeat.feature | 85 ++++++++++++++++++++++++- 8 files changed, 150 insertions(+), 21 deletions(-) diff --git a/internal/app/app.go b/internal/app/app.go index dcd963d1..16487e51 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -268,6 +268,7 @@ func run(c *cli.Context) error { logrus.Warn("The vault is corrupt and has been wiped") b.PushError(bridge.ErrVaultCorrupt) } + // Start telemetry heartbeat process b.StartHeartbeat(b) diff --git a/internal/bridge/heartbeat.go b/internal/bridge/heartbeat.go index eb37503d..508a237b 100644 --- a/internal/bridge/heartbeat.go +++ b/internal/bridge/heartbeat.go @@ -83,18 +83,6 @@ func (bridge *Bridge) SetLastHeartbeatSent(timestamp time.Time) error { func (bridge *Bridge) StartHeartbeat(manager telemetry.HeartbeatManager) { bridge.heartbeat = telemetry.NewHeartbeat(manager, 1143, 1025, bridge.GetGluonCacheDir(), keychain.DefaultHelper) - safe.RLock(func() { - var splitMode = false - for _, user := range bridge.users { - if user.GetAddressMode() == vault.SplitMode { - splitMode = true - break - } - } - bridge.heartbeat.SetNbAccount(len(bridge.users)) - bridge.heartbeat.SetSplitMode(splitMode) - }, bridge.usersLock) - bridge.heartbeat.SetRollout(bridge.GetUpdateRollout()) bridge.heartbeat.SetAutoStart(bridge.GetAutostart()) bridge.heartbeat.SetAutoUpdate(bridge.GetAutoUpdate()) @@ -113,5 +101,21 @@ func (bridge *Bridge) StartHeartbeat(manager telemetry.HeartbeatManager) { } bridge.heartbeat.SetPrevVersion(bridge.GetLastVersion().String()) - bridge.heartbeat.TrySending() + safe.RLock(func() { + var splitMode = false + for _, user := range bridge.users { + if user.GetAddressMode() == vault.SplitMode { + splitMode = true + break + } + } + var nbAccount = len(bridge.users) + bridge.heartbeat.SetNbAccount(nbAccount) + bridge.heartbeat.SetSplitMode(splitMode) + + // Do not try to send if there is no user yet. + if nbAccount > 0 { + bridge.heartbeat.TrySending() + } + }, bridge.usersLock) } diff --git a/internal/bridge/settings.go b/internal/bridge/settings.go index 0da85403..f5ebae26 100644 --- a/internal/bridge/settings.go +++ b/internal/bridge/settings.go @@ -300,7 +300,14 @@ func (bridge *Bridge) GetTelemetryDisabled() bool { } func (bridge *Bridge) SetTelemetryDisabled(isDisabled bool) error { - return bridge.vault.SetTelemetryDisabled(isDisabled) + if err := bridge.vault.SetTelemetryDisabled(isDisabled); err != nil { + return err + } + // If telemetry is re-enabled locally, try to send the heartbeat. + if !isDisabled { + bridge.heartbeat.TrySending() + } + return nil } func (bridge *Bridge) GetUpdateChannel() updater.Channel { diff --git a/internal/telemetry/heartbeat.go b/internal/telemetry/heartbeat.go index 8d8c8f2d..c2a7b888 100644 --- a/internal/telemetry/heartbeat.go +++ b/internal/telemetry/heartbeat.go @@ -157,7 +157,13 @@ func (heartbeat *Heartbeat) TrySending() { heartbeat.log.WithFields(logrus.Fields{ "metrics": heartbeat.metrics, }).Error("Failed to send heartbeat") - } else if err := heartbeat.manager.SetLastHeartbeatSent(now); err != nil { + return + } + heartbeat.log.WithFields(logrus.Fields{ + "metrics": heartbeat.metrics, + }).Debug("Heartbeat sent") + + if err := heartbeat.manager.SetLastHeartbeatSent(now); err != nil { heartbeat.log.WithError(err).Warn("Cannot save last heartbeat sent to the vault.") } } diff --git a/tests/bdd_test.go b/tests/bdd_test.go index e560b7a8..92a73bed 100644 --- a/tests/bdd_test.go +++ b/tests/bdd_test.go @@ -135,9 +135,14 @@ func TestFeatures(testingT *testing.T) { ctx.Step(`^bridge stops$`, s.bridgeStops) ctx.Step(`^bridge is version "([^"]*)" and the latest available version is "([^"]*)" reachable from "([^"]*)"$`, s.bridgeVersionIsAndTheLatestAvailableVersionIsReachableFrom) ctx.Step(`^the user has disabled automatic updates$`, s.theUserHasDisabledAutomaticUpdates) + ctx.Step(`^the user has disabled automatic start`, s.theUserHasDisabledAutomaticStart) + ctx.Step(`^the user has enabled alternative routing`, s.theUserHasEnabledAlternativeRouting) + ctx.Step(`^the user set IMAP mode to SSL`, s.theUserSetIMAPModeToSSL) + ctx.Step(`^the user set SMTP mode to SSL`, s.theUserSetSMTPModeToSSL) ctx.Step(`^the user changes the IMAP port to (\d+)$`, s.theUserChangesTheIMAPPortTo) ctx.Step(`^the user changes the SMTP port to (\d+)$`, s.theUserChangesTheSMTPPortTo) ctx.Step(`^the user sets the address mode of user "([^"]*)" to "([^"]*)"$`, s.theUserSetsTheAddressModeOfUserTo) + ctx.Step(`^the user changes the default keychain application`, s.theUserChangesTheDefaultKeychainApplication) ctx.Step(`^the user changes the gluon path$`, s.theUserChangesTheGluonPath) ctx.Step(`^the user deletes the gluon files$`, s.theUserDeletesTheGluonFiles) ctx.Step(`^the user deletes the gluon cache$`, s.theUserDeletesTheGluonCache) @@ -234,9 +239,9 @@ func TestFeatures(testingT *testing.T) { // ==== TELEMETRY ==== ctx.Step(`^bridge eventually sends the following heartbeat:$`, s.bridgeEventuallySendsTheFollowingHeartbeat) - ctx.Step(`^bridge needs to send heartbeat$`, s.bridgeNeedsToSendHeartbeat) - ctx.Step(`^bridge do not need to send heartbeat$`, s.bridgeDoNotNeedToSendHeartbeat) - ctx.Step(`^heartbeat is not whitelisted$`, s.heartbeatIsNotwhitelisted) + ctx.Step(`^bridge needs to send heartbeat`, s.bridgeNeedsToSendHeartbeat) + ctx.Step(`^bridge do not need to send heartbeat`, s.bridgeDoNotNeedToSendHeartbeat) + ctx.Step(`^heartbeat is not whitelisted`, s.heartbeatIsNotwhitelisted) }, Options: &godog.Options{ Format: "pretty", diff --git a/tests/bridge_test.go b/tests/bridge_test.go index 87c719e9..7d3edd14 100644 --- a/tests/bridge_test.go +++ b/tests/bridge_test.go @@ -80,6 +80,10 @@ func (s *scenario) theUserSetsTheAddressModeOfUserTo(user, mode string) error { } } +func (s *scenario) theUserChangesTheDefaultKeychainApplication() error { + return s.t.bridge.SetKeychainApp("CustomKeychainApp") +} + func (s *scenario) theUserChangesTheGluonPath() error { gluonDir, err := os.MkdirTemp(s.t.dir, "gluon") if err != nil { @@ -118,7 +122,6 @@ func (s *scenario) theUserHasDisabledAutomaticUpdates() error { started = true } - if err := s.t.bridge.SetAutoUpdate(false); err != nil { return err } @@ -128,10 +131,26 @@ func (s *scenario) theUserHasDisabledAutomaticUpdates() error { return err } } - return nil } +func (s *scenario) theUserHasDisabledAutomaticStart() error { + return s.t.bridge.SetAutostart(false) +} + +func (s *scenario) theUserHasEnabledAlternativeRouting() error { + s.t.expectProxyCtlAllowProxy() + return s.t.bridge.SetProxyAllowed(true) +} + +func (s *scenario) theUserSetIMAPModeToSSL() error { + return s.t.bridge.SetIMAPSSL(true) +} + +func (s *scenario) theUserSetSMTPModeToSSL() error { + return s.t.bridge.SetSMTPSSL(true) +} + func (s *scenario) theUserReportsABug() error { return s.t.bridge.ReportBug(context.Background(), "osType", "osVersion", "description", "username", "email", "client", false) } diff --git a/tests/ctx_bridge_test.go b/tests/ctx_bridge_test.go index de4ed616..0afe93d7 100644 --- a/tests/ctx_bridge_test.go +++ b/tests/ctx_bridge_test.go @@ -175,6 +175,7 @@ func (t *testCtx) initBridge() (<-chan events.Event, error) { t.bridge = bridge t.heartbeat.setBridge(bridge) + bridge.StartHeartbeat(t.heartbeat) return t.events.collectFrom(eventCh), nil @@ -344,6 +345,9 @@ func (t *testCtx) closeFrontendClient() error { return nil } +func (t *testCtx) expectProxyCtlAllowProxy() { + t.mocks.ProxyCtl.EXPECT().AllowProxy() +} type mockRestarter struct{} diff --git a/tests/features/bridge/heartbeat.feature b/tests/features/bridge/heartbeat.feature index a1ce7372..3675cd38 100644 --- a/tests/features/bridge/heartbeat.feature +++ b/tests/features/bridge/heartbeat.feature @@ -4,7 +4,7 @@ Feature: Send Telemetry Heartbeat And bridge starts - Scenario: Send at first start - one user + Scenario: Send at first start - one user default settings Then bridge telemetry feature is enabled And bridge needs to send heartbeat When the user logs in with username "[user:user1]" and password "password" @@ -37,6 +37,89 @@ Feature: Send Telemetry Heartbeat """ And bridge do not need to send heartbeat + + Scenario: Send at first start - one user modified settings + Then bridge telemetry feature is enabled + And bridge needs to send heartbeat + When the user has disabled automatic updates + And the user has disabled automatic start + And the user has enabled alternative routing + And the user hides All Mail + And the user set IMAP mode to SSL + And the user set SMTP mode to SSL + And the user changes the IMAP port to 42695 + And the user changes the SMTP port to 56942 + And the user changes the gluon path + And the user changes the default keychain application + When the user logs in with username "[user:user1]" and password "password" + And user "[user:user1]" finishes syncing + Then bridge eventually sends the following heartbeat: + """ + { + "MeasurementGroup": "bridge.any.usage", + "Event": "bridge_heartbeat", + "Values": { + "nb_account": 1 + }, + "Dimensions": { + "auto_update": "off", + "auto_start": "off", + "beta": "off", + "doh": "on", + "split_mode": "off", + "show_all_mail": "off", + "imap_connection_mode": "ssl", + "smtp_connection_mode": "ssl", + "imap_port": "custom", + "smtp_port": "custom", + "cache_location": "custom", + "keychain_pref": "custom", + "prev_version": "0.0.0", + "rollout": 42 + } + } + """ + And bridge do not need to send heartbeat + + + Scenario: Send at first start - one user telemetry disabled + Then bridge telemetry feature is enabled + And bridge needs to send heartbeat + When the user disables telemetry in bridge settings + And the user logs in with username "[user:user1]" and password "password" + And user "[user:user1]" finishes syncing + And bridge needs to send heartbeat + Then the user sets the address mode of user "[user:user1]" to "split" + And the user enables telemetry in bridge settings + Then bridge eventually sends the following heartbeat: + """ + { + "MeasurementGroup": "bridge.any.usage", + "Event": "bridge_heartbeat", + "Values": { + "nb_account": 1 + }, + "Dimensions": { + "auto_update": "on", + "auto_start": "on", + "beta": "off", + "doh": "off", + "split_mode": "on", + "show_all_mail": "on", + "imap_connection_mode": "starttls", + "smtp_connection_mode": "starttls", + "imap_port": "default", + "smtp_port": "default", + "cache_location": "default", + "keychain_pref": "default", + "prev_version": "0.0.0", + "rollout": 42 + } + } + """ + And bridge do not need to send heartbeat + + Scenario: GroupMeasurement rejected by API Given heartbeat is not whitelisted Then bridge telemetry feature is enabled