diff --git a/internal/metrics/exporter_test.go b/internal/metrics/exporter_test.go index 52faae96..11343237 100644 --- a/internal/metrics/exporter_test.go +++ b/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 diff --git a/internal/metrics/metrics.go b/internal/metrics/metrics.go index 7284ea76..46c24663 100644 --- a/internal/metrics/metrics.go +++ b/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) diff --git a/internal/metrics/update.go b/internal/metrics/update.go index 58f739f2..043382c0 100644 --- a/internal/metrics/update.go +++ b/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 diff --git a/internal/metrics/update_test.go b/internal/metrics/update_test.go index d2bb65a2..af38905b 100644 --- a/internal/metrics/update_test.go +++ b/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 TestInitialiseMetrics_DuplicateKeys(t *testing.T) { + + teardownTestCase := setupTestCase(true) + defer teardownTestCase() + + _, err := initialiseMetrics(getTestLogger()) + + if err == nil { + t.Error("Expected duplicate keys error") + } +} + func TestUpdateMetrics(t *testing.T) { - teardownTestCase := setupTestCase() + teardownTestCase := setupTestCase(false) defer teardownTestCase() - metrics := initialiseMetrics() + 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 +}