MM-18115: fix segment v3 usage (#12317)

* diagnostics_test.go: fix spacing

* diagnostics_test.go: explicitly assert payload

This fails, since the package is currently receiving struct pointers and won't set the MessageId or Timestamp on the corresponding Message.

* MM-18115: fix segment v3 usage

In v5.14, we updated [github.com/segmentio/analytics-go](https://github.com/segmentio/analytics-go) to v3 as part of https://mattermost.atlassian.net/browse/MM-12389. As noted in the [migration guide](https://segment.com/docs/sources/server/go/#migrating-from-v2), the API subtly changed to expect a struct value and not a struct pointer:

```go
// in v2, you would call the `Track` method with a `Track` struct.
client.Track(&track)

// in v3, you would call the `Enqueue` method with a `Track` struct.
// Note that a pointer is not used here.
client.Enqueue(track)
```

Unfortunately, we kept passing a pointer, and the package didn't complain since it only required an interface -- which the pointer to these structs still implemented. Internally, it only checked for the value types, and failed to annotate our payloads with the requisite metadata. Upstream, segment.io accepted the payload, but then discarded it silently.

This has since been reported and fixed in https://github.com/segmentio/analytics-go/pull/146, but isn't yet part of a tagged release of the package.

Fix our code to pass struct values instead.

Fixes: MM-18115

app/diagnostics.go

@@ -75,7 +75,7 @@ func (a *App) sendDailyDiagnostics(override bool) {
 }
 
 func (a *App) SendDiagnostic(event string, properties map[string]interface{}) {
-	a.Srv.diagnosticClient.Enqueue(&analytics.Track{
+	a.Srv.diagnosticClient.Enqueue(analytics.Track{
 		Event:      event,
 		UserId:     a.DiagnosticId(),
 		Properties: properties,

app/diagnostics_test.go

@@ -4,6 +4,7 @@
 package app
 
 import (
+	"encoding/json"
 	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
@@ -50,12 +51,34 @@ func TestDiagnostics(t *testing.T) {
 	th := Setup(t).InitBasic()
 	defer th.TearDown()
 
-	data := make(chan string, 100)
+	type payload struct {
+		MessageId string
+		SentAt    time.Time
+		Batch     []struct {
+			MessageId  string
+			UserId     string
+			Event      string
+			Timestamp  time.Time
+			Properties map[string]interface{}
+		}
+		Context struct {
+			Library struct {
+				Name    string
+				Version string
+			}
+		}
+	}
+
+	data := make(chan payload, 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)
+		var p payload
+		err = json.Unmarshal(body, &p)
+		require.NoError(t, err)
+
+		data <- p
 	}))
 	defer server.Close()
 
@@ -63,12 +86,30 @@ func TestDiagnostics(t *testing.T) {
 	th.App.SetDiagnosticId(diagnosticID)
 	th.Server.initDiagnostics(server.URL)
 
+	assertPayload := func(t *testing.T, actual payload, event string, properties map[string]interface{}) {
+		assert.NotEmpty(t, actual.MessageId)
+		assert.False(t, actual.SentAt.IsZero())
+		if assert.Len(t, actual.Batch, 1) {
+			assert.NotEmpty(t, actual.Batch[0].MessageId, "message id should not be empty")
+			assert.Equal(t, diagnosticID, actual.Batch[0].UserId)
+			if event != "" {
+				assert.Equal(t, event, actual.Batch[0].Event)
+			}
+			assert.False(t, actual.Batch[0].Timestamp.IsZero(), "batch timestamp should not be the zero value")
+			if properties != nil {
+				assert.Equal(t, properties, actual.Batch[0].Properties)
+			}
+		}
+		assert.Equal(t, "analytics-go", actual.Context.Library.Name)
+		assert.Equal(t, "3.0.0", actual.Context.Library.Version)
+	}
+
 	// Should send a client identify message
 	select {
 	case identifyMessage := <-data:
-		require.Contains(t, identifyMessage, diagnosticID)
+		assertPayload(t, identifyMessage, "", nil)
 	case <-time.After(time.Second * 1):
-		require.Fail(t,"Did not receive ID message")
+		require.Fail(t, "Did not receive ID message")
 	}
 
 	t.Run("Send", func(t *testing.T) {
@@ -78,30 +119,31 @@ func TestDiagnostics(t *testing.T) {
 		})
 		select {
 		case result := <-data:
-			require.Contains(t, result, testValue)
+			assertPayload(t, result, "Testing Diagnostic", map[string]interface{}{
+				"hey": testValue,
+			})
 		case <-time.After(time.Second * 1):
-			require.Fail(t,"Did not receive diagnostic")
+			require.Fail(t, "Did not receive diagnostic")
 		}
 	})
 
 	t.Run("SendDailyDiagnostics", func(t *testing.T) {
 		th.App.sendDailyDiagnostics(true)
 
-		var info string
+		var info []string
 		// Collect the info sent.
 	Loop:
 		for {
 			select {
 			case result := <-data:
-				info += result
+				assertPayload(t, result, "", nil)
+				info = append(info, result.Batch[0].Event)
 			case <-time.After(time.Second * 1):
 				break Loop
 			}
 		}
 
 		for _, item := range []string{
-			TRACK_CONFIG_SERVICE,
-			TRACK_CONFIG_TEAM,
 			TRACK_CONFIG_SERVICE,
 			TRACK_CONFIG_TEAM,
 			TRACK_CONFIG_SQL,
@@ -137,7 +179,7 @@ func TestDiagnostics(t *testing.T) {
 
 		select {
 		case <-data:
-			require.Fail(t,"Should not send diagnostics when the segment key is not set")
+			require.Fail(t, "Should not send diagnostics when the segment key is not set")
 		case <-time.After(time.Second * 1):
 			// Did not receive diagnostics
 		}
@@ -150,7 +192,7 @@ func TestDiagnostics(t *testing.T) {
 
 		select {
 		case <-data:
-			require.Fail(t,"Should not send diagnostics when they are disabled")
+			require.Fail(t, "Should not send diagnostics when they are disabled")
 		case <-time.After(time.Second * 1):
 			// Did not receive diagnostics
 		}

app/server.go

@@ -760,7 +760,7 @@ func (s *Server) initDiagnostics(endpoint string) {
 			config.BatchSize = 1
 		}
 		client, _ := analytics.NewWithConfig(SEGMENT_KEY, config)
-		client.Enqueue(&analytics.Identify{
+		client.Enqueue(analytics.Identify{
 			UserId: s.diagnosticId,
 		})