align flow with previous telemetry flow

tested and none panic:
telemetry service running, lock acquired
telemetry service not running, lock acquired
telemetry service running, lock not acquired
telemetry service not running, lock not acquired
stateless if telemetry service not running
stateless if telemetry service is running

Author: QxBytes

cni/network/network.go

@@ -973,7 +973,7 @@ func (plugin *NetPlugin) Delete(args *cniSkel.CmdArgs) error {
 		logger.Info("DEL command completed",
 			zap.String("pod", k8sPodName),
 			zap.Error(log.NewErrorWithoutStackTrace(err)))
-		telemetryClient.SendEvent(fmt.Sprintf("DEL command completed: [released ip]: %+v [podname]: %s [namespace]: %s [error]: %v", nwCfg.IPAM.Address, k8sPodName, k8sNamespace, err))
+		telemetryClient.SendEvent(fmt.Sprintf("DEL command completed: [podname]: %s [namespace]: %s [error]: %v", k8sPodName, k8sNamespace, err))
 	}()
 
 	// Parse network configuration from stdin.

cni/network/plugin/main.go

@@ -8,15 +8,13 @@ import (
 	"os"
 	"time"
 
-	"github.com/Azure/azure-container-networking/aitelemetry"
 	"github.com/Azure/azure-container-networking/cni"
 	"github.com/Azure/azure-container-networking/cni/api"
 	zaplog "github.com/Azure/azure-container-networking/cni/log"
 	"github.com/Azure/azure-container-networking/cni/network"
 	"github.com/Azure/azure-container-networking/common"
 	"github.com/Azure/azure-container-networking/nns"
 	"github.com/Azure/azure-container-networking/platform"
-	"github.com/Azure/azure-container-networking/store"
 	"github.com/Azure/azure-container-networking/telemetry"
 	"github.com/pkg/errors"
 	"go.uber.org/zap"
@@ -93,30 +91,19 @@ func rootExecute() error {
 			cniReport.VMUptime = upTime.Format("2006-01-02 15:04:05")
 		}
 
-		// Start telemetry process if not already started. This should be done inside lock, otherwise multiple process
-		// end up creating/killing telemetry process results in undesired state.
-		telemetry.AIClient.StartAndConnectTelemetry(logger)
-		defer telemetry.AIClient.DisconnectTelemetry()
-		telemetry.AIClient.SetSettings(cniReport)
-
-		// CNI Acquires lock
+		// CNI attempts to acquire lock
 		if err = netPlugin.Plugin.InitializeKeyValueStore(&config); err != nil {
+			// Error acquiring lock
 			network.PrintCNIError(fmt.Sprintf("Failed to initialize key-value store of network plugin: %v", err))
 
-			if !telemetry.AIClient.IsConnected() {
-				logger.Error("Not connected to telemetry service, skipping sending error to application insights")
-				return errors.Wrap(err, "lock acquire error")
-			}
-			telemetry.AIClient.SendError(err)
+			// Connect to telemetry service if it is running, otherwise skips telemetry
+			telemetry.AIClient.ConnectTelemetry(logger)
+			defer telemetry.AIClient.DisconnectTelemetry()
 
-			if errors.Is(err, store.ErrTimeoutLockingStore) {
-				var cniMetric telemetry.AIMetric
-				cniMetric.Metric = aitelemetry.Metric{
-					Name:             telemetry.CNILockTimeoutStr,
-					Value:            1.0,
-					CustomDimensions: make(map[string]string),
-				}
-				telemetry.AIClient.SendMetric(&cniMetric)
+			if telemetry.AIClient.IsConnected() {
+				telemetry.AIClient.SendError(err)
+			} else {
+				logger.Error("Not connected to telemetry service, skipping sending error to application insights")
 			}
 			return errors.Wrap(err, "lock acquire error")
 		}
@@ -130,6 +117,12 @@ func rootExecute() error {
 				os.Exit(1)
 			}
 		}()
+		// At this point, lock is acquired
+		// Start telemetry process if not already started. This should be done inside lock, otherwise multiple process
+		// end up creating/killing telemetry process results in undesired state.
+		telemetry.AIClient.StartAndConnectTelemetry(logger)
+		defer telemetry.AIClient.DisconnectTelemetry()
+		telemetry.AIClient.SetSettings(cniReport)
 
 		t := time.Now()
 		cniReport.Timestamp = t.Format("2006-01-02 15:04:05")

telemetry/telemetry_client_test.go

@@ -54,4 +54,12 @@ func TestClient(t *testing.T) {
 	// ...and doesn't affect the cni report
 	require.Regexp(t, allowedEventMsg, emptyClient.Settings().EventMessage)
 	require.Equal(t, "", emptyClient.Settings().ErrorMessage)
+
+	emptyClient.Settings().Context = "abc"
+	require.Equal(t, "abc", emptyClient.Settings().Context)
+
+	myClient := &Client{
+		tb: &TelemetryBuffer{},
+	}
+	require.NotPanics(t, func() { myClient.DisconnectTelemetry() })
 }

telemetry/telemetrybuffer_test.go

@@ -188,3 +188,22 @@ func TestStartTelemetryService(t *testing.T) {
 	err := tb.StartTelemetryService("", nil)
 	require.Error(t, err)
 }
+
+// TestExtraneousClose checks that closing potentially multiple times after a failed connect won't panic
+func TestExtraneousClose(t *testing.T) {
+	tb := NewTelemetryBuffer(nil)
+
+	tb.Close()
+	tb.Close()
+
+	tb.ConnectToTelemetry()
+
+	tb.Close()
+	tb.Close()
+
+	tb = NewTelemetryBuffer(nil)
+	tb.ConnectToTelemetryService(telemetryNumberRetries, telemetryWaitTimeInMilliseconds)
+
+	tb.Close()
+	tb.Close()
+}