Repository: NetApp/harvest
Pull request #1810 (2023-03-10 11:08:59)

fix: handle array as comma separated value in zapi
Tested with Volume and NetworkInterface object.

**For Volume, with changes:**
```
hardikl@hardikl-mac-2 ~ % curl -s 'http://localhost:13001/metrics'  | grep volume_labels | grep aggr | wc -l
     164
```
```
hardikl@hardikl-mac-2 ~ % curl -s 'http://localhost:13001/metrics'  | grep volume_labels | grep flexvol | grep aggr | wc -l
     163
hardikl@hardikl-mac-2 ~ % curl -s 'http://localhost:13001/metrics'  |grep volume_labels | grep flexvol | grep aggr     
volume_labels{aggr="umeng_aff300_aggr2",cluster="umeng-aff300-01-02",datacenter="u2",isEncrypted="false",isHardwareEncrypted="false",is_sis_volume="true",node="umeng-aff300-01",node_root="false",root_volume="No",snapshot_policy="none",state="online",style="flexvol",svm="astra_300",svm_root="false",type="rw",volume="trident_pvc_4e7e77ff_39a3_43e2_b4c8_69e31ce90b93"} 1.0
volume_labels{aggr="umeng_aff300_aggr2",cluster="umeng-aff300-01-02",datacenter="u2",isEncrypted="false",isHardwareEncrypted="false",is_sis_volume="true",node="umeng-aff300-01",node_root="false",root_volume="No",snapshot_policy="none",state="online",style="flexvol",svm="astra_300",svm_root="false",type="rw",volume="trident_pvc_9b4f1397_106d_4359_8076_2f3d89ba447f"} 1.0
volume_labels{aggr="umeng_aff300_aggr2",cluster="umeng-aff300-01-02",datacenter="u2",isEncrypted="false",isHardwareEncrypted="false",is_sis_volume="true",node="umeng-aff300-01",node_root="false",root_volume="No",snapshot_policy="none",state="online",style="flexvol",svm="astra_300",svm_root="false",type="rw",volume="trident_pvc_c37ce9f9_2b8d_406a_8610_2f511868882c"} 1.0
volume_labels{aggr="test",cluster="umeng-aff300-01-02",datacenter="u2",isEncrypted="false",isHardwareEncrypted="false",is_sis_volume="true",node="umeng-aff300-02",node_root="false",root_volume="No",snapshot_policy="default",state="online",style="flexvol",svm="astra_304",svm_root="false",type="rw",volume="dummy"} 1.0
volume_labels{aggr="test",cluster="umeng-aff300-01-02",datacenter="u2",isEncrypted="false",isHardwareEncrypted="false",is_sis_volume="true",node="umeng-aff300-02",node_root="false",root_volume="No",snapshot_policy="none",state="online",style="flexvol",svm="astra_304",svm_root="false",type="rw",volume="trident_pvc_ce04e59a_9073_407c_bbce_c149b0e6a32c"} 1.0
volume_labels{aggr="test",cluster="umeng-aff300-01-02",datacenter="u2",isEncrypted="false",isHardwareEncrypted="false",is_sis_volume="false",node="umeng-aff300-02",node_root="false",root_volume="Yes",snapshot_policy="default",state="online",style="flexvol",svm="test_banner_154_1",svm_root="true",type="rw",volume="test_banner_154_1_root"} 1.0
...
```
```
hardikl@hardikl-mac-2 ~ % curl -s 'http://localhost:13001/metrics'  | grep volume_labels | grep flexgroup | grep aggr | wc -l
       1
hardikl@hardikl-mac-2 ~ % curl -s 'http://localhost:13001/metrics'  |grep volume_labels | grep flexgroup | grep aggr        
volume_labels{aggr="test,umeng_aff300_aggr2",cluster="umeng-aff300-01-02",datacenter="u2",isEncrypted="false",isHardwareEncrypted="false",is_sis_volume="true",node="",node_root="false",root_volume="No",snapshot_policy="none",state="online",style="flexgroup",svm="osc",svm_root="false",type="rw",volume="RahulTest"} 1.0
```

**For NetworkInterface, custom Rest:**
```
name:               networkinterface
query:              api/private/cli/network/interface
object:             netint

counters:
  - ^^data_protocol => data_protocol
  - ^address        => address
  - ^^lif           => interface

plugins:
  LabelAgent:
    value_to_num:
      - test_status status ok ok 1

export_options:
  include_all_labels: true
```

<img width="1775" alt="image" src="https://user-images.githubusercontent.com/83282894/224300424-577cceb6-624e-42fd-beb7-c1a1297f35dc.png">


**For NetworkInterface, custom zapi, with changes,**
```
name: networkinterface
query: net-interface-get-iter
object: netint

collect_only_labels: true

counters:
  net-interface-info:
    - ^address
    - ^^interface-name => interface
    - data-protocols:
      - ^data-protocol => data_protocol

plugins:
  LabelAgent:
    value_to_num:
      - test_status status ok ok 1

export_options:
  include_all_labels: true
```
<img width="1772" alt="image" src="https://user-images.githubusercontent.com/83282894/224300575-2ae9cc23-0b5c-4f04-9df9-486e15d7f3f5.png">


- [X] code changes
- [ ] unit test case for array

Hardikl: Fix #1360 
Hardikl: As no extra symbol for array handling in Zapi, we don't need any documentation change for this.

Patch:
diff --git a/cmd/collectors/zapi/collector/zapi.go b/cmd/collectors/zapi/collector/zapi.go
index 7d11d65d..038aef64 100644
--- a/cmd/collectors/zapi/collector/zapi.go
+++ b/cmd/collectors/zapi/collector/zapi.go
@@ -17,6 +17,7 @@ import (
 	"github.com/netapp/harvest/v2/cmd/poller/plugin"
 	"github.com/netapp/harvest/v2/pkg/conf"
 	"github.com/netapp/harvest/v2/pkg/set"
+	"github.com/netapp/harvest/v2/pkg/util"
 	"sort"
 	"strconv"
 	"strings"
@@ -234,7 +235,7 @@ func (z *Zapi) PollData() (map[string]*matrix.Matrix, error) {
 		tag               string
 		err               error
 		ad, pd            time.Duration // Request/API time, Parse time, Fetch time
-		fetch             func(*matrix.Instance, *node.Node, []string)
+		fetch             func(*matrix.Instance, *node.Node, []string, bool)
 		instances         []*node.Node
 	)
 
@@ -251,15 +252,23 @@ func (z *Zapi) PollData() (map[string]*matrix.Matrix, error) {
 		oldInstances.Add(key)
 	}
 
-	fetch = func(instance *matrix.Instance, node *node.Node, path []string) {
+	fetch = func(instance *matrix.Instance, node *node.Node, path []string, isAppend bool) {
 
 		newpath := append(path, node.GetNameS())
 		key := strings.Join(newpath, ".")
 		z.Logger.Trace().Msgf(" > %s(%s)%s <%s%d%s> name=[%s%s%s%s] value=[%s%s%s]", color.Grey, newpath, color.End, color.Red, len(node.GetChildren()), color.End, color.Bold, color.Cyan, node.GetNameS(), color.End, color.Yellow, node.GetContentS(), color.End)
+
 		if value := node.GetContentS(); value != "" {
 			if label, has := z.instanceLabelPaths[key]; has {
-				instance.SetLabel(label, value)
-				z.Logger.Trace().Msgf(" > %slabel (%s) [%s] set value (%s)%s", color.Yellow, key, label, value, color.End)
+				// Handling array with comma separated values
+				previousValue := instance.GetLabel(label)
+				if isAppend && previousValue != "" {
+					instance.SetLabel(label, previousValue+","+value)
+					z.Logger.Trace().Msgf(" > %slabel (%s) [%s] set value (%s)%s", color.Yellow, key, label, instance.GetLabel(label)+","+value, color.End)
+				} else {
+					instance.SetLabel(label, value)
+					z.Logger.Trace().Msgf(" > %slabel (%s) [%s] set value (%s)%s", color.Yellow, key, label, value, color.End)
+				}
 				count++
 			} else if metric := mat.GetMetric(key); metric != nil {
 				if err := metric.SetValueString(instance, value); err != nil {
@@ -279,7 +288,12 @@ func (z *Zapi) PollData() (map[string]*matrix.Matrix, error) {
 		}
 
 		for _, child := range node.GetChildren() {
-			fetch(instance, child, newpath)
+			if util.HasDuplicates(child.GetAllChildNamesS()) {
+				z.Logger.Debug().Msgf("Array detected for %s", child.GetNameS())
+				fetch(instance, child, newpath, true)
+			} else {
+				fetch(instance, child, newpath, isAppend)
+			}
 		}
 	}
 
@@ -333,7 +347,7 @@ func (z *Zapi) PollData() (map[string]*matrix.Matrix, error) {
 					return nil, err
 				}
 			}
-			fetch(instance, instances[0], make([]string, 0))
+			fetch(instance, instances[0], make([]string, 0), false)
 			oldInstances.Remove("cluster")
 			break
 		}
@@ -360,7 +374,7 @@ func (z *Zapi) PollData() (map[string]*matrix.Matrix, error) {
 			oldInstances.Remove(key)
 			// clear all instance labels as there are some fields which may be missing between polls
 			instance.ClearLabels()
-			fetch(instance, instanceElem, make([]string, 0))
+			fetch(instance, instanceElem, make([]string, 0), false)
 		}
 	}
 
diff --git a/cmd/poller/collector/collector.go b/cmd/poller/collector/collector.go
index 3459299f..634855db 100644
--- a/cmd/poller/collector/collector.go
+++ b/cmd/poller/collector/collector.go
@@ -448,9 +448,9 @@ func (c *AbstractCollector) Start(wg *sync.WaitGroup) {
 		if nd := c.Schedule.NextDue(); nd > 0 {
 			c.Logger.Debug().Msgf("sleeping %s until next poll", nd.String()) //DEBUG
 			c.Schedule.Sleep()
-			// log if lagging by more than 50 ms
+			// log if lagging by more than 500 ms
 			// < is used since larger durations are more negative
-		} else if nd.Milliseconds() <= -50 && !c.Schedule.IsStandBy() {
+		} else if nd.Milliseconds() <= -500 && !c.Schedule.IsStandBy() {
 			c.Logger.Warn().
 				Str("lag", (-nd).String()).
 				Msg("lagging behind schedule")
diff --git a/conf/rest/9.12.0/volume.yaml b/conf/rest/9.12.0/volume.yaml
index c91e9763..d8be3182 100644
--- a/conf/rest/9.12.0/volume.yaml
+++ b/conf/rest/9.12.0/volume.yaml
@@ -9,6 +9,7 @@ counters:
   - ^name                                         => volume
   - ^svm.name                                     => svm
   - ^aggregates.#.uuid                            => aggrUuid        # handled in plugin for flexgroup
+  - ^aggregates.#.name                            => aggr
   - ^style                                        => style
   - ^type                                         => type
   - ^snapshot_policy.name                         => snapshot_policy
@@ -54,8 +55,7 @@ endpoints:
   - query: api/private/cli/volume
     counters:
       - ^^instance_uuid                           => instance_uuid
-      - ^node                                     => node
-      - ^aggregate                                => aggr
+      - ^nodes                                    => node
       - ^is_sis_volume                            => is_sis_volume
       - compression_space_saved_percent           => sis_compress_saved_percent
       - dedupe_space_saved_percent                => sis_dedup_saved_percent
diff --git a/conf/zapi/cdot/9.8.0/volume.yaml b/conf/zapi/cdot/9.8.0/volume.yaml
index e1f7506f..f4418c70 100644
--- a/conf/zapi/cdot/9.8.0/volume.yaml
+++ b/conf/zapi/cdot/9.8.0/volume.yaml
@@ -14,12 +14,14 @@ counters:
     - volume-id-attributes:
       - ^^instance-uuid             => instance_uuid
       - ^name                       => volume
-      - ^node                       => node
       - ^owning-vserver-name        => svm
-      - ^containing-aggregate-name  => aggr
+      - aggr-list:
+          - ^aggr-name              => aggr
       - ^containing-aggregate-uuid  => aggrUuid
       - ^style-extended             => style
       - ^type                       => type
+      - nodes:
+          - ^node-name              => node
 
     - volume-inode-attributes:
       - files-used
diff --git a/pkg/util/util.go b/pkg/util/util.go
index f96bc6fa..e5297274 100644
--- a/pkg/util/util.go
+++ b/pkg/util/util.go
@@ -391,3 +391,17 @@ func EncodeURL(href string) (string, error) {
 	u.RawQuery = u.Query().Encode()
 	return u.RequestURI(), nil
 }
+
+func HasDuplicates(slice []string) bool {
+	encountered := map[string]bool{}
+
+	for _, v := range slice {
+		if encountered[v] {
+			return true
+		} else {
+			encountered[v] = true
+		}
+	}
+
+	return false
+}


Test patch:
diff --git a/pkg/util/util_test.go b/pkg/util/util_test.go
index 05ff60be..a3ba1ca1 100644
--- a/pkg/util/util_test.go
+++ b/pkg/util/util_test.go
@@ -85,3 +85,41 @@ func TestParseMetricType(t *testing.T) {
 		})
 	}
 }
+
+func TestHasDuplicates(t *testing.T) {
+	type test struct {
+		testCase      string
+		childNames    []string
+		expectedArray bool
+	}
+
+	tests := []test{
+		{
+			testCase:      "testcasenochild",
+			childNames:    []string{},
+			expectedArray: false,
+		},
+		{
+			testCase:      "testcaseonechild",
+			childNames:    []string{"type"},
+			expectedArray: false,
+		},
+		{
+			testCase:      "testcasemultipleChild",
+			childNames:    []string{"type", "style", "aggr"},
+			expectedArray: false,
+		},
+		{
+			testCase:      "testcasearray",
+			childNames:    []string{"aggr-name", "aggr-name", "aggr-name", "aggr-name"},
+			expectedArray: true,
+		},
+	}
+
+	for _, testcase := range tests {
+		isArray := HasDuplicates(testcase.childNames)
+		if isArray != testcase.expectedArray {
+			t.Errorf("array hasn't been detected properly for %s, isArray is = %v, isArray should = %v", testcase.testCase, isArray, testcase.expectedArray)
+		}
+	}
+}