add unit tests and simplify

we use SendError where we would have previously called reportPluginError (no log emitted)
we don't set error message in cni report because the error message and event message fields both end up in the Message field in the cni telemetry service

Author: QxBytes

cni/network/plugin/main.go

@@ -53,9 +53,7 @@ func printVersion() {
 }
 
 func rootExecute() error {
-	var (
-		config common.PluginConfig
-	)
+	var config common.PluginConfig
 
 	config.Version = version
 

cni/network/stateless/main.go

@@ -51,9 +51,7 @@ func printVersion() {
 }
 
 func rootExecute() error {
-	var (
-		config common.PluginConfig
-	)
+	var config common.PluginConfig
 
 	log.SetName(name)
 	log.SetLevel(log.LevelInfo)

cni/telemetry/client/telemetry_client.go

@@ -21,17 +21,32 @@ type TelemetryClient struct {
 	lock              sync.Mutex
 }
 
-var Telemetry = NewTelemetryClient(&telemetry.CNIReport{})
+type TelemetryInterface interface {
+	// Settings gets a pointer to the cni report struct, used to modify individual fields
+	Settings() *telemetry.CNIReport
+	// SetSettings REPLACES the pointer to the cni report struct and should only be used on startup
+	SetSettings(settings *telemetry.CNIReport)
+	IsConnected() bool
+	ConnectTelemetry(logger *zap.Logger)
+	StartAndConnectTelemetry(logger *zap.Logger)
+	DisconnectTelemetry()
+	SendEvent(msg string)
+	SendError(err error)
+	SendMetric(cniMetric *telemetry.AIMetric)
+}
+
+var Telemetry TelemetryInterface = NewTelemetryClient()
 
-func NewTelemetryClient(report *telemetry.CNIReport) *TelemetryClient {
+func NewTelemetryClient() *TelemetryClient {
 	return &TelemetryClient{
-		cniReportSettings: report,
+		cniReportSettings: &telemetry.CNIReport{},
 	}
 }
 
 func (c *TelemetryClient) Settings() *telemetry.CNIReport {
 	return c.cniReportSettings
 }
+
 func (c *TelemetryClient) SetSettings(settings *telemetry.CNIReport) {
 	c.cniReportSettings = settings
 }
@@ -59,15 +74,14 @@ func (c *TelemetryClient) DisconnectTelemetry() {
 	c.tb.Close()
 }
 
-func (c *TelemetryClient) sendTelemetry(msg string, errMsg string) {
+func (c *TelemetryClient) sendEvent(msg string) {
 	if c.tb == nil {
 		return
 	}
 	c.lock.Lock()
 	defer c.lock.Unlock()
 	eventMsg := fmt.Sprintf("[%d] %s", os.Getpid(), msg)
 	c.cniReportSettings.EventMessage = eventMsg
-	c.cniReportSettings.ErrorMessage = errMsg
 	telemetry.SendCNIEvent(c.tb, c.cniReportSettings)
 }
 
@@ -80,14 +94,19 @@ func (c *TelemetryClient) sendLog(msg string) {
 
 func (c *TelemetryClient) SendEvent(msg string) {
 	c.sendLog(msg)
-	c.sendTelemetry(msg, "")
+	c.sendEvent(msg)
 }
+
 func (c *TelemetryClient) SendError(err error) {
 	if err == nil {
 		return
 	}
-	c.sendTelemetry("", err.Error())
+	// when the cni report reaches the telemetry service, the ai log message
+	// is set to either the cni report's event message or error message,
+	// whichever is not empty, so we can always just set the event message
+	c.sendEvent(err.Error())
 }
+
 func (c *TelemetryClient) SendMetric(cniMetric *telemetry.AIMetric) {
 	if c.tb == nil || cniMetric == nil {
 		return

cni/telemetry/client/telemetry_client_test.go

@@ -1,23 +1,33 @@
 package telemetryclient
 
 import (
+	"errors"
+	"regexp"
 	"testing"
 
+	"github.com/Azure/azure-container-networking/telemetry"
 	"github.com/stretchr/testify/require"
 	"go.uber.org/zap"
 )
 
+var errMockTelemetryClient = errors.New("mock telemetry client error")
+
 func TestClient(t *testing.T) {
-	emptyClient := NewTelemetryClient(nil)
+	allowedErrorMsg := regexp.MustCompile(`^\[\d+\] mock telemetry client error`)
+	allowedEventMsg := regexp.MustCompile(`^\[\d+\] telemetry event`)
+
+	emptyClient := NewTelemetryClient()
 
 	// an empty client should not cause panics
 	require.NotPanics(t, func() { emptyClient.SendEvent("no errors") })
 
+	require.NotPanics(t, func() { emptyClient.SendError(errMockTelemetryClient) })
+
 	require.NotPanics(t, func() { emptyClient.DisconnectTelemetry() })
 
 	require.NotPanics(t, func() { emptyClient.sendLog("no errors") })
 
-	require.NotPanics(t, func() { emptyClient.sendTelemetry("no errors", "") })
+	require.NotPanics(t, func() { emptyClient.sendEvent("no errors") })
 
 	logger, err := zap.NewDevelopment()
 	require.NoError(t, err)
@@ -27,4 +37,22 @@ func TestClient(t *testing.T) {
 
 	// should set logger during connection
 	require.Equal(t, logger, emptyClient.logger)
+
+	// for testing, we create a new telemetry buffer and assign it
+	emptyClient.tb = &telemetry.TelemetryBuffer{}
+
+	// test sending error, event is empty
+	require.NotPanics(t, func() { emptyClient.SendError(errMockTelemetryClient) })
+	require.Regexp(t, allowedErrorMsg, emptyClient.Settings().EventMessage)
+
+	// test sending event, error is empty
+	require.NotPanics(t, func() { emptyClient.SendEvent("telemetry event") })
+	require.Regexp(t, allowedEventMsg, emptyClient.Settings().EventMessage)
+	require.Equal(t, "", emptyClient.Settings().ErrorMessage)
+
+	// test sending aimetrics doesn't panic...
+	require.NotPanics(t, func() { emptyClient.SendMetric(&telemetry.AIMetric{}) })
+	// ...and doesn't affect the cni report
+	require.Regexp(t, allowedEventMsg, emptyClient.Settings().EventMessage)
+	require.Equal(t, "", emptyClient.Settings().ErrorMessage)
 }