From 64f4695297b5f7eb756aad312ef2921c2515f36f Mon Sep 17 00:00:00 2001
From: Rahul Gupta <rahul.gupta@netapp.com>
Date: Wed, 4 Oct 2023 10:51:50 +0530
Subject: [PATCH 1/9] refactor: update href creation

---
 cmd/collectors/commonutils.go                 |  7 +-
 cmd/collectors/ems/ems.go                     | 13 ++-
 cmd/collectors/power.go                       |  6 +-
 .../rest/plugins/certificate/certificate.go   | 12 ++-
 cmd/collectors/rest/plugins/health/health.go  | 57 +++++++++---
 .../plugins/ontaps3service/ontaps3service.go  |  5 +-
 cmd/collectors/rest/plugins/qtree/qtree.go    |  6 +-
 .../securityaccount/securityaccount.go        |  5 +-
 .../rest/plugins/snapmirror/snapmirror.go     |  6 +-
 cmd/collectors/rest/plugins/svm/svm.go        | 20 ++++-
 cmd/collectors/rest/plugins/volume/volume.go  |  6 +-
 .../volumeanalytics/volumeanalytics.go        |  7 +-
 cmd/collectors/rest/rest.go                   | 22 ++++-
 cmd/collectors/restperf/plugins/disk/disk.go  | 15 +++-
 cmd/collectors/restperf/plugins/fcvi/fcvi.go  |  6 +-
 .../restperf/plugins/volumetag/volumetag.go   |  5 +-
 cmd/collectors/restperf/restperf.go           | 25 +++++-
 cmd/tools/generate/counter.go                 |  4 +-
 cmd/tools/rest/HrefBuilder.go                 | 88 +++++++++++++++++++
 cmd/tools/rest/client.go                      | 35 ++------
 cmd/tools/rest/rest.go                        | 11 ++-
 21 files changed, 289 insertions(+), 72 deletions(-)
 create mode 100644 cmd/tools/rest/HrefBuilder.go

