You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jmeter.apache.org by "vperaltac (via GitHub)" <gi...@apache.org> on 2023/03/06 00:07:50 UTC
[GitHub] [jmeter] vperaltac opened a new pull request, #5785: Fix InfluxDBRawBackendListenerClient missing data
vperaltac opened a new pull request, #5785:
URL: https://github.com/apache/jmeter/pull/5785
## Description
<!--- Provide a general summary of your changes in the Title above -->
<!--- Describe your changes in detail here -->
This PR includes a new tag `threadName` for InfluxDBRawBackendListenerClient. This allows InfluxDB to insert multiple entries with the same `timestamp` but with different `threadName`.
## Motivation and Context
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here. -->
fixes #5654
There is currently an issue where Jmeter will send requests coming from multiple threads where the timestamp for those requests is the exact same, this causes InfluxDB to override that data and only keep 1 entry.
Here is an example with a test that runs 12 requests:
```
time connectTime duration status transaction ttfb
---- ----------- -------- ------ ----------- ----
1678045696972000000 56 72 ok user2 72
1678045698040000000 0 16 ok userlist 16
1678045698044000000 0 25 ok userlist 25
1678045699056000000 0 19 ok listresource 19
1678045699070000000 0 15 ok listresource 14
1678045700076000000 30 46 ok user2 46
1678045700085000000 34 50 ok user2 50
1678045701122000000 0 14 ok userlist 14
1678045701136000000 0 15 ok userlist 15
1678045702136000000 0 16 ok listresource 16
1678045702150000000 0 16 ok listresource 16
```
In this example there is 1 entry missing at the beginning, there should be 2 entries for the transaction `user2`. The reason why the second entry is not there is because both had the same `timestamp`.
After adding the tag `threadName`, InfluxDB will save both entries since they differ in the value for that tag. Notice how the time for the 2 first entries is the same:
```
time connectTime duration status threadName transaction ttfb
---- ----------- -------- ------ ---------- ----------- ----
1678046919362000000 39 55 ok ThreadGroup1-1 user2 55
1678046919362000000 36 59 ok ThreadGroup1-2 user2 59
1678046920418000000 0 19 ok ThreadGroup1-1 userlist 19
1678046920421000000 0 18 ok ThreadGroup1-2 userlist 18
1678046921438000000 0 18 ok ThreadGroup1-1 listresource 18
1678046921439000000 0 21 ok ThreadGroup1-2 listresource 21
1678046922457000000 38 53 ok ThreadGroup1-1 user2 53
1678046922461000000 37 56 ok ThreadGroup1-2 user2 56
1678046923511000000 0 17 ok ThreadGroup1-1 userlist 17
1678046923517000000 0 18 ok ThreadGroup1-2 userlist 18
1678046924528000000 0 26 ok ThreadGroup1-1 listresource 26
1678046924536000000 0 16 ok ThreadGroup1-2 listresource 16
```
This change removes any white spaces in the thread name, but given that this value can be set by the user, maybe this is not enough, I would appreciate some feedback here.
## How Has This Been Tested?
<!--- Please describe in detail how you tested your changes. -->
<!--- Include details of your testing environment, tests ran to see how -->
<!--- your change affects other areas of the code, etc. -->
Ran multiple tests after making the changes with over 3000 requests, did not lose any data this time.
## Types of changes
<!--- What types of changes does your code introduce? Delete as appropriate -->
- Bug fix (non-breaking change which fixes an issue)
## Checklist:
<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->
- [x] My code follows the [code style][style-guide] of this project.
- [ ] 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.
To unsubscribe, e-mail: dev-unsubscribe@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] FSchumacher commented on pull request #5785: Fix InfluxDBRawBackendListenerClient missing data
Posted by "FSchumacher (via GitHub)" <gi...@apache.org>.
FSchumacher commented on PR #5785:
URL: https://github.com/apache/jmeter/pull/5785#issuecomment-1528820282
PR has been merged.
Can you test current trunk or next nightly?
--
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.
To unsubscribe, e-mail: dev-unsubscribe@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] vperaltac commented on a diff in pull request #5785: Fix InfluxDBRawBackendListenerClient missing data
Posted by "vperaltac (via GitHub)" <gi...@apache.org>.
vperaltac commented on code in PR #5785:
URL: https://github.com/apache/jmeter/pull/5785#discussion_r1202972376
##########
src/components/src/main/java/org/apache/jmeter/visualizers/backend/influxdb/InfluxDBRawBackendListenerClient.java:
##########
@@ -122,8 +122,10 @@ private String createTags(SampleResult sampleResult) {
// remove surrounding quotes and spaces from sample label
String label = StringUtils.strip(sampleResult.getSampleLabel(), "\" ");
String transaction = AbstractInfluxdbMetricsSender.tagToStringValue(label);
+ String threadName = StringUtils.deleteWhitespace(sampleResult.getThreadName());
Review Comment:
Sure, if the thread ID is unique at the Test Plan level, it should work.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] vlsi commented on a diff in pull request #5785: Fix InfluxDBRawBackendListenerClient missing data
Posted by "vlsi (via GitHub)" <gi...@apache.org>.
vlsi commented on code in PR #5785:
URL: https://github.com/apache/jmeter/pull/5785#discussion_r1198899521
##########
src/components/src/main/java/org/apache/jmeter/visualizers/backend/influxdb/InfluxDBRawBackendListenerClient.java:
##########
@@ -122,8 +122,10 @@ private String createTags(SampleResult sampleResult) {
// remove surrounding quotes and spaces from sample label
String label = StringUtils.strip(sampleResult.getSampleLabel(), "\" ");
String transaction = AbstractInfluxdbMetricsSender.tagToStringValue(label);
+ String threadName = StringUtils.deleteWhitespace(sampleResult.getThreadName());
Review Comment:
This might be time-consuming.
Do you really need a thread name?
Would it be workable if we included a "unique thread id"?
--
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.
To unsubscribe, e-mail: dev-unsubscribe@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] ham1 commented on pull request #5785: Fix InfluxDBRawBackendListenerClient missing data
Posted by "ham1 (via GitHub)" <gi...@apache.org>.
ham1 commented on PR #5785:
URL: https://github.com/apache/jmeter/pull/5785#issuecomment-1519098326
I think the only implication is that a run will now use slightly more storage due to adding a thread tag (but also by not throwing away data points!)
--
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.
To unsubscribe, e-mail: dev-unsubscribe@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] ham1 commented on pull request #5785: Fix InfluxDBRawBackendListenerClient missing data
Posted by "ham1 (via GitHub)" <gi...@apache.org>.
ham1 commented on PR #5785:
URL: https://github.com/apache/jmeter/pull/5785#issuecomment-1463090933
LGTM :) Thanks for investigating and fixing this issue!
--
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.
To unsubscribe, e-mail: dev-unsubscribe@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] vperaltac commented on pull request #5785: Fix InfluxDBRawBackendListenerClient missing data
Posted by "vperaltac (via GitHub)" <gi...@apache.org>.
vperaltac commented on PR #5785:
URL: https://github.com/apache/jmeter/pull/5785#issuecomment-1548638631
The change should be backwards compatible, no need to modify the setup.
@FSchumacher I just tested the latest trunk and it works great!
--
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.
To unsubscribe, e-mail: dev-unsubscribe@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] FSchumacher commented on pull request #5785: Fix InfluxDBRawBackendListenerClient missing data
Posted by "FSchumacher (via GitHub)" <gi...@apache.org>.
FSchumacher commented on PR #5785:
URL: https://github.com/apache/jmeter/pull/5785#issuecomment-1519026526
Looks good to me, but one question to those using influx. Is the change backwards compatible, or has everyone to change their setup afterwards?
--
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.
To unsubscribe, e-mail: dev-unsubscribe@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org