Add check for duplicate metric keys (#106)

* Remove vet of test code

* Add check for duplicate metric keys

internal/metrics/exporter_test.go

@@ -25,7 +25,7 @@ import (
 
 func TestDescribe(t *testing.T) {
 
-	teardownTestCase := setupTestCase()
+	teardownTestCase := setupTestCase(false)
 	defer teardownTestCase()
 
 	ch := make(chan *prometheus.Desc)
@@ -39,7 +39,7 @@ func TestDescribe(t *testing.T) {
 		t.Errorf("Received unexpected collect request")
 	}
 
-	metrics := initialiseMetrics()
+	metrics, _ := initialiseMetrics(getTestLogger())
 	responseChannel <- metrics
 
 	select {
@@ -56,7 +56,7 @@ func TestDescribe(t *testing.T) {
 
 func TestCollect(t *testing.T) {
 
-	teardownTestCase := setupTestCase()
+	teardownTestCase := setupTestCase(false)
 	defer teardownTestCase()
 
 	exporter := newExporter("qmName")
@@ -75,8 +75,8 @@ func TestCollect(t *testing.T) {
 			t.Errorf("Received unexpected describe request")
 		}
 
-		populateTestMetrics(i)
-		metrics := initialiseMetrics()
+		populateTestMetrics(i, false)
+		metrics, _ := initialiseMetrics(getTestLogger())
 		updateMetrics(metrics)
 		responseChannel <- metrics
 

internal/metrics/metrics.go

@@ -59,6 +59,8 @@ func startMetricsGathering(qmName string, log *logger.Logger) error {
 		}
 	}()
 
+	log.Println("Starting metrics gathering")
+
 	// Start processing metrics
 	wg.Add(1)
 	go processMetrics(log, qmName, &wg)

internal/metrics/update.go

@@ -81,11 +81,12 @@ func processMetrics(log *logger.Logger, qmName string, wg *sync.WaitGroup) {
 				first = false
 				wg.Done()
 			}
-			metrics = initialiseMetrics()
+			metrics, _ = initialiseMetrics(log)
 		}
 
-		// now loop until something goes wrong
+		// Now loop until something goes wrong
 		for err == nil {
+
 			// Process publications of metric data
 			err = mqmetric.ProcessPublications()
 
@@ -100,19 +101,21 @@ func processMetrics(log *logger.Logger, qmName string, wg *sync.WaitGroup) {
 				log.Debugf("Metrics: No requests received within timeout period (%d seconds)", requestTimeout)
 			}
 		}
+		log.Errorf("Metrics Error: %s", err.Error())
 
 		// Close the connection
 		mqmetric.EndConnection()
 
-		//If we're told to keep runnign sleep for a bit before trying again
+		// If we're told to keep running sleep for a bit before trying again
 		time.Sleep(10 * time.Second)
 	}
 }
 
 // initialiseMetrics sets initial details for all available metrics
-func initialiseMetrics() map[string]*metricData {
+func initialiseMetrics(log *logger.Logger) (map[string]*metricData, error) {
 
 	metrics := make(map[string]*metricData)
+	validMetrics := true
 
 	for _, metricClass := range mqmetric.Metrics.Classes {
 		for _, metricType := range metricClass.Types {
@@ -123,12 +126,21 @@ func initialiseMetrics() map[string]*metricData {
 						description: metricElement.Description,
 					}
 					key := makeKey(metricElement)
-					metrics[key] = &metric
+					if _, exists := metrics[key]; !exists {
+						metrics[key] = &metric
+					} else {
+						log.Errorf("Metrics Error: Found duplicate metric key %s", key)
+						validMetrics = false
+					}
 				}
 			}
 		}
 	}
-	return metrics
+
+	if !validMetrics {
+		return metrics, fmt.Errorf("Invalid metrics data - found duplicate metric keys")
+	}
+	return metrics, nil
 }
 
 // updateMetrics updates values for all available metrics

internal/metrics/update_test.go

@@ -16,19 +16,24 @@ limitations under the License.
 package metrics
 
 import (
+	"os"
 	"testing"
 
+	"github.com/ibm-messaging/mq-container/internal/logger"
 	"github.com/ibm-messaging/mq-golang/mqmetric"
 )
 
 func TestInitialiseMetrics(t *testing.T) {
 
-	teardownTestCase := setupTestCase()
+	teardownTestCase := setupTestCase(false)
 	defer teardownTestCase()
 
-	metrics := initialiseMetrics()
+	metrics, err := initialiseMetrics(getTestLogger())
 	metric, ok := metrics["ClassName/Type1Name/Element1Name"]
 
+	if err != nil {
+		t.Errorf("Unexpected error %s", err.Error())
+	}
 	if !ok {
 		t.Error("Expected metric not found in map")
 	} else {
@@ -55,12 +60,24 @@ func TestInitialiseMetrics(t *testing.T) {
 	}
 }
 
-func TestUpdateMetrics(t *testing.T) {
+func TestInitialiseMetrics_DuplicateKeys(t *testing.T) {
 
-	teardownTestCase := setupTestCase()
+	teardownTestCase := setupTestCase(true)
 	defer teardownTestCase()
 
-	metrics := initialiseMetrics()
+	_, err := initialiseMetrics(getTestLogger())
+
+	if err == nil {
+		t.Error("Expected duplicate keys error")
+	}
+}
+
+func TestUpdateMetrics(t *testing.T) {
+
+	teardownTestCase := setupTestCase(false)
+	defer teardownTestCase()
+
+	metrics, _ := initialiseMetrics(getTestLogger())
 	updateMetrics(metrics)
 
 	metric, _ := metrics["ClassName/Type1Name/Element1Name"]
@@ -90,7 +107,7 @@ func TestUpdateMetrics(t *testing.T) {
 
 func TestMakeKey(t *testing.T) {
 
-	teardownTestCase := setupTestCase()
+	teardownTestCase := setupTestCase(false)
 	defer teardownTestCase()
 
 	expected := "ClassName/Type1Name/Element1Name"
@@ -100,14 +117,14 @@ func TestMakeKey(t *testing.T) {
 	}
 }
 
-func setupTestCase() func() {
-	populateTestMetrics(1)
+func setupTestCase(duplicateKey bool) func() {
+	populateTestMetrics(1, duplicateKey)
 	return func() {
 		cleanTestMetrics()
 	}
 }
 
-func populateTestMetrics(testValue int) {
+func populateTestMetrics(testValue int, duplicateKey bool) {
 
 	metricClass := new(mqmetric.MonClass)
 	metricType1 := new(mqmetric.MonType)
@@ -135,6 +152,9 @@ func populateTestMetrics(testValue int) {
 	metricType1.Elements = make(map[int]*mqmetric.MonElement)
 	metricType2.Elements = make(map[int]*mqmetric.MonElement)
 	metricType1.Elements[0] = metricElement1
+	if duplicateKey {
+		metricType1.Elements[1] = metricElement1
+	}
 	metricType2.Elements[0] = metricElement2
 	metricClass.Types = make(map[int]*mqmetric.MonType)
 	metricClass.Types[0] = metricType1
@@ -146,3 +166,8 @@ func populateTestMetrics(testValue int) {
 func cleanTestMetrics() {
 	mqmetric.Metrics.Classes = make(map[int]*mqmetric.MonClass)
 }
+
+func getTestLogger() *logger.Logger {
+	log, _ := logger.NewLogger(os.Stdout, false, false, "test")
+	return log
+}