Skip to content

Add escaping for new lines in AbstractInfluxdbMetricsSender#645

Closed
dgetzlaf wants to merge 2 commits intoapache:masterfrom
dgetzlaf:fix-linebreaks-in-influxdb
Closed

Add escaping for new lines in AbstractInfluxdbMetricsSender#645
dgetzlaf wants to merge 2 commits intoapache:masterfrom
dgetzlaf:fix-linebreaks-in-influxdb

Conversation

@dgetzlaf
Copy link
Copy Markdown
Contributor

Description

Add escaping for new lines in AbstractInfluxdbMetricsSender.tagToStringValue()

Motivation and Context

For load- and performance testing (distributed testing) we send metrics to the InfluxDB.
From time to time, no metrics occur in the dashboard while testing:
Missing metrics

In the log file of the JMeter controller, I find this line pretty often:

ERROR o.a.j.v.b.i.HttpMetricsSender: Error writing metrics to influxDB Url: https://<URL>, responseCode: 400, responseBody: {"code":"invalid","message":"unable to parse 'jmeter,application=<application name>,transaction=<transaction name>,responseCode=500,responseMessage=javax.script.ScriptException:\\ org.openqa.selenium.ElementClickInterceptedException:\\ element\\ click\\ intercepted:\\ Element\\ \u003cbutton\\ type\\=\"submit\"\\ class\\=\"btn\\ btn-primary\\ btn-next\"\u003e...\u003c/button\u003e\\ is\\ not\\ clickable\\ at\\ point\\ (313\\,\\ 620).\\ Other\\ element\\ would\\ receive\\ the\\ click:\\ \u003cdiv\\ class\\=\"dhl\\ redesign\\ frankieren-loading-overlay\"\u003e...\u003c/div\u003e': missing fields\nunable to parse '\\ \\ (Session\\ info:\\ headless\\ chrome\\=88.0.4324.150)': missing fields\nunable to parse 'Build\\ info:\\ version:\\ '3.14.0'\\,\\ revision:\\ 'aacccce0'\\,\\ time:\\ '2018-08-02T20:19:58.91Z'': missing fields\nunable to parse 'System\\ info:\\ host:\\ 'ip-172-31-45-153.internal'\\,\\ ip:\\ '172.31.45.153'\\,\\ os.name:\\ 'Linux'\\,\\ os.arch:\\ 'amd64'\\,\\ os.version:\\ '4.14.214-160.339.amzn2.x86_64'\\,\\ java.version:\\ '11.0.7'': missing fields\nunable to parse 'Driver\\ info:\\ org.openqa.selenium.chrome.ChromeDriver': missing fields\nunable to parse 'Capabilities\\ {acceptInsecureCerts:\\ false\\,\\ browserName:\\ chrome\\,\\ browserVersion:\\ 88.0.4324.150\\,\\ chrome:\\ {chromedriverVersion:\\ 88.0.4324.96\\ (68dba2d8a0b14...\\,\\ userDataDir:\\ /tmp/.com.google.Chrome.L3v1yY}\\,\\ goog:chromeOptions:\\ {debuggerAddress:\\ localhost:46619}\\,\\ javascriptEnabled:\\ true\\,\\ networkConnectionEnabled:\\ false\\,\\ pageLoadStrategy:\\ normal\\,\\ platform:\\ LINUX\\,\\ platformName:\\ LINUX\\,\\ proxy:\\ Proxy(manual\\,\\ http\\=localhos...\\,\\ setWindowRect:\\ true\\,\\ strictFileInteractability:\\ false\\,\\ timeouts:\\ {implicit:\\ 0\\,\\ pageLoad:\\ 300000\\,\\ script:\\ 30000}\\,\\ unhandledPromptBehavior:\\ dismiss\\ and\\ notify\\,\\ webauthn:extension:largeBlob:\\ true\\,\\ webauthn:virtualAuthenticators:\\ true}': missing fields"}

It seems that the escaping of the line breaks is not working properly. By that, invalid requests are sent to the InfluxDB, resulting in 'missing fields'.

How Has This Been Tested?

  1. Configure InfluxDB connection
  2. Add a new JSR223 with the body
throw new Exception('''Line 0
Line 1
Line 2
End
''')

Current JMeter 5.4.1 Release:
3. Looking at the console output / jmeter log file --> The error occurs
4. influxDB : No metrics received

JMeter 5.4.1 Release + this fix:
3. Looking at the console output / jmeter log file --> No error occurs
4. influxDB : metrics are received

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

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

@codecov-io
Copy link
Copy Markdown

Codecov Report

Merging #645 (429d67f) into master (b89c454) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #645      +/-   ##
============================================
+ Coverage     55.43%   55.46%   +0.02%     
- Complexity    10157    10180      +23     
============================================
  Files          1044     1046       +2     
  Lines         64082    64151      +69     
  Branches       7250     7265      +15     
============================================
+ Hits          35527    35579      +52     
- Misses        26048    26069      +21     
+ Partials       2507     2503       -4     
Impacted Files Coverage Δ Complexity Δ
...ackend/influxdb/AbstractInfluxdbMetricsSender.java 100.00% <100.00%> (ø) 3.00 <1.00> (ø)
...ter/protocol/http/proxy/SamplerCreatorFactory.java 27.90% <0.00%> (-13.96%) 4.00% <0.00%> (-1.00%)
.../apache/jmeter/protocol/http/util/HTTPFileArg.java 87.93% <0.00%> (-2.55%) 25.00% <0.00%> (+6.00%) ⬇️
...n/java/org/apache/jmeter/sampler/DebugSampler.java 77.27% <0.00%> (-1.46%) 13.00% <0.00%> (-1.00%)
...rg/apache/jmeter/extractor/DebugPostProcessor.java 80.00% <0.00%> (-0.96%) 14.00% <0.00%> (-1.00%)
...n/java/org/apache/jmeter/reporters/Summariser.java 85.49% <0.00%> (-0.77%) 18.00% <0.00%> (-1.00%)
...ter/protocol/http/proxy/DefaultSamplerCreator.java 54.12% <0.00%> (-0.52%) 28.00% <0.00%> (-1.00%)
...in/java/org/apache/jmeter/save/CSVSaveService.java 44.53% <0.00%> (-0.05%) 61.00% <0.00%> (+1.00%) ⬇️
...pache/jmeter/protocol/http/sampler/PostWriter.java 86.70% <0.00%> (ø) 31.00% <0.00%> (-1.00%)
...ache/jmeter/protocol/http/sampler/HTTPHC4Impl.java 60.46% <0.00%> (ø) 89.00% <0.00%> (-1.00%)
... and 5 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 b89c454...429d67f. Read the comment docs.

.replaceAll(" ", "\\\\ ")
.replaceAll(",", "\\\\,")
.replaceAll("=", "\\\\=")
.replaceAll("\n", "\\\\n");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks ok to me, but while we are here, I believe we should use replace as replaceAll is only needed when using a regex as the pattern to find (it should also perform better).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ham1 thanks for your feedback. I've updated this code block according to your recommendation.

@asfgit asfgit closed this in 2c2f611 Feb 27, 2021
@FSchumacher
Copy link
Copy Markdown
Contributor

Thanks for the PR. Could you test next nightly or build from trunk and report back, if it solves your problem?

kkalinin pushed a commit to kkalinin/jmeter that referenced this pull request Mar 11, 2021
Contributed by David Getzlaff (david.getzlaff at t-systems.com>)

Closes apache#645 on github
@dgetzlaf
Copy link
Copy Markdown
Contributor Author

Hi @FSchumacher,
I'm sorry for the late answer. I've just verified this fix with the SNAPSHOT from https://ci-builds.apache.org/job/JMeter/job/JMeter-trunk/266/

@coeyc
Copy link
Copy Markdown

coeyc commented Dec 6, 2021

has this fix made its way into a downloadable build of JMeter?

@dgetzlaf
Copy link
Copy Markdown
Contributor Author

dgetzlaf commented Dec 6, 2021

@coeyc this fix was approved and merged. Sadly JMeter wasn't released yet 😿
Maybe you can use a Nightly build instead https://ci-builds.apache.org/job/JMeter/job/JMeter-trunk/

@vlsi
Copy link
Copy Markdown
Collaborator

vlsi commented Dec 6, 2021

AFAIK it should be included in v5.4.1+. Am I mistaken?

@vlsi
Copy link
Copy Markdown
Collaborator

vlsi commented Dec 6, 2021

Oh, there were no releases since the fix has been merged. That is quite some time now.

@coeyc
Copy link
Copy Markdown

coeyc commented Dec 6, 2021

so, i guess the next build, we will look at the nightly build

@vlsi
Copy link
Copy Markdown
Collaborator

vlsi commented Dec 6, 2021

Just in case, the nightly would include Open Model Thread Group, and I would be glad to hear your feedback :)

@dgetzlaf
Copy link
Copy Markdown
Contributor Author

dgetzlaf commented Dec 6, 2021

@vlsi do you know someone, who could initiate a JMeter 5.5 release?

Your Open Model Thread Group looks good to me; sadly I had no chance in testing it yet.

@vlsi
Copy link
Copy Markdown
Collaborator

vlsi commented Dec 6, 2021

A mail to dev@jmeter.apache.org would do

@dgetzlaf dgetzlaf deleted the fix-linebreaks-in-influxdb branch March 16, 2023 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants