Simplify proxy selection for the REST Client #19261

gsmet · 2021-08-05T15:10:01Z

do not use regexp pattern matching for http.nonProxyHosts env variable
given it is not supposed to be a valid regexp (* is the wildcard)
give precedence to the builder configuration over the env variables
delegate the whole proxy resolution via env variables to ProxySelector

Fixes #19240
Fixes #18548

gsmet · 2021-08-05T15:10:49Z

@andrzejszywala any chance you could test this PR in your environment?

Just get this branch and you can follow the instructions here: https://github.com/quarkusio/quarkus/blob/main/CONTRIBUTING.md#building-main

quarkus-bot · 2021-08-05T18:24:02Z

This workflow status is outdated as a new workflow run has been triggered.

Failing Jobs - Building `529aba7`

Status	Name	Step	Test failures	Logs	Raw logs
✖	JVM Tests - JDK 11	`Build`	Test failures	Logs	Raw logs
✖	JVM Tests - JDK 11 Windows	`Build`	Test failures	Logs	Raw logs
✖	JVM Tests - JDK 16	`Build`	Test failures	Logs	Raw logs

Full information is available in the Build summary check run.

Test Failures

⚙️ JVM Tests - JDK 11 #

📦 extensions/scheduler/deployment

✖ io.quarkus.scheduler.test.metrics.MicrometerTimedTest.testTimedMethod line 51 - More details - Source on GitHub

io.micrometer.core.instrument.search.MeterNotFoundException: 
Unable to find a meter that matches all the requirements at once. Here's what was found:
   FAIL: No meter with name 'scheduled.methods' was found.
   OK: A meter with type timer was found.
   FAIL: No meters have the required tag 'method'.
   OK: A meter with name 'scheduled.methods' has a tag 'class' with value 'io.quarkus.scheduler.test.metrics.MicrometerTimedTest$Jobs'.
   OK: A meter with name 'scheduled.methods' has a tag 'exception' with value 'none'.
	at io.micrometer.core.instrument.search.MeterNotFoundException$FromRequiredSearch.build(MeterNotFoundException.java:228)
	at io.micrometer.core.instrument.search.MeterNotFoundException$FromRequiredSearch.access$100(MeterNotFoundException.java:46)
	at io.micrometer.core.instrument.search.MeterNotFoundException.forSearch(MeterNotFoundException.java:43)
	at io.micrometer.core.instrument.search.RequiredSearch.findOne(RequiredSearch.java:199)
	at io.micrometer.core.instru...

⚙️ JVM Tests - JDK 11 Windows #

📦 extensions/scheduler/deployment

✖ io.quarkus.scheduler.test.metrics.MicrometerTimedTest.testTimedMethod line 51 - More details - Source on GitHub