diff --git a/cmd/collectors/commonutils.go b/cmd/collectors/commonutils.go
index 70748e1c0..873ea502d 100644
--- a/cmd/collectors/commonutils.go
+++ b/cmd/collectors/commonutils.go
@@ -72,7 +72,12 @@ func GetClusterTime(client *rest.Client, returnTimeOut string, logger *logging.L
 	query := "private/cli/cluster/date"
 	fields := []string{"date"}
 
-	href := rest.BuildHref(query, strings.Join(fields, ","), nil, "", "", "1", returnTimeOut, "")
+	href := rest.NewHrefBuilder().
+		APIPath(query).
+		Fields(strings.Join(fields, ",")).
+		MaxRecords("1").
+		ReturnTimeout(returnTimeOut).
+		Build()
 
 	if records, err = rest.Fetch(client, href); err != nil {
 		return clusterTime, err
diff --git a/cmd/collectors/ems/ems.go b/cmd/collectors/ems/ems.go
index 5468336bc..a11d7c74b 100644
--- a/cmd/collectors/ems/ems.go
+++ b/cmd/collectors/ems/ems.go
@@ -283,7 +283,11 @@ func (e *Ems) PollInstance() (map[string]*matrix.Matrix, error) {
 	query := "api/support/ems/messages"
 	fields := []string{"name"}
 
-	href := rest.BuildHref(query, strings.Join(fields, ","), nil, "", "", "", e.ReturnTimeOut, query)
+	href := rest.NewHrefBuilder().
+		APIPath(query).
+		Fields(strings.Join(fields, ",")).
+		ReturnTimeout(e.ReturnTimeOut).
+		Build()
 
 	if records, err = e.GetRestData(href); err != nil {
 		return nil, err
@@ -415,7 +419,12 @@ func (e *Ems) getHref(names []string, filter []string) string {
 	orderByIndexFilter := "order_by=" + "index%20asc"
 	filter = append(filter, orderByIndexFilter)
 
-	href := rest.BuildHref(e.Query, strings.Join(e.Fields, ","), filter, "", "", "", e.ReturnTimeOut, e.Query)
+	href := rest.NewHrefBuilder().
+		APIPath(e.Query).
+		Fields(strings.Join(e.Fields, ",")).
+		Filter(filter).
+		ReturnTimeout(e.ReturnTimeOut).
+		Build()
 	return href
 }
 
diff --git a/cmd/collectors/power.go b/cmd/collectors/power.go
index 30728672f..c05e6daa0 100644
--- a/cmd/collectors/power.go
+++ b/cmd/collectors/power.go
@@ -26,7 +26,11 @@ func collectChassisFRU(client *rest.Client, logger *logging.Logger) (map[string]
 	fields := "fru-name,type,status,connected-nodes,num-nodes"
 	query := "api/private/cli/system/chassis/fru"
 	filter := []string{"type=psu"}
-	href := rest.BuildHref("", fields, filter, "", "", "", "", query)
+	href := rest.NewHrefBuilder().
+		APIPath(query).
+		Fields(fields).
+		Filter(filter).
+		Build()
 
 	result, err := rest.Fetch(client, href)
 	if err != nil {
diff --git a/cmd/collectors/rest/plugins/certificate/certificate.go b/cmd/collectors/rest/plugins/certificate/certificate.go
index 643c4092a..004d0e508 100644
--- a/cmd/collectors/rest/plugins/certificate/certificate.go
+++ b/cmd/collectors/rest/plugins/certificate/certificate.go
@@ -186,7 +186,11 @@ func (my *Certificate) GetAdminVserver() (string, error) {
 	)
 
 	query := "api/private/cli/vserver"
-	href := rest.BuildHref("", "type", []string{"type=admin"}, "", "", "", "", query)
+	href := rest.NewHrefBuilder().
+		APIPath(query).
+		Fields("type").
+		Filter([]string{"type=admin"}).
+		Build()
 
 	if result, err = collectors.InvokeRestCall(my.client, href, my.Logger); err != nil {
 		return "", err
@@ -208,7 +212,11 @@ func (my *Certificate) GetSecuritySsl(adminSvm string) (string, error) {
 	)
 
 	query := "api/private/cli/security/ssl"
-	href := rest.BuildHref("", "serial", []string{"vserver=" + adminSvm}, "", "", "", "", query)
+	href := rest.NewHrefBuilder().
+		APIPath(query).
+		Fields("serial").
+		Filter([]string{"vserver=" + adminSvm}).
+		Build()
 
 	if result, err = collectors.InvokeRestCall(my.client, href, my.Logger); err != nil {
 		return "", err
diff --git a/cmd/collectors/rest/plugins/health/health.go b/cmd/collectors/rest/plugins/health/health.go
index 01eb40153..b34e6bc63 100644
--- a/cmd/collectors/rest/plugins/health/health.go
+++ b/cmd/collectors/rest/plugins/health/health.go
@@ -533,7 +533,11 @@ func (h *Health) getDisks() ([]gjson.Result, error) {
 
 	fields := []string{"name", "container_type"}
 	query := "api/storage/disks"
-	href := rest.BuildHref(query, strings.Join(fields, ","), []string{"container_type=broken|unassigned"}, "", "", "", "", query)
+	href := rest.NewHrefBuilder().
+		APIPath(query).
+		Fields(strings.Join(fields, ",")).
+		Filter([]string{"container_type=broken|unassigned"}).
+		Build()
 
 	if result, err = collectors.InvokeRestCall(h.client, href, h.Logger); err != nil {
 		return nil, err
@@ -549,7 +553,10 @@ func (h *Health) getShelves() ([]gjson.Result, error) {
 
 	fields := []string{"error_type", "error_severity", "error_text"}
 	query := "api/private/cli/storage/shelf"
-	href := rest.BuildHref(query, strings.Join(fields, ","), nil, "", "", "", "", query)
+	href := rest.NewHrefBuilder().
+		APIPath(query).
+		Fields(strings.Join(fields, ",")).
+		Build()
 
 	if result, err = collectors.InvokeRestCall(h.client, href, h.Logger); err != nil {
 		return nil, err
@@ -565,7 +572,11 @@ func (h *Health) getNodes() ([]gjson.Result, error) {
 
 	fields := []string{"health"}
 	query := "api/private/cli/node"
-	href := rest.BuildHref(query, strings.Join(fields, ","), []string{"health=false"}, "", "", "", "", query)
+	href := rest.NewHrefBuilder().
+		APIPath(query).
+		Fields(strings.Join(fields, ",")).
+		Filter([]string{"health=false"}).
+		Build()
 
 	if result, err = collectors.InvokeRestCall(h.client, href, h.Logger); err != nil {
 		return nil, err
@@ -580,7 +591,10 @@ func (h *Health) getRansomwareVolumes() ([]gjson.Result, error) {
 	)
 
 	query := "api/storage/volumes"
-	href := rest.BuildHref(query, "", []string{"anti_ransomware.state=enabled", "anti_ransomware.attack_probability=low|moderate|high"}, "", "", "", "", query)
+	href := rest.NewHrefBuilder().
+		APIPath(query).
+		Filter([]string{"anti_ransomware.state=enabled", "anti_ransomware.attack_probability=low|moderate|high"}).
+		Build()
 
 	if result, err = collectors.InvokeRestCall(h.client, href, h.Logger); err != nil {
 		return nil, err
@@ -596,7 +610,11 @@ func (h *Health) getNonCompliantLicense() ([]gjson.Result, error) {
 
 	query := "api/cluster/licensing/licenses"
 	fields := []string{"name,scope,state"}
-	href := rest.BuildHref(query, strings.Join(fields, ","), []string{"state=noncompliant"}, "", "", "", "", query)
+	href := rest.NewHrefBuilder().
+		APIPath(query).
+		Fields(strings.Join(fields, ",")).
+		Filter([]string{"state=noncompliant"}).
+		Build()
 
 	if result, err = collectors.InvokeRestCall(h.client, href, h.Logger); err != nil {
 		return nil, err
@@ -612,7 +630,11 @@ func (h *Health) getMoveFailedVolumes() ([]gjson.Result, error) {
 
 	query := "api/storage/volumes"
 	fields := []string{"uuid,name,movement.state,svm"}
-	href := rest.BuildHref(query, strings.Join(fields, ","), []string{"movement.state=cutover_wait|failed|cutover_pending"}, "", "", "", "", query)
+	href := rest.NewHrefBuilder().
+		APIPath(query).
+		Fields(strings.Join(fields, ",")).
+		Filter([]string{"movement.state=cutover_wait|failed|cutover_pending"}).
+		Build()
 
 	if result, err = collectors.InvokeRestCall(h.client, href, h.Logger); err != nil {
 		return nil, err
@@ -627,7 +649,11 @@ func (h *Health) getNonHomeLIFs() ([]gjson.Result, error) {
 	)
 
 	query := "api/network/ip/interfaces"
-	href := rest.BuildHref(query, "svm,location", []string{"location.is_home=false"}, "", "", "", "", query)
+	href := rest.NewHrefBuilder().
+		APIPath(query).
+		Fields("svm,location").
+		Filter([]string{"location.is_home=false"}).
+		Build()
 
 	if result, err = collectors.InvokeRestCall(h.client, href, h.Logger); err != nil {
 		return nil, err
@@ -643,7 +669,11 @@ func (h *Health) getFCPorts() ([]gjson.Result, error) {
 
 	fields := []string{"name,node"}
 	query := "api/network/fc/ports"
-	href := rest.BuildHref(query, strings.Join(fields, ","), []string{"enabled=true", "state=offlined_by_system"}, "", "", "", "", query)
+	href := rest.NewHrefBuilder().
+		APIPath(query).
+		Fields(strings.Join(fields, ",")).
+		Filter([]string{"enabled=true", "state=offlined_by_system"}).
+		Build()
 
 	if result, err = collectors.InvokeRestCall(h.client, href, h.Logger); err != nil {
 		return nil, err
@@ -659,7 +689,11 @@ func (h *Health) getEthernetPorts() ([]gjson.Result, error) {
 
 	fields := []string{"name,node"}
 	query := "api/network/ethernet/ports"
-	href := rest.BuildHref(query, strings.Join(fields, ","), []string{"enabled=true", "state=down"}, "", "", "", "", query)
+	href := rest.NewHrefBuilder().
+		APIPath(query).
+		Fields(strings.Join(fields, ",")).
+		Filter([]string{"enabled=true", "state=down"}).
+		Build()
 
 	if result, err = collectors.InvokeRestCall(h.client, href, h.Logger); err != nil {
 		return nil, err
@@ -673,7 +707,10 @@ func (h *Health) getSupportAlerts(filter []string) ([]gjson.Result, error) {
 		err    error
 	)
 	query := "api/private/support/alerts"
-	href := rest.BuildHref(query, "", filter, "", "", "", "", query)
+	href := rest.NewHrefBuilder().
+		APIPath(query).
+		Filter(filter).
+		Build()
 
 	if result, err = collectors.InvokeRestCall(h.client, href, h.Logger); err != nil {
 		return nil, err
diff --git a/cmd/collectors/rest/plugins/ontaps3service/ontaps3service.go b/cmd/collectors/rest/plugins/ontaps3service/ontaps3service.go
index 59e4119a0..a0fc3d4ea 100644
--- a/cmd/collectors/rest/plugins/ontaps3service/ontaps3service.go
+++ b/cmd/collectors/rest/plugins/ontaps3service/ontaps3service.go
@@ -67,7 +67,10 @@ func (o *OntapS3Service) Run(dataMap map[string]*matrix.Matrix) ([]*matrix.Matri
 	data := dataMap[o.Object]
 
 	fields := []string{"svm.name", "name", "is_http_enabled", "is_https_enabled", "secure_port", "port"}
-	href := rest.BuildHref("", strings.Join(fields, ","), nil, "", "", "", "", o.query)
+	href := rest.NewHrefBuilder().
+		APIPath(o.query).
+		Fields(strings.Join(fields, ",")).
+		Build()
 
 	if result, err = collectors.InvokeRestCall(o.client, href, o.Logger); err != nil {
 		return nil, err
diff --git a/cmd/collectors/rest/plugins/qtree/qtree.go b/cmd/collectors/rest/plugins/qtree/qtree.go
index cf780cbeb..eb7f38d98 100644
--- a/cmd/collectors/rest/plugins/qtree/qtree.go
+++ b/cmd/collectors/rest/plugins/qtree/qtree.go
@@ -159,7 +159,11 @@ func (q *Qtree) Run(dataMap map[string]*matrix.Matrix) ([]*matrix.Matrix, error)
 		filter = []string{"return_unmatched_nested_array_objects=true", "show_default_records=true", "type=" + strings.Join(q.quotaType[:], "|")}
 	}
 
-	href := rest.BuildHref("", "*", filter, "", "", "", "", q.query)
+	href := rest.NewHrefBuilder().
+		APIPath(q.query).
+		Fields("*").
+		Filter(filter).
+		Build()
 
 	if result, err = collectors.InvokeRestCall(q.client, href, q.Logger); err != nil {
 		return nil, err
diff --git a/cmd/collectors/rest/plugins/securityaccount/securityaccount.go b/cmd/collectors/rest/plugins/securityaccount/securityaccount.go
index 6ab0797a7..22ac6230e 100644
--- a/cmd/collectors/rest/plugins/securityaccount/securityaccount.go
+++ b/cmd/collectors/rest/plugins/securityaccount/securityaccount.go
@@ -61,7 +61,10 @@ func (s *SecurityAccount) Run(dataMap map[string]*matrix.Matrix) ([]*matrix.Matr
 	)
 
 	data := dataMap[s.Object]
-	href := rest.BuildHref("", "applications", nil, "", "", "", "", s.query)
+	href := rest.NewHrefBuilder().
+		APIPath(s.query).
+		Fields("applications").
+		Build()
 
 	if result, err = collectors.InvokeRestCall(s.client, href, s.Logger); err != nil {
 		return nil, err
diff --git a/cmd/collectors/rest/plugins/snapmirror/snapmirror.go b/cmd/collectors/rest/plugins/snapmirror/snapmirror.go
index b6efab821..db6ed9e44 100644
--- a/cmd/collectors/rest/plugins/snapmirror/snapmirror.go
+++ b/cmd/collectors/rest/plugins/snapmirror/snapmirror.go
@@ -111,7 +111,11 @@ func (my *SnapMirror) getSVMPeerData(cluster string) error {
 	my.svmPeerDataMap = make(map[string]Peer)
 	fields := []string{"name", "peer.svm.name", "peer.cluster.name"}
 	query := "api/svm/peers"
-	href := rest.BuildHref("", strings.Join(fields, ","), []string{"peer.cluster.name=!" + cluster}, "", "", "", "", query)
+	href := rest.NewHrefBuilder().
+		APIPath(query).
+		Fields(strings.Join(fields, ",")).
+		Filter([]string{"peer.cluster.name=!" + cluster}).
+		Build()
 
 	result, err := rest.Fetch(my.client, href)
 	if err != nil {
diff --git a/cmd/collectors/rest/plugins/svm/svm.go b/cmd/collectors/rest/plugins/svm/svm.go
index 016d704a7..36eea8bf9 100644
--- a/cmd/collectors/rest/plugins/svm/svm.go
+++ b/cmd/collectors/rest/plugins/svm/svm.go
@@ -209,7 +209,10 @@ func (my *SVM) GetKerberosConfig() (map[string]string, error) {
 	svmKerberosMap = make(map[string]string)
 	query := "api/protocols/nfs/kerberos/interfaces"
 	kerberosFields := []string{"svm.name", "enabled"}
-	href := rest.BuildHref("", strings.Join(kerberosFields, ","), nil, "", "", "", "", query)
+	href := rest.NewHrefBuilder().
+		APIPath(query).
+		Fields(strings.Join(kerberosFields, ",")).
+		Build()
 
 	if result, err = collectors.InvokeRestCall(my.client, href, my.Logger); err != nil {
 		return nil, err
@@ -241,7 +244,10 @@ func (my *SVM) GetFpolicy() (map[string]Fpolicy, error) {
 	svmFpolicyMap = make(map[string]Fpolicy)
 	query := "api/protocols/fpolicy"
 	fpolicyFields := []string{"svm.name", "policies.enabled", "policies.name"}
-	href := rest.BuildHref("", strings.Join(fpolicyFields, ","), nil, "", "", "", "", query)
+	href := rest.NewHrefBuilder().
+		APIPath(query).
+		Fields(strings.Join(fpolicyFields, ",")).
+		Build()
 
 	if result, err = collectors.InvokeRestCall(my.client, href, my.Logger); err != nil {
 		return nil, err
@@ -274,7 +280,10 @@ func (my *SVM) GetIscsiServices() (map[string]string, error) {
 	svmIscsiServiceMap = make(map[string]string)
 	query := "api/protocols/san/iscsi/services"
 	iscsiServiceFields := []string{"svm.name", "enabled"}
-	href := rest.BuildHref("", strings.Join(iscsiServiceFields, ","), nil, "", "", "", "", query)
+	href := rest.NewHrefBuilder().
+		APIPath(query).
+		Fields(strings.Join(iscsiServiceFields, ",")).
+		Build()
 
 	if result, err = collectors.InvokeRestCall(my.client, href, my.Logger); err != nil {
 		return nil, err
@@ -306,7 +315,10 @@ func (my *SVM) GetIscsiCredentials() (map[string]string, error) {
 	svmIscsiCredentialMap = make(map[string]string)
 	query := "api/protocols/san/iscsi/credentials"
 	iscsiCredentialFields := []string{"svm.name", "authentication_type"}
-	href := rest.BuildHref("", strings.Join(iscsiCredentialFields, ","), nil, "", "", "", "", query)
+	href := rest.NewHrefBuilder().
+		APIPath(query).
+		Fields(strings.Join(iscsiCredentialFields, ",")).
+		Build()
 
 	if result, err = collectors.InvokeRestCall(my.client, href, my.Logger); err != nil {
 		return nil, err
diff --git a/cmd/collectors/rest/plugins/volume/volume.go b/cmd/collectors/rest/plugins/volume/volume.go
index d57e4c19b..fdae5af40 100644
--- a/cmd/collectors/rest/plugins/volume/volume.go
+++ b/cmd/collectors/rest/plugins/volume/volume.go
@@ -190,7 +190,11 @@ func (my *Volume) getEncryptedDisks() ([]gjson.Result, error) {
 
 	diskFields := []string{"aggregates.name", "aggregates.uuid"}
 	query := "api/storage/disks"
-	href := rest.BuildHref("", strings.Join(diskFields, ","), []string{"protection_mode=!data|full"}, "", "", "", "", query)
+	href := rest.NewHrefBuilder().
+		APIPath(query).
+		Fields(strings.Join(diskFields, ",")).
+		Filter([]string{"protection_mode=!data|full"}).
+		Build()
 
 	if result, err = collectors.InvokeRestCall(my.client, href, my.Logger); err != nil {
 		return nil, err
diff --git a/cmd/collectors/rest/plugins/volumeanalytics/volumeanalytics.go b/cmd/collectors/rest/plugins/volumeanalytics/volumeanalytics.go
index d89a3e14a..8f526e86b 100644
--- a/cmd/collectors/rest/plugins/volumeanalytics/volumeanalytics.go
+++ b/cmd/collectors/rest/plugins/volumeanalytics/volumeanalytics.go
@@ -282,8 +282,13 @@ func (v *VolumeAnalytics) getAnalyticsData(instanceID string) ([]gjson.Result, g
 
 	fields := []string{"analytics.file_count", "analytics.bytes_used", "analytics.subdir_count", "analytics.by_modified_time.bytes_used", "analytics.by_accessed_time.bytes_used"}
 	query := path.Join("api/storage/volumes", instanceID, "files/")
-	href := rest.BuildHref(query, strings.Join(fields, ","), []string{"order_by=analytics.bytes_used+desc", "type=directory"}, "", "", MaxDirCollectCount, "", query)
 
+	href := rest.NewHrefBuilder().
+		APIPath(query).
+		Fields(strings.Join(fields, ",")).
+		Filter([]string{"order_by=analytics.bytes_used+desc", "type=directory"}).
+		MaxRecords(MaxDirCollectCount).
+		Build()
 	if result, analytics, err = rest.FetchAnalytics(v.client, href); err != nil {
 		return nil, gjson.Result{}, err
 	}
diff --git a/cmd/collectors/rest/rest.go b/cmd/collectors/rest/rest.go
index 9e43cab72..1be6dc9b2 100644
--- a/cmd/collectors/rest/rest.go
+++ b/cmd/collectors/rest/rest.go
@@ -297,7 +297,12 @@ func (r *Rest) PollData() (map[string]*matrix.Matrix, error) {
 
 	startTime = time.Now()
 
-	href := rest.BuildHref(r.Prop.Query, strings.Join(r.Prop.Fields, ","), r.Prop.Filter, "", "", "", r.Prop.ReturnTimeOut, r.Prop.Query)
+	href := rest.NewHrefBuilder().
+		APIPath(r.Prop.Query).
+		Fields(strings.Join(r.Prop.Fields, ",")).
+		Filter(r.Prop.Filter).
+		ReturnTimeout(r.Prop.ReturnTimeOut).
+		Build()
 
 	if records, err = r.GetRestData(href); err != nil {
 		return nil, err
@@ -341,7 +346,12 @@ func (r *Rest) pollData(startTime time.Time, records []gjson.Result, endpointFun
 }
 
 func (r *Rest) processEndPoint(e *endPoint) ([]gjson.Result, error) {
-	href := rest.BuildHref(r.query(e), strings.Join(r.fields(e), ","), r.filter(e), "", "", "", r.Prop.ReturnTimeOut, r.query(e))
+	href := rest.NewHrefBuilder().
+		APIPath(r.query(e)).
+		Fields(strings.Join(r.fields(e), ",")).
+		Filter(r.filter(e)).
+		ReturnTimeout(r.Prop.ReturnTimeOut).
+		Build()
 
 	return r.GetRestData(href)
 }
@@ -365,7 +375,7 @@ func (r *Rest) processEndPoints(endpointFunc func(e *endPoint) ([]gjson.Result,
 		}
 
 		if len(records) == 0 {
-			r.Logger.Debug().Str("ApiPath", endpoint.prop.Query).Msg("no instances on cluster")
+			r.Logger.Debug().Str("APIPath", endpoint.prop.Query).Msg("no instances on cluster")
 			continue
 		}
 		count = r.HandleResults(records, endpoint.prop, true)
@@ -665,7 +675,11 @@ func (r *Rest) getNodeUuids() ([]collector.ID, error) {
 	)
 	query := "api/cluster/nodes"
 
-	href := rest.BuildHref(query, "serial_number,system_id", nil, "", "", "", r.Prop.ReturnTimeOut, query)
+	href := rest.NewHrefBuilder().
+		APIPath(query).
+		Fields("serial_number,system_id").
+		ReturnTimeout(r.Prop.ReturnTimeOut).
+		Build()
 
 	if records, err = r.GetRestData(href); err != nil {
 		return nil, err
diff --git a/cmd/collectors/restperf/plugins/disk/disk.go b/cmd/collectors/restperf/plugins/disk/disk.go
index 6d586df6f..fd7323eff 100644
--- a/cmd/collectors/restperf/plugins/disk/disk.go
+++ b/cmd/collectors/restperf/plugins/disk/disk.go
@@ -244,7 +244,10 @@ func (d *Disk) Run(dataMap map[string]*matrix.Matrix) ([]*matrix.Matrix, error)
 		d.powerData[a].SetGlobalLabels(data.GetGlobalLabels())
 	}
 
-	href := rest.BuildHref("", "*", nil, "", "", "", "", d.query)
+	href := rest.NewHrefBuilder().
+		APIPath(d.query).
+		Fields("*").
+		Build()
 
 	records, err := rest.Fetch(d.client, href)
 	if err != nil {
@@ -544,7 +547,10 @@ func (d *Disk) getDisks() error {
 
 	query := "api/storage/disks"
 
-	href := rest.BuildHref("", "name,uid,shelf.uid,type,aggregates", nil, "", "", "", "", query)
+	href := rest.NewHrefBuilder().
+		APIPath(query).
+		Fields("name,uid,shelf.uid,type,aggregates").
+		Build()
 
 	records, err := rest.Fetch(d.client, href)
 	if err != nil {
@@ -599,7 +605,10 @@ func (d *Disk) getAggregates() error {
 
 	query := "api/private/cli/aggr"
 
-	href := rest.BuildHref("", "aggregate,composite,node,uses_shared_disks,storage_type", nil, "", "", "", "", query)
+	href := rest.NewHrefBuilder().
+		APIPath(query).
+		Fields("aggregate,composite,node,uses_shared_disks,storage_type").
+		Build()
 
 	records, err := rest.Fetch(d.client, href)
 	if err != nil {
diff --git a/cmd/collectors/restperf/plugins/fcvi/fcvi.go b/cmd/collectors/restperf/plugins/fcvi/fcvi.go
index 6dbba32ad..632a03b23 100644
--- a/cmd/collectors/restperf/plugins/fcvi/fcvi.go
+++ b/cmd/collectors/restperf/plugins/fcvi/fcvi.go
@@ -37,8 +37,10 @@ func (f *FCVI) Run(dataMap map[string]*matrix.Matrix) ([]*matrix.Matrix, error)
 	data := dataMap[f.Object]
 	query := "api/private/cli/metrocluster/interconnect/adapter"
 	fields := []string{"node", "adapter", "port_name"}
-	href := rest.BuildHref("", strings.Join(fields, ","), nil, "", "", "", "", query)
-
+	href := rest.NewHrefBuilder().
+		APIPath(query).
+		Fields(strings.Join(fields, ",")).
+		Build()
 	records, err := rest.Fetch(f.client, href)
 	if err != nil {
 		f.Logger.Error().Err(err).Str("href", href).Msg("Failed to fetch data")
diff --git a/cmd/collectors/restperf/plugins/volumetag/volumetag.go b/cmd/collectors/restperf/plugins/volumetag/volumetag.go
index 77628ea86..a8ab88c8c 100644
--- a/cmd/collectors/restperf/plugins/volumetag/volumetag.go
+++ b/cmd/collectors/restperf/plugins/volumetag/volumetag.go
@@ -41,7 +41,10 @@ func (v *VolumeTag) Run(dataMap map[string]*matrix.Matrix) ([]*matrix.Matrix, er
 	data := dataMap[v.Object]
 	query := "api/storage/volumes"
 
-	href := rest.BuildHref("", "comment", nil, "", "", "", "", query)
+	href := rest.NewHrefBuilder().
+		APIPath(query).
+		Fields("comment").
+		Build()
 
 	records, err := rest.Fetch(v.client, href)
 	if err != nil {
diff --git a/cmd/collectors/restperf/restperf.go b/cmd/collectors/restperf/restperf.go
index 60a05be31..3e67837e2 100644
--- a/cmd/collectors/restperf/restperf.go
+++ b/cmd/collectors/restperf/restperf.go
@@ -235,7 +235,10 @@ func (r *RestPerf) PollCounter() (map[string]*matrix.Matrix, error) {
 		records []gjson.Result
 	)
 
-	href := rest.BuildHref(r.Prop.Query, "", nil, "", "", "", r.Prop.ReturnTimeOut, r.Prop.Query)
+	href := rest.NewHrefBuilder().
+		APIPath(r.Prop.Query).
+		ReturnTimeout(r.Prop.ReturnTimeOut).
+		Build()
 	r.Logger.Debug().Str("href", href).Msg("")
 	if href == "" {
 		return nil, errs.New(errs.ErrConfig, "empty url")
@@ -639,7 +642,11 @@ func (r *RestPerf) PollData() (map[string]*matrix.Matrix, error) {
 
 	dataQuery := path.Join(r.Prop.Query, "rows")
 
-	href := rest.BuildHref(dataQuery, strings.Join(r.Prop.Fields, ","), nil, "", "", "", r.Prop.ReturnTimeOut, dataQuery)
+	href := rest.NewHrefBuilder().
+		APIPath(dataQuery).
+		Fields(strings.Join(r.Prop.Fields, ",")).
+		ReturnTimeout(r.Prop.ReturnTimeOut).
+		Build()
 
 	r.Logger.Debug().Str("href", href).Msg("")
 	if href == "" {
@@ -1228,7 +1235,12 @@ func (r *RestPerf) getParentOpsCounters(data *matrix.Matrix) error {
 
 	var filter []string
 	filter = append(filter, "counters.name=ops")
-	href := rest.BuildHref(dataQuery, "*", filter, "", "", "", r.Prop.ReturnTimeOut, dataQuery)
+	href := rest.NewHrefBuilder().
+		APIPath(dataQuery).
+		Fields("*").
+		Filter(filter).
+		ReturnTimeout(r.Prop.ReturnTimeOut).
+		Build()
 
 	r.Logger.Debug().Str("href", href).Msg("")
 	if href == "" {
@@ -1342,7 +1354,12 @@ func (r *RestPerf) PollInstance() (map[string]*matrix.Matrix, error) {
 		}
 	}
 
-	href := rest.BuildHref(dataQuery, fields, filter, "", "", "", r.Prop.ReturnTimeOut, dataQuery)
+	href := rest.NewHrefBuilder().
+		APIPath(dataQuery).
+		Fields(fields).
+		Filter(filter).
+		ReturnTimeout(r.Prop.ReturnTimeOut).
+		Build()
 
 	r.Logger.Debug().Str("href", href).Msg("")
 	if href == "" {
diff --git a/cmd/tools/generate/counter.go b/cmd/tools/generate/counter.go
index 45d5a478a..f10e3eb0a 100644
--- a/cmd/tools/generate/counter.go
+++ b/cmd/tools/generate/counter.go
@@ -622,7 +622,9 @@ func processRestPerfCounters(path string, client *rest.Client) map[string]Counte
 			}
 		}
 	}
-	href := rest.BuildHref(query, "", nil, "", "", "", "", query)
+	href := rest.NewHrefBuilder().
+		APIPath(query).
+		Build()
 	records, err = rest.Fetch(client, href)
 	if err != nil {
 		fmt.Printf("error while invoking api %+v\n", err)
diff --git a/cmd/tools/rest/HrefBuilder.go b/cmd/tools/rest/HrefBuilder.go
new file mode 100644
index 000000000..1b7446113
--- /dev/null
+++ b/cmd/tools/rest/HrefBuilder.go
@@ -0,0 +1,88 @@
+package rest
+
+import "strings"
+
+type HrefBuilder struct {
+	apiPath       string
+	fields        string
+	filter        []string
+	queryFields   string
+	queryValue    string
+	maxRecords    string
+	returnTimeout string
+	endpoint      string
+}
+
+func NewHrefBuilder() *HrefBuilder {
+	return &HrefBuilder{}
+}
+
+func (b *HrefBuilder) APIPath(apiPath string) *HrefBuilder {
+	b.apiPath = apiPath
+	return b
+}
+
+func (b *HrefBuilder) Fields(fields string) *HrefBuilder {
+	b.fields = fields
+	return b
+}
+
+func (b *HrefBuilder) Filter(filter []string) *HrefBuilder {
+	b.filter = filter
+	return b
+}
+
+func (b *HrefBuilder) QueryFields(queryFields string) *HrefBuilder {
+	b.queryFields = queryFields
+	return b
+}
+
+func (b *HrefBuilder) QueryValue(queryValue string) *HrefBuilder {
+	b.queryValue = queryValue
+	return b
+}
+
+func (b *HrefBuilder) MaxRecords(maxRecords string) *HrefBuilder {
+	b.maxRecords = maxRecords
+	return b
+}
+
+func (b *HrefBuilder) ReturnTimeout(returnTimeout string) *HrefBuilder {
+	b.returnTimeout = returnTimeout
+	return b
+}
+
+func (b *HrefBuilder) Endpoint(endpoint string) *HrefBuilder {
+	b.endpoint = endpoint
+	return b
+}
+
+func (b *HrefBuilder) Build() string {
+	href := strings.Builder{}
+	if b.endpoint == "" {
+		if !strings.HasPrefix(b.apiPath, "api/") {
+			href.WriteString("api/")
+		}
+		href.WriteString(b.apiPath)
+	} else {
+		href.WriteString(b.endpoint)
+	}
+	href.WriteString("?return_records=true")
+	addArg(&href, "&fields=", b.fields)
+	for _, f := range b.filter {
+		addArg(&href, "&", f)
+	}
+	addArg(&href, "&query_fields=", b.queryFields)
+	addArg(&href, "&query=", b.queryValue)
+	addArg(&href, "&max_records=", b.maxRecords)
+	addArg(&href, "&return_timeout=", b.returnTimeout)
+	return href.String()
+}
+
+func addArg(href *strings.Builder, field string, value string) {
+	if value == "" {
+		return
+	}
+	href.WriteString(field)
+	href.WriteString(value)
+}
diff --git a/cmd/tools/rest/client.go b/cmd/tools/rest/client.go
index 0aaff5ce7..89d20046b 100644
--- a/cmd/tools/rest/client.go
+++ b/cmd/tools/rest/client.go
@@ -292,8 +292,11 @@ func (c *Client) Init(retries int) error {
 	)
 
 	for i = 0; i < retries; i++ {
-
-		content, err = c.GetRest(BuildHref("cluster", "*", nil, "", "", "", "", ""))
+		href := NewHrefBuilder().
+			APIPath("api/cluster").
+			Fields("*").
+			Build()
+		content, err = c.GetRest(href)
 		if err != nil {
 			if errors.Is(err, errs.ErrPermissionDenied) {
 				return err
@@ -313,34 +316,6 @@ func (c *Client) Init(retries int) error {
 	return err
 }
 
-func BuildHref(apiPath string, fields string, field []string, queryFields string, queryValue string, maxRecords string, returnTimeout string, endpoint string) string {
-	href := strings.Builder{}
-	if endpoint == "" {
-		href.WriteString("api/")
-		href.WriteString(apiPath)
-	} else {
-		href.WriteString(endpoint)
-	}
-	href.WriteString("?return_records=true")
-	addArg(&href, "&fields=", fields)
-	for _, f := range field {
-		addArg(&href, "&", f)
-	}
-	addArg(&href, "&query_fields=", queryFields)
-	addArg(&href, "&query=", queryValue)
-	addArg(&href, "&max_records=", maxRecords)
-	addArg(&href, "&return_timeout=", returnTimeout)
-	return href.String()
-}
-
-func addArg(href *strings.Builder, field string, value string) {
-	if value == "" {
-		return
-	}
-	href.WriteString(field)
-	href.WriteString(value)
-}
-
 func (c *Client) Cluster() Cluster {
 	return c.cluster
 }
diff --git a/cmd/tools/rest/rest.go b/cmd/tools/rest/rest.go
index a42a402dc..95050c7be 100644
--- a/cmd/tools/rest/rest.go
+++ b/cmd/tools/rest/rest.go
@@ -184,7 +184,16 @@ func fetchData(poller *conf.Poller, timeout time.Duration) (*Results, error) {
 	now := time.Now()
 	var records []any
 	var curls []string
-	href := BuildHref(args.API, args.Fields, args.Field, args.QueryField, args.QueryValue, args.MaxRecords, "", args.Endpoint)
+
+	href := NewHrefBuilder().
+		APIPath(args.API).
+		Fields(args.Fields).
+		Filter(args.Field).
+		QueryFields(args.QueryField).
+		QueryValue(args.QueryValue).
+		MaxRecords(args.MaxRecords).
+		Endpoint(args.Endpoint).
+		Build()
 
 	err = FetchForCli(client, href, &records, args.DownloadAll, &curls)
 	if err != nil {

From 19d770a362b08bbdd611eda23ee4b46ba290e887 Mon Sep 17 00:00:00 2001
From: Rahul Gupta <rahul.gupta@netapp.com>
Date: Thu, 5 Oct 2023 19:26:05 +0530
Subject: [PATCH 2/9] refactor: update href creation

---
 cmd/tools/rest/HrefBuilder.go | 17 ++++-------------
 cmd/tools/rest/rest.go        |  2 --
 2 files changed, 4 insertions(+), 15 deletions(-)

diff --git a/cmd/tools/rest/HrefBuilder.go b/cmd/tools/rest/HrefBuilder.go
index 1b7446113..5387c9ddf 100644
--- a/cmd/tools/rest/HrefBuilder.go
+++ b/cmd/tools/rest/HrefBuilder.go
@@ -10,7 +10,6 @@ type HrefBuilder struct {
 	queryValue    string
 	maxRecords    string
 	returnTimeout string
-	endpoint      string
 }
 
 func NewHrefBuilder() *HrefBuilder {
@@ -52,21 +51,13 @@ func (b *HrefBuilder) ReturnTimeout(returnTimeout string) *HrefBuilder {
 	return b
 }
 
-func (b *HrefBuilder) Endpoint(endpoint string) *HrefBuilder {
-	b.endpoint = endpoint
-	return b
-}
-
 func (b *HrefBuilder) Build() string {
 	href := strings.Builder{}
-	if b.endpoint == "" {
-		if !strings.HasPrefix(b.apiPath, "api/") {
-			href.WriteString("api/")
-		}
-		href.WriteString(b.apiPath)
-	} else {
-		href.WriteString(b.endpoint)
+	if !strings.HasPrefix(b.apiPath, "api/") {
+		href.WriteString("api/")
 	}
+	href.WriteString(b.apiPath)
+
 	href.WriteString("?return_records=true")
 	addArg(&href, "&fields=", b.fields)
 	for _, f := range b.filter {
diff --git a/cmd/tools/rest/rest.go b/cmd/tools/rest/rest.go
index 95050c7be..413169ee5 100644
--- a/cmd/tools/rest/rest.go
+++ b/cmd/tools/rest/rest.go
@@ -192,7 +192,6 @@ func fetchData(poller *conf.Poller, timeout time.Duration) (*Results, error) {
 		QueryFields(args.QueryField).
 		QueryValue(args.QueryValue).
 		MaxRecords(args.MaxRecords).
-		Endpoint(args.Endpoint).
 		Build()
 
 	err = FetchForCli(client, href, &records, args.DownloadAll, &curls)
@@ -594,7 +593,6 @@ func init() {
 
 	showFlags := showCmd.Flags()
 	showFlags.StringVarP(&args.API, "api", "a", "", "REST API PATTERN to show")
-	showFlags.StringVar(&args.Endpoint, "endpoint", "", "By default, /api is appended to passed argument in --api. Use --endpoint instead to pass absolute path of url")
 	showFlags.BoolVar(&args.DownloadAll, "all", false, "Collect all records by walking pagination links")
 	showFlags.BoolVarP(&args.Verbose, "verbose", "v", false, "Be verbose")
 	showFlags.StringVarP(&args.MaxRecords, "max-records", "m", "", "Limit the number of records returned before providing pagination link")

From be78a13e635630aaa87c7a8cc6026d4711ced108 Mon Sep 17 00:00:00 2001
From: Rahul Gupta <rahul.gupta@netapp.com>
Date: Mon, 9 Oct 2023 16:37:53 +0530
Subject: [PATCH 3/9] feat: address review comments

---
 cmd/collectors/commonutils.go                 | 16 +++++++-----
 cmd/collectors/ems/ems.go                     | 13 +++++++---
 cmd/collectors/power.go                       |  2 +-
 .../rest/plugins/certificate/certificate.go   |  4 +--
 cmd/collectors/rest/plugins/health/health.go  | 19 +++++++-------
 .../plugins/ontaps3service/ontaps3service.go  |  2 +-
 cmd/collectors/rest/plugins/qtree/qtree.go    |  2 +-
 .../securityaccount/securityaccount.go        |  2 +-
 .../rest/plugins/snapmirror/snapmirror.go     |  2 +-
 cmd/collectors/rest/plugins/svm/svm.go        | 16 ++++++------
 cmd/collectors/rest/plugins/volume/volume.go  |  5 ++--
 .../volumeanalytics/volumeanalytics.go        | 12 ++++++---
 cmd/collectors/rest/rest.go                   |  8 +++---
 cmd/collectors/rest/templating.go             |  7 +++++-
 cmd/collectors/restperf/plugins/disk/disk.go  |  6 ++---
 cmd/collectors/restperf/plugins/fcvi/fcvi.go  |  3 +--
 .../restperf/plugins/volumetag/volumetag.go   |  2 +-
 cmd/collectors/restperf/restperf.go           |  6 ++---
 cmd/tools/rest/HrefBuilder.go                 | 25 ++++++++++++-------
 cmd/tools/rest/client.go                      |  2 +-
 cmd/tools/rest/rest.go                        |  6 ++---
 21 files changed, 92 insertions(+), 68 deletions(-)

diff --git a/cmd/collectors/commonutils.go b/cmd/collectors/commonutils.go
index 873ea502d..09a2a03d7 100644
--- a/cmd/collectors/commonutils.go
+++ b/cmd/collectors/commonutils.go
@@ -61,7 +61,7 @@ func InvokeRestCall(client *rest.Client, href string, logger *logging.Logger) ([
 	return result, nil
 }
 
-func GetClusterTime(client *rest.Client, returnTimeOut string, logger *logging.Logger) (time.Time, error) {
+func GetClusterTime(client *rest.Client, returnTimeOut *int, logger *logging.Logger) (time.Time, error) {
 	var (
 		err         error
 		records     []gjson.Result
@@ -72,12 +72,16 @@ func GetClusterTime(client *rest.Client, returnTimeOut string, logger *logging.L
 	query := "private/cli/cluster/date"
 	fields := []string{"date"}
 
-	href := rest.NewHrefBuilder().
+	hrefBuilder := rest.NewHrefBuilder().
 		APIPath(query).
-		Fields(strings.Join(fields, ",")).
-		MaxRecords("1").
-		ReturnTimeout(returnTimeOut).
-		Build()
+		Fields(fields).
+		MaxRecords(1)
+
+	if returnTimeOut != nil {
+		hrefBuilder.ReturnTimeout(returnTimeOut)
+	}
+
+	href := hrefBuilder.Build()
 
 	if records, err = rest.Fetch(client, href); err != nil {
 		return clusterTime, err
diff --git a/cmd/collectors/ems/ems.go b/cmd/collectors/ems/ems.go
index a11d7c74b..4ad136484 100644
--- a/cmd/collectors/ems/ems.go
+++ b/cmd/collectors/ems/ems.go
@@ -34,7 +34,7 @@ type Ems struct {
 	emsProp        map[string][]*emsProp // array is used here to handle same ems written with different ops, matches or exports. Example: arw.volume.state ems with op as disabled or dry-run
 	Filter         []string
 	Fields         []string
-	ReturnTimeOut  string
+	ReturnTimeOut  *int
 	lastFilterTime int64
 	maxURLSize     int
 	DefaultLabels  []string
@@ -187,7 +187,12 @@ func (e *Ems) InitCache() error {
 
 	// default value for ONTAP is 15 sec
 	if returnTimeout := e.Params.GetChildContentS("return_timeout"); returnTimeout != "" {
-		e.ReturnTimeOut = returnTimeout
+		iReturnTimeout, err := strconv.Atoi(returnTimeout)
+		if err != nil {
+			e.Logger.Warn().Str("returnTimeout", returnTimeout).Msg("Invalid value of returnTimeout")
+		} else {
+			e.ReturnTimeOut = &iReturnTimeout
+		}
 	}
 
 	// init plugins
@@ -285,7 +290,7 @@ func (e *Ems) PollInstance() (map[string]*matrix.Matrix, error) {
 
 	href := rest.NewHrefBuilder().
 		APIPath(query).
-		Fields(strings.Join(fields, ",")).
+		Fields(fields).
 		ReturnTimeout(e.ReturnTimeOut).
 		Build()
 
@@ -421,7 +426,7 @@ func (e *Ems) getHref(names []string, filter []string) string {
 
 	href := rest.NewHrefBuilder().
 		APIPath(e.Query).
-		Fields(strings.Join(e.Fields, ",")).
+		Fields(e.Fields).
 		Filter(filter).
 		ReturnTimeout(e.ReturnTimeOut).
 		Build()
diff --git a/cmd/collectors/power.go b/cmd/collectors/power.go
index c05e6daa0..bcbe9156b 100644
--- a/cmd/collectors/power.go
+++ b/cmd/collectors/power.go
@@ -23,7 +23,7 @@ const (
 // `system chassis fru show`.
 // Chassis FRU information is only available via private CLI
 func collectChassisFRU(client *rest.Client, logger *logging.Logger) (map[string]int, error) {
-	fields := "fru-name,type,status,connected-nodes,num-nodes"
+	fields := []string{"fru-name", "type", "status", "connected-nodes", "num-nodes"}
 	query := "api/private/cli/system/chassis/fru"
 	filter := []string{"type=psu"}
 	href := rest.NewHrefBuilder().
diff --git a/cmd/collectors/rest/plugins/certificate/certificate.go b/cmd/collectors/rest/plugins/certificate/certificate.go
index 004d0e508..2f7a769fc 100644
--- a/cmd/collectors/rest/plugins/certificate/certificate.go
+++ b/cmd/collectors/rest/plugins/certificate/certificate.go
@@ -188,7 +188,7 @@ func (my *Certificate) GetAdminVserver() (string, error) {
 	query := "api/private/cli/vserver"
 	href := rest.NewHrefBuilder().
 		APIPath(query).
-		Fields("type").
+		Fields([]string{"type"}).
 		Filter([]string{"type=admin"}).
 		Build()
 
@@ -214,7 +214,7 @@ func (my *Certificate) GetSecuritySsl(adminSvm string) (string, error) {
 	query := "api/private/cli/security/ssl"
 	href := rest.NewHrefBuilder().
 		APIPath(query).
-		Fields("serial").
+		Fields([]string{"serial"}).
 		Filter([]string{"vserver=" + adminSvm}).
 		Build()
 
diff --git a/cmd/collectors/rest/plugins/health/health.go b/cmd/collectors/rest/plugins/health/health.go
index b34e6bc63..5d9624f03 100644
--- a/cmd/collectors/rest/plugins/health/health.go
+++ b/cmd/collectors/rest/plugins/health/health.go
@@ -11,7 +11,6 @@ import (
 	"github.com/netapp/harvest/v2/pkg/matrix"
 	"github.com/tidwall/gjson"
 	"strconv"
-	"strings"
 	"time"
 )
 
@@ -446,7 +445,7 @@ func (h *Health) collectSupportAlerts() {
 	var (
 		instance *matrix.Instance
 	)
-	clusterTime, err := collectors.GetClusterTime(h.client, "", h.Logger)
+	clusterTime, err := collectors.GetClusterTime(h.client, nil, h.Logger)
 	if err != nil {
 		h.Logger.Error().Err(err).Msg("Failed to collect cluster time")
 		return
@@ -535,7 +534,7 @@ func (h *Health) getDisks() ([]gjson.Result, error) {
 	query := "api/storage/disks"
 	href := rest.NewHrefBuilder().
 		APIPath(query).
-		Fields(strings.Join(fields, ",")).
+		Fields(fields).
 		Filter([]string{"container_type=broken|unassigned"}).
 		Build()
 
@@ -555,7 +554,7 @@ func (h *Health) getShelves() ([]gjson.Result, error) {
 	query := "api/private/cli/storage/shelf"
 	href := rest.NewHrefBuilder().
 		APIPath(query).
-		Fields(strings.Join(fields, ",")).
+		Fields(fields).
 		Build()
 
 	if result, err = collectors.InvokeRestCall(h.client, href, h.Logger); err != nil {
@@ -574,7 +573,7 @@ func (h *Health) getNodes() ([]gjson.Result, error) {
 	query := "api/private/cli/node"
 	href := rest.NewHrefBuilder().
 		APIPath(query).
-		Fields(strings.Join(fields, ",")).
+		Fields(fields).
 		Filter([]string{"health=false"}).
 		Build()
 
@@ -612,7 +611,7 @@ func (h *Health) getNonCompliantLicense() ([]gjson.Result, error) {
 	fields := []string{"name,scope,state"}
 	href := rest.NewHrefBuilder().
 		APIPath(query).
-		Fields(strings.Join(fields, ",")).
+		Fields(fields).
 		Filter([]string{"state=noncompliant"}).
 		Build()
 
@@ -632,7 +631,7 @@ func (h *Health) getMoveFailedVolumes() ([]gjson.Result, error) {
 	fields := []string{"uuid,name,movement.state,svm"}
 	href := rest.NewHrefBuilder().
 		APIPath(query).
-		Fields(strings.Join(fields, ",")).
+		Fields(fields).
 		Filter([]string{"movement.state=cutover_wait|failed|cutover_pending"}).
 		Build()
 
@@ -651,7 +650,7 @@ func (h *Health) getNonHomeLIFs() ([]gjson.Result, error) {
 	query := "api/network/ip/interfaces"
 	href := rest.NewHrefBuilder().
 		APIPath(query).
-		Fields("svm,location").
+		Fields([]string{"svm", "location"}).
 		Filter([]string{"location.is_home=false"}).
 		Build()
 
@@ -671,7 +670,7 @@ func (h *Health) getFCPorts() ([]gjson.Result, error) {
 	query := "api/network/fc/ports"
 	href := rest.NewHrefBuilder().
 		APIPath(query).
-		Fields(strings.Join(fields, ",")).
+		Fields(fields).
 		Filter([]string{"enabled=true", "state=offlined_by_system"}).
 		Build()
 
@@ -691,7 +690,7 @@ func (h *Health) getEthernetPorts() ([]gjson.Result, error) {
 	query := "api/network/ethernet/ports"
 	href := rest.NewHrefBuilder().
 		APIPath(query).
-		Fields(strings.Join(fields, ",")).
+		Fields(fields).
 		Filter([]string{"enabled=true", "state=down"}).
 		Build()
 
diff --git a/cmd/collectors/rest/plugins/ontaps3service/ontaps3service.go b/cmd/collectors/rest/plugins/ontaps3service/ontaps3service.go
index a0fc3d4ea..85c280e5e 100644
--- a/cmd/collectors/rest/plugins/ontaps3service/ontaps3service.go
+++ b/cmd/collectors/rest/plugins/ontaps3service/ontaps3service.go
@@ -69,7 +69,7 @@ func (o *OntapS3Service) Run(dataMap map[string]*matrix.Matrix) ([]*matrix.Matri
 	fields := []string{"svm.name", "name", "is_http_enabled", "is_https_enabled", "secure_port", "port"}
 	href := rest.NewHrefBuilder().
 		APIPath(o.query).
-		Fields(strings.Join(fields, ",")).
+		Fields(fields).
 		Build()
 
 	if result, err = collectors.InvokeRestCall(o.client, href, o.Logger); err != nil {
diff --git a/cmd/collectors/rest/plugins/qtree/qtree.go b/cmd/collectors/rest/plugins/qtree/qtree.go
index eb7f38d98..aceb0633c 100644
--- a/cmd/collectors/rest/plugins/qtree/qtree.go
+++ b/cmd/collectors/rest/plugins/qtree/qtree.go
@@ -161,7 +161,7 @@ func (q *Qtree) Run(dataMap map[string]*matrix.Matrix) ([]*matrix.Matrix, error)
 
 	href := rest.NewHrefBuilder().
 		APIPath(q.query).
-		Fields("*").
+		Fields([]string{"*"}).
 		Filter(filter).
 		Build()
 
diff --git a/cmd/collectors/rest/plugins/securityaccount/securityaccount.go b/cmd/collectors/rest/plugins/securityaccount/securityaccount.go
index 22ac6230e..aefc0668c 100644
--- a/cmd/collectors/rest/plugins/securityaccount/securityaccount.go
+++ b/cmd/collectors/rest/plugins/securityaccount/securityaccount.go
@@ -63,7 +63,7 @@ func (s *SecurityAccount) Run(dataMap map[string]*matrix.Matrix) ([]*matrix.Matr
 	data := dataMap[s.Object]
 	href := rest.NewHrefBuilder().
 		APIPath(s.query).
-		Fields("applications").
+		Fields([]string{"applications"}).
 		Build()
 
 	if result, err = collectors.InvokeRestCall(s.client, href, s.Logger); err != nil {
diff --git a/cmd/collectors/rest/plugins/snapmirror/snapmirror.go b/cmd/collectors/rest/plugins/snapmirror/snapmirror.go
index db6ed9e44..bd3e307a7 100644
--- a/cmd/collectors/rest/plugins/snapmirror/snapmirror.go
+++ b/cmd/collectors/rest/plugins/snapmirror/snapmirror.go
@@ -113,7 +113,7 @@ func (my *SnapMirror) getSVMPeerData(cluster string) error {
 	query := "api/svm/peers"
 	href := rest.NewHrefBuilder().
 		APIPath(query).
-		Fields(strings.Join(fields, ",")).
+		Fields(fields).
 		Filter([]string{"peer.cluster.name=!" + cluster}).
 		Build()
 
diff --git a/cmd/collectors/rest/plugins/svm/svm.go b/cmd/collectors/rest/plugins/svm/svm.go
index 36eea8bf9..df197568f 100644
--- a/cmd/collectors/rest/plugins/svm/svm.go
+++ b/cmd/collectors/rest/plugins/svm/svm.go
@@ -208,10 +208,10 @@ func (my *SVM) GetKerberosConfig() (map[string]string, error) {
 
 	svmKerberosMap = make(map[string]string)
 	query := "api/protocols/nfs/kerberos/interfaces"
-	kerberosFields := []string{"svm.name", "enabled"}
+	fields := []string{"svm.name", "enabled"}
 	href := rest.NewHrefBuilder().
 		APIPath(query).
-		Fields(strings.Join(kerberosFields, ",")).
+		Fields(fields).
 		Build()
 
 	if result, err = collectors.InvokeRestCall(my.client, href, my.Logger); err != nil {
@@ -243,10 +243,10 @@ func (my *SVM) GetFpolicy() (map[string]Fpolicy, error) {
 
 	svmFpolicyMap = make(map[string]Fpolicy)
 	query := "api/protocols/fpolicy"
-	fpolicyFields := []string{"svm.name", "policies.enabled", "policies.name"}
+	fields := []string{"svm.name", "policies.enabled", "policies.name"}
 	href := rest.NewHrefBuilder().
 		APIPath(query).
-		Fields(strings.Join(fpolicyFields, ",")).
+		Fields(fields).
 		Build()
 
 	if result, err = collectors.InvokeRestCall(my.client, href, my.Logger); err != nil {
@@ -279,10 +279,10 @@ func (my *SVM) GetIscsiServices() (map[string]string, error) {
 
 	svmIscsiServiceMap = make(map[string]string)
 	query := "api/protocols/san/iscsi/services"
-	iscsiServiceFields := []string{"svm.name", "enabled"}
+	fields := []string{"svm.name", "enabled"}
 	href := rest.NewHrefBuilder().
 		APIPath(query).
-		Fields(strings.Join(iscsiServiceFields, ",")).
+		Fields(fields).
 		Build()
 
 	if result, err = collectors.InvokeRestCall(my.client, href, my.Logger); err != nil {
@@ -314,10 +314,10 @@ func (my *SVM) GetIscsiCredentials() (map[string]string, error) {
 
 	svmIscsiCredentialMap = make(map[string]string)
 	query := "api/protocols/san/iscsi/credentials"
-	iscsiCredentialFields := []string{"svm.name", "authentication_type"}
+	fields := []string{"svm.name", "authentication_type"}
 	href := rest.NewHrefBuilder().
 		APIPath(query).
-		Fields(strings.Join(iscsiCredentialFields, ",")).
+		Fields(fields).
 		Build()
 
 	if result, err = collectors.InvokeRestCall(my.client, href, my.Logger); err != nil {
diff --git a/cmd/collectors/rest/plugins/volume/volume.go b/cmd/collectors/rest/plugins/volume/volume.go
index fdae5af40..abcb54b26 100644
--- a/cmd/collectors/rest/plugins/volume/volume.go
+++ b/cmd/collectors/rest/plugins/volume/volume.go
@@ -14,7 +14,6 @@ import (
 	"github.com/netapp/harvest/v2/pkg/tree/node"
 	"github.com/tidwall/gjson"
 	"strconv"
-	"strings"
 	"time"
 )
 
@@ -188,11 +187,11 @@ func (my *Volume) getEncryptedDisks() ([]gjson.Result, error) {
 		err    error
 	)
 
-	diskFields := []string{"aggregates.name", "aggregates.uuid"}
+	fields := []string{"aggregates.name", "aggregates.uuid"}
 	query := "api/storage/disks"
 	href := rest.NewHrefBuilder().
 		APIPath(query).
-		Fields(strings.Join(diskFields, ",")).
+		Fields(fields).
 		Filter([]string{"protection_mode=!data|full"}).
 		Build()
 
diff --git a/cmd/collectors/rest/plugins/volumeanalytics/volumeanalytics.go b/cmd/collectors/rest/plugins/volumeanalytics/volumeanalytics.go
index 8f526e86b..c6fb31b1d 100644
--- a/cmd/collectors/rest/plugins/volumeanalytics/volumeanalytics.go
+++ b/cmd/collectors/rest/plugins/volumeanalytics/volumeanalytics.go
@@ -17,7 +17,7 @@ import (
 
 const explorer = "volume_analytics"
 
-var MaxDirCollectCount = "100"
+var MaxDirCollectCount = 100
 
 type VolumeAnalytics struct {
 	*plugin.AbstractPlugin
@@ -50,7 +50,13 @@ func (v *VolumeAnalytics) Init() error {
 
 	m := v.Params.GetChildS("MaxDirectoryCount")
 	if m != nil {
-		MaxDirCollectCount = m.GetContentS()
+		count := m.GetContentS()
+		i, err := strconv.Atoi(count)
+		if err != nil {
+			v.Logger.Warn().Str("MaxDirectoryCount", count).Msg("using default")
+		} else {
+			MaxDirCollectCount = i
+		}
 	}
 
 	timeout, _ := time.ParseDuration(rest.DefaultTimeout)
@@ -285,7 +291,7 @@ func (v *VolumeAnalytics) getAnalyticsData(instanceID string) ([]gjson.Result, g
 
 	href := rest.NewHrefBuilder().
 		APIPath(query).
-		Fields(strings.Join(fields, ",")).
+		Fields(fields).
 		Filter([]string{"order_by=analytics.bytes_used+desc", "type=directory"}).
 		MaxRecords(MaxDirCollectCount).
 		Build()
diff --git a/cmd/collectors/rest/rest.go b/cmd/collectors/rest/rest.go
index a6f8a1228..38376fd8b 100644
--- a/cmd/collectors/rest/rest.go
+++ b/cmd/collectors/rest/rest.go
@@ -54,7 +54,7 @@ type prop struct {
 	InstanceLabels map[string]string
 	Metrics        map[string]*Metric
 	Counters       map[string]string
-	ReturnTimeOut  string
+	ReturnTimeOut  *int
 	Fields         []string
 	APIType        string // public, private
 	Filter         []string
@@ -299,7 +299,7 @@ func (r *Rest) PollData() (map[string]*matrix.Matrix, error) {
 
 	href := rest.NewHrefBuilder().
 		APIPath(r.Prop.Query).
-		Fields(strings.Join(r.Prop.Fields, ",")).
+		Fields(r.Prop.Fields).
 		Filter(r.Prop.Filter).
 		ReturnTimeout(r.Prop.ReturnTimeOut).
 		Build()
@@ -348,7 +348,7 @@ func (r *Rest) pollData(startTime time.Time, records []gjson.Result, endpointFun
 func (r *Rest) processEndPoint(e *endPoint) ([]gjson.Result, error) {
 	href := rest.NewHrefBuilder().
 		APIPath(r.query(e)).
-		Fields(strings.Join(r.fields(e), ",")).
+		Fields(r.fields(e)).
 		Filter(r.filter(e)).
 		ReturnTimeout(r.Prop.ReturnTimeOut).
 		Build()
@@ -678,7 +678,7 @@ func (r *Rest) getNodeUuids() ([]collector.ID, error) {
 
 	href := rest.NewHrefBuilder().
 		APIPath(query).
-		Fields("serial_number,system_id").
+		Fields([]string{"serial_number", "system_id"}).
 		ReturnTimeout(r.Prop.ReturnTimeOut).
 		Build()
 
diff --git a/cmd/collectors/rest/templating.go b/cmd/collectors/rest/templating.go
index a29dc9242..8b8afe777 100644
--- a/cmd/collectors/rest/templating.go
+++ b/cmd/collectors/rest/templating.go
@@ -49,7 +49,12 @@ func (r *Rest) InitCache() error {
 
 	// default value for ONTAP is 15 sec
 	if returnTimeout := r.Params.GetChildContentS("return_timeout"); returnTimeout != "" {
-		r.Prop.ReturnTimeOut = returnTimeout
+		iReturnTimeout, err := strconv.Atoi(returnTimeout)
+		if err != nil {
+			r.Logger.Warn().Str("returnTimeout", returnTimeout).Msg("Invalid value of returnTimeout")
+		} else {
+			r.Prop.ReturnTimeOut = &iReturnTimeout
+		}
 	}
 
 	// private end point do not support * as fields. We need to pass fields in endpoint
diff --git a/cmd/collectors/restperf/plugins/disk/disk.go b/cmd/collectors/restperf/plugins/disk/disk.go
index fd7323eff..d828ec804 100644
--- a/cmd/collectors/restperf/plugins/disk/disk.go
+++ b/cmd/collectors/restperf/plugins/disk/disk.go
@@ -246,7 +246,7 @@ func (d *Disk) Run(dataMap map[string]*matrix.Matrix) ([]*matrix.Matrix, error)
 
 	href := rest.NewHrefBuilder().
 		APIPath(d.query).
-		Fields("*").
+		Fields([]string{"*"}).
 		Build()
 
 	records, err := rest.Fetch(d.client, href)
@@ -549,7 +549,7 @@ func (d *Disk) getDisks() error {
 
 	href := rest.NewHrefBuilder().
 		APIPath(query).
-		Fields("name,uid,shelf.uid,type,aggregates").
+		Fields([]string{"name", "uid", "shelf.uid", "type", "aggregates"}).
 		Build()
 
 	records, err := rest.Fetch(d.client, href)
@@ -607,7 +607,7 @@ func (d *Disk) getAggregates() error {
 
 	href := rest.NewHrefBuilder().
 		APIPath(query).
-		Fields("aggregate,composite,node,uses_shared_disks,storage_type").
+		Fields([]string{"aggregate", "composite", "node", "uses_shared_disks", "storage_type"}).
 		Build()
 
 	records, err := rest.Fetch(d.client, href)
diff --git a/cmd/collectors/restperf/plugins/fcvi/fcvi.go b/cmd/collectors/restperf/plugins/fcvi/fcvi.go
index 632a03b23..adbc242ca 100644
--- a/cmd/collectors/restperf/plugins/fcvi/fcvi.go
+++ b/cmd/collectors/restperf/plugins/fcvi/fcvi.go
@@ -5,7 +5,6 @@ import (
 	"github.com/netapp/harvest/v2/cmd/tools/rest"
 	"github.com/netapp/harvest/v2/pkg/conf"
 	"github.com/netapp/harvest/v2/pkg/matrix"
-	"strings"
 	"time"
 )
 
@@ -39,7 +38,7 @@ func (f *FCVI) Run(dataMap map[string]*matrix.Matrix) ([]*matrix.Matrix, error)
 	fields := []string{"node", "adapter", "port_name"}
 	href := rest.NewHrefBuilder().
 		APIPath(query).
-		Fields(strings.Join(fields, ",")).
+		Fields(fields).
 		Build()
 	records, err := rest.Fetch(f.client, href)
 	if err != nil {
diff --git a/cmd/collectors/restperf/plugins/volumetag/volumetag.go b/cmd/collectors/restperf/plugins/volumetag/volumetag.go
index a8ab88c8c..643712736 100644
--- a/cmd/collectors/restperf/plugins/volumetag/volumetag.go
+++ b/cmd/collectors/restperf/plugins/volumetag/volumetag.go
@@ -43,7 +43,7 @@ func (v *VolumeTag) Run(dataMap map[string]*matrix.Matrix) ([]*matrix.Matrix, er
 
 	href := rest.NewHrefBuilder().
 		APIPath(query).
-		Fields("comment").
+		Fields([]string{"comment"}).
 		Build()
 
 	records, err := rest.Fetch(v.client, href)
diff --git a/cmd/collectors/restperf/restperf.go b/cmd/collectors/restperf/restperf.go
index 3e67837e2..5f2e22b65 100644
--- a/cmd/collectors/restperf/restperf.go
+++ b/cmd/collectors/restperf/restperf.go
@@ -644,7 +644,7 @@ func (r *RestPerf) PollData() (map[string]*matrix.Matrix, error) {
 
 	href := rest.NewHrefBuilder().
 		APIPath(dataQuery).
-		Fields(strings.Join(r.Prop.Fields, ",")).
+		Fields(r.Prop.Fields).
 		ReturnTimeout(r.Prop.ReturnTimeOut).
 		Build()
 
@@ -1237,7 +1237,7 @@ func (r *RestPerf) getParentOpsCounters(data *matrix.Matrix) error {
 	filter = append(filter, "counters.name=ops")
 	href := rest.NewHrefBuilder().
 		APIPath(dataQuery).
-		Fields("*").
+		Fields([]string{"*"}).
 		Filter(filter).
 		ReturnTimeout(r.Prop.ReturnTimeOut).
 		Build()
@@ -1356,7 +1356,7 @@ func (r *RestPerf) PollInstance() (map[string]*matrix.Matrix, error) {
 
 	href := rest.NewHrefBuilder().
 		APIPath(dataQuery).
-		Fields(fields).
+		Fields([]string{fields}).
 		Filter(filter).
 		ReturnTimeout(r.Prop.ReturnTimeOut).
 		Build()
diff --git a/cmd/tools/rest/HrefBuilder.go b/cmd/tools/rest/HrefBuilder.go
index 5387c9ddf..8f10cc7d0 100644
--- a/cmd/tools/rest/HrefBuilder.go
+++ b/cmd/tools/rest/HrefBuilder.go
@@ -1,6 +1,9 @@
 package rest
 
-import "strings"
+import (
+	"strconv"
+	"strings"
+)
 
 type HrefBuilder struct {
 	apiPath       string
@@ -8,8 +11,8 @@ type HrefBuilder struct {
 	filter        []string
 	queryFields   string
 	queryValue    string
-	maxRecords    string
-	returnTimeout string
+	maxRecords    int
+	returnTimeout *int
 }
 
 func NewHrefBuilder() *HrefBuilder {
@@ -21,8 +24,8 @@ func (b *HrefBuilder) APIPath(apiPath string) *HrefBuilder {
 	return b
 }
 
-func (b *HrefBuilder) Fields(fields string) *HrefBuilder {
-	b.fields = fields
+func (b *HrefBuilder) Fields(fields []string) *HrefBuilder {
+	b.fields = strings.Join(fields, ",")
 	return b
 }
 
@@ -41,12 +44,12 @@ func (b *HrefBuilder) QueryValue(queryValue string) *HrefBuilder {
 	return b
 }
 
-func (b *HrefBuilder) MaxRecords(maxRecords string) *HrefBuilder {
+func (b *HrefBuilder) MaxRecords(maxRecords int) *HrefBuilder {
 	b.maxRecords = maxRecords
 	return b
 }
 
-func (b *HrefBuilder) ReturnTimeout(returnTimeout string) *HrefBuilder {
+func (b *HrefBuilder) ReturnTimeout(returnTimeout *int) *HrefBuilder {
 	b.returnTimeout = returnTimeout
 	return b
 }
@@ -65,8 +68,12 @@ func (b *HrefBuilder) Build() string {
 	}
 	addArg(&href, "&query_fields=", b.queryFields)
 	addArg(&href, "&query=", b.queryValue)
-	addArg(&href, "&max_records=", b.maxRecords)
-	addArg(&href, "&return_timeout=", b.returnTimeout)
+	if b.maxRecords > 0 {
+		addArg(&href, "&max_records=", strconv.Itoa(b.maxRecords))
+	}
+	if b.returnTimeout != nil {
+		addArg(&href, "&return_timeout=", strconv.Itoa(*b.returnTimeout))
+	}
 	return href.String()
 }
 
diff --git a/cmd/tools/rest/client.go b/cmd/tools/rest/client.go
index 89d20046b..1ef654580 100644
--- a/cmd/tools/rest/client.go
+++ b/cmd/tools/rest/client.go
@@ -294,7 +294,7 @@ func (c *Client) Init(retries int) error {
 	for i = 0; i < retries; i++ {
 		href := NewHrefBuilder().
 			APIPath("api/cluster").
-			Fields("*").
+			Fields([]string{"*"}).
 			Build()
 		content, err = c.GetRest(href)
 		if err != nil {
diff --git a/cmd/tools/rest/rest.go b/cmd/tools/rest/rest.go
index 413169ee5..fa1af83de 100644
--- a/cmd/tools/rest/rest.go
+++ b/cmd/tools/rest/rest.go
@@ -46,7 +46,7 @@ type Args struct {
 	QueryField    string
 	QueryValue    string
 	DownloadAll   bool
-	MaxRecords    string
+	MaxRecords    int
 	ForceDownload bool
 	Verbose       bool
 	Timeout       string
@@ -187,7 +187,7 @@ func fetchData(poller *conf.Poller, timeout time.Duration) (*Results, error) {
 
 	href := NewHrefBuilder().
 		APIPath(args.API).
-		Fields(args.Fields).
+		Fields(strings.Split(args.Fields, ",")).
 		Filter(args.Field).
 		QueryFields(args.QueryField).
 		QueryValue(args.QueryValue).
@@ -595,7 +595,7 @@ func init() {
 	showFlags.StringVarP(&args.API, "api", "a", "", "REST API PATTERN to show")
 	showFlags.BoolVar(&args.DownloadAll, "all", false, "Collect all records by walking pagination links")
 	showFlags.BoolVarP(&args.Verbose, "verbose", "v", false, "Be verbose")
-	showFlags.StringVarP(&args.MaxRecords, "max-records", "m", "", "Limit the number of records returned before providing pagination link")
+	showFlags.IntVarP(&args.MaxRecords, "max-records", "m", -1, "Limit the number of records returned before providing pagination link")
 	showFlags.BoolVar(&args.ForceDownload, "download", false, "Force download Swagger file instead of using local copy")
 	showFlags.StringVarP(&args.Fields, "fields", "f", "*", "Fields to return in the response <field>[,...].")
 	showFlags.StringArrayVar(&args.Field, "field", []string{}, "Query a field by value (can be specified multiple times.)\n"+

From ae7d5c8ebd1c186f3ab561e22ea1b93396aac874 Mon Sep 17 00:00:00 2001
From: Rahul Gupta <rahul.gupta@netapp.com>
Date: Mon, 9 Oct 2023 17:56:52 +0530
Subject: [PATCH 4/9] feat: address review comments

---
 cmd/collectors/commonutils.go                      | 14 +++++---------
 cmd/collectors/ems/ems.go                          |  4 ++--
 cmd/collectors/rest/plugins/health/health.go       |  2 +-
 cmd/collectors/rest/rest.go                        |  2 +-
 cmd/collectors/rest/templating.go                  |  2 +-
 cmd/tools/rest/{HrefBuilder.go => href_builder.go} |  8 ++++----
 6 files changed, 14 insertions(+), 18 deletions(-)
 rename cmd/tools/rest/{HrefBuilder.go => href_builder.go} (89%)

diff --git a/cmd/collectors/commonutils.go b/cmd/collectors/commonutils.go
index 09a2a03d7..31ad2f098 100644
--- a/cmd/collectors/commonutils.go
+++ b/cmd/collectors/commonutils.go
@@ -61,7 +61,7 @@ func InvokeRestCall(client *rest.Client, href string, logger *logging.Logger) ([
 	return result, nil
 }
 
-func GetClusterTime(client *rest.Client, returnTimeOut *int, logger *logging.Logger) (time.Time, error) {
+func GetClusterTime(client *rest.Client, returnTimeOut int, logger *logging.Logger) (time.Time, error) {
 	var (
 		err         error
 		records     []gjson.Result
@@ -72,16 +72,12 @@ func GetClusterTime(client *rest.Client, returnTimeOut *int, logger *logging.Log
 	query := "private/cli/cluster/date"
 	fields := []string{"date"}
 
-	hrefBuilder := rest.NewHrefBuilder().
+	href := rest.NewHrefBuilder().
 		APIPath(query).
 		Fields(fields).
-		MaxRecords(1)
-
-	if returnTimeOut != nil {
-		hrefBuilder.ReturnTimeout(returnTimeOut)
-	}
-
-	href := hrefBuilder.Build()
+		ReturnTimeout(returnTimeOut).
+		MaxRecords(1).
+		Build()
 
 	if records, err = rest.Fetch(client, href); err != nil {
 		return clusterTime, err
diff --git a/cmd/collectors/ems/ems.go b/cmd/collectors/ems/ems.go
index 4ad136484..fdbc9ee4a 100644
--- a/cmd/collectors/ems/ems.go
+++ b/cmd/collectors/ems/ems.go
@@ -34,7 +34,7 @@ type Ems struct {
 	emsProp        map[string][]*emsProp // array is used here to handle same ems written with different ops, matches or exports. Example: arw.volume.state ems with op as disabled or dry-run
 	Filter         []string
 	Fields         []string
-	ReturnTimeOut  *int
+	ReturnTimeOut  int
 	lastFilterTime int64
 	maxURLSize     int
 	DefaultLabels  []string
@@ -191,7 +191,7 @@ func (e *Ems) InitCache() error {
 		if err != nil {
 			e.Logger.Warn().Str("returnTimeout", returnTimeout).Msg("Invalid value of returnTimeout")
 		} else {
-			e.ReturnTimeOut = &iReturnTimeout
+			e.ReturnTimeOut = iReturnTimeout
 		}
 	}
 
diff --git a/cmd/collectors/rest/plugins/health/health.go b/cmd/collectors/rest/plugins/health/health.go
index 5d9624f03..612deb121 100644
--- a/cmd/collectors/rest/plugins/health/health.go
+++ b/cmd/collectors/rest/plugins/health/health.go
@@ -445,7 +445,7 @@ func (h *Health) collectSupportAlerts() {
 	var (
 		instance *matrix.Instance
 	)
-	clusterTime, err := collectors.GetClusterTime(h.client, nil, h.Logger)
+	clusterTime, err := collectors.GetClusterTime(h.client, -1, h.Logger)
 	if err != nil {
 		h.Logger.Error().Err(err).Msg("Failed to collect cluster time")
 		return
diff --git a/cmd/collectors/rest/rest.go b/cmd/collectors/rest/rest.go
index 38376fd8b..d94607e30 100644
--- a/cmd/collectors/rest/rest.go
+++ b/cmd/collectors/rest/rest.go
@@ -54,7 +54,7 @@ type prop struct {
 	InstanceLabels map[string]string
 	Metrics        map[string]*Metric
 	Counters       map[string]string
-	ReturnTimeOut  *int
+	ReturnTimeOut  int
 	Fields         []string
 	APIType        string // public, private
 	Filter         []string
diff --git a/cmd/collectors/rest/templating.go b/cmd/collectors/rest/templating.go
index 8b8afe777..676efe560 100644
--- a/cmd/collectors/rest/templating.go
+++ b/cmd/collectors/rest/templating.go
@@ -53,7 +53,7 @@ func (r *Rest) InitCache() error {
 		if err != nil {
 			r.Logger.Warn().Str("returnTimeout", returnTimeout).Msg("Invalid value of returnTimeout")
 		} else {
-			r.Prop.ReturnTimeOut = &iReturnTimeout
+			r.Prop.ReturnTimeOut = iReturnTimeout
 		}
 	}
 
diff --git a/cmd/tools/rest/HrefBuilder.go b/cmd/tools/rest/href_builder.go
similarity index 89%
rename from cmd/tools/rest/HrefBuilder.go
rename to cmd/tools/rest/href_builder.go
index 8f10cc7d0..e9973a8f0 100644
--- a/cmd/tools/rest/HrefBuilder.go
+++ b/cmd/tools/rest/href_builder.go
@@ -12,7 +12,7 @@ type HrefBuilder struct {
 	queryFields   string
 	queryValue    string
 	maxRecords    int
-	returnTimeout *int
+	returnTimeout int
 }
 
 func NewHrefBuilder() *HrefBuilder {
@@ -49,7 +49,7 @@ func (b *HrefBuilder) MaxRecords(maxRecords int) *HrefBuilder {
 	return b
 }
 
-func (b *HrefBuilder) ReturnTimeout(returnTimeout *int) *HrefBuilder {
+func (b *HrefBuilder) ReturnTimeout(returnTimeout int) *HrefBuilder {
 	b.returnTimeout = returnTimeout
 	return b
 }
@@ -71,8 +71,8 @@ func (b *HrefBuilder) Build() string {
 	if b.maxRecords > 0 {
 		addArg(&href, "&max_records=", strconv.Itoa(b.maxRecords))
 	}
-	if b.returnTimeout != nil {
-		addArg(&href, "&return_timeout=", strconv.Itoa(*b.returnTimeout))
+	if b.returnTimeout > 0 {
+		addArg(&href, "&return_timeout=", strconv.Itoa(b.returnTimeout))
 	}
 	return href.String()
 }

From bc1a913a187d4ac608daba08c441466a7498be84 Mon Sep 17 00:00:00 2001
From: Rahul Gupta <rahul.gupta@netapp.com>
Date: Mon, 9 Oct 2023 19:28:15 +0530
Subject: [PATCH 5/9] Revert "feat: address review comments"

This reverts commit ae7d5c8ebd1c186f3ab561e22ea1b93396aac874.
---
 cmd/collectors/commonutils.go                      | 14 +++++++++-----
 cmd/collectors/ems/ems.go                          |  4 ++--
 cmd/collectors/rest/plugins/health/health.go       |  2 +-
 cmd/collectors/rest/rest.go                        |  2 +-
 cmd/collectors/rest/templating.go                  |  2 +-
 cmd/tools/rest/{href_builder.go => HrefBuilder.go} |  8 ++++----
 6 files changed, 18 insertions(+), 14 deletions(-)
 rename cmd/tools/rest/{href_builder.go => HrefBuilder.go} (89%)

diff --git a/cmd/collectors/commonutils.go b/cmd/collectors/commonutils.go
index 31ad2f098..09a2a03d7 100644
--- a/cmd/collectors/commonutils.go
+++ b/cmd/collectors/commonutils.go
@@ -61,7 +61,7 @@ func InvokeRestCall(client *rest.Client, href string, logger *logging.Logger) ([
 	return result, nil
 }
 
-func GetClusterTime(client *rest.Client, returnTimeOut int, logger *logging.Logger) (time.Time, error) {
+func GetClusterTime(client *rest.Client, returnTimeOut *int, logger *logging.Logger) (time.Time, error) {
 	var (
 		err         error
 		records     []gjson.Result
@@ -72,12 +72,16 @@ func GetClusterTime(client *rest.Client, returnTimeOut int, logger *logging.Logg
 	query := "private/cli/cluster/date"
 	fields := []string{"date"}
 
-	href := rest.NewHrefBuilder().
+	hrefBuilder := rest.NewHrefBuilder().
 		APIPath(query).
 		Fields(fields).
-		ReturnTimeout(returnTimeOut).
-		MaxRecords(1).
-		Build()
+		MaxRecords(1)
+
+	if returnTimeOut != nil {
+		hrefBuilder.ReturnTimeout(returnTimeOut)
+	}
+
+	href := hrefBuilder.Build()
 
 	if records, err = rest.Fetch(client, href); err != nil {
 		return clusterTime, err
diff --git a/cmd/collectors/ems/ems.go b/cmd/collectors/ems/ems.go
index fdbc9ee4a..4ad136484 100644
--- a/cmd/collectors/ems/ems.go
+++ b/cmd/collectors/ems/ems.go
@@ -34,7 +34,7 @@ type Ems struct {
 	emsProp        map[string][]*emsProp // array is used here to handle same ems written with different ops, matches or exports. Example: arw.volume.state ems with op as disabled or dry-run
 	Filter         []string
 	Fields         []string
-	ReturnTimeOut  int
+	ReturnTimeOut  *int
 	lastFilterTime int64
 	maxURLSize     int
 	DefaultLabels  []string
@@ -191,7 +191,7 @@ func (e *Ems) InitCache() error {
 		if err != nil {
 			e.Logger.Warn().Str("returnTimeout", returnTimeout).Msg("Invalid value of returnTimeout")
 		} else {
-			e.ReturnTimeOut = iReturnTimeout
+			e.ReturnTimeOut = &iReturnTimeout
 		}
 	}
 
diff --git a/cmd/collectors/rest/plugins/health/health.go b/cmd/collectors/rest/plugins/health/health.go
index 612deb121..5d9624f03 100644
--- a/cmd/collectors/rest/plugins/health/health.go
+++ b/cmd/collectors/rest/plugins/health/health.go
@@ -445,7 +445,7 @@ func (h *Health) collectSupportAlerts() {
 	var (
 		instance *matrix.Instance
 	)
-	clusterTime, err := collectors.GetClusterTime(h.client, -1, h.Logger)
+	clusterTime, err := collectors.GetClusterTime(h.client, nil, h.Logger)
 	if err != nil {
 		h.Logger.Error().Err(err).Msg("Failed to collect cluster time")
 		return
diff --git a/cmd/collectors/rest/rest.go b/cmd/collectors/rest/rest.go
index d94607e30..38376fd8b 100644
--- a/cmd/collectors/rest/rest.go
+++ b/cmd/collectors/rest/rest.go
@@ -54,7 +54,7 @@ type prop struct {
 	InstanceLabels map[string]string
 	Metrics        map[string]*Metric
 	Counters       map[string]string
-	ReturnTimeOut  int
+	ReturnTimeOut  *int
 	Fields         []string
 	APIType        string // public, private
 	Filter         []string
diff --git a/cmd/collectors/rest/templating.go b/cmd/collectors/rest/templating.go
index 676efe560..8b8afe777 100644
--- a/cmd/collectors/rest/templating.go
+++ b/cmd/collectors/rest/templating.go
@@ -53,7 +53,7 @@ func (r *Rest) InitCache() error {
 		if err != nil {
 			r.Logger.Warn().Str("returnTimeout", returnTimeout).Msg("Invalid value of returnTimeout")
 		} else {
-			r.Prop.ReturnTimeOut = iReturnTimeout
+			r.Prop.ReturnTimeOut = &iReturnTimeout
 		}
 	}
 
diff --git a/cmd/tools/rest/href_builder.go b/cmd/tools/rest/HrefBuilder.go
similarity index 89%
rename from cmd/tools/rest/href_builder.go
rename to cmd/tools/rest/HrefBuilder.go
index e9973a8f0..8f10cc7d0 100644
--- a/cmd/tools/rest/href_builder.go
+++ b/cmd/tools/rest/HrefBuilder.go
@@ -12,7 +12,7 @@ type HrefBuilder struct {
 	queryFields   string
 	queryValue    string
 	maxRecords    int
-	returnTimeout int
+	returnTimeout *int
 }
 
 func NewHrefBuilder() *HrefBuilder {
@@ -49,7 +49,7 @@ func (b *HrefBuilder) MaxRecords(maxRecords int) *HrefBuilder {
 	return b
 }
 
-func (b *HrefBuilder) ReturnTimeout(returnTimeout int) *HrefBuilder {
+func (b *HrefBuilder) ReturnTimeout(returnTimeout *int) *HrefBuilder {
 	b.returnTimeout = returnTimeout
 	return b
 }
@@ -71,8 +71,8 @@ func (b *HrefBuilder) Build() string {
 	if b.maxRecords > 0 {
 		addArg(&href, "&max_records=", strconv.Itoa(b.maxRecords))
 	}
-	if b.returnTimeout > 0 {
-		addArg(&href, "&return_timeout=", strconv.Itoa(b.returnTimeout))
+	if b.returnTimeout != nil {
+		addArg(&href, "&return_timeout=", strconv.Itoa(*b.returnTimeout))
 	}
 	return href.String()
 }

From 11eb76ff9826c9c336b1e2309b3856f01272cb59 Mon Sep 17 00:00:00 2001
From: Rahul Gupta <rahul.gupta@netapp.com>
Date: Mon, 9 Oct 2023 19:28:50 +0530
Subject: [PATCH 6/9] feat: address review comments

---
 cmd/tools/rest/{HrefBuilder.go => href_builder.go} | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename cmd/tools/rest/{HrefBuilder.go => href_builder.go} (100%)

diff --git a/cmd/tools/rest/HrefBuilder.go b/cmd/tools/rest/href_builder.go
similarity index 100%
rename from cmd/tools/rest/HrefBuilder.go
rename to cmd/tools/rest/href_builder.go

From 772e3955dc107184f8bcaec8954a776561765644 Mon Sep 17 00:00:00 2001
From: Rahul Gupta <rahul.gupta@netapp.com>
Date: Mon, 9 Oct 2023 19:31:58 +0530
Subject: [PATCH 7/9] feat: address review comments

---
 cmd/collectors/commonutils.go | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/cmd/collectors/commonutils.go b/cmd/collectors/commonutils.go
index 09a2a03d7..58bee452b 100644
--- a/cmd/collectors/commonutils.go
+++ b/cmd/collectors/commonutils.go
@@ -72,16 +72,12 @@ func GetClusterTime(client *rest.Client, returnTimeOut *int, logger *logging.Log
 	query := "private/cli/cluster/date"
 	fields := []string{"date"}
 
-	hrefBuilder := rest.NewHrefBuilder().
+	href := rest.NewHrefBuilder().
 		APIPath(query).
 		Fields(fields).
-		MaxRecords(1)
-
-	if returnTimeOut != nil {
-		hrefBuilder.ReturnTimeout(returnTimeOut)
-	}
-
-	href := hrefBuilder.Build()
+		MaxRecords(1).
+		ReturnTimeout(returnTimeOut).
+		Build()
 
 	if records, err = rest.Fetch(client, href); err != nil {
 		return clusterTime, err

From c23cdb5f188e8bab8dceb561fd474798b3bf63fb Mon Sep 17 00:00:00 2001
From: Rahul Gupta <rahul.gupta@netapp.com>
Date: Mon, 9 Oct 2023 20:31:31 +0530
Subject: [PATCH 8/9] feat: update maxRecords to *

---
 cmd/collectors/commonutils.go                         |  4 +++-
 .../rest/plugins/volumeanalytics/volumeanalytics.go   |  2 +-
 cmd/tools/rest/href_builder.go                        |  8 ++++----
 cmd/tools/rest/rest.go                                | 11 ++++++++---
 4 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/cmd/collectors/commonutils.go b/cmd/collectors/commonutils.go
index 58bee452b..ce1abdb5f 100644
--- a/cmd/collectors/commonutils.go
+++ b/cmd/collectors/commonutils.go
@@ -72,10 +72,12 @@ func GetClusterTime(client *rest.Client, returnTimeOut *int, logger *logging.Log
 	query := "private/cli/cluster/date"
 	fields := []string{"date"}
 
+	maxRecords := 1
+
 	href := rest.NewHrefBuilder().
 		APIPath(query).
 		Fields(fields).
-		MaxRecords(1).
+		MaxRecords(&maxRecords).
 		ReturnTimeout(returnTimeOut).
 		Build()
 
diff --git a/cmd/collectors/rest/plugins/volumeanalytics/volumeanalytics.go b/cmd/collectors/rest/plugins/volumeanalytics/volumeanalytics.go
index c6fb31b1d..eacdb8aaa 100644
--- a/cmd/collectors/rest/plugins/volumeanalytics/volumeanalytics.go
+++ b/cmd/collectors/rest/plugins/volumeanalytics/volumeanalytics.go
@@ -293,7 +293,7 @@ func (v *VolumeAnalytics) getAnalyticsData(instanceID string) ([]gjson.Result, g
 		APIPath(query).
 		Fields(fields).
 		Filter([]string{"order_by=analytics.bytes_used+desc", "type=directory"}).
-		MaxRecords(MaxDirCollectCount).
+		MaxRecords(&MaxDirCollectCount).
 		Build()
 	if result, analytics, err = rest.FetchAnalytics(v.client, href); err != nil {
 		return nil, gjson.Result{}, err
diff --git a/cmd/tools/rest/href_builder.go b/cmd/tools/rest/href_builder.go
index 8f10cc7d0..662025315 100644
--- a/cmd/tools/rest/href_builder.go
+++ b/cmd/tools/rest/href_builder.go
@@ -11,7 +11,7 @@ type HrefBuilder struct {
 	filter        []string
 	queryFields   string
 	queryValue    string
-	maxRecords    int
+	maxRecords    *int
 	returnTimeout *int
 }
 
@@ -44,7 +44,7 @@ func (b *HrefBuilder) QueryValue(queryValue string) *HrefBuilder {
 	return b
 }
 
-func (b *HrefBuilder) MaxRecords(maxRecords int) *HrefBuilder {
+func (b *HrefBuilder) MaxRecords(maxRecords *int) *HrefBuilder {
 	b.maxRecords = maxRecords
 	return b
 }
@@ -68,8 +68,8 @@ func (b *HrefBuilder) Build() string {
 	}
 	addArg(&href, "&query_fields=", b.queryFields)
 	addArg(&href, "&query=", b.queryValue)
-	if b.maxRecords > 0 {
-		addArg(&href, "&max_records=", strconv.Itoa(b.maxRecords))
+	if b.maxRecords != nil {
+		addArg(&href, "&max_records=", strconv.Itoa(*b.maxRecords))
 	}
 	if b.returnTimeout != nil {
 		addArg(&href, "&return_timeout=", strconv.Itoa(*b.returnTimeout))
diff --git a/cmd/tools/rest/rest.go b/cmd/tools/rest/rest.go
index fa1af83de..d78c4c438 100644
--- a/cmd/tools/rest/rest.go
+++ b/cmd/tools/rest/rest.go
@@ -46,7 +46,7 @@ type Args struct {
 	QueryField    string
 	QueryValue    string
 	DownloadAll   bool
-	MaxRecords    int
+	MaxRecords    *int
 	ForceDownload bool
 	Verbose       bool
 	Timeout       string
@@ -107,7 +107,7 @@ func ReadOrDownloadSwagger(pName string) (string, error) {
 	return swaggerPath, nil
 }
 
-func doShow(_ *cobra.Command, a []string) {
+func doShow(cmd *cobra.Command, a []string) {
 	c := validateArgs(a)
 	if !c.isValid {
 		return
@@ -119,6 +119,9 @@ func doShow(_ *cobra.Command, a []string) {
 	if args.SwaggerPath != "" {
 		doSwagger(*args)
 	} else {
+		if !cmd.Flags().Changed("max-records") {
+			args.MaxRecords = nil
+		}
 		doCmd()
 	}
 }
@@ -595,7 +598,9 @@ func init() {
 	showFlags.StringVarP(&args.API, "api", "a", "", "REST API PATTERN to show")
 	showFlags.BoolVar(&args.DownloadAll, "all", false, "Collect all records by walking pagination links")
 	showFlags.BoolVarP(&args.Verbose, "verbose", "v", false, "Be verbose")
-	showFlags.IntVarP(&args.MaxRecords, "max-records", "m", -1, "Limit the number of records returned before providing pagination link")
+	var maxRecords int
+	args.MaxRecords = &maxRecords
+	showFlags.IntVarP(args.MaxRecords, "max-records", "m", 0, "Limit the number of records returned before providing pagination link")
 	showFlags.BoolVar(&args.ForceDownload, "download", false, "Force download Swagger file instead of using local copy")
 	showFlags.StringVarP(&args.Fields, "fields", "f", "*", "Fields to return in the response <field>[,...].")
 	showFlags.StringArrayVar(&args.Field, "field", []string{}, "Query a field by value (can be specified multiple times.)\n"+

From e8dd4719f6e868d1a7a342926054122f583f86eb Mon Sep 17 00:00:00 2001
From: Rahul Gupta <rahul.gupta@netapp.com>
Date: Mon, 9 Oct 2023 21:26:41 +0530
Subject: [PATCH 9/9] feat: change to str

---
 cmd/tools/rest/rest.go | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/cmd/tools/rest/rest.go b/cmd/tools/rest/rest.go
index d78c4c438..87b921313 100644
--- a/cmd/tools/rest/rest.go
+++ b/cmd/tools/rest/rest.go
@@ -46,7 +46,7 @@ type Args struct {
 	QueryField    string
 	QueryValue    string
 	DownloadAll   bool
-	MaxRecords    *int
+	MaxRecords    string
 	ForceDownload bool
 	Verbose       bool
 	Timeout       string
@@ -107,7 +107,7 @@ func ReadOrDownloadSwagger(pName string) (string, error) {
 	return swaggerPath, nil
 }
 
-func doShow(cmd *cobra.Command, a []string) {
+func doShow(_ *cobra.Command, a []string) {
 	c := validateArgs(a)
 	if !c.isValid {
 		return
@@ -119,9 +119,6 @@ func doShow(cmd *cobra.Command, a []string) {
 	if args.SwaggerPath != "" {
 		doSwagger(*args)
 	} else {
-		if !cmd.Flags().Changed("max-records") {
-			args.MaxRecords = nil
-		}
 		doCmd()
 	}
 }
@@ -188,14 +185,22 @@ func fetchData(poller *conf.Poller, timeout time.Duration) (*Results, error) {
 	var records []any
 	var curls []string
 
-	href := NewHrefBuilder().
+	hrefBuilder := NewHrefBuilder().
 		APIPath(args.API).
 		Fields(strings.Split(args.Fields, ",")).
 		Filter(args.Field).
 		QueryFields(args.QueryField).
-		QueryValue(args.QueryValue).
-		MaxRecords(args.MaxRecords).
-		Build()
+		QueryValue(args.QueryValue)
+
+	if args.MaxRecords != "" {
+		maxRecords, err := strconv.Atoi(args.MaxRecords)
+		if err != nil {
+			return nil, fmt.Errorf("--max-records should be numeric %s", args.MaxRecords)
+		}
+		hrefBuilder.MaxRecords(&maxRecords)
+	}
+
+	href := hrefBuilder.Build()
 
 	err = FetchForCli(client, href, &records, args.DownloadAll, &curls)
 	if err != nil {
@@ -598,9 +603,7 @@ func init() {
 	showFlags.StringVarP(&args.API, "api", "a", "", "REST API PATTERN to show")
 	showFlags.BoolVar(&args.DownloadAll, "all", false, "Collect all records by walking pagination links")
 	showFlags.BoolVarP(&args.Verbose, "verbose", "v", false, "Be verbose")
-	var maxRecords int
-	args.MaxRecords = &maxRecords
-	showFlags.IntVarP(args.MaxRecords, "max-records", "m", 0, "Limit the number of records returned before providing pagination link")
+	showFlags.StringVarP(&args.MaxRecords, "max-records", "m", "", "Limit the number of records returned before providing pagination link")
 	showFlags.BoolVar(&args.ForceDownload, "download", false, "Force download Swagger file instead of using local copy")
 	showFlags.StringVarP(&args.Fields, "fields", "f", "*", "Fields to return in the response <field>[,...].")
 	showFlags.StringArrayVar(&args.Field, "field", []string{}, "Query a field by value (can be specified multiple times.)\n"+