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 trailing slash to jolokia context #2105

Merged
merged 1 commit into from
Dec 17, 2016

Conversation

stevenpall
Copy link
Contributor

Required for all PRs:

  • CHANGELOG.md updated (we recommend not updating this until the PR has been approved by a maintainer)
  • Sign CLA (if not already signed)
  • README.md updated (if adding a new plugin)

If one is using the suggested Jolokia plugin configuration, the following will cause the plugin to fail:

context = "/jolokia"

The server requires a trailing slash (i.e. http://127.0.0.1:8778/jolokia/). This change adds a slash to the end of the context parameter if it is not present.

@sparrc
Copy link
Contributor

sparrc commented Nov 30, 2016

instead of modifying the input the user provides, why not just make the /jolokia/ the default? you could log an info message if you don't find a trailing slash.

@stevenpall
Copy link
Contributor Author

@sparrc That makes sense. I'll update the sampleConfig and README.md.

@sparrc sparrc added this to the 1.2.0 milestone Nov 30, 2016
@stevenpall stevenpall force-pushed the jolokia-context branch 4 times, most recently from b882bdd to 0d3487d Compare November 30, 2016 18:33
@@ -6,6 +6,7 @@
# Read JMX metrics through Jolokia
[[inputs.jolokia]]
## This is the context root used to compose the jolokia url
## NOTE that Jolokia requires a trailing slash at the end of the context root
context = "/jolokia"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add the trailing slash here as well

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@sparrc sparrc merged commit 0e8122a into influxdata:master Dec 17, 2016
njwhite pushed a commit to njwhite/telegraf that referenced this pull request Jan 31, 2017
maxunt pushed a commit that referenced this pull request Jun 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants