From 3482256b6942d57705a522eef0d3b4c887a3434b Mon Sep 17 00:00:00 2001 From: Stephen Marshall Date: Fri, 1 Jun 2018 09:58:12 +0100 Subject: [PATCH 1/2] Remove vet of test code --- Dockerfile-server | 3 +-- Makefile | 6 +++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/Dockerfile-server b/Dockerfile-server index 0bb95671..60017f45 100644 --- a/Dockerfile-server +++ b/Dockerfile-server @@ -22,7 +22,6 @@ WORKDIR /go/src/github.com/ibm-messaging/mq-container/ COPY cmd/ ./cmd COPY internal/ ./internal COPY vendor/ ./vendor -COPY test/ ./test RUN go build ./cmd/runmqserver/ RUN go build ./cmd/chkmqready/ RUN go build ./cmd/chkmqhealthy/ @@ -31,7 +30,7 @@ RUN go test -v ./cmd/runmqserver/ RUN go test -v ./cmd/chkmqready/ RUN go test -v ./cmd/chkmqhealthy/ RUN go test -v ./internal/... -RUN go vet ./cmd/... ./internal/... ./test/... +RUN go vet ./cmd/... ./internal/... ############################################################################### # Main build stage, to build MQ image diff --git a/Makefile b/Makefile index b90725e0..dac6f1f1 100644 --- a/Makefile +++ b/Makefile @@ -142,7 +142,7 @@ build-cov: # Shortcut to just run the unit tests .PHONY: test-unit -test-unit: test/docker/vendor +test-unit: docker build --target builder --file Dockerfile-server . .PHONY: test-advancedserver @@ -220,14 +220,14 @@ docker-version: @test "$(word 1,$(subst ., ,$(DOCKER_SERVER_VERSION)))" -ge "17" || ("$(word 1,$(subst ., ,$(DOCKER_SERVER_VERSION)))" -eq "17" && "$(word 2,$(subst ., ,$(DOCKER_CLIENT_VERSION)))" -ge "05") || (echo "Error: Docker server 17.05 or greater is required" && exit 1) .PHONY: build-advancedserver -build-advancedserver: downloads/$(MQ_ARCHIVE) docker-version build-golang-sdk test/docker/vendor +build-advancedserver: downloads/$(MQ_ARCHIVE) docker-version build-golang-sdk $(info $(SPACER)$(shell printf $(TITLE)"Build $(MQ_IMAGE_ADVANCEDSERVER)"$(END))) $(call docker-build-mq,$(MQ_IMAGE_ADVANCEDSERVER),Dockerfile-server,$(MQ_ARCHIVE),"4486e8c4cc9146fd9b3ce1f14a2dfc5b","IBM MQ Advanced",$(MQ_VERSION)) .PHONY: build-devserver # Target-specific variable to add web server into devserver image build-devserver: MQ_PACKAGES=ibmmq-server ibmmq-java ibmmq-jre ibmmq-gskit ibmmq-msg-.* ibmmq-samples ibmmq-ams ibmmq-web -build-devserver: downloads/$(MQ_ARCHIVE_DEV) docker-version build-golang-sdk test/docker/vendor +build-devserver: downloads/$(MQ_ARCHIVE_DEV) docker-version build-golang-sdk $(info $(shell printf $(TITLE)"Build $(MQ_IMAGE_DEVSERVER_BASE)"$(END))) $(call docker-build-mq,$(MQ_IMAGE_DEVSERVER_BASE),Dockerfile-server,$(MQ_ARCHIVE_DEV),"98102d16795c4263ad9ca075190a2d4d","IBM MQ Advanced for Developers (Non-Warranted)",$(MQ_VERSION)) docker build --tag $(MQ_IMAGE_DEVSERVER) --file incubating/mqadvanced-server-dev/Dockerfile . From b453d45c0231871597aa75045c2e1c72d8b201fa Mon Sep 17 00:00:00 2001 From: Stephen Marshall Date: Fri, 1 Jun 2018 14:40:58 +0100 Subject: [PATCH 2/2] Add check for duplicate metric keys --- internal/metrics/exporter_test.go | 10 ++++---- internal/metrics/metrics.go | 2 ++ internal/metrics/update.go | 24 +++++++++++++----- internal/metrics/update_test.go | 41 +++++++++++++++++++++++++------ 4 files changed, 58 insertions(+), 19 deletions(-) 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 +}