diff --git a/app/diagnostics.go b/app/diagnostics.go index 8beb957f3f..09b01c3dd9 100644 --- a/app/diagnostics.go +++ b/app/diagnostics.go @@ -58,15 +58,13 @@ const ( TRACK_PLUGINS = "plugins" ) -var client *analytics.Client - func (a *App) SendDailyDiagnostics() { a.sendDailyDiagnostics(false) } func (a *App) sendDailyDiagnostics(override bool) { if *a.Config().LogSettings.EnableDiagnostics && a.IsLeader() && (!strings.Contains(SEGMENT_KEY, "placeholder") || override) { - a.initDiagnostics("") + a.Srv.initDiagnostics("") a.trackActivity() a.trackConfig() a.trackLicense() @@ -76,24 +74,8 @@ func (a *App) sendDailyDiagnostics(override bool) { } } -func (a *App) initDiagnostics(endpoint string) { - if client == nil { - client = analytics.New(SEGMENT_KEY) - client.Logger = a.Log.StdLog(mlog.String("source", "segment")) - // For testing - if endpoint != "" { - client.Endpoint = endpoint - client.Verbose = true - client.Size = 1 - } - client.Identify(&analytics.Identify{ - UserId: a.DiagnosticId(), - }) - } -} - func (a *App) SendDiagnostic(event string, properties map[string]interface{}) { - client.Track(&analytics.Track{ + a.Srv.diagnosticClient.Track(&analytics.Track{ Event: event, UserId: a.DiagnosticId(), Properties: properties, diff --git a/app/diagnostics_test.go b/app/diagnostics_test.go index 9bc82a7489..b0e13b1be8 100644 --- a/app/diagnostics_test.go +++ b/app/diagnostics_test.go @@ -4,32 +4,18 @@ package app import ( - "bytes" - "io" + "io/ioutil" "net/http" "net/http/httptest" - "strings" "testing" "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/mattermost/mattermost-server/model" ) -func newTestServer() (chan string, *httptest.Server) { - result := make(chan string, 100) - - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - buf := bytes.NewBuffer(nil) - io.Copy(buf, r.Body) - - result <- buf.String() - })) - - return result, server -} - func TestPluginSetting(t *testing.T) { settings := &model.PluginSettings{ Plugins: map[string]map[string]interface{}{ @@ -57,42 +43,42 @@ func TestPluginActivated(t *testing.T) { } func TestDiagnostics(t *testing.T) { - th := Setup(t).InitBasic() - defer th.TearDown() - if testing.Short() { t.SkipNow() } - data, server := newTestServer() + th := Setup(t).InitBasic() + defer th.TearDown() + + data := make(chan string, 100) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, err := ioutil.ReadAll(r.Body) + require.NoError(t, err) + + data <- string(body) + })) defer server.Close() - diagnosticId := "i am not real" - th.App.SetDiagnosticId(diagnosticId) - th.App.initDiagnostics(server.URL) + diagnosticID := "test-diagnostic-id-12345" + th.App.SetDiagnosticId(diagnosticID) + th.Server.initDiagnostics(server.URL) // Should send a client identify message select { case identifyMessage := <-data: - t.Log("Got idmessage:\n" + identifyMessage) - if !strings.Contains(identifyMessage, diagnosticId) { - t.Fail() - } + require.Contains(t, identifyMessage, diagnosticID) case <-time.After(time.Second * 1): t.Fatal("Did not receive ID message") } t.Run("Send", func(t *testing.T) { - const TEST_VALUE = "stuff548959847" + testValue := "test-send-value-6789" th.App.SendDiagnostic("Testing Diagnostic", map[string]interface{}{ - "hey": TEST_VALUE, + "hey": testValue, }) select { case result := <-data: - t.Log("Got diagnostic:\n" + result) - if !strings.Contains(result, TEST_VALUE) { - t.Fail() - } + require.Contains(t, result, testValue) case <-time.After(time.Second * 1): t.Fatal("Did not receive diagnostic") } @@ -101,7 +87,7 @@ func TestDiagnostics(t *testing.T) { t.Run("SendDailyDiagnostics", func(t *testing.T) { th.App.sendDailyDiagnostics(true) - info := "" + var info string // Collect the info sent. Loop: for { @@ -142,9 +128,7 @@ func TestDiagnostics(t *testing.T) { TRACK_CONFIG_MESSAGE_EXPORT, TRACK_PLUGINS, } { - if !strings.Contains(info, item) { - t.Fatal("Sent diagnostics missing item: " + item) - } + require.Contains(t, info, item) } }) diff --git a/app/server.go b/app/server.go index 6bfbc06d5d..6335f86174 100644 --- a/app/server.go +++ b/app/server.go @@ -20,6 +20,7 @@ import ( "github.com/gorilla/mux" "github.com/pkg/errors" "github.com/rs/cors" + analytics "github.com/segmentio/analytics-go" "github.com/throttled/throttled" "golang.org/x/crypto/acme/autocert" @@ -100,7 +101,9 @@ type Server struct { clientConfig map[string]string clientConfigHash string limitedClientConfig map[string]string - diagnosticId string + + diagnosticId string + diagnosticClient *analytics.Client phase2PermissionsMigrationComplete bool @@ -321,6 +324,11 @@ func (s *Server) Shutdown() error { s.RunOldAppShutdown() + err := s.shutdownDiagnostics() + if err != nil { + mlog.Error(fmt.Sprintf("Unable to cleanly shutdown diagnostic client: %s", err)) + } + s.StopHTTPServer() s.WaitForGoroutines() @@ -734,3 +742,30 @@ func (s *Server) StartElasticsearch() { } }) } + +func (s *Server) initDiagnostics(endpoint string) { + if s.diagnosticClient == nil { + client := analytics.New(SEGMENT_KEY) + client.Logger = s.Log.StdLog(mlog.String("source", "segment")) + // For testing + if endpoint != "" { + client.Endpoint = endpoint + client.Verbose = true + client.Size = 1 + } + client.Identify(&analytics.Identify{ + UserId: s.diagnosticId, + }) + + s.diagnosticClient = client + } +} + +// ShutdownDiagnostics closes the diagnostic client. +func (s *Server) shutdownDiagnostics() error { + if s.diagnosticClient != nil { + return s.diagnosticClient.Close() + } + + return nil +}