From 4ea36cd352696afe558ff0f26889b33f863722cf Mon Sep 17 00:00:00 2001
From: Jaime Soriano Pastor <jaime.soriano@elastic.co>
Date: Wed, 9 May 2018 08:15:24 +0200
Subject: [PATCH] Metricbeat: Ensure canonical naming for JMX beans is disabled
 (#7047)

Canonical naming for JMX beans in Jolokia is enabled by default, this
orders mbean fields, what breaks metricbeat internal mapping.

We were already setting these options in the url, but url parameters are
ignored in POST requests. This change adds these parameters as part of
the body.
---
 CHANGELOG.asciidoc                       |  2 ++
 metricbeat/module/jolokia/jmx/config.go  | 16 +++++++++++-----
 metricbeat/module/jolokia/jmx/data.go    |  2 +-
 metricbeat/module/jolokia/jmx/jmx.go     |  2 +-
 metricbeat/tests/system/requirements.txt |  1 +
 metricbeat/tests/system/test_jolokia.py  | 17 +++++++++++------
 6 files changed, 27 insertions(+), 13 deletions(-)

diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 84bb6d53463c..523306c346a6 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -81,6 +81,8 @@ https://github.com/elastic/beats/compare/v6.2.4...6.2[Check the HEAD diff]
 
 *Metricbeat*
 
+- Ensure canonical naming for JMX beans is disabled in Jolokia module. {pull}7047[7047]
+
 *Packetbeat*
 
 *Winlogbeat*
diff --git a/metricbeat/module/jolokia/jmx/config.go b/metricbeat/module/jolokia/jmx/config.go
index a711f3059d67..95ed3ac2d427 100644
--- a/metricbeat/module/jolokia/jmx/config.go
+++ b/metricbeat/module/jolokia/jmx/config.go
@@ -32,19 +32,25 @@ type Attribute struct {
 //    }
 // ]
 type RequestBlock struct {
-	Type      string   `json:"type"`
-	MBean     string   `json:"mbean"`
-	Attribute []string `json:"attribute"`
+	Type      string                 `json:"type"`
+	MBean     string                 `json:"mbean"`
+	Attribute []string               `json:"attribute"`
+	Config    map[string]interface{} `json:"config"`
 }
 
 func buildRequestBodyAndMapping(mappings []JMXMapping) ([]byte, map[string]string, error) {
 	responseMapping := map[string]string{}
 	var blocks []RequestBlock
 
+	config := map[string]interface{}{
+		"ignoreErrors":    true,
+		"canonicalNaming": false,
+	}
 	for _, mapping := range mappings {
 		rb := RequestBlock{
-			Type:  "read",
-			MBean: mapping.MBean,
+			Type:   "read",
+			MBean:  mapping.MBean,
+			Config: config,
 		}
 
 		for _, attribute := range mapping.Attributes {
diff --git a/metricbeat/module/jolokia/jmx/data.go b/metricbeat/module/jolokia/jmx/data.go
index 294c426004b6..7360dbf47337 100644
--- a/metricbeat/module/jolokia/jmx/data.go
+++ b/metricbeat/module/jolokia/jmx/data.go
@@ -81,7 +81,7 @@ func parseResponseEntry(
 
 	key, exists := mapping[metricName]
 	if !exists {
-		return errors.Errorf("metric key '%v' not found in response", metricName)
+		return errors.Errorf("metric key '%v' for mbean '%s' not found in mapping (%+v)", attributeName, mbeanName, mapping)
 	}
 
 	var err error
diff --git a/metricbeat/module/jolokia/jmx/jmx.go b/metricbeat/module/jolokia/jmx/jmx.go
index 5f0f4c1009eb..e37de23604a9 100644
--- a/metricbeat/module/jolokia/jmx/jmx.go
+++ b/metricbeat/module/jolokia/jmx/jmx.go
@@ -27,7 +27,7 @@ const (
 	defaultScheme = "http"
 
 	// defaultPath is the default path to the ngx_http_stub_status_module endpoint on Nginx.
-	defaultPath = "/jolokia/?ignoreErrors=true&canonicalNaming=false"
+	defaultPath = "/jolokia/"
 )
 
 var (
diff --git a/metricbeat/tests/system/requirements.txt b/metricbeat/tests/system/requirements.txt
index 470c2e0da233..cc3c0ad31a2a 100644
--- a/metricbeat/tests/system/requirements.txt
+++ b/metricbeat/tests/system/requirements.txt
@@ -1 +1,2 @@
 kafka-python==1.4.2
+parameterized==0.6.1
diff --git a/metricbeat/tests/system/test_jolokia.py b/metricbeat/tests/system/test_jolokia.py
index 189dcc911e4d..64c9f42742db 100644
--- a/metricbeat/tests/system/test_jolokia.py
+++ b/metricbeat/tests/system/test_jolokia.py
@@ -2,25 +2,30 @@
 import metricbeat
 import unittest
 from nose.plugins.attrib import attr
+from parameterized import parameterized
 
 
 class Test(metricbeat.BaseTest):
 
     COMPOSE_SERVICES = ['jolokia']
 
+    @parameterized.expand([
+        'java.lang:name=PS MarkSweep,type=GarbageCollector',
+        'java.lang:type=GarbageCollector,name=PS MarkSweep'
+    ])
     @unittest.skipUnless(metricbeat.INTEGRATION_TESTS, "integration test")
-    def test_jmx(self):
+    def test_jmx(self, mbean):
         """
         jolokia jmx  metricset test
         """
 
         additional_content = """
   jmx.mappings:
-    - mbean: 'java.lang:type=Runtime'
+    - mbean: '%s'
       attributes:
-         - attr: Uptime
-           field: uptime
-"""
+         - attr: CollectionCount
+           field: gc.collection_count
+""" % (mbean)
 
         self.render_config_template(modules=[{
             "name": "jolokia",
@@ -40,7 +45,7 @@ def test_jmx(self):
         evt = output[0]
         print(evt)
 
-        assert evt["jolokia"]["test"]["uptime"] > 0
+        assert evt["jolokia"]["test"]["gc"]["collection_count"] >= 0
 
     def get_hosts(self):
         return [os.getenv('JOLOKIA_HOST', 'localhost') + ':' +