Add BackendListener that sends "raw" results to InfluxDB #544

ham1 · 2019-11-09T10:07:29Z

Description

Add BackendListener that sends "raw" results to InfluxDB.
Instead of summarised statistics this listener sends the raw results for each sample to InfluxDB.

In its current form it sends connect time, ttfb (latency) and duration.
Are there other metrics that would be essential to send?

Added extra method to InfluxdbMetricsSender: public void addMetric(String measurement, String tag, String field, long timestamp).

This is a draft PR to start a discussion about what more needs to be done or could be done before including.

Motivation and Context

As can be seen from the below graphs, in a low TPS situation the current InfluxDB listener has a misleading max value and you lose a lot of detail with avg etc.

In a slightly higher TPS test you can see that the time is slightly off (due to the listener using System.currentTimeMillis() instead of the sample end (or start) time) and also interesting details are lost due to the summary nature of the listener.

How Has This Been Tested?

Tested with a few simple test plans against a local InxlufDB and Grafana, and also the Influx cloud 2.0.0.

Screenshots (if appropriate):

Types of changes

New feature (non-breaking change which adds functionality)

Checklist:

My code follows the code style of this project.
I have updated the documentation accordingly.

codecov-io · 2019-11-09T10:21:31Z

Codecov Report

Merging #544 into master will increase coverage by 0.04%.
The diff coverage is 79.12%.

@@             Coverage Diff              @@
##             master     #544      +/-   ##
============================================
+ Coverage     55.33%   55.37%   +0.04%     
- Complexity     9960     9976      +16     
============================================
  Files          1033     1035       +2     
  Lines         63345    63399      +54     
  Branches       7157     7159       +2     
============================================
+ Hits          35052    35109      +57     
+ Misses        25818    25815       -3     
  Partials       2475     2475

Impacted Files	Coverage Δ	Complexity Δ
...lizers/backend/influxdb/InfluxdbMetricsSender.java	`100% <ø> (ø)`	`0 <0> (ø)`	⬇️
...alizers/backend/AbstractBackendListenerClient.java	`23.07% <ø> (+1.64%)`	`3 <0> (ø)`	⬇️
...ackend/influxdb/InfluxdbBackendListenerClient.java	`39.91% <0%> (+0.35%)`	`12 <0> (ø)`	⬇️
...visualizers/backend/influxdb/UdpMetricsSender.java	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
...ter/visualizers/backend/BackendListenerClient.java	`0% <0%> (ø)`	`0 <0> (?)`
...end/influxdb/InfluxDBRawBackendListenerClient.java	`100% <100%> (ø)`	`14 <14> (?)`
...isualizers/backend/influxdb/HttpMetricsSender.java	`76.84% <70%> (-0.34%)`	`11 <1> (+1)`
...er/visualizers/backend/BackendListenerContext.java	`23.68% <0%> (ø)`	`7% <0%> (ø)`	⬇️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2314361...2a4737a. Read the comment docs.

ham1 · 2020-03-21T17:50:23Z

Any thoughts on this one - is there more that needs doing or is this ok to mergE?

pmouawad

Hello @ham1 ,
It looks good to me.

@vlsi , @FSchumacher any thoughts ?
Thanks

vlsi · 2020-03-24T20:29:07Z

...y/org/apache/jmeter/visualizers/backend/influxdb/InfluxDBRawBackendListenerClientSpec.groovy

+            def koSample = new SampleResult()
+            koSample.setSampleLabel("myLabel")
+        expect:
+            sut.createTags(koSample) == "status=ko,transaction=myLabel"


Does this look like a call to a private method?

Frankly speaking, I don't think we should encourage that type of tests.

It is, I'd be interested in any ideas you have to test this, would it be better to make the method public marked with @PublicForTesting?

vlsi · 2020-03-24T20:29:45Z

...y/org/apache/jmeter/visualizers/backend/influxdb/InfluxDBRawBackendListenerClientSpec.groovy

+    def defaultContext = new BackendListenerContext(sut.getDefaultParameters())
+
+    def createOkSample() {
+        def now = System.currentTimeMillis()


Is currentTimeMillis needed here?
Can a constant be used instead?

Could be, what would be the advantage of using say 1 or 1585413149344, vs. this call?

Using a fixed value in test helps to keep the test reproducible, and it helps to avoid unexpected timezone issues.

If you want to exercise timezone issues, please create a separate test for that.

vlsi · 2020-03-24T20:31:56Z

I'm not sure it is worth adding yet another Listerer implementation.

It looks like the users would have to copy-paste the configuration if they want to switch between raw and grouped data.

vlsi · 2020-03-24T20:33:54Z

In a parallel universe, there's https://gitlab.com/testload/jmeter-listener (MIT)

pmouawad · 2020-03-24T20:47:09Z

I'm not sure it is worth adding yet another Listerer implementation.

IMO this one complements the other one and will be more precise

It looks like the users would have to copy-paste the configuration if they want to switch between raw and grouped data.

Is this a big deal ?
Will this really happen ?

pmouawad · 2020-03-24T20:48:54Z

In a parallel universe, there's https://gitlab.com/testload/jmeter-listener (MIT)

Have you tried it ?
I think it's better to have things in CORE for what seems to be a "core" feature, making live metrics available.
Not all users of JMeter are aware of 3rd party plugins.

vlsi · 2020-03-24T20:53:19Z

Is this a big deal ?

I think it is.
From my perspective, the use case is: "user wants to send the data to InfluxDB".
The way to send the data looks more like a configuration option rather than selecting a different listener.

For instance, what if we add BackendListener that sends percentiles and BackendListener that sends errors?
It does look not user-friendly, especially, taking into account that JMeter does not allow copy-pasting multiple fields.

IMO this one complements the other one and will be more precise

Having 3-4 "similar in-core listeners that send data to InfluxDB" would be confusing.

vlsi · 2020-03-24T20:56:28Z

I think it's better to have things in CORE for what seems to be a "core" feature, making live metrics available.

One of the options is to bundle the plugin into the final distribution (like we bundle lots of third-party dependencies).

I have not tried that myself, however, their wiki says there are reasons to group data before sending:

Activation of GROUP_BY function (only couple parameters in settings) could be very useful on highload (thousands of transaction per second) because on such intensity InfluxDB/ElasticSearch/Clickhouse can be bottleneck (IOPS). In this mode all Samplers grouping by name and only aggregates

pmouawad · 2020-03-28T07:50:31Z

I think it's better to have things in CORE for what seems to be a "core" feature, making live metrics available.

One of the options is to bundle the plugin into the final distribution (like we bundle lots of third-party dependencies).

I have not tried that myself, however, their wiki says there are reasons to group data before sending:

Activation of GROUP_BY function (only couple parameters in settings) could be very useful on highload (thousands of transaction per second) because on such intensity InfluxDB/ElasticSearch/Clickhouse can be bottleneck (IOPS). In this mode all Samplers grouping by name and only aggregates

This plugin has indeed good ideas, but implementation should be improved if embedded in JMeter (https://gitlab.com/testload/jmeter-listener/-/issues).

ham1 · 2020-03-28T16:34:10Z

If we don't like the idea of another listener for this use case how might we allow the current implementation to be customised?

pmouawad · 2020-09-13T20:01:19Z

Hello @ham1 ,

Regarding this PR, did you have the opportunity to test it on a "real life" performance test ?

What were the use cases ? and related volumes ?

Thanks

ham1 · 2020-09-13T20:30:16Z

Hello. Yes I did. But I seem to have messed up rebasing so I'll have a look at it again towards the end of the month.

I can try to compare the performance of this vs. the existing one, but I'd expect it to be slightly worse but that's a choice for the user based on their use case and need for higher resolution of results in Influx.

Added default impl of BackendListenerClient#createSampleResult Added overloaded UdpMetricsSender and HttpMetricsSender#addMetric to include custom timestamp Improved JavaDocs

Also, removing these defaults from AbstractBackendListenerClient.

eostermueller · 2020-11-24T18:56:23Z

Is there a way to submit the raw requests in batches?
If performance is decent, we'll try this.

ham1 · 2020-11-24T21:37:44Z

Is there a way to submit the raw requests in batches?
If performance is decent, we'll try this.

Afraid not.

How many req/s are you currently testing with? Would be interested to know what you discover if you try it out.

This PR was to allow more accurate results, especially with low RPS tests, so I didn't add the extra complexity of batching.

Sorry for missing this earlier, Graham Russell. Relates to #544

* Implemented InfluxDBRawBackendListenerClient and tests Added default impl of BackendListenerClient#createSampleResult Added overloaded UdpMetricsSender and HttpMetricsSender#addMetric to include custom timestamp Improved JavaDocs * Added documentation for InfluxDBRawBackendListenerClient * Add whitespace and improved InfluxdbBackendListenerClientSpec * Add default implementation to BackendListenerClient. Also, removing these defaults from AbstractBackendListenerClient. * Improve BackendListenerClient JavaDoc * http -> https in docs plus some formatting. * Influxdb -> InfluxDB and whitespace * Update InfluxDB Raw Listener docs * Used fixed timestamp for test and remove unused import * Fix error prone error

Sorry for missing this earlier, Graham Russell. Relates to apache#544

sergiodegrazia · 2021-11-25T17:33:25Z

Hi everyone!
At this time, this new Backend Listener only reports on response time data. It would be great if this could also include some of the details the 'old' Listener sent to Influx.
For example:

The status of each request (ok or ko)
The response code of each request
Information about the active threads (e.g. meanAT), sent every second
Event information such as test start and end
An 'Application' tag similar to the one used in the previous Backend Listener, so it is possible to filter each test.

ham1 force-pushed the raw-influx-listener branch from fed71d1 to bbcf292 Compare November 11, 2019 16:09

ham1 marked this pull request as ready for review November 11, 2019 17:29

ham1 force-pushed the raw-influx-listener branch 2 times, most recently from e3ba370 to f477c5e Compare November 11, 2019 17:49

ham1 force-pushed the raw-influx-listener branch 2 times, most recently from 6c3e04d to af53f31 Compare November 21, 2019 17:30

ham1 force-pushed the raw-influx-listener branch from af53f31 to df4d26e Compare December 16, 2019 19:17

ham1 force-pushed the raw-influx-listener branch from df4d26e to eba22f2 Compare January 1, 2020 19:59

ham1 force-pushed the raw-influx-listener branch from eba22f2 to be98a04 Compare February 16, 2020 22:24

ham1 force-pushed the raw-influx-listener branch 2 times, most recently from 4711d5a to f07e070 Compare February 27, 2020 13:11

ham1 force-pushed the raw-influx-listener branch from f07e070 to 4bb3ec4 Compare March 13, 2020 23:27

ham1 force-pushed the raw-influx-listener branch from 4bb3ec4 to 62ceeb9 Compare March 21, 2020 17:42

pmouawad approved these changes Mar 24, 2020

View reviewed changes

vlsi reviewed Mar 24, 2020

View reviewed changes

ham1 force-pushed the raw-influx-listener branch 2 times, most recently from fea4cb4 to 8585791 Compare March 28, 2020 00:02

ham1 force-pushed the raw-influx-listener branch 2 times, most recently from 53c6a8b to 3686ad7 Compare March 29, 2020 20:50

ham1 force-pushed the raw-influx-listener branch 2 times, most recently from bd7b3bc to 2a4737a Compare April 5, 2020 16:07

ham1 force-pushed the raw-influx-listener branch from 2a4737a to 6f8b730 Compare June 6, 2020 17:12

ham1 force-pushed the raw-influx-listener branch from 6f8b730 to 13e9db1 Compare July 2, 2020 22:31

ham1 force-pushed the raw-influx-listener branch from 13e9db1 to 121c259 Compare July 17, 2020 12:50

ham1 force-pushed the raw-influx-listener branch from e51b436 to ee541c8 Compare September 28, 2020 18:14

ham1 added 8 commits November 1, 2020 00:18

Implemented InfluxDBRawBackendListenerClient and tests

e7662f4

Added default impl of BackendListenerClient#createSampleResult Added overloaded UdpMetricsSender and HttpMetricsSender#addMetric to include custom timestamp Improved JavaDocs

Added documentation for InfluxDBRawBackendListenerClient

17ddb9f

Add whitespace and improved InfluxdbBackendListenerClientSpec

f17da5e

Add default implementation to BackendListenerClient.

6c59ded

Also, removing these defaults from AbstractBackendListenerClient.

Improve BackendListenerClient JavaDoc

70be128

http -> https in docs plus some formatting.

45faac9

Influxdb -> InfluxDB and whitespace

1e36c5f

Update InfluxDB Raw Listener docs

0a41fd6

ham1 force-pushed the raw-influx-listener branch from ee541c8 to f414151 Compare November 1, 2020 00:36

Used fixed timestamp for test and remove unused import

1c89b04

ham1 force-pushed the raw-influx-listener branch from f414151 to 1c89b04 Compare November 1, 2020 00:54

Fix error prone error

b41fc52

pmouawad merged commit 166800a into apache:master Nov 14, 2020

ham1 deleted the raw-influx-listener branch November 14, 2020 16:48

asfgit pushed a commit that referenced this pull request Nov 26, 2020

Mention PR 544 in changes

c39571c

Sorry for missing this earlier, Graham Russell. Relates to #544

kkalinin pushed a commit to kkalinin/jmeter that referenced this pull request Mar 11, 2021

Mention PR 544 in changes

8bab174

Sorry for missing this earlier, Graham Russell. Relates to apache#544

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add BackendListener that sends "raw" results to InfluxDB #544

Add BackendListener that sends "raw" results to InfluxDB #544

ham1 commented Nov 9, 2019 •

edited

Loading

codecov-io commented Nov 9, 2019 •

edited

Loading

ham1 commented Mar 21, 2020

pmouawad left a comment

vlsi Mar 24, 2020

ham1 Sep 28, 2020

vlsi Mar 24, 2020

ham1 Mar 28, 2020

vlsi Oct 21, 2020

ham1 Nov 3, 2020

vlsi commented Mar 24, 2020

vlsi commented Mar 24, 2020 •

edited

Loading

pmouawad commented Mar 24, 2020

pmouawad commented Mar 24, 2020

vlsi commented Mar 24, 2020

vlsi commented Mar 24, 2020

pmouawad commented Mar 28, 2020

ham1 commented Mar 28, 2020 •

edited

Loading

pmouawad commented Sep 13, 2020

ham1 commented Sep 13, 2020

eostermueller commented Nov 24, 2020

ham1 commented Nov 24, 2020

sergiodegrazia commented Nov 25, 2021

Add BackendListener that sends "raw" results to InfluxDB #544

Add BackendListener that sends "raw" results to InfluxDB #544

Conversation

ham1 commented Nov 9, 2019 • edited Loading

Description

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

codecov-io commented Nov 9, 2019 • edited Loading

Codecov Report

ham1 commented Mar 21, 2020

pmouawad left a comment

Choose a reason for hiding this comment

vlsi Mar 24, 2020

Choose a reason for hiding this comment

ham1 Sep 28, 2020

Choose a reason for hiding this comment

vlsi Mar 24, 2020

Choose a reason for hiding this comment

ham1 Mar 28, 2020

Choose a reason for hiding this comment

vlsi Oct 21, 2020

Choose a reason for hiding this comment

ham1 Nov 3, 2020

Choose a reason for hiding this comment

vlsi commented Mar 24, 2020

vlsi commented Mar 24, 2020 • edited Loading

pmouawad commented Mar 24, 2020

pmouawad commented Mar 24, 2020

vlsi commented Mar 24, 2020

vlsi commented Mar 24, 2020

pmouawad commented Mar 28, 2020

ham1 commented Mar 28, 2020 • edited Loading

pmouawad commented Sep 13, 2020

ham1 commented Sep 13, 2020

eostermueller commented Nov 24, 2020

ham1 commented Nov 24, 2020

sergiodegrazia commented Nov 25, 2021

ham1 commented Nov 9, 2019 •

edited

Loading

codecov-io commented Nov 9, 2019 •

edited

Loading

vlsi commented Mar 24, 2020 •

edited

Loading

ham1 commented Mar 28, 2020 •

edited

Loading