-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Fix InfluxDBRawBackendListenerClient missing data #5785
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
base: master
Are you sure you want to change the base?
Conversation
LGTM :) Thanks for investigating and fixing this issue! |
Looks good to me, but one question to those using influx. Is the change backwards compatible, or has everyone to change their setup afterwards? |
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!) |
PR has been merged. |
The change should be backwards compatible, no need to modify the setup. |
@@ -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()); |
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.
This might be time-consuming.
Do you really need a thread name?
Would it be workable if we included a "unique thread id"?
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.
Sure, if the thread ID is unique at the Test Plan level, it should work.
Description
This PR includes a new tag
threadName
for InfluxDBRawBackendListenerClient. This allows InfluxDB to insert multiple entries with the sametimestamp
but with differentthreadName
.Motivation and Context
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:
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 sametimestamp
,status
andtransaction
values.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: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?
Ran multiple tests after making the changes with over 3000 requests, did not lose any data this time.
Types of changes
Checklist: