Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for GET method in Jolokia metricbeat module #8566

Closed
jsoriano opened this issue Oct 4, 2018 · 4 comments
Closed

Add support for GET method in Jolokia metricbeat module #8566

jsoriano opened this issue Oct 4, 2018 · 4 comments
Labels

Comments

@jsoriano
Copy link
Member

jsoriano commented Oct 4, 2018

Current Jolokia module makes requests using POST method, but depending on the environment where Jolokia agent is deployed, POST method can be unavilable.

Jolokia also supports GET method, the module could have an option to select the method used.

Using GET method with Jolokia is a bit tricky, as parameters need to be carefully escaped and sent in the query parameters instead of using the request body. Also, according to documentation, not all Jolokia features are available when using the GET method, we'd have to check for these limitations when using this method.

@manios
Copy link
Contributor

manios commented Oct 4, 2018

Hello!

May I try to take up this issue? Some first thoughts/questions for discussion:

  1. Regarding configuration, shall we add a new http_method parameter to config.yml which by default will have the value POST and can accept GET as well?
  2. Regarding the special path parameter treatment of GET method, is GET request going to use this form <base-url>/<type>/<arg1>/<arg2>/..../ as described in 6.1.1. GET requests abiding by Table 6.1. Escaping rules?
  3. Regarding features unavalable in GET read request:
    1. Proxy requests are not supported in GET, so if they exist in configuration shall we ignore them, log a warning message and continue or throw an error?
    2. Bulk requests are only possible with POST. What will we do for multiple jmx.mappings in beat configuration?. Shall we log a warning message and use only the first mapping, make multiple GET requests or throw an error?

Thanks in advance.

@jsoriano
Copy link
Member Author

jsoriano commented Oct 5, 2018

Hi @manios, thanks for offering your help on this issue! 🙂

Regarding configuration, shall we add a new http_method parameter to config.yml which by default will have the value POST and can accept GET as well?

Yes, we should add a new parameter, http_method with POST by default looks good to me.

Regarding the special path parameter treatment of GET method, is GET request going to use this form <base-url>/<type>/<arg1>/<arg2>/..../ as described in 6.1.1. GET requests abiding by Table 6.1. Escaping rules?

Yes, I guess we will have to build this kind of urls.

Regarding features unavalable in GET read request:

Thanks for looking for the unavailable features! 😄

Proxy requests are not supported in GET, so if they exist in configuration shall we ignore them, log a warning message and continue or throw an error?

I'd throw an error as the configuration should be considered invalid, we could mention in the error message that this is supported using POST method.

Bulk requests are only possible with POST. What will we do for multiple jmx.mappings in beat configuration?. Shall we log a warning message and use only the first mapping, make multiple GET requests or throw an error?

I'd try to workaround this by making multiple GET requests.

@jsoriano
Copy link
Member Author

jsoriano commented Oct 5, 2018

@manios I think I cannot assign this issue to you, but you can take it, feel free to open a PR when ready.

@manios
Copy link
Contributor

manios commented Oct 5, 2018

@jsoriano Thanks for the clarifications! I will start it and make a PR when ready! Cheers!

@ruflin ruflin added the Team:Integrations Label for the Integrations team label Nov 21, 2018
jsoriano added a commit to jsoriano/beats that referenced this issue Dec 4, 2018
jsoriano pushed a commit to jsoriano/beats that referenced this issue Dec 4, 2018
…c#9226)

Add a new `http_method` parameter to Jolokia module which by default
will have the value POST and can accept GET as well.

Fixes elastic#8566.

(cherry picked from commit 463ce3e)
jsoriano added a commit to jsoriano/beats that referenced this issue Dec 4, 2018
(cherry picked from commit d434b47)
jasontedor added a commit to jasontedor/beats that referenced this issue Dec 4, 2018
…yslog_host

* elastic/master:
  Metricbeat can call Jolokia with GET requests. (elastic#8566)  (elastic#9226)
  Add some missing references to PRs in changelog (elastic#9358)
jsoriano added a commit that referenced this issue Dec 4, 2018
jsoriano added a commit that referenced this issue Dec 12, 2018
…ts. (#8566)  (#9375)

Add a new `http_method` parameter to Jolokia module which by default
will have the value POST and can accept GET as well.

Fixes #8566.

(cherry picked from commit 463ce3e)

Add changelog for #8566

(cherry picked from commit d434b47)

Co-authored-by: Christos Manios <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants