You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jmeter.apache.org by GitBox <gi...@apache.org> on 2020/09/02 12:51:53 UTC
[GitHub] [jmeter] delirius325 opened a new pull request #615: Integrated ElasticSearch backend listener to main codebase
delirius325 opened a new pull request #615:
URL: https://github.com/apache/jmeter/pull/615
## Description
After 3 years of developing and maintaining the ElasticSearch Backend Listener plugin for JMeter, I believe it's time for it to be integrated to the main codebase.
* Added `org.apache.jmeter.visualizers.backend.elasticsearch` package
* Added the following classes:
* ElasticsearchBackendClient.java
* ElasticsearchMetric.java
* ElasticsearchMetricSender.java
* ElasticsearchRequests.java
* Added unit tests
* TestElasticsearchBackend.java
## Motivation and Context
As I mentioned above, after 3 years of developing, publishing and maintaining the plugin "Elasticsearch Backend Listener", I believe it is time for it to be added to the main codebase. As a result of this, Elasticsearch would officially be supported by JMeter and would evolve nicely with it.
It is clearly a used and needed integration since as it stands there are over 2500 users installing the plugin [source](https://jmeter-plugins.org/stats/).
## How Has This Been Tested?
The plugin itself has been tested through and through for over 3 years. As for how it's been tested in the main codebase; I've changed all the packages names, added the necessary dependencies in the `build.gradle.kts` of the `components` workspace. I have applied the necessary coding styles, corrected and adjusted if necessary.
My testing environment consisted of the following:
* locally built JMeter (5.3.1-SNAPSHOT + my code modifications)
* ElasticSearch 7.9.0
* Kibana 7.9.0
To test, I built JMeter using `gw runGui`. I then added a sample `Thread Group` as well as `Debug samplers` and a `Backend Listener` to which I setup the config to point to my local ElasticSearch engine.
I executed the test and verified that samples showed up.
## Screenshots
## Types of changes
- New feature (non-breaking change which adds functionality)
## Checklist:
- [x] My code follows the [code style][style-guide] of this project.
- [x] I have updated the documentation accordingly.
[style-guide]: https://wiki.apache.org/jmeter/CodeStyleGuidelines
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] delirius325 edited a comment on pull request #615: Add: ElasticSearch backend listener
Posted by GitBox <gi...@apache.org>.
delirius325 edited a comment on pull request #615:
URL: https://github.com/apache/jmeter/pull/615#issuecomment-691688501
Hey there @pmouawad , anything to change in your opinion?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] pmouawad commented on pull request #615: Add: ElasticSearch backend listener
Posted by GitBox <gi...@apache.org>.
pmouawad commented on pull request #615:
URL: https://github.com/apache/jmeter/pull/615#issuecomment-705181585
Hello @delirius325 ,
The drawbacks I see currently are:
- For signing on AWS ElasticSearch, It introduces a dependency on aws-java-sdk and google guava which might not be acceptable. This should be available as a pluggable thing instead
- We need to maintain in core a dependency on a 3rd party (Elastic Search) that has a faster pace in terms of releases
- is there a reason for not keeping the plugin as is and contributing it to JMeter core ?
The addition looks really useful, but I am wondering if it's not better that this one remains as a plugin, as its pace of release can be faster than JMeter one.
What's your thoughts ?
Regards
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] vlsi commented on pull request #615: Add: ElasticSearch backend listener
Posted by GitBox <gi...@apache.org>.
vlsi commented on pull request #615:
URL: https://github.com/apache/jmeter/pull/615#issuecomment-694509648
Just in case, here's dependency impact.
+5MiB is not that very expensive, however, it would be great if there was a brief explanation of why the dependency is needed and why existing classes are not enough.
For instance, we already have `jackson-*`, so do we really need `gson`?
Do we need `guava`?
Do we need AWS clients?
```
55421535 => 60544843 bytes (+5123308 bytes)
98 => 108 files (+10)
+ 1013903 aws-java-sdk-core-1.11.851.jar
+ 272295 aws-java-sdk-secretsmanager-1.11.851.jar
+ 12691 aws-signing-request-interceptor-0.0.22.jar
+ 64552 elasticsearch-rest-client-7.9.0.jar
+ 240255 gson-2.8.6.jar
+ 2256213 guava-18.0.jar
+ 565410 ion-java-1.0.2.jar
+ 48468 jackson-dataformat-cbor-2.6.7.jar
+ 27590 jmespath-java-1.11.851.jar
+ 621931 joda-time-2.8.1.jar
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] pmouawad closed pull request #615: Add: ElasticSearch backend listener
Posted by GitBox <gi...@apache.org>.
pmouawad closed pull request #615:
URL: https://github.com/apache/jmeter/pull/615
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] delirius325 commented on pull request #615: Add: ElasticSearch backend listener
Posted by GitBox <gi...@apache.org>.
delirius325 commented on pull request #615:
URL: https://github.com/apache/jmeter/pull/615#issuecomment-707943229
Hey @pmouawad ,
You bring up some good points; including the plugin's JAR probably would be the more logical and flexible approach to this. Otherwise, I agree that I would have to cut back on some of the features to remove certain dependencies (i.e. for AWS). Honestly, it's a good thing you brought this up, I hadn't even thought about doing it like that.
I'll look into making that happen instead of having an "official" implementation.
Thanks for the feedback!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] delirius325 commented on pull request #615: Add: ElasticSearch backend listener
Posted by GitBox <gi...@apache.org>.
delirius325 commented on pull request #615:
URL: https://github.com/apache/jmeter/pull/615#issuecomment-694894905
@vlsi Thanks for the feedback! I've added comments concerning AWS and I've switched from Gson to Jackson, therefore avoiding the need to add a new dependency.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] delirius325 commented on pull request #615: Add: ElasticSearch backend listener
Posted by GitBox <gi...@apache.org>.
delirius325 commented on pull request #615:
URL: https://github.com/apache/jmeter/pull/615#issuecomment-691688501
AHey there @pmouawad , anything to change in your opinion?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] pmouawad commented on pull request #615: Add: ElasticSearch backend listener
Posted by GitBox <gi...@apache.org>.
pmouawad commented on pull request #615:
URL: https://github.com/apache/jmeter/pull/615#issuecomment-713529253
Thanks for your answer.
I'll close this PR for now.
Thanks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] pmouawad commented on pull request #615: Add: ElasticSearch backend listener
Posted by GitBox <gi...@apache.org>.
pmouawad commented on pull request #615:
URL: https://github.com/apache/jmeter/pull/615#issuecomment-691717692
Hello @delirius325 ,
Thanks a lot for your contribution.
Can you open a discussion on dev mailing list for this PR ?
Thanks
Regards
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org