Cciutea/improvements2 (#125)

* avoid querying mbeannames when not neccessary

* fetch attributes only when no attribute is provided

* added nrjmx internal stats for troubleshooting

* added disconnect

* report missing attributes

* added ability to auto-reconnect to jmx endpoint, improved error handling

commons/nrjmx.thrift

@@ -1,7 +1,7 @@
 namespace java org.newrelic.nrjmx.v2.nrprotocol
 
 struct JMXConfig {
-  1: string connectionURL
+  1: string connectionURL,
   2: string hostname,
   3: i32 port,
   4: optional string uriPath,
@@ -12,10 +12,12 @@ struct JMXConfig {
   9: string trustStore,
   10: string trustStorePassword,
   11: bool isRemote,
-  12: bool isJBossStandaloneMode
-  13: bool useSSL
-  14: i64 requestTimeoutMs
-  15: bool verbose
+  12: bool isJBossStandaloneMode,
+  13: bool useSSL,
+  14: i64 requestTimeoutMs,
+  15: bool verbose,
+  16: bool enableInternalStats,
+  17: i64 maxInternalStatsSize
 }
 
 enum ResponseType {
@@ -36,6 +38,16 @@ struct AttributeResponse {
   7: bool boolValue
 }
 
+struct InternalStat {
+    1: string statType,
+    2: string mBean,
+    3: list<string> attrs,
+    4: i64 responseCount,
+    5: double milliseconds,
+    6: i64 startTimestamp,
+    7: bool successful
+}
+
 exception JMXError {
   1: string message,
   2: string causeMessage
@@ -59,5 +71,7 @@ service JMXService {
 
     list<AttributeResponse> getMBeanAttributes(1:string mBeanName, 2:list<string> attributes) throws (1:JMXConnectionError connErr, 2:JMXError jmxErr),
 
-    list<AttributeResponse> queryMBeanAttributes(1:string mBeanNamePattern, 2:list<string> attributes) throws (1:JMXConnectionError connErr, 2:JMXError jmxErr)
+    list<AttributeResponse> queryMBeanAttributes(1:string mBeanNamePattern, 2:list<string> attributes) throws (1:JMXConnectionError connErr, 2:JMXError jmxErr),
+
+    list<InternalStat> getInternalStats() throws (1:JMXError jmxErr)
 }

gojmx/README.md

@@ -87,3 +87,40 @@ JMX allows the use of custom connectors to communicate with the application. In
 By default, the sub-folder connectors is in the classpath. If this folder does not exist, create it under the folder where nrjmx is installed.
 
 For example, to add support for JBoss, create a folder named connectors under the default (Linux) library path /usr/lib/nrjmx/ (/usr/lib/nrjmx/connectors/) and copy the custom connector jar ($JBOSS_HOME/bin/client/jboss-cli-client.jar) into it. You can now execute JMX queries against JBoss.
+
+# Internal nrjmx query stats
+If `InternalStats` feature is enabled (gojmx.JMXConfig.EnableInternalStats = true), the `nrjmx` java subprocess will collect
+information about each JMX request. This feature can be used to give more insights while troubleshooting performance issues.
+
+The `InternalStats` are collected in memory (up to 10000 samples by default). When the maximum limit is reached, old samples
+will be discarded. You can increase the maximum limit using `gojmx.JMXConfig.MaxInternalStatsSize` config option.
+After the stats are retrieved (using `client.GetInternalStats()`) `nrjmx` java subprocess will clean the sample storage.
+
+e.g.:
+
+```go
+    // JMX Client configuration.
+	config := &gojmx.JMXConfig{
+		Hostname:         "localhost",
+		Port:             7199,
+
+		// Enable internal gojmx stats for troubleshooting.
+		EnableInternalStats: true,
+	}
+
+    // Connect to JMX endpoint.
+	client, err := gojmx.NewClient(context.Background()).Open(config)
+	handleError(err)
+
+	defer client.Close()
+
+    ... queries ...
+
+	// Collecting gojmx internal query stats. Use this only for troubleshooting.
+	internalStats, err := client.GetInternalStats()
+	handleError(err)
+
+	for _, internalStat := range internalStats {
+		fmt.Println(internalStat.String())
+	}
+```

gojmx/examples/main.go

@@ -16,7 +16,7 @@ import (
 
 func init() {
 	// Uncomment this when you want use the nrjmx.jar build from the project bin directory.
-	_ = os.Setenv("NR_JMX_TOOL", filepath.Join("..", "bin", "nrjmx"))
+	_ = os.Setenv("NR_JMX_TOOL", filepath.Join("../../", "bin", "nrjmx"))
 
 	// Uncomment this when you want to run both: golang debugger and java debugger.
 	//_ = os.Setenv("NRIA_NRJMX_DEBUG", "true")
@@ -28,6 +28,9 @@ func main() {
 		Hostname:         "localhost",
 		Port:             7199,
 		RequestTimeoutMs: 10000,
+
+		// Enable internal gojmx stats for troubleshooting.
+		EnableInternalStats: true,
 	}
 
 	// Connect to JMX endpoint.
@@ -53,7 +56,7 @@ func main() {
 		}
 		for _, attr := range jmxAttrs {
 			if attr.ResponseType == gojmx.ResponseTypeErr {
-				fmt.Println(attr.StatusMsg)
+				fmt.Println(attr.Name, attr.StatusMsg)
 				continue
 			}
 			printAttr(attr)
@@ -67,11 +70,19 @@ func main() {
 	handleError(err)
 	for _, attr := range response {
 		if attr.ResponseType == gojmx.ResponseTypeErr {
-			fmt.Println(attr.StatusMsg)
+			fmt.Println(attr.Name, attr.StatusMsg)
 			continue
 		}
 		printAttr(attr)
 	}
+
+	// Collecting gojmx internal query stats. Use this only for troubleshooting.
+	internalStats, err := client.GetInternalStats()
+	handleError(err)
+
+	for _, internalStat := range internalStats {
+		fmt.Println(internalStat.String())
+	}
 }
 
 func printAttr(jmxAttr *gojmx.AttributeResponse) {

gojmx/gojmx.go

@@ -24,9 +24,6 @@ const (
 	unknownNRJMXVersion = "<unknown>"
 )
 
-// errPingTimeout returned if pingTimeout exceeded.
-var errPingTimeout = newJMXConnectionError("could not establish communication with nrjmx process: ping timeout")
-
 // Client to connect with a JMX endpoint.
 type Client struct {
 	// jmxService is the thrift implementation to communicate with nrjmx subprocess.
@@ -52,29 +49,42 @@ func (c *Client) Open(config *JMXConfig) (client *Client, err error) {
 		return c, err
 	}
 
-	defer func() {
-		if err != nil {
-			c.Close()
-		}
-	}()
-
 	c.jmxService, err = c.configureJMXServiceClient()
 	if err != nil {
+		c.nrJMXProcess.waitExit(nrJMXExitTimeout)
 		return c, err
 	}
 
 	c.version, err = c.ping(pingTimeout)
 	if err != nil {
+		c.nrJMXProcess.waitExit(nrJMXExitTimeout)
 		return c, err
 	}
 
 	return c, c.connect(config)
 }
 
+// IsClientRunning returns if the nrjmx client is running.
+func (c *Client) IsRunning() bool {
+	if c.nrJMXProcess == nil {
+		return false
+	}
+
+	return c.nrJMXProcess.state.IsRunning()
+}
+
+// checkNRJMXProccessError will check if the nrjmx subprocess returned any error.
+func (c *Client) checkNRJMXProccessError() error {
+	if c.nrJMXProcess == nil {
+		return errProcessNotRunning
+	}
+	return c.nrJMXProcess.error()
+}
+
 // QueryMBeanNames returns all the mbeans that match the glob pattern DOMAIN:BEAN.
 // e.g *:* or jboss.as:subsystem=remoting,configuration=endpoint
 func (c *Client) QueryMBeanNames(mBeanGlobPattern string) ([]string, error) {
-	if err := c.nrJMXProcess.error(); err != nil {
+	if err := c.checkNRJMXProccessError(); err != nil {
 		return nil, err
 	}
 	result, err := c.jmxService.QueryMBeanNames(c.ctx, mBeanGlobPattern)
@@ -84,7 +94,7 @@ func (c *Client) QueryMBeanNames(mBeanGlobPattern string) ([]string, error) {
 
 // GetMBeanAttributeNames returns all the available JMX attribute names for a given mBeanName.
 func (c *Client) GetMBeanAttributeNames(mBeanName string) ([]string, error) {
-	if err := c.nrJMXProcess.error(); err != nil {
+	if err := c.checkNRJMXProccessError(); err != nil {
 		return nil, err
 	}
 	result, err := c.jmxService.GetMBeanAttributeNames(c.ctx, mBeanName)
@@ -93,7 +103,7 @@ func (c *Client) GetMBeanAttributeNames(mBeanName string) ([]string, error) {
 
 // GetMBeanAttributes returns the JMX attribute values.
 func (c *Client) GetMBeanAttributes(mBeanName string, mBeanAttrName ...string) ([]*AttributeResponse, error) {
-	if err := c.nrJMXProcess.error(); err != nil {
+	if err := c.checkNRJMXProccessError(); err != nil {
 		return nil, err
 	}
 
@@ -103,7 +113,7 @@ func (c *Client) GetMBeanAttributes(mBeanName string, mBeanAttrName ...string) (
 
 // Close will stop the connection with the JMX endpoint.
 func (c *Client) Close() error {
-	if err := c.nrJMXProcess.error(); err != nil {
+	if err := c.checkNRJMXProccessError(); err != nil {
 		return err
 	}
 	c.jmxService.Disconnect(c.ctx)
@@ -124,18 +134,31 @@ func (c *Client) GetClientVersion() string {
 // 3. GetMBeanAttributes
 // If an error occur it checks if it's a collection error (it can recover) or a connection error (that blocks all the collection).
 func (c *Client) QueryMBeanAttributes(mBeanNamePattern string, mBeanAttrName ...string) ([]*AttributeResponse, error) {
-	if err := c.nrJMXProcess.error(); err != nil {
+	if err := c.checkNRJMXProccessError(); err != nil {
 		return nil, err
 	}
 
 	result, err := c.jmxService.QueryMBeanAttributes(c.ctx, mBeanNamePattern, mBeanAttrName)
 	return toAttributeResponseList(result), c.handleError(err)
 }
 
+// GetInternalStats returns the nrjmx internal query statistics for troubleshooting.
+// Internal statistics must be enabled using JMXConfig.EnableInternalStats flag.
+// Additionally you can set a maximum size for the collected stats using JMXConfig.MaxInternalStatsSize. (default: 100000)
+// Each time you retrieve GetInternalStats, the internal stats will be cleaned.
+func (c *Client) GetInternalStats() ([]*InternalStat, error) {
+	if err := c.checkNRJMXProccessError(); err != nil {
+		return nil, err
+	}
+	result, err := c.jmxService.GetInternalStats(c.ctx)
+
+	return toInternalStatList(result), c.handleError(err)
+}
+
 // connect will pass the JMXConfig to nrjmx subprocess and establish the
 // connection with the JMX endpoint.
 func (c *Client) connect(config *JMXConfig) (err error) {
-	if err = c.nrJMXProcess.error(); err != nil {
+	if err = c.checkNRJMXProccessError(); err != nil {
 		return err
 	}
 	err = c.jmxService.Connect(c.ctx, config.convertToProtocol())
@@ -147,6 +170,7 @@ func (c *Client) connect(config *JMXConfig) (err error) {
 func (c *Client) ping(timeout time.Duration) (string, error) {
 	ctx, cancel := context.WithCancel(c.ctx)
 	defer cancel()
+
 	done := make(chan string, 1)
 	go func() {
 		for ctx.Err() == nil {
@@ -158,6 +182,7 @@ func (c *Client) ping(timeout time.Duration) (string, error) {
 			break
 		}
 	}()
+
 	select {
 	case <-time.After(timeout):
 		return "<nil>", errPingTimeout