Skip to content

Commit 787eaef

Browse files
QxBytesQxBytes
committed
refactor plugin main
remove reflect remove duplicated telemetry and telemetry buffer remove unused fields in report manager force access to telemetry client fields through methods move telemetry start/connect code closer to start of plugin execution
1 parent dd9ca83 commit 787eaef

File tree

6 files changed

+61
-75
lines changed

6 files changed

+61
-75
lines changed

‎cni/network/network.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -295,10 +295,10 @@ func (plugin *NetPlugin) getPodInfo(args string) (name, ns string, err error) {
295295
}
296296

297297
func (plugin *NetPlugin) setCNIReportDetails(containerID, opType, msg string) {
298-
telemetry.CNIReportSettings.OperationType = opType
299-
telemetry.CNIReportSettings.SubContext = containerID
300-
telemetry.CNIReportSettings.EventMessage = msg
301-
telemetry.CNIReportSettings.Version = plugin.Version
298+
telemetry.Settings().OperationType = opType
299+
telemetry.Settings().SubContext = containerID
300+
telemetry.Settings().EventMessage = msg
301+
telemetry.Settings().Version = plugin.Version
302302
}
303303

304304
func addNatIPV6SubnetInfo(nwCfg *cni.NetworkConfig,
@@ -383,7 +383,7 @@ func (plugin *NetPlugin) Add(args *cniSkel.CmdArgs) error {
383383
if err != nil {
384384
return err
385385
}
386-
telemetry.CNIReportSettings.ContainerName = k8sPodName + ":" + k8sNamespace
386+
telemetry.Settings().ContainerName = k8sPodName + ":" + k8sNamespace
387387

388388
plugin.setCNIReportDetails(args.ContainerID, CNI_ADD, "")
389389
telemetry.SendEvent(fmt.Sprintf("[cni-net] Processing ADD command with args {ContainerID:%v Netns:%v IfName:%v Args:%v Path:%v StdinData:%s}.",
@@ -493,7 +493,7 @@ func (plugin *NetPlugin) Add(args *cniSkel.CmdArgs) error {
493493
// triggered only in swift v1 multitenancy
494494
// dual nic multitenancy -> two interface infos
495495
// multitenancy (swift v1) -> one interface info
496-
telemetry.CNIReportSettings.Context = "AzureCNIMultitenancy"
496+
telemetry.Settings().Context = "AzureCNIMultitenancy"
497497
plugin.multitenancyClient.Init(cnsClient, AzureNetIOShim{})
498498

499499
// Temporary if block to determining whether we disable SNAT on host (for multi-tenant scenario only)
@@ -987,7 +987,7 @@ func (plugin *NetPlugin) Delete(args *cniSkel.CmdArgs) error {
987987
if k8sPodName, k8sNamespace, err = plugin.getPodInfo(args.Args); err != nil {
988988
logger.Error("Failed to get POD info", zap.Error(err))
989989
}
990-
telemetry.CNIReportSettings.ContainerName = k8sPodName + ":" + k8sNamespace
990+
telemetry.Settings().ContainerName = k8sPodName + ":" + k8sNamespace
991991

992992
plugin.setCNIReportDetails(args.ContainerID, CNI_DEL, "")
993993
telemetry.SendEvent(fmt.Sprintf("[cni-net] Processing DEL command with args {ContainerID:%v Netns:%v IfName:%v Args:%v Path:%v, StdinData:%s}.",

‎cni/network/plugin/main.go‎

Lines changed: 12 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ package main
66
import (
77
"fmt"
88
"os"
9-
"reflect"
109
"time"
1110

1211
"github.com/Azure/azure-container-networking/aitelemetry"
@@ -25,7 +24,6 @@ import (
2524
)
2625

2726
const (
28-
hostNetAgentURL = "http://168.63.129.16/machine/plugins?comp=netagent&type=cnireport"
2927
ipamQueryURL = "http://168.63.129.16/machine/plugins?comp=nmagent&type=getinterfaceinfov1"
3028
pluginName = "CNI"
3129
telemetryNumRetries = 5
@@ -54,27 +52,14 @@ func printVersion() {
5452
fmt.Printf("Azure CNI Version %v\n", version)
5553
}
5654

57-
// send error report to hostnetagent if CNI encounters any error.
58-
func reportPluginError(reportManager *telemetry.ReportManager, tb *telemetry.TelemetryBuffer, err error) {
59-
logger.Error("Report plugin error")
60-
reflect.ValueOf(reportManager.Report).Elem().FieldByName("ErrorMessage").SetString(err.Error())
61-
62-
if err := reportManager.SendReport(tb); err != nil {
63-
logger.Error("SendReport failed", zap.Error(err))
64-
}
65-
}
66-
6755
func rootExecute() error {
6856
var (
6957
config common.PluginConfig
70-
tb *telemetry.TelemetryBuffer
7158
)
7259

7360
config.Version = version
7461

7562
reportManager := &telemetry.ReportManager{
76-
HostNetAgentURL: hostNetAgentURL,
77-
ContentType: telemetry.ContentType,
7863
Report: &telemetry.CNIReport{
7964
Context: "AzureCNI",
8065
SystemDetails: telemetry.SystemInfo{},
@@ -111,17 +96,21 @@ func rootExecute() error {
11196
cniReport.VMUptime = upTime.Format("2006-01-02 15:04:05")
11297
}
11398

99+
// Start telemetry process if not already started. This should be done inside lock, otherwise multiple process
100+
// end up creating/killing telemetry process results in undesired state.
101+
telemetryclient.Telemetry.StartAndConnectTelemetry(logger)
102+
defer telemetryclient.Telemetry.DisconnectTelemetry()
103+
telemetryclient.Telemetry.SetSettings(cniReport)
104+
114105
// CNI Acquires lock
115106
if err = netPlugin.Plugin.InitializeKeyValueStore(&config); err != nil {
116107
network.PrintCNIError(fmt.Sprintf("Failed to initialize key-value store of network plugin: %v", err))
117108

118-
tb = telemetry.NewTelemetryBuffer(logger)
119-
if tberr := tb.Connect(); tberr != nil {
120-
logger.Error("Cannot connect to telemetry service", zap.Error(tberr))
109+
if telemetryclient.Telemetry.IsConnected() {
110+
logger.Error("Not connected to telemetry service")
121111
return errors.Wrap(err, "lock acquire error")
122112
}
123-
124-
reportPluginError(reportManager, tb, err)
113+
telemetryclient.Telemetry.SendError(err)
125114

126115
if errors.Is(err, store.ErrTimeoutLockingStore) {
127116
var cniMetric telemetry.AIMetric
@@ -130,13 +119,8 @@ func rootExecute() error {
130119
Value: 1.0,
131120
CustomDimensions: make(map[string]string),
132121
}
133-
sendErr := telemetry.SendCNIMetric(&cniMetric, tb)
134-
if sendErr != nil {
135-
logger.Error("Couldn't send cnilocktimeout metric", zap.Error(sendErr))
136-
}
122+
telemetryclient.Telemetry.SendMetric(&cniMetric)
137123
}
138-
139-
tb.Close()
140124
return errors.Wrap(err, "lock acquire error")
141125
}
142126

@@ -150,19 +134,12 @@ func rootExecute() error {
150134
}
151135
}()
152136

153-
// Start telemetry process if not already started. This should be done inside lock, otherwise multiple process
154-
// end up creating/killing telemetry process results in undesired state.
155-
telemetryclient.Telemetry.StartAndConnectTelemetry(logger)
156-
defer telemetryclient.Telemetry.DisconnectTelemetry()
157-
158-
telemetryclient.Telemetry.CNIReportSettings = cniReport
159-
160137
t := time.Now()
161138
cniReport.Timestamp = t.Format("2006-01-02 15:04:05")
162139

163140
if err = netPlugin.Start(&config); err != nil {
164141
network.PrintCNIError(fmt.Sprintf("Failed to start network plugin, err:%v.\n", err))
165-
reportPluginError(reportManager, tb, err)
142+
telemetryclient.Telemetry.SendError(err)
166143
panic("network plugin start fatal error")
167144
}
168145

@@ -199,7 +176,7 @@ func rootExecute() error {
199176
netPlugin.Stop()
200177

201178
if err != nil {
202-
reportPluginError(reportManager, tb, err)
179+
telemetryclient.Telemetry.SendError(err)
203180
}
204181

205182
return errors.Wrap(err, "Execute netplugin failure")

‎cni/network/stateless/main.go‎

Lines changed: 7 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ package main
66
import (
77
"fmt"
88
"os"
9-
"reflect"
109
"time"
1110

1211
"github.com/Azure/azure-container-networking/cni"
@@ -26,11 +25,10 @@ import (
2625
var logger = zapLog.CNILogger.With(zap.String("component", "cni-main"))
2726

2827
const (
29-
hostNetAgentURL = "http://168.63.129.16/machine/plugins?comp=netagent&type=cnireport"
30-
ipamQueryURL = "http://168.63.129.16/machine/plugins?comp=nmagent&type=getinterfaceinfov1"
31-
pluginName = "CNI"
32-
name = "azure-vnet"
33-
stateless = true
28+
ipamQueryURL = "http://168.63.129.16/machine/plugins?comp=nmagent&type=getinterfaceinfov1"
29+
pluginName = "CNI"
30+
name = "azure-vnet"
31+
stateless = true
3432
)
3533

3634
// Version is populated by make during build.
@@ -52,20 +50,9 @@ func printVersion() {
5250
fmt.Printf("Azure CNI Version %v\n", version)
5351
}
5452

55-
// send error report to hostnetagent if CNI encounters any error.
56-
func reportPluginError(reportManager *telemetry.ReportManager, tb *telemetry.TelemetryBuffer, err error) {
57-
logger.Error("Report plugin error")
58-
reflect.ValueOf(reportManager.Report).Elem().FieldByName("ErrorMessage").SetString(err.Error())
59-
60-
if err := reportManager.SendReport(tb); err != nil {
61-
logger.Error("SendReport failed", zap.Error(err))
62-
}
63-
}
64-
6553
func rootExecute() error {
6654
var (
6755
config common.PluginConfig
68-
tb *telemetry.TelemetryBuffer
6956
)
7057

7158
log.SetName(name)
@@ -79,8 +66,6 @@ func rootExecute() error {
7966
config.Stateless = stateless
8067

8168
reportManager := &telemetry.ReportManager{
82-
HostNetAgentURL: hostNetAgentURL,
83-
ContentType: telemetry.ContentType,
8469
Report: &telemetry.CNIReport{
8570
Context: "AzureCNI",
8671
SystemDetails: telemetry.SystemInfo{},
@@ -125,15 +110,14 @@ func rootExecute() error {
125110
// Connect to the telemetry process. Does not start the telemetry service if it is not running.
126111
telemetryclient.Telemetry.ConnectTelemetry(logger)
127112
defer telemetryclient.Telemetry.DisconnectTelemetry()
128-
129-
telemetryclient.Telemetry.CNIReportSettings = cniReport
113+
telemetryclient.Telemetry.SetSettings(cniReport)
130114

131115
t := time.Now()
132116
cniReport.Timestamp = t.Format("2006-01-02 15:04:05")
133117

134118
if err = netPlugin.Start(&config); err != nil {
135119
network.PrintCNIError(fmt.Sprintf("Failed to start network plugin, err:%v.\n", err))
136-
reportPluginError(reportManager, tb, err)
120+
telemetryclient.Telemetry.SendError(err)
137121
panic("network plugin start fatal error")
138122
}
139123
}
@@ -161,7 +145,7 @@ func rootExecute() error {
161145
netPlugin.Stop()
162146

163147
if err != nil {
164-
reportPluginError(reportManager, tb, err)
148+
telemetryclient.Telemetry.SendError(err)
165149
}
166150

167151
return errors.Wrap(err, "Execute netplugin failure")

‎cni/telemetry/client/telemetry_client.go‎

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ const (
1515
)
1616

1717
type TelemetryClient struct {
18-
CNIReportSettings *telemetry.CNIReport
18+
cniReportSettings *telemetry.CNIReport
1919
tb *telemetry.TelemetryBuffer
2020
logger *zap.Logger
2121
lock sync.Mutex
@@ -25,10 +25,21 @@ var Telemetry = NewTelemetryClient(&telemetry.CNIReport{})
2525

2626
func NewTelemetryClient(report *telemetry.CNIReport) *TelemetryClient {
2727
return &TelemetryClient{
28-
CNIReportSettings: report,
28+
cniReportSettings: report,
2929
}
3030
}
3131

32+
func (c *TelemetryClient) Settings() *telemetry.CNIReport {
33+
return c.cniReportSettings
34+
}
35+
func (c *TelemetryClient) SetSettings(settings *telemetry.CNIReport) {
36+
c.cniReportSettings = settings
37+
}
38+
39+
func (c *TelemetryClient) IsConnected() bool {
40+
return c.tb != nil && c.tb.Connected
41+
}
42+
3243
func (c *TelemetryClient) ConnectTelemetry(logger *zap.Logger) {
3344
c.tb = telemetry.NewTelemetryBuffer(logger)
3445
c.tb.ConnectToTelemetry()
@@ -48,15 +59,16 @@ func (c *TelemetryClient) DisconnectTelemetry() {
4859
c.tb.Close()
4960
}
5061

51-
func (c *TelemetryClient) sendTelemetry(msg string) {
62+
func (c *TelemetryClient) sendTelemetry(msg string, errMsg string) {
5263
if c.tb == nil {
5364
return
5465
}
5566
c.lock.Lock()
5667
defer c.lock.Unlock()
5768
eventMsg := fmt.Sprintf("[%d] %s", os.Getpid(), msg)
58-
c.CNIReportSettings.EventMessage = eventMsg
59-
telemetry.SendCNIEvent(c.tb, c.CNIReportSettings)
69+
c.cniReportSettings.EventMessage = eventMsg
70+
c.cniReportSettings.ErrorMessage = errMsg
71+
telemetry.SendCNIEvent(c.tb, c.cniReportSettings)
6072
}
6173

6274
func (c *TelemetryClient) sendLog(msg string) {
@@ -68,5 +80,20 @@ func (c *TelemetryClient) sendLog(msg string) {
6880

6981
func (c *TelemetryClient) SendEvent(msg string) {
7082
c.sendLog(msg)
71-
c.sendTelemetry(msg)
83+
c.sendTelemetry(msg, "")
84+
}
85+
func (c *TelemetryClient) SendError(err error) {
86+
if err == nil {
87+
return
88+
}
89+
c.sendTelemetry("", err.Error())
90+
}
91+
func (c *TelemetryClient) SendMetric(cniMetric *telemetry.AIMetric) {
92+
if c.tb == nil || cniMetric == nil {
93+
return
94+
}
95+
err := telemetry.SendCNIMetric(cniMetric, c.tb)
96+
if err != nil {
97+
c.logger.Error("Couldn't send metric", zap.Error(err))
98+
}
7299
}

‎cni/telemetry/client/telemetry_client_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ func TestClient(t *testing.T) {
1717

1818
require.NotPanics(t, func() { emptyClient.sendLog("no errors") })
1919

20-
require.NotPanics(t, func() { emptyClient.sendTelemetry("no errors") })
20+
require.NotPanics(t, func() { emptyClient.sendTelemetry("no errors", "") })
2121

2222
logger, err := zap.NewDevelopment()
2323
require.NoError(t, err)

‎telemetry/telemetry.go‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -89,9 +89,7 @@ type AIMetric struct {
8989

9090
// ReportManager structure.
9191
type ReportManager struct {
92-
HostNetAgentURL string
93-
ContentType string
94-
Report interface{}
92+
Report interface{}
9593
}
9694

9795
// GetReport retrieves orchestrator, system, OS and Interface details and create a report structure.

0 commit comments

Comments
 (0)