Skip to content

Commit 27832a1

Browse files
QxBytesQxBytes
committed
address feedback
1 parent c783032 commit 27832a1

File tree

5 files changed

+44
-60
lines changed

5 files changed

+44
-60
lines changed

‎cni/network/network.go‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import (
1515
"github.com/Azure/azure-container-networking/cni"
1616
"github.com/Azure/azure-container-networking/cni/api"
1717
"github.com/Azure/azure-container-networking/cni/log"
18-
telemetryclient "github.com/Azure/azure-container-networking/cni/telemetry/client"
1918
"github.com/Azure/azure-container-networking/cni/util"
2019
"github.com/Azure/azure-container-networking/cns"
2120
cnscli "github.com/Azure/azure-container-networking/cns/client"
@@ -30,6 +29,7 @@ import (
3029
"github.com/Azure/azure-container-networking/platform"
3130
nnscontracts "github.com/Azure/azure-container-networking/proto/nodenetworkservice/3.302.0.744"
3231
"github.com/Azure/azure-container-networking/store"
32+
"github.com/Azure/azure-container-networking/telemetry"
3333
cniSkel "github.com/containernetworking/cni/pkg/skel"
3434
cniTypes "github.com/containernetworking/cni/pkg/types"
3535
cniTypesCurr "github.com/containernetworking/cni/pkg/types/100"
@@ -39,8 +39,8 @@ import (
3939

4040
// matches if the string fully consists of zero or more alphanumeric, dots, dashes, parentheses, spaces, or underscores
4141
var (
42-
allowedInput = regexp.MustCompile(`^[a-zA-Z0-9._\-\(\) ]*$`)
43-
telemetry = telemetryclient.Telemetry
42+
allowedInput = regexp.MustCompile(`^[a-zA-Z0-9._\-\(\) ]*$`)
43+
telemetryClient = telemetry.Client
4444
)
4545

4646
const (
@@ -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.Settings().OperationType = opType
299-
telemetry.Settings().SubContext = containerID
300-
telemetry.Settings().EventMessage = msg
301-
telemetry.Settings().Version = plugin.Version
298+
telemetryClient.Settings().OperationType = opType
299+
telemetryClient.Settings().SubContext = containerID
300+
telemetryClient.Settings().EventMessage = msg
301+
telemetryClient.Settings().Version = plugin.Version
302302
}
303303

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

388388
plugin.setCNIReportDetails(args.ContainerID, CNI_ADD, "")
389-
telemetry.SendEvent(fmt.Sprintf("[cni-net] Processing ADD command with args {ContainerID:%v Netns:%v IfName:%v Args:%v Path:%v StdinData:%s}.",
389+
telemetryClient.SendEvent(fmt.Sprintf("[cni-net] Processing ADD command with args {ContainerID:%v Netns:%v IfName:%v Args:%v Path:%v StdinData:%s}.",
390390
args.ContainerID, args.Netns, args.IfName, args.Args, args.Path, args.StdinData))
391391

392392
iptables.DisableIPTableLock = nwCfg.DisableIPTableLock
@@ -437,7 +437,7 @@ func (plugin *NetPlugin) Add(args *cniSkel.CmdArgs) error {
437437
zap.Any("IPs", cniResult.IPs),
438438
zap.Error(log.NewErrorWithoutStackTrace(err)))
439439

440-
telemetry.SendEvent(fmt.Sprintf("ADD command completed with [ipamAddResult]: %s [epInfos]: %s [error]: %v ", ipamAddResult.PrettyString(), network.FormatStructPointers(epInfos), err))
440+
telemetryClient.SendEvent(fmt.Sprintf("ADD command completed with [ipamAddResult]: %s [epInfos]: %s [error]: %v ", ipamAddResult.PrettyString(), network.FormatStructPointers(epInfos), err))
441441
}()
442442

443443
ipamAddResult = IPAMAddResult{interfaceInfo: make(map[string]network.InterfaceInfo)}
@@ -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.Settings().Context = "AzureCNIMultitenancy"
496+
telemetryClient.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)
@@ -969,7 +969,7 @@ func (plugin *NetPlugin) Delete(args *cniSkel.CmdArgs) error {
969969
logger.Info("DEL command completed",
970970
zap.String("pod", k8sPodName),
971971
zap.Error(log.NewErrorWithoutStackTrace(err)))
972-
telemetry.SendEvent(fmt.Sprintf("DEL command completed: [released ip]: %+v [podname]: %s [namespace]: %s [error]: %v", nwCfg.IPAM.Address, k8sPodName, k8sNamespace, err))
972+
telemetryClient.SendEvent(fmt.Sprintf("DEL command completed: [released ip]: %+v [podname]: %s [namespace]: %s [error]: %v", nwCfg.IPAM.Address, k8sPodName, k8sNamespace, err))
973973
}()
974974

975975
// Parse network configuration from stdin.
@@ -987,10 +987,10 @@ 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.Settings().ContainerName = k8sPodName + ":" + k8sNamespace
990+
telemetryClient.Settings().ContainerName = k8sPodName + ":" + k8sNamespace
991991

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

996996
iptables.DisableIPTableLock = nwCfg.DisableIPTableLock
@@ -1120,14 +1120,14 @@ func (plugin *NetPlugin) Delete(args *cniSkel.CmdArgs) error {
11201120
for _, epInfo := range epInfos {
11211121
logger.Info("Deleting endpoint",
11221122
zap.String("endpointID", epInfo.EndpointID))
1123-
telemetry.SendEvent("Deleting endpoint: " + epInfo.EndpointID)
1123+
telemetryClient.SendEvent("Deleting endpoint: " + epInfo.EndpointID)
11241124

11251125
if !nwCfg.MultiTenancy && (epInfo.NICType == cns.InfraNIC || epInfo.NICType == "") {
11261126
// Delegated/secondary nic ips are statically allocated so we don't need to release
11271127
// Call into IPAM plugin to release the endpoint's addresses.
11281128
for i := range epInfo.IPAddresses {
11291129
logger.Info("Release ip", zap.String("ip", epInfo.IPAddresses[i].IP.String()))
1130-
telemetry.SendEvent(fmt.Sprintf("Release ip: %s container id: %s endpoint id: %s", epInfo.IPAddresses[i].IP.String(), args.ContainerID, epInfo.EndpointID))
1130+
telemetryClient.SendEvent(fmt.Sprintf("Release ip: %s container id: %s endpoint id: %s", epInfo.IPAddresses[i].IP.String(), args.ContainerID, epInfo.EndpointID))
11311131
err = plugin.ipamInvoker.Delete(&epInfo.IPAddresses[i], nwCfg, args, nwInfo.Options)
11321132
if err != nil {
11331133
return plugin.RetriableError(fmt.Errorf("failed to release address: %w", err))

‎cni/network/plugin/main.go‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import (
1313
"github.com/Azure/azure-container-networking/cni/api"
1414
zaplog "github.com/Azure/azure-container-networking/cni/log"
1515
"github.com/Azure/azure-container-networking/cni/network"
16-
telemetryclient "github.com/Azure/azure-container-networking/cni/telemetry/client"
1716
"github.com/Azure/azure-container-networking/common"
1817
"github.com/Azure/azure-container-networking/nns"
1918
"github.com/Azure/azure-container-networking/platform"
@@ -96,19 +95,19 @@ func rootExecute() error {
9695

9796
// Start telemetry process if not already started. This should be done inside lock, otherwise multiple process
9897
// end up creating/killing telemetry process results in undesired state.
99-
telemetryclient.Telemetry.StartAndConnectTelemetry(logger)
100-
defer telemetryclient.Telemetry.DisconnectTelemetry()
101-
telemetryclient.Telemetry.SetSettings(cniReport)
98+
telemetry.Client.StartAndConnectTelemetry(logger)
99+
defer telemetry.Client.DisconnectTelemetry()
100+
telemetry.Client.SetSettings(cniReport)
102101

103102
// CNI Acquires lock
104103
if err = netPlugin.Plugin.InitializeKeyValueStore(&config); err != nil {
105104
network.PrintCNIError(fmt.Sprintf("Failed to initialize key-value store of network plugin: %v", err))
106105

107-
if telemetryclient.Telemetry.IsConnected() {
106+
if telemetry.Client.IsConnected() {
108107
logger.Error("Not connected to telemetry service")
109108
return errors.Wrap(err, "lock acquire error")
110109
}
111-
telemetryclient.Telemetry.SendError(err)
110+
telemetry.Client.SendError(err)
112111

113112
if errors.Is(err, store.ErrTimeoutLockingStore) {
114113
var cniMetric telemetry.AIMetric
@@ -117,7 +116,7 @@ func rootExecute() error {
117116
Value: 1.0,
118117
CustomDimensions: make(map[string]string),
119118
}
120-
telemetryclient.Telemetry.SendMetric(&cniMetric)
119+
telemetry.Client.SendMetric(&cniMetric)
121120
}
122121
return errors.Wrap(err, "lock acquire error")
123122
}
@@ -137,7 +136,7 @@ func rootExecute() error {
137136

138137
if err = netPlugin.Start(&config); err != nil {
139138
network.PrintCNIError(fmt.Sprintf("Failed to start network plugin, err:%v.\n", err))
140-
telemetryclient.Telemetry.SendError(err)
139+
telemetry.Client.SendError(err)
141140
panic("network plugin start fatal error")
142141
}
143142

‎cni/network/stateless/main.go‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import (
1212
"github.com/Azure/azure-container-networking/cni/api"
1313
zapLog "github.com/Azure/azure-container-networking/cni/log"
1414
"github.com/Azure/azure-container-networking/cni/network"
15-
telemetryclient "github.com/Azure/azure-container-networking/cni/telemetry/client"
1615
"github.com/Azure/azure-container-networking/common"
1716
"github.com/Azure/azure-container-networking/log"
1817
"github.com/Azure/azure-container-networking/nns"
@@ -106,16 +105,16 @@ func rootExecute() error {
106105
}()
107106

108107
// Connect to the telemetry process. Does not start the telemetry service if it is not running.
109-
telemetryclient.Telemetry.ConnectTelemetry(logger)
110-
defer telemetryclient.Telemetry.DisconnectTelemetry()
111-
telemetryclient.Telemetry.SetSettings(cniReport)
108+
telemetry.Client.ConnectTelemetry(logger)
109+
defer telemetry.Client.DisconnectTelemetry()
110+
telemetry.Client.SetSettings(cniReport)
112111

113112
t := time.Now()
114113
cniReport.Timestamp = t.Format("2006-01-02 15:04:05")
115114

116115
if err = netPlugin.Start(&config); err != nil {
117116
network.PrintCNIError(fmt.Sprintf("Failed to start network plugin, err:%v.\n", err))
118-
telemetryclient.Telemetry.SendError(err)
117+
telemetry.Client.SendError(err)
119118
panic("network plugin start fatal error")
120119
}
121120
}

‎cni/telemetry/client/telemetry_client.go‎ renamed to ‎telemetry/telemetry_client.go‎

Lines changed: 14 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
1-
package telemetryclient
1+
package telemetry
22

33
import (
44
"fmt"
55
"os"
66
"sync"
77

8-
"github.com/Azure/azure-container-networking/telemetry"
98
"go.uber.org/zap"
109
)
1110

@@ -15,39 +14,27 @@ const (
1514
)
1615

1716
type TelemetryClient struct {
18-
cniReportSettings *telemetry.CNIReport
19-
tb *telemetry.TelemetryBuffer
17+
cniReportSettings *CNIReport
18+
tb *TelemetryBuffer
2019
logger *zap.Logger
2120
lock sync.Mutex
2221
}
2322

24-
type TelemetryInterface interface {
25-
// Settings gets a pointer to the cni report struct, used to modify individual fields
26-
Settings() *telemetry.CNIReport
27-
// SetSettings REPLACES the pointer to the cni report struct and should only be used on startup
28-
SetSettings(settings *telemetry.CNIReport)
29-
IsConnected() bool
30-
ConnectTelemetry(logger *zap.Logger)
31-
StartAndConnectTelemetry(logger *zap.Logger)
32-
DisconnectTelemetry()
33-
SendEvent(msg string)
34-
SendError(err error)
35-
SendMetric(cniMetric *telemetry.AIMetric)
36-
}
37-
38-
var Telemetry TelemetryInterface = NewTelemetryClient()
23+
var Client = NewTelemetryClient()
3924

4025
func NewTelemetryClient() *TelemetryClient {
4126
return &TelemetryClient{
42-
cniReportSettings: &telemetry.CNIReport{},
27+
cniReportSettings: &CNIReport{},
4328
}
4429
}
4530

46-
func (c *TelemetryClient) Settings() *telemetry.CNIReport {
31+
// Settings gets a pointer to the cni report struct, used to modify individual fields
32+
func (c *TelemetryClient) Settings() *CNIReport {
4733
return c.cniReportSettings
4834
}
4935

50-
func (c *TelemetryClient) SetSettings(settings *telemetry.CNIReport) {
36+
// SetSettings REPLACES the pointer to the cni report struct and should only be used on startup
37+
func (c *TelemetryClient) SetSettings(settings *CNIReport) {
5138
c.cniReportSettings = settings
5239
}
5340

@@ -56,13 +43,13 @@ func (c *TelemetryClient) IsConnected() bool {
5643
}
5744

5845
func (c *TelemetryClient) ConnectTelemetry(logger *zap.Logger) {
59-
c.tb = telemetry.NewTelemetryBuffer(logger)
46+
c.tb = NewTelemetryBuffer(logger)
6047
c.tb.ConnectToTelemetry()
6148
c.logger = logger
6249
}
6350

6451
func (c *TelemetryClient) StartAndConnectTelemetry(logger *zap.Logger) {
65-
c.tb = telemetry.NewTelemetryBuffer(logger)
52+
c.tb = NewTelemetryBuffer(logger)
6653
c.tb.ConnectToTelemetryService(telemetryNumberRetries, telemetryWaitTimeInMilliseconds)
6754
c.logger = logger
6855
}
@@ -82,7 +69,7 @@ func (c *TelemetryClient) sendEvent(msg string) {
8269
defer c.lock.Unlock()
8370
eventMsg := fmt.Sprintf("[%d] %s", os.Getpid(), msg)
8471
c.cniReportSettings.EventMessage = eventMsg
85-
telemetry.SendCNIEvent(c.tb, c.cniReportSettings)
72+
SendCNIEvent(c.tb, c.cniReportSettings)
8673
}
8774

8875
func (c *TelemetryClient) sendLog(msg string) {
@@ -106,11 +93,11 @@ func (c *TelemetryClient) SendError(err error) {
10693
c.sendEvent(err.Error())
10794
}
10895

109-
func (c *TelemetryClient) SendMetric(cniMetric *telemetry.AIMetric) {
96+
func (c *TelemetryClient) SendMetric(cniMetric *AIMetric) {
11097
if c.tb == nil || cniMetric == nil {
11198
return
11299
}
113-
err := telemetry.SendCNIMetric(cniMetric, c.tb)
100+
err := SendCNIMetric(cniMetric, c.tb)
114101
if err != nil {
115102
c.sendLog("Couldn't send metric: " + err.Error())
116103
}

‎cni/telemetry/client/telemetry_client_test.go‎ renamed to ‎telemetry/telemetry_client_test.go‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
1-
package telemetryclient
1+
package telemetry
22

33
import (
44
"errors"
55
"regexp"
66
"testing"
77

8-
"github.com/Azure/azure-container-networking/telemetry"
98
"github.com/stretchr/testify/require"
109
"go.uber.org/zap"
1110
)
@@ -39,7 +38,7 @@ func TestClient(t *testing.T) {
3938
require.Equal(t, logger, emptyClient.logger)
4039

4140
// for testing, we create a new telemetry buffer and assign it
42-
emptyClient.tb = &telemetry.TelemetryBuffer{}
41+
emptyClient.tb = &TelemetryBuffer{}
4342

4443
// test sending error
4544
require.NotPanics(t, func() { emptyClient.SendError(errMockTelemetryClient) })
@@ -51,7 +50,7 @@ func TestClient(t *testing.T) {
5150
require.Equal(t, "", emptyClient.Settings().ErrorMessage)
5251

5352
// test sending aimetrics doesn't panic...
54-
require.NotPanics(t, func() { emptyClient.SendMetric(&telemetry.AIMetric{}) })
53+
require.NotPanics(t, func() { emptyClient.SendMetric(&AIMetric{}) })
5554
// ...and doesn't affect the cni report
5655
require.Regexp(t, allowedEventMsg, emptyClient.Settings().EventMessage)
5756
require.Equal(t, "", emptyClient.Settings().ErrorMessage)

0 commit comments

Comments
 (0)