[VOL-3803] : Provide a global flag for enable/disable metrics collection at startup
- Provide a global flag for enable/disable metrics collection at startup
- Minor fixes in metrics collection code.

Change-Id: I80f89aa3416e94fc0cd64d54ba4d885139b623d1
diff --git a/PM_Notes.md b/PM_Notes.md
index ecabc17..e0a6ba5 100644
--- a/PM_Notes.md
+++ b/PM_Notes.md
@@ -146,12 +146,27 @@
 
 Note2: default sample rate is applicable only if frequency override is false
 
+Note3 : The `frequency` has to be greater than 0 and a multiple of _FrequencyGranularity_ which is currently set to 5.
+
 ### Disable a group metric
 
 ```
 voltctl device pmconfig group disable <onu-device-id> <group-name>
 ```
 
+### Enable a group metric
+
+```
+voltctl device pmconfig group enable <onu-device-id> <group-name>
+```
+
+### Set group frequency
+
+```
+voltctl device pmconfig group set <frequency> <onu-device-id> <group-name>
+```
+Note1 : The `frequency` has to be greater than 0 and a multiple of _FrequencyGranularity_ which is currently set to 5.
+
 ### Listen for KPI events on kafka
 
 ```
diff --git a/VERSION b/VERSION
index 8869ce2..ba0e451 100644
--- a/VERSION
+++ b/VERSION
@@ -1 +1 @@
-1.1.1-dev
+1.1.1-dev159
diff --git a/internal/pkg/config/config.go b/internal/pkg/config/config.go
index 59b8da6..9ceca40 100644
--- a/internal/pkg/config/config.go
+++ b/internal/pkg/config/config.go
@@ -63,6 +63,8 @@
 	defaultTraceEnabled          = false
 	defaultTraceAgentAddress     = "127.0.0.1:6831"
 	defaultLogCorrelationEnabled = true
+
+	defaultMetricsEnabled = false
 )
 
 // AdapterFlags represents the set of configurations used by the read-write adaptercore service
@@ -99,6 +101,7 @@
 	TraceAgentAddress           string
 	LogCorrelationEnabled       bool
 	OnuVendorIds                string
+	MetricsEnabled              bool
 }
 
 // NewAdapterFlags returns a new RWCore config
@@ -135,6 +138,7 @@
 		TraceAgentAddress:           defaultTraceAgentAddress,
 		LogCorrelationEnabled:       defaultLogCorrelationEnabled,
 		OnuVendorIds:                defaultOnuVendorIds,
+		MetricsEnabled:              defaultMetricsEnabled,
 	}
 	return &adapterFlags
 }
@@ -233,6 +237,9 @@
 	help = fmt.Sprintf("List of Allowed ONU Vendor Ids")
 	flag.StringVar(&(so.OnuVendorIds), "allowed_onu_vendors", defaultOnuVendorIds, help)
 
+	help = fmt.Sprintf("Whether to enable metrics collection")
+	flag.BoolVar(&(so.MetricsEnabled), "metrics_enabled", defaultMetricsEnabled, help)
+
 	flag.Parse()
 	containerName := getContainerInfo()
 	if len(containerName) > 0 {
diff --git a/internal/pkg/onuadaptercore/device_handler.go b/internal/pkg/onuadaptercore/device_handler.go
index 7405f20..bc8a185 100644
--- a/internal/pkg/onuadaptercore/device_handler.go
+++ b/internal/pkg/onuadaptercore/device_handler.go
@@ -256,7 +256,8 @@
 			logger.Errorw(ctx, "Device FSM: Can't go to state DeviceInit", log.Fields{"err": err})
 		}
 		logger.Debugw(ctx, "Device FSM: ", log.Fields{"state": string(dh.pDeviceStateFsm.Current())})
-		if device.PmConfigs == nil { // device.PmConfigs is not nil in cases when adapter restarts. We should not re-set the core again.
+		// device.PmConfigs is not nil in cases when adapter restarts. We should not re-set the core again.
+		if device.PmConfigs == nil {
 			// Now, set the initial PM configuration for that device
 			if err := dh.coreProxy.DevicePMConfigUpdate(ctx, dh.pmConfigs); err != nil {
 				logger.Errorw(ctx, "error updating pm config to core", log.Fields{"device-id": dh.deviceID, "err": err})
@@ -1542,7 +1543,8 @@
 			}
 		}
 	}
-	// Stop collector routine for PM Counters
+
+	// Stop collector routine
 	dh.stopCollector <- true
 
 	return nil
@@ -2587,7 +2589,7 @@
 	// Check if standalone metric related config is updated
 	for _, v := range pmConfigs.Metrics {
 		dh.pOnuMetricsMgr.onuMetricsManagerLock.RLock()
-		m, ok := dh.pOnuMetricsMgr.groupMetricMap[v.Name]
+		m, ok := dh.pOnuMetricsMgr.standaloneMetricMap[v.Name]
 		dh.pOnuMetricsMgr.onuMetricsManagerLock.RUnlock()
 
 		if ok && m.frequency != v.SampleFreq {
@@ -2625,9 +2627,9 @@
 				// If the current time is eqaul to or greater than the nextGlobalMetricCollectionTime, collect the group and standalone metrics
 				if time.Now().Equal(dh.pOnuMetricsMgr.nextGlobalMetricCollectionTime) || time.Now().After(dh.pOnuMetricsMgr.nextGlobalMetricCollectionTime) {
 					go dh.pOnuMetricsMgr.collectAllGroupAndStandaloneMetrics(ctx)
+					// Update the next metric collection time.
+					dh.pOnuMetricsMgr.nextGlobalMetricCollectionTime = time.Now().Add(time.Duration(dh.pmConfigs.DefaultFreq) * time.Second)
 				}
-				// Update the next metric collection time.
-				dh.pOnuMetricsMgr.nextGlobalMetricCollectionTime = time.Now().Add(time.Duration(dh.pmConfigs.DefaultFreq) * time.Second)
 			} else {
 				if dh.pmConfigs.Grouped { // metrics are managed as a group
 					// parse through the group and standalone metrics to see it is time to collect their metrics
diff --git a/internal/pkg/onuadaptercore/onu_metrics_manager.go b/internal/pkg/onuadaptercore/onu_metrics_manager.go
index 4316186..68c628c 100644
--- a/internal/pkg/onuadaptercore/onu_metrics_manager.go
+++ b/internal/pkg/onuadaptercore/onu_metrics_manager.go
@@ -135,7 +135,7 @@
 		}
 		opticalPowerGroupMetric := voltha.PmGroupConfig{
 			GroupName: OpticalPowerGroupMetricName,
-			Enabled:   OpticalPowerGroupMetricEnabled,
+			Enabled:   OpticalPowerGroupMetricEnabled && dh.pOpenOnuAc.metricsEnabled,
 			GroupFreq: OpticalPowerMetricGroupCollectionFrequency,
 			Metrics:   opPmConfigSlice,
 		}
@@ -148,7 +148,7 @@
 		}
 		uniStatusGroupMetric := voltha.PmGroupConfig{
 			GroupName: UniStatusGroupMetricName,
-			Enabled:   UniStatusGroupMetricEnabled,
+			Enabled:   UniStatusGroupMetricEnabled && dh.pOpenOnuAc.metricsEnabled,
 			GroupFreq: UniStatusMetricGroupCollectionFrequency,
 			Metrics:   uniStPmConfigSlice,
 		}
@@ -220,7 +220,7 @@
 
 func (mm *onuMetricsManager) updateDefaultFrequency(ctx context.Context, pmConfigs *voltha.PmConfigs) error {
 	// Verify that the configured DefaultFrequency is > 0 and is a multiple of FrequencyGranularity
-	if pmConfigs.DefaultFreq == 0 && pmConfigs.DefaultFreq%FrequencyGranularity != 0 {
+	if pmConfigs.DefaultFreq == 0 || (pmConfigs.DefaultFreq > 0 && pmConfigs.DefaultFreq%FrequencyGranularity != 0) {
 		logger.Errorf(ctx, "frequency-%u-should-be-a-multiple-of-%u", pmConfigs.DefaultFreq, FrequencyGranularity)
 		return fmt.Errorf("frequency-%d-should-be-a-multiple-of-%d", pmConfigs.DefaultFreq, FrequencyGranularity)
 	}
@@ -238,11 +238,14 @@
 	var group *voltha.PmGroupConfig
 	for groupSliceIdx, group = range pmConfigs.Groups {
 		if group.GroupName == aGroupName {
-			if group.GroupFreq != 0 { // freq 0 not allowed
-				newGroupFreq = group.GroupFreq
-				found = true
-				break
+			// freq 0 is not allowed and it should be multiple of FrequencyGranularity
+			if group.GroupFreq == 0 || (group.GroupFreq > 0 && group.GroupFreq%FrequencyGranularity != 0) {
+				logger.Errorf(ctx, "frequency-%u-should-be-a-multiple-of-%u", group.GroupFreq, FrequencyGranularity)
+				return fmt.Errorf("frequency-%d-should-be-a-multiple-of-%d", group.GroupFreq, FrequencyGranularity)
 			}
+			newGroupFreq = group.GroupFreq
+			found = true
+			break
 		}
 	}
 	// if not found update group freq and next collection interval for the group
@@ -255,7 +258,7 @@
 	mm.onuMetricsManagerLock.Lock()
 	defer mm.onuMetricsManagerLock.Unlock()
 	for k, v := range mm.groupMetricMap {
-		if k == aGroupName && newGroupFreq != 0 { // freq 0 not allowed
+		if k == aGroupName {
 			v.frequency = newGroupFreq
 			// update internal pm config
 			mm.pDeviceHandler.pmConfigs.Groups[groupSliceIdx].GroupFreq = newGroupFreq
@@ -279,11 +282,14 @@
 	var metric *voltha.PmConfig
 	for metricSliceIdx, metric = range pmConfigs.Metrics {
 		if metric.Name == aMetricName {
-			if metric.SampleFreq != 0 { // freq 0 not allowed
-				newMetricFreq = metric.SampleFreq
-				found = true
-				break
+			// freq 0 is not allowed and it should be multiple of FrequencyGranularity
+			if metric.SampleFreq == 0 || (metric.SampleFreq > 0 && metric.SampleFreq%FrequencyGranularity != 0) {
+				logger.Errorf(ctx, "frequency-%u-should-be-a-multiple-of-%u", metric.SampleFreq, FrequencyGranularity)
+				return fmt.Errorf("frequency-%d-should-be-a-multiple-of-%d", metric.SampleFreq, FrequencyGranularity)
 			}
+			newMetricFreq = metric.SampleFreq
+			found = true
+			break
 		}
 	}
 	if !found {
@@ -295,7 +301,7 @@
 	mm.onuMetricsManagerLock.Lock()
 	defer mm.onuMetricsManagerLock.Unlock()
 	for k, v := range mm.groupMetricMap {
-		if k == aMetricName && newMetricFreq != 0 {
+		if k == aMetricName {
 			v.frequency = newMetricFreq
 			// update internal pm config
 			mm.pDeviceHandler.pmConfigs.Metrics[metricSliceIdx].SampleFreq = newMetricFreq
diff --git a/internal/pkg/onuadaptercore/openonu.go b/internal/pkg/onuadaptercore/openonu.go
index 2bfb3c9..e933173 100644
--- a/internal/pkg/onuadaptercore/openonu.go
+++ b/internal/pkg/onuadaptercore/openonu.go
@@ -68,6 +68,7 @@
 	pSupportedFsms             *OmciDeviceFsms
 	maxTimeoutInterAdapterComm time.Duration
 	pDownloadManager           *adapterDownloadManager
+	metricsEnabled             bool
 }
 
 //NewOpenONUAC returns a new instance of OpenONU_AC
@@ -98,6 +99,7 @@
 	openOnuAc.AcceptIncrementalEvto = cfg.AccIncrEvto
 	openOnuAc.maxTimeoutInterAdapterComm = cfg.MaxTimeoutInterAdapterComm
 	//openOnuAc.GrpcTimeoutInterval = cfg.GrpcTimeoutInterval
+	openOnuAc.metricsEnabled = cfg.MetricsEnabled
 
 	openOnuAc.pSupportedFsms = &OmciDeviceFsms{
 		"mib-synchronizer": {