From d3b08dcf275ba5da2e8973c870aa140e4ece5f2e Mon Sep 17 00:00:00 2001 From: Grigorii Merkushev Date: Thu, 7 Jan 2021 12:58:51 +0100 Subject: [PATCH 1/5] fix: warning stops mbeans processing --- jmx/jmx.go | 21 ++++++++++++++++----- jmx/jmx_test.go | 20 +++++++++++++++----- log/log.go | 4 ++++ 3 files changed, 35 insertions(+), 10 deletions(-) diff --git a/jmx/jmx.go b/jmx/jmx.go index 6cb16fd9..01fab0f9 100644 --- a/jmx/jmx.go +++ b/jmx/jmx.go @@ -341,7 +341,7 @@ func Query(objectPattern string, timeoutMillis int) (result map[string]interface // receiveResult checks for channels to receive result from nrjmx command. func receiveResult(lineC chan []byte, queryErrC chan error, cancelFn context.CancelFunc, objectPattern string, timeout time.Duration) (result map[string]interface{}, err error) { - var warn string + defer logAvailableWarnings() for { select { case line := <-lineC: @@ -361,11 +361,7 @@ func receiveResult(lineC chan []byte, queryErrC chan error, cancelFn context.Can for k, v := range r { result[k] = v } - return - case warn = <-cmdWarnC: - // change on the API is required to return warnings - log.Warn(warn) return case err = <-cmdErrC: @@ -383,3 +379,18 @@ func receiveResult(lineC chan []byte, queryErrC chan error, cancelFn context.Can } } } + +func logAvailableWarnings() { + var warn string + for { + + select { + case warn = <-cmdWarnC: + { + log.Warn(warn) + } + default: + return + } + } +} diff --git a/jmx/jmx_test.go b/jmx/jmx_test.go index 7e4634ec..3e0ea8fa 100644 --- a/jmx/jmx_test.go +++ b/jmx/jmx_test.go @@ -2,9 +2,11 @@ package jmx import ( "bufio" + "bytes" "context" "flag" "fmt" + "github.com/newrelic/infra-integrations-sdk/v4/log" "os" "strings" "testing" @@ -179,17 +181,25 @@ func openWaitWithSSL(hostname, port, username, password, keyStore, keyStorePassw } func Test_receiveResult_warningsDoNotBreakResultReception(t *testing.T) { + + var buf bytes.Buffer + log.SetOutput(&buf) + _, cancelFn := context.WithCancel(context.Background()) resultCh := make(chan []byte, 1) queryErrCh := make(chan error) outTimeout := time.Duration(timeoutMillis) * time.Millisecond + warningMessage := fmt.Sprint("WARNING foo bar") + cmdWarnC <- warningMessage - _, _ = receiveResult(resultCh, queryErrCh, cancelFn, "empty", outTimeout) + resultCh <- []byte("{\"foo\":1}") - cmdErrC <- fmt.Errorf("WARNING foo bar") - assert.Equal(t, <-cmdErrC, fmt.Errorf("WARNING foo bar")) + result, err := receiveResult(resultCh, queryErrCh, cancelFn, "foo", outTimeout) - resultCh <- []byte("{foo}") - assert.Equal(t, string(<-resultCh), "{foo}") + assert.NoError(t, err) + assert.Equal(t, map[string]interface{}{ + "foo": 1., + }, result) + assert.Equal(t, fmt.Sprintf("[WARN] %s\n", warningMessage), buf.String()) } diff --git a/log/log.go b/log/log.go index 6bd9996a..1b1aa4a6 100644 --- a/log/log.go +++ b/log/log.go @@ -81,6 +81,10 @@ func SetupLogging(verbose bool) { } } +func SetOutput(w io.Writer) { + globalLogger.logger.SetOutput(w) +} + // Debug logs a formatted message at level Debug. func Debug(format string, args ...interface{}) { globalLogger.Debugf(format, args...) From 3a12c25ab7d6a7eacea4e261adfc7ed3401ee7b8 Mon Sep 17 00:00:00 2001 From: Grigorii Merkushev Date: Fri, 8 Jan 2021 10:39:51 +0100 Subject: [PATCH 2/5] fix: restore warnings handling --- jmx/jmx.go | 4 ++++ jmx/jmx_test.go | 2 +- log/log.go | 1 + 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/jmx/jmx.go b/jmx/jmx.go index 01fab0f9..079e958d 100644 --- a/jmx/jmx.go +++ b/jmx/jmx.go @@ -342,6 +342,7 @@ func Query(objectPattern string, timeoutMillis int) (result map[string]interface // receiveResult checks for channels to receive result from nrjmx command. func receiveResult(lineC chan []byte, queryErrC chan error, cancelFn context.CancelFunc, objectPattern string, timeout time.Duration) (result map[string]interface{}, err error) { defer logAvailableWarnings() + var warn string for { select { case line := <-lineC: @@ -364,6 +365,9 @@ func receiveResult(lineC chan []byte, queryErrC chan error, cancelFn context.Can return + case warn = <-cmdWarnC: + log.Warn(warn) + case err = <-cmdErrC: return diff --git a/jmx/jmx_test.go b/jmx/jmx_test.go index 3e0ea8fa..0b168562 100644 --- a/jmx/jmx_test.go +++ b/jmx/jmx_test.go @@ -6,12 +6,12 @@ import ( "context" "flag" "fmt" - "github.com/newrelic/infra-integrations-sdk/v4/log" "os" "strings" "testing" "time" + "github.com/newrelic/infra-integrations-sdk/v4/log" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) diff --git a/log/log.go b/log/log.go index 1b1aa4a6..35b82a36 100644 --- a/log/log.go +++ b/log/log.go @@ -81,6 +81,7 @@ func SetupLogging(verbose bool) { } } +// SetOutput sets output stream func SetOutput(w io.Writer) { globalLogger.logger.SetOutput(w) } From 8e8c6d3875ef6b6bed10265713a3152af3d76339 Mon Sep 17 00:00:00 2001 From: Grigorii Merkushev Date: Fri, 8 Jan 2021 12:18:46 +0100 Subject: [PATCH 3/5] fix: doQuery loop exit --- jmx/jmx.go | 1 + 1 file changed, 1 insertion(+) diff --git a/jmx/jmx.go b/jmx/jmx.go index 079e958d..45aaf94f 100644 --- a/jmx/jmx.go +++ b/jmx/jmx.go @@ -323,6 +323,7 @@ func doQuery(ctx context.Context, out chan []byte, queryErrC chan error, querySt queryErrC <- fmt.Errorf("reading nrjmx stdout: %s", err.Error()) } out <- b + return } } From e5892b6ca40ef362b9d63a0fe77fa382378e4b05 Mon Sep 17 00:00:00 2001 From: Grigorii Merkushev Date: Fri, 8 Jan 2021 15:23:34 +0100 Subject: [PATCH 4/5] fix: move global variable as a param for logAvailableWarnings --- jmx/jmx.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/jmx/jmx.go b/jmx/jmx.go index 45aaf94f..73eab25b 100644 --- a/jmx/jmx.go +++ b/jmx/jmx.go @@ -342,7 +342,7 @@ func Query(objectPattern string, timeoutMillis int) (result map[string]interface // receiveResult checks for channels to receive result from nrjmx command. func receiveResult(lineC chan []byte, queryErrC chan error, cancelFn context.CancelFunc, objectPattern string, timeout time.Duration) (result map[string]interface{}, err error) { - defer logAvailableWarnings() + defer logAvailableWarnings(cmdWarnC) var warn string for { select { @@ -385,12 +385,11 @@ func receiveResult(lineC chan []byte, queryErrC chan error, cancelFn context.Can } } -func logAvailableWarnings() { +func logAvailableWarnings(channel chan string) { var warn string for { - select { - case warn = <-cmdWarnC: + case warn = <-channel: { log.Warn(warn) } From 22f38a2613fdcdcf65b102053a9a404b85b2667b Mon Sep 17 00:00:00 2001 From: Grigorii Merkushev Date: Mon, 11 Jan 2021 16:11:44 +0100 Subject: [PATCH 5/5] fix: review remarks --- jmx/jmx.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/jmx/jmx.go b/jmx/jmx.go index 73eab25b..e4750ea6 100644 --- a/jmx/jmx.go +++ b/jmx/jmx.go @@ -390,9 +390,7 @@ func logAvailableWarnings(channel chan string) { for { select { case warn = <-channel: - { - log.Warn(warn) - } + log.Warn(warn) default: return }