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