From 0ceb9bb81bc5f068c5cea6f3d1840c949bf5e14b Mon Sep 17 00:00:00 2001 From: hardikl Date: Mon, 25 Sep 2023 18:09:19 +0530 Subject: [PATCH] feat: handle review comments --- .../zapi/plugins/aggregate/aggregate.go | 29 ++++++++-------- cmd/collectors/zapi/plugins/volume/volume.go | 34 +++++++++---------- 2 files changed, 32 insertions(+), 31 deletions(-) diff --git a/cmd/collectors/zapi/plugins/aggregate/aggregate.go b/cmd/collectors/zapi/plugins/aggregate/aggregate.go index 3bdc644de..38023ae7c 100644 --- a/cmd/collectors/zapi/plugins/aggregate/aggregate.go +++ b/cmd/collectors/zapi/plugins/aggregate/aggregate.go @@ -17,7 +17,8 @@ import ( type Aggregate struct { *plugin.AbstractPlugin client *zapi.Client - aggrCloudStoresMap map[string][]string // aggregate-uuid -> slice of cloud stores map + aggrCloudStoresMap map[string][]string // aggregate-uuid -> slice of cloud stores map + aggrFootprintMap map[string]map[string]string // aggr -> map of footprint metric name and value } func New(p *plugin.AbstractPlugin) plugin.Plugin { @@ -42,21 +43,22 @@ func (a *Aggregate) Init() error { } a.aggrCloudStoresMap = make(map[string][]string) + a.aggrFootprintMap = make(map[string]map[string]string) return nil } func (a *Aggregate) Run(dataMap map[string]*matrix.Matrix) ([]*matrix.Matrix, error) { data := dataMap[a.Object] + var err error // invoke aggr-object-store-get-iter zapi and populate cloud stores info - if err := a.getCloudStores(); err != nil { + if err = a.getCloudStores(); err != nil { if errors.Is(err, errs.ErrNoInstance) { a.Logger.Debug().Err(err).Msg("Failed to collect cloud store data") } } - aggrFootprintMap, err := a.getAggrFootprint() - if err != nil { + if err = a.getAggrFootprint(); err != nil { a.Logger.Error().Err(err).Msg("Failed to update footprint data") } @@ -69,7 +71,7 @@ func (a *Aggregate) Run(dataMap map[string]*matrix.Matrix) ([]*matrix.Matrix, er // Handling aggr footprint metrics aggrName := aggr.GetLabel("aggr") - if af, ok := aggrFootprintMap[aggrName]; ok { + if af, ok := a.aggrFootprintMap[aggrName]; ok { for afKey, afVal := range af { vfMetric := data.GetMetric(afKey) if vfMetric == nil { @@ -152,14 +154,13 @@ func (a *Aggregate) getCloudStores() error { return nil } -func (a *Aggregate) getAggrFootprint() (map[string]map[string]string, error) { +func (a *Aggregate) getAggrFootprint() error { var ( - result []*node.Node - aggrFootprintMap map[string]map[string]string - err error + result []*node.Node + err error ) - aggrFootprintMap = make(map[string]map[string]string) + a.aggrFootprintMap = make(map[string]map[string]string) request := node.NewXMLS("aggr-space-get-iter") request.NewChildS("max-records", collectors.DefaultBatchSize) desired := node.NewXMLS("desired-attributes") @@ -171,11 +172,11 @@ func (a *Aggregate) getAggrFootprint() (map[string]map[string]string, error) { request.AddChild(desired) if result, err = a.client.InvokeZapiCall(request); err != nil { - return aggrFootprintMap, err + return err } if len(result) == 0 { - return aggrFootprintMap, nil + return nil } for _, footprint := range result { @@ -186,9 +187,9 @@ func (a *Aggregate) getAggrFootprint() (map[string]map[string]string, error) { if performanceTierUsed != "" || performanceTierUsedPerc != "" { footprintMatrics["space_performance_tier_used"] = performanceTierUsed footprintMatrics["space_performance_tier_used_percent"] = performanceTierUsedPerc - aggrFootprintMap[aggr] = footprintMatrics + a.aggrFootprintMap[aggr] = footprintMatrics } } - return aggrFootprintMap, nil + return nil } diff --git a/cmd/collectors/zapi/plugins/volume/volume.go b/cmd/collectors/zapi/plugins/volume/volume.go index 01b7f8590..be488720a 100644 --- a/cmd/collectors/zapi/plugins/volume/volume.go +++ b/cmd/collectors/zapi/plugins/volume/volume.go @@ -14,9 +14,10 @@ import ( type Volume struct { *plugin.AbstractPlugin - currentVal int - client *zapi.Client - aggrsMap map[string]string // aggregate-uuid -> aggregate-name map + currentVal int + client *zapi.Client + aggrsMap map[string]string // aggregate-uuid -> aggregate-name map + volumeFootprintMap map[string]map[string]string // volume+svm -> map of footprint metric name and value } type aggrData struct { @@ -55,6 +56,7 @@ func (v *Volume) Init() error { } v.aggrsMap = make(map[string]string) + v.volumeFootprintMap = make(map[string]map[string]string) // Assigned the value to currentVal so that plugin would be invoked first time to populate cache. v.currentVal = v.SetPluginInterval() @@ -96,19 +98,18 @@ func (v *Volume) Run(dataMap map[string]*matrix.Matrix) ([]*matrix.Matrix, error v.Logger.Error().Err(err).Msg("Failed to update clone data") } - volumeFootprintMap, err := v.getVolumeFootprint() - if err != nil { + if err := v.getVolumeFootprint(); err != nil { v.Logger.Error().Err(err).Msg("Failed to update footprint data") } // update volume instance labels - v.updateVolumeLabels(data, volumeCloneMap, volumeFootprintMap) + v.updateVolumeLabels(data, volumeCloneMap) v.currentVal++ return nil, nil } -func (v *Volume) updateVolumeLabels(data *matrix.Matrix, volumeCloneMap map[string]volumeClone, volumeFootprintMap map[string]map[string]string) { +func (v *Volume) updateVolumeLabels(data *matrix.Matrix, volumeCloneMap map[string]volumeClone) { var err error for _, volume := range data.GetInstances() { if !volume.IsExportable() { @@ -149,7 +150,7 @@ func (v *Volume) updateVolumeLabels(data *matrix.Matrix, volumeCloneMap map[stri } // Handling volume footprint metrics - if vf, ok := volumeFootprintMap[key]; ok { + if vf, ok := v.volumeFootprintMap[key]; ok { for vfKey, vfVal := range vf { vfMetric := data.GetMetric(vfKey) if vfMetric == nil { @@ -215,14 +216,13 @@ func (v *Volume) getVolumeCloneInfo() (map[string]volumeClone, error) { return volumeCloneMap, nil } -func (v *Volume) getVolumeFootprint() (map[string]map[string]string, error) { +func (v *Volume) getVolumeFootprint() error { var ( - result []*node.Node - volumeFootprintMap map[string]map[string]string - err error + result []*node.Node + err error ) - volumeFootprintMap = make(map[string]map[string]string) + v.volumeFootprintMap = make(map[string]map[string]string) request := node.NewXMLS("volume-footprint-get-iter") request.NewChildS("max-records", collectors.DefaultBatchSize) desired := node.NewXMLS("desired-attributes") @@ -237,11 +237,11 @@ func (v *Volume) getVolumeFootprint() (map[string]map[string]string, error) { request.AddChild(desired) if result, err = v.client.InvokeZapiCall(request); err != nil { - return volumeFootprintMap, err + return err } if len(result) == 0 { - return volumeFootprintMap, nil + return nil } for _, footprint := range result { @@ -256,10 +256,10 @@ func (v *Volume) getVolumeFootprint() (map[string]map[string]string, error) { footprintMatrics["performance_tier_footprint_percent"] = performanceTierFootprintPerc footprintMatrics["capacity_tier_footprint"] = capacityTierFootprint footprintMatrics["capacity_tier_footprint_percent"] = capacityTierFootprintPerc - volumeFootprintMap[volume+svm] = footprintMatrics + v.volumeFootprintMap[volume+svm] = footprintMatrics } - return volumeFootprintMap, nil + return nil } func (v *Volume) getEncryptedDisks() ([]string, error) {