Allow prometheus namespace and subsystem to be empty (#115)

Co-authored-by: Phil Godzin <pgodzin@jwplayer.com>
prebid · Sep 26, 2022 · 0f2ccc2 · 0f2ccc2
1 parent 3f7ff1e
commit 0f2ccc2
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 30 deletions.
diff --git a/config/config.go b/config/config.go
@@ -293,19 +293,12 @@ type PrometheusMetrics struct {
 	Enabled          bool   `mapstructure:"enabled"`
 }
 
+// validateAndLog will error out when the value of port is 0
 func (promMetricsConfig *PrometheusMetrics) validateAndLog() {
-	// validate
 	if promMetricsConfig.Port == 0 {
 		log.Fatalf(`Despite being enabled, prometheus metrics came with an empty port number: config.metrics.prometheus.port = 0`)
 	}
-	if promMetricsConfig.Namespace == "" {
-		log.Fatalf(`Despite being enabled, prometheus metrics came with an empty name space: config.metrics.prometheus.namespace = %s.`, promMetricsConfig.Namespace)
-	}
-	if promMetricsConfig.Subsystem == "" {
-		log.Fatalf(`Despite being enabled, prometheus metrics came with an empty subsystem value: config.metrics.prometheus.subsystem = \"\".`)
-	}
 
-	// log
 	log.Infof("config.metrics.prometheus.namespace: %s", promMetricsConfig.Namespace)
 	log.Infof("config.metrics.prometheus.subsystem: %s", promMetricsConfig.Subsystem)
 	log.Infof("config.metrics.prometheus.port: %d", promMetricsConfig.Port)

diff --git a/config/config_test.go b/config/config_test.go
@@ -645,13 +645,12 @@ func TestPrometheusValidateAndLog(t *testing.T) {
 	}
 	testCases := []aTest{
 		{
-			description: "[1] Port invalid, Namespace valid, Subsystem valid. Expect error",
+			description: "Port invalid, both Namespace and Subsystem were set. Expect error",
 			prometheusConfig: &PrometheusMetrics{
 				Port:      0,
 				Namespace: "prebid",
 				Subsystem: "cache",
 			},
-			//out
 			expectError: true,
 			expectedLogInfo: []logComponents{
 				{
@@ -673,19 +672,14 @@ func TestPrometheusValidateAndLog(t *testing.T) {
 			},
 		},
 		{
-			description: "[2] Port valid, Namespace invalid, Subsystem valid. Expect error",
+			description: "Port valid, Namespace empty, Subsystem set. Don't expect error",
 			prometheusConfig: &PrometheusMetrics{
 				Port:      8080,
 				Namespace: "",
 				Subsystem: "cache",
 			},
-			//out
-			expectError: true,
+			expectError: false,
 			expectedLogInfo: []logComponents{
-				{
-					msg: `Despite being enabled, prometheus metrics came with an empty name space: config.metrics.prometheus.namespace = .`,
-					lvl: logrus.FatalLevel,
-				},
 				{
 					msg: "config.metrics.prometheus.namespace: ",
 					lvl: logrus.InfoLevel,
@@ -701,19 +695,14 @@ func TestPrometheusValidateAndLog(t *testing.T) {
 			},
 		},
 		{
-			description: "[3] Port valid, Namespace valid, Subsystem invalid. Expect error",
+			description: "Port valid, Namespace set, Subsystem empty. Don't expect error",
 			prometheusConfig: &PrometheusMetrics{
 				Port:      8080,
 				Namespace: "prebid",
 				Subsystem: "",
 			},
-			//out
-			expectError: true,
+			expectError: false,
 			expectedLogInfo: []logComponents{
-				{
-					msg: `Despite being enabled, prometheus metrics came with an empty subsystem value: config.metrics.prometheus.subsystem = \"\".`,
-					lvl: logrus.FatalLevel,
-				},
 				{
 					msg: "config.metrics.prometheus.namespace: prebid",
 					lvl: logrus.InfoLevel,
@@ -729,13 +718,12 @@ func TestPrometheusValidateAndLog(t *testing.T) {
 			},
 		},
 		{
-			description: "[4] Port valid, Namespace valid, Subsystem valid. Expect elements in log",
+			description: "Port valid, both Namespace and Subsystem set. Expect elements in log",
 			prometheusConfig: &PrometheusMetrics{
 				Port:      8080,
 				Namespace: "prebid",
 				Subsystem: "cache",
 			},
-			//out
 			expectError: false,
 			expectedLogInfo: []logComponents{
 				{
@@ -752,6 +740,29 @@ func TestPrometheusValidateAndLog(t *testing.T) {
 				},
 			},
 		},
+		{
+			description: "Port valid, Namespace and Subsystem empty. Expect log messages with blank Namespace and Subsystem",
+			prometheusConfig: &PrometheusMetrics{
+				Port:      8080,
+				Namespace: "",
+				Subsystem: "",
+			},
+			expectError: false,
+			expectedLogInfo: []logComponents{
+				{
+					msg: "config.metrics.prometheus.namespace: ",
+					lvl: logrus.InfoLevel,
+				},
+				{
+					msg: "config.metrics.prometheus.subsystem: ",
+					lvl: logrus.InfoLevel,
+				},
+				{
+					msg: "config.metrics.prometheus.port: 8080",
+					lvl: logrus.InfoLevel,
+				},
+			},
+		},
 	}
 
 	// logrus entries will be recorded to this `hook` object so we can compare and assert them
@@ -762,18 +773,18 @@ func TestPrometheusValidateAndLog(t *testing.T) {
 	var fatal bool
 	logrus.StandardLogger().ExitFunc = func(int) { fatal = true }
 
-	for j, tc := range testCases {
+	for _, tc := range testCases {
 		// Reset the fatal flag to false every test
 		fatal = false
 
 		//run test
 		tc.prometheusConfig.validateAndLog()
 
 		// Assert logrus expected entries
-		if assert.Equal(t, len(tc.expectedLogInfo), len(hook.Entries), "Incorrect number of entries were logged to logrus in test %d: len(tc.expectedLogInfo) = %d len(hook.Entries) = %d", j, len(tc.expectedLogInfo), len(hook.Entries)) {
+		if assert.Equal(t, len(tc.expectedLogInfo), len(hook.Entries), "Incorrect number of entries were logged to logrus in test %s.", tc.description) {
 			for i := 0; i < len(tc.expectedLogInfo); i++ {
 				assert.Equal(t, tc.expectedLogInfo[i].msg, hook.Entries[i].Message)
-				assert.Equal(t, tc.expectedLogInfo[i].lvl, hook.Entries[i].Level, "Expected Info entry in log")
+				assert.Equal(t, tc.expectedLogInfo[i].lvl, hook.Entries[i].Level, "Expected Info entry in log. Test %s.", tc.description)
 			}
 		} else {
 			return

diff --git a/metrics/prometheus/prometheus.go b/metrics/prometheus/prometheus.go
@@ -161,7 +161,14 @@ func CreatePrometheusMetrics(cfg config.PrometheusMetrics) *PrometheusMetrics {
 	// Should be the equivalent of the following influx collectors
 	// go metrics.CaptureRuntimeMemStats(m.Registry, flushTime)
 	// go metrics.CaptureDebugGCStats(m.Registry, flushTime)
-	collectorNamespace := fmt.Sprintf("%s_%s", cfg.Namespace, cfg.Subsystem)
+	collectorNamespace := ""
+	if len(cfg.Namespace) > 0 {
+		collectorNamespace += fmt.Sprintf("%s_", cfg.Namespace)
+	}
+	if len(cfg.Subsystem) > 0 {
+		collectorNamespace += fmt.Sprintf("%s", cfg.Subsystem)
+	}
+
 	promMetrics.Registry.MustRegister(
 		prometheus.NewProcessCollector(prometheus.ProcessCollectorOpts{Namespace: collectorNamespace}),
 	)