fix: prevent potential misconfiguration of metricsInterval (#284) (#638)

# Description

I have added a new field `metricsIntervalDuration` of type
`time.Duration`. It will super cede the `metricsInterval` configuration
of the same type. The older key is not removed to keep backward
compatibility. The change is done to prevent potential misconfiguration
that can happen by assigning a duration to `metricsInterval` when it is
converted to seconds.

## Related Issue

Issue #284 

## Checklist

- [X] I have read the [contributing
documentation](https://retina.sh/docs/contributing).
- [X] I signed and signed-off the commits (`git commit -S -s ...`). See
[this
documentation](https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification)
on signing commits.
- [X] I have correctly attributed the author(s) of the code.
- [X] I have tested the changes locally.
- [X] I have followed the project's style guidelines.
- [X] I have updated the documentation, if necessary.
- [X] I have added tests, if applicable.

## Screenshots (if applicable) or Testing Completed

The logs showing that retina agent runs successffully after making the
changes.

![retina-after-chnage](https://github.com/user-attachments/assets/b4b1c643-6ebe-421f-9d32-f0f3d5dac8f6)

This shows the log line that `metricsInterval` should not be used.

![retina-using-metrics-interval](https://github.com/user-attachments/assets/9345ac4e-339e-4818-9571-324c38124abd)


## Additional Notes

I was not able to us the `zap` logger as it is probably not initialized
when the `GetConfig` is called or I might be missing how to use it so i
used the go `log` library. Do let me know if we need to change that.

---

Please refer to the [CONTRIBUTING.md](../CONTRIBUTING.md) file for more
information on how to contribute to this project.

Signed-off-by: Ritwik Ranjan <[email protected]>

Author: ritwikranjan

deploy/hubble/manifests/controller/helm/retina/templates/agent/configmap.yaml

@@ -105,6 +105,7 @@ data:
     logLevel: {{ .Values.logLevel }}
     enabledPlugin: {{ .Values.enabledPlugin_linux }}
     metricsInterval: {{ .Values.metricsInterval }}
+    metricsIntervalDuration: {{ .Values.metricsIntervalDuration }}
     enableTelemetry: {{ .Values.enableTelemetry }}
     enablePodLevel: {{ .Values.enablePodLevel }}
     remoteContext: {{ .Values.remoteContext }}
@@ -128,6 +129,7 @@ data:
     logLevel: {{ .Values.logLevel }}
     enabledPlugin: {{ .Values.enabledPlugin_win }}
     metricsInterval: {{ .Values.metricsInterval }}
+    metricsIntervalDuration: {{ .Values.metricsIntervalDuration }}
     enableTelemetry: {{ .Values.enableTelemetry }}
     enablePodLevel: {{ .Values.enablePodLevel }}
     remoteContext: {{ .Values.remoteContext }}

deploy/hubble/manifests/controller/helm/retina/values.yaml

@@ -86,8 +86,8 @@ enabledPlugin_win: '["hnsstats"]'
 
 enableTelemetry: true
 
-# Interval, in seconds, to scrape/publish metrics.
-metricsInterval: 10
+# Interval, in duration, to scrape/publish metrics.
+metricsIntervalDuration: "10s"
 
 azure:
   appinsights:

deploy/legacy/manifests/controller/helm/retina/templates/configmap.yaml

@@ -16,6 +16,7 @@ data:
     logLevel: {{ .Values.logLevel }}
     enabledPlugin: {{ .Values.enabledPlugin_linux }}
     metricsInterval: {{ .Values.metricsInterval }}
+    metricsIntervalDuration: {{ .Values.metricsIntervalDuration }}
     enableTelemetry: {{ .Values.enableTelemetry }}
     enablePodLevel: {{ .Values.enablePodLevel }}
     remoteContext: {{ .Values.remoteContext }}
@@ -42,6 +43,7 @@ data:
     logLevel: {{ .Values.logLevel }}
     enabledPlugin: {{ .Values.enabledPlugin_win }}
     metricsInterval: {{ .Values.metricsInterval }}
+    metricsIntervalDuration: {{ .Values.metricsIntervalDuration }}
     enableTelemetry: {{ .Values.enableTelemetry }}
     enablePodLevel: {{ .Values.enablePodLevel }}
     remoteContext: {{ .Values.remoteContext }}

deploy/legacy/manifests/controller/helm/retina/values.yaml

@@ -66,8 +66,8 @@ enabledPlugin_win: '["hnsstats"]'
 
 enableTelemetry: false
 
-# Interval, in seconds, to scrape/publish metrics.
-metricsInterval: 10
+# Interval, in duration, to scrape/publish metrics.
+metricsIntervalDuration: "10s"
 
 azure:
   appinsights:
@@ -191,3 +191,4 @@ metrics:
     ## @param metrics.podMonitor.relabelings [array] Prometheus relabeling rules
     ##
     relabelings: []
+

docs/02-Installation/03-Config.md

@@ -12,7 +12,8 @@ Defaults are specified for each component in *deploy/legacy/manifests/controller
 * `enableAnnotations`: When this toggle is set to true, retina will gather metrics for the annotated resources. Namespaces or Pods can be annotated with `retina.sh/v1alpha=observe`. The operator and enableRetinaEndpoint for the operator should be enabled.
 * `enabledPlugin_linux`: Array of enabled plugins for linux.
 * `enabledPlugin_win`: Array of enabled plugins for windows.
-* `metricsInterval`: the interval for which metrics will be gathered.
+* `metricsInterval`: the interval for which metrics will be gathered (in seconds). (@deprecated, use metricsIntervalDuration instead)
+* `metricsIntervalDuration`: the interval for which metrics will be gathered (in duration)
 * `dataAggregationLevel`: This config defines the level of data aggregation for Retina. See [Data Aggregation](../05-Concepts/data-aggregation.md) for more details.
 
 ## Operator Config

docs/06-Troubleshooting/basic-metrics.md

@@ -56,7 +56,7 @@ data:
       port: 10093
     logLevel: info
     enabledPlugin: ["dropreason","packetforward","linuxutil"]
-    metricsInterval: 10
+    metricsIntervalDuration: "10s"
     enableTelemetry: false
 kind: ConfigMap
 metadata:

pkg/config/config.go

@@ -4,6 +4,7 @@ package config
 
 import (
 	"fmt"
+	"log"
 	"reflect"
 	"strings"
 	"time"
@@ -51,10 +52,12 @@ type Server struct {
 }
 
 type Config struct {
-	APIServer                Server        `yaml:"apiServer"`
-	LogLevel                 string        `yaml:"logLevel"`
-	EnabledPlugin            []string      `yaml:"enabledPlugin"`
-	MetricsInterval          time.Duration `yaml:"metricsInterval"`
+	APIServer       Server        `yaml:"apiServer"`
+	LogLevel        string        `yaml:"logLevel"`
+	EnabledPlugin   []string      `yaml:"enabledPlugin"`
+	MetricsInterval time.Duration `yaml:"metricsInterval"`
+	// Deprecated: Use only MetricsInterval instead in the go code.
+	MetricsIntervalDuration  time.Duration `yaml:"metricsIntervalDuration"`
 	EnableTelemetry          bool          `yaml:"enableTelemetry"`
 	EnableRetinaEndpoint     bool          `yaml:"enableRetinaEndpoint"`
 	EnablePodLevel           bool          `yaml:"enablePodLevel"`
@@ -95,8 +98,13 @@ func GetConfig(cfgFilename string) (*Config, error) {
 	if err != nil {
 		return nil, fmt.Errorf("fatal error config file: %s", err)
 	}
-	// Convert to second.
-	config.MetricsInterval = config.MetricsInterval * time.Second
+
+	if config.MetricsIntervalDuration != 0 {
+		config.MetricsInterval = config.MetricsIntervalDuration
+	} else if config.MetricsInterval != 0 {
+		config.MetricsInterval *= time.Second
+		log.Print("metricsInterval is deprecated, please use metricsIntervalDuration instead")
+	}
 
 	return &config, nil
 }

pkg/config/testwith/config-mock.yaml

@@ -5,6 +5,6 @@ apiServer:
 logLevel: info
 enabledPlugin: ["mockplugin"]
 # Interval, in seconds, to scrape/publish metrics.
-metricsInterval: 10
+metricsIntervalDuration: "10s"
 # used to export telemetry to AppInsights
 telemetryEnabled: true

pkg/config/testwith/config-win.yaml

@@ -5,6 +5,6 @@ apiServer:
 logLevel: info
 enabledPlugin: ["hnsstats"]
 # Interval, in seconds, to scrape/publish metrics.
-metricsInterval: 10
+metricsIntervalDuration: "10s"
 # used to export telemetry to AppInsights
 telemetryEnabled: true

pkg/config/testwith/config.yaml

@@ -5,7 +5,7 @@ apiServer:
 logLevel: info
 enabledPlugin: ["dropreason", "packetforward", "linuxutil"]
 # Interval, in seconds, to scrape/publish metrics.
-metricsInterval: 10
+metricsIntervalDuration: "10s"
 # used to export telemetry to AppInsights
 telemetryEnabled: true
 dataAggregationLevel: "low"

windows/manifests/windows.yaml

@@ -67,4 +67,4 @@ data:
       port: 10093
     logLevel: info
     enabledPlugin: ["hnsstats"] 
-    metricsInterval: 10
+    metricsIntervalDuration: "10s"