-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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 Jolokia agent in proxy mode #6475
Conversation
Correct mapping to mappings
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
} | ||
|
||
type TargetBlock struct { | ||
Url string `json:"url"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
struct field Url should be URL
Target *TargetBlock `json:"target,omitempty"` | ||
} | ||
|
||
type TargetBlock struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
exported type TargetBlock should have comment or be unexported
} | ||
|
||
type Attribute struct { | ||
Attr string | ||
Field string | ||
} | ||
|
||
type Target struct { | ||
Url string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
struct field Url should be URL
} | ||
|
||
type Attribute struct { | ||
Attr string | ||
Field string | ||
} | ||
|
||
type Target struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
exported type Target should have comment or be unexported
@whale-shark Thanks a lot for opening this PR. Could you make Houndci happy and add an entry to the CHANGELOG.asciidoc file? I'm pretty sure this change will make quite a few people happy. I was wondering if there is a good way to have automated tests for this? |
@ruflin I plan to carry out CI correspondence and document addition on the weekend. I've just started Go language, and I'm also studying about testing. Absolutely test is necessary? |
I wonder if jolokia can be run in proxy mode also if it's run on the same machine? Best would be to have also a docker container with the full setup we then can connect to to test this the same way as we do it now. Perhaps we could even have 2 instances of Jolokia running, one in proxy mode and one not? The main reason I'm asking for tests is because in case it breaks I'm looking for an easy way to have a setup to also test it manually. And there is obviously the benefit of the automated way of knowing when it breaks. Please post any questions you have for the setup or beats here, more then happy to help you get going. |
@whale-shark Any update here? Would be great to move this forward. |
Sorry for late reply. I am considering adding an instance of docker to connect to remote JMX. Please wait just a little longer. |
I am sorry to be late. I changed Dockerfile. |
@whale-shark Any chance you could rebase this on the most recent version of master? |
…atch # Conflicts: # metricbeat/module/jolokia/_meta/Dockerfile # metricbeat/module/jolokia/jmx/_meta/docs.asciidoc # metricbeat/module/jolokia/jmx/data_test.go
@ruflin I merged with the most recent master. Would you verification please. |
CHANGELOG.asciidoc
Outdated
- Update the MySQL dashboard to use the Time Series Visual Builder. {pull}5996[5996] | ||
- Add experimental uwsgi module. {pull}6006[6006] | ||
- Docker and Kubernetes modules are now GA, instead of Beta. {pull}6105[6105] | ||
- Add Jolokia agent in proxy mode. {pull}6475[6475] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@whale-shark something weird happened with the changelog after updating the branch, could you remove the added lines that are not related with your change?
jenkins, test this please |
Jolokia agent in proxy mode
@jsoriano Thank you for pointing out I revert to latest branch and added my fix line. |
jenkins, test this |
|
||
# Set up a server to be connected in proxy mode | ||
RUN wget http://archive.apache.org/dist/tomcat/tomcat-9/v${PROXT_TARGET_TOMCAT_VERSION}/bin/${PX_TC}.tar.gz;\ | ||
tar xzf ${PX_TC}.tar.gz -C /usr;\ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you also remove these .tar.gz files that are not needed in the final image?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I deleted the unnecessary tar.gz file from the final image.
-Dcom.sun.management.jmxremote.ssl=false\ | ||
-Dcom.sun.management.jmxremote.authenticate=true"\ | ||
/usr/${PX_TC}/bin/catalina.sh run & \ | ||
env CATALINA_OPTS="-Dorg.jolokia.jsr160ProxyEnabled" /usr/${TC}/bin/catalina.sh run |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be possible to run everything in an only tomcat?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In order to check the connection in Proxy mode, Tomcat to be connected is added.
If you use in an only Tomcat, you will not know if you are connected to another Tomcat in the process of connecting in Proxy mode of the integration test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, makes sense, maybe we could think in a two container scenario, but I think we can continue as is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed the connecting to another Tomcat to a pattern where jmx password authentication cannot be performed correctly.
Fix to be possible to run everything in an only Tomcat.
jenkins, test this please |
jenkins, test this please |
…atch # Conflicts: # metricbeat/module/jolokia/jmx/config.go
@jsoriano Fixed a file conflicting with the latest source. |
jenkins, test this, please |
jenkins, test this again, please |
Failing tests are not related. |
Modified the object of sending to support Jolokia proxy mode.