io.micrometer.core.instrument.search.MeterNotFoundException: 
Unable to find a meter that matches all the requirements at once. Here's what was found:
   FAIL: No meter with name 'scheduled.methods' was found.
   OK: Meters with type timer were found.
   OK: A meter with name 'scheduled.methods' has a tag 'method' with value 'everySecond'.
   OK: A meter with name 'scheduled.methods' has a tag 'class' with value 'io.quarkus.scheduler.test.metrics.MicrometerTimedTest$Jobs'.
   OK: A meter with name 'scheduled.methods' has a tag 'exception' with value 'none'.
	at io.micrometer.core.instrument.search.MeterNotFoundException$FromRequiredSearch.build(MeterNotFoundException.java:228)
	at io.micrometer.core.instrument.search.MeterNotFoundException$FromRequiredSearch.access$100(MeterNotFoundException.java:46)
	at io.micrometer.core.instrument.search.MeterNotFoundException.forSearch(MeterNotFoundException.java:43)
	at io.micrometer.core.instrument.search.RequiredSearch.findOne(RequiredSearch....

⚙️ JVM Tests - JDK 16 #

📦 extensions/scheduler/deployment

✖ io.quarkus.scheduler.test.metrics.MicrometerTimedTest.testTimedMethod line 51 - More details - Source on GitHub

io.micrometer.core.instrument.search.MeterNotFoundException: 
Unable to find a meter that matches all the requirements at once. Here's what was found:
   FAIL: No meter with name 'scheduled.methods' was found.
   FAIL: No meters with type timer were found.
   FAIL: No meters have the required tag 'method'.
   FAIL: No meters have the required tag 'class'.
   FAIL: No meters have the required tag 'exception'.
	at io.micrometer.core.instrument.search.MeterNotFoundException$FromRequiredSearch.build(MeterNotFoundException.java:228)
	at io.micrometer.core.instrument.search.MeterNotFoundException$FromRequiredSearch.access$100(MeterNotFoundException.java:46)
	at io.micrometer.core.instrument.search.MeterNotFoundException.forSearch(MeterNotFoundException.java:43)
	at io.micrometer.core.instrument.search.RequiredSearch.findOne(RequiredSearch.java:199)
	at io.micrometer.core.instrument.search.RequiredSearch.timer(RequiredSearch.java:122)
	at io.quarkus.scheduler.test.metrics.MicrometerTimedTe...

gsmet · 2021-08-05T18:26:04Z

@mkouba the failures above looks related to #19192 ?

mkouba · 2021-08-06T06:43:14Z

@mkouba the failures above looks related to #19192 ?

@gsmet Yes, it's related. Seems like a flaky test.

@ebullient Do you happen to know what exactly does the MeterNotFoundException message mean? I'm a bit confused. It says that there's no meter with name scheduled.methods but then it says "A meter with name scheduled.methods has a tag 'method' with value 'everySecond'."

FAIL: No meter with name 'scheduled.methods' was found.
OK: Meters with type timer were found.
OK: A meter with name 'scheduled.methods' has a tag 'method' with value 'everySecond'.
OK: A meter with name 'scheduled.methods' has a tag 'class' with value 'io.quarkus.scheduler.test.metrics.MicrometerTimedTest$Jobs'.

ebullient · 2021-08-06T12:20:55Z

@ebullient Do you happen to know what exactly does the MeterNotFoundException message mean? I'm a bit confused. It says that there's no meter with name scheduled.methods but then it says "A meter with name scheduled.methods has a tag 'method' with value 'everySecond'."
FAIL: No meter with name 'scheduled.methods' was found.
OK: Meters with type timer were found.
OK: A meter with name 'scheduled.methods' has a tag 'method' with value 'everySecond'.
OK: A meter with name 'scheduled.methods' has a tag 'class' with value 'io.quarkus.scheduler.test.metrics.MicrometerTimedTest$Jobs'.

Meters must have the same labels (class, method, ...). It looks like we somehow have two timers? That is a little odd.

I usually do a bit of waiting for meters to show up for testing..

quarkus/extensions/micrometer/deployment/src/test/java/io/quarkus/micrometer/deployment/binder/UriTagWithHttpRootTest.java

Line 66 in 0616e26

Util.waitForMeters(registry.find("http.server.requests").timers(), 5);

famod · 2021-08-06T23:29:19Z

@gsmet did you mean "Fixes #19240"?

Btw, I ran into a problem in the same code a few days ago because I had my nonProxyHosts (in MAVEN_OPTS) wrapped in double quotes. Without those, gradlew fails in the Quarkus build (sic!) due to the pipe separating 127.0.0.1 and localhost.
I'll try to test whether this PR changes anything. I'm less than sure that those quotes are even allowed, but the JDK parsing logic doesn't seem to mind.

- do not use regexp pattern matching for http.nonProxyHosts env variable given it is not supposed to be a valid regexp (* is the wildcard) - give precedence to the builder configuration over the env variables - delegate the whole proxy resolution via env variables to ProxySelector Fixes quarkusio#19240

gsmet · 2021-08-09T09:41:22Z

@famod yup, fixed.

quarkus-bot · 2021-08-09T10:59:11Z

Failing Jobs - Building `1e517c8`

Status	Name	Step	Test failures	Logs	Raw logs
✖	Gradle Tests - JDK 11 Windows	`Build`	⚠️ Check →	Logs	Raw logs

famod · 2021-08-16T21:47:00Z

Btw, this also seems to fix #18548 (in which @michalszynkiewicz and @radcortez discussed how to move forward).

michalszynkiewicz · 2021-08-17T08:21:22Z

@gsmet with your changes, is it still possible to set nonProxyHosts?

gsmet · 2021-08-18T13:23:32Z

@michalszynkiewicz the JDK proxy selector should take care of everything and consume the proxy system properties to determine what to do. The only thing I was a bit unsure of is that I changed the ordering slightly for something that sounds more logical to me... but I might have missed something.

gsmet requested a review from michalszynkiewicz August 5, 2021 15:10

gsmet force-pushed the fix-rest-client-proxy branch from 529aba7 to 1e517c8 Compare August 9, 2021 09:40

gsmet added triage/backport? labels Aug 18, 2021

michalszynkiewicz approved these changes Aug 19, 2021

View reviewed changes

gsmet merged commit 2ae8c51 into quarkusio:main Aug 19, 2021

quarkus-bot bot added this to the 2.3 - main milestone Aug 19, 2021

gsmet modified the milestones: 2.3 - main, 2.1.4.Final Aug 23, 2021

gsmet removed triage/backport-2.1? labels Aug 23, 2021

maxandersen mentioned this pull request Dec 22, 2021

REST Client Reactive proxy enhancements #22466

Closed

3 tasks

gsmet mentioned this pull request Apr 6, 2022

[DON'T MERGE] Quarkus changes to RestClientBuilderImpl resteasy/resteasy#3061

Closed

Simplify proxy selection for the REST Client #19261

Simplify proxy selection for the REST Client #19261

Uh oh!

Conversation

gsmet commented Aug 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gsmet commented Aug 5, 2021

Uh oh!

quarkus-bot bot commented Aug 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Failing Jobs - Building 529aba7

Test Failures

⚙️ JVM Tests - JDK 11 #

📦 extensions/scheduler/deployment

⚙️ JVM Tests - JDK 11 Windows #

📦 extensions/scheduler/deployment

⚙️ JVM Tests - JDK 16 #

📦 extensions/scheduler/deployment

Uh oh!

gsmet commented Aug 5, 2021

Uh oh!

mkouba commented Aug 6, 2021

Uh oh!

ebullient commented Aug 6, 2021

Uh oh!

famod commented Aug 6, 2021

Uh oh!

gsmet commented Aug 9, 2021

Uh oh!

quarkus-bot bot commented Aug 9, 2021

Failing Jobs - Building 1e517c8

Uh oh!

famod commented Aug 16, 2021

Uh oh!

michalszynkiewicz commented Aug 17, 2021

Uh oh!

gsmet commented Aug 18, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

gsmet commented Aug 5, 2021 •

edited

Loading

quarkus-bot bot commented Aug 5, 2021 •

edited

Loading

Failing Jobs - Building `529aba7`

Failing Jobs - Building `1e517c8`