quarkus icon indicating copy to clipboard operation
quarkus copied to clipboard

Add access log handler to the main router if non-app root-path is different from root-path

Open xstefank opened this issue 2 years ago • 4 comments

Fixes #26782

I wanted to write a test but in test mode the framework router is not created and everything is under HTTP root router so I think that adding an if case for test mode would be overkill.

xstefank avatar Jul 18 '22 11:07 xstefank

Thanks for your pull request!

The title of your pull request does not follow our editorial rules. Could you have a look?

  • title should not end up with ellipsis (make sure the title is complete)

This message is automatically generated by a bot.

quarkus-bot[bot] avatar Jul 18 '22 11:07 quarkus-bot[bot]

@ebullient could you have a look at this one? I think it makes sense but would prefer you having a look.

gsmet avatar Aug 03 '22 08:08 gsmet

@ebullient @gsmet I did this over a morning coffee to have fresh eyes. So original was wrong IMO because the root router included in case both root path and non-root path was specified also 404s logs of any accesses, which is not something we have now and probably don't want. So I rather changed it to pull created framework router into the recorder and just add the handler to the framework router if it's not mounted under the root path router. Here I did some more extensive testing - https://gist.github.com/xstefank/036ad52c5e40644ad1228915f22f5369 which shows the behavior of this PR, which I think is correct.

xstefank avatar Aug 10 '22 07:08 xstefank

Failing Jobs - Building 767eedab15fa679940a3c9641e01635fd9820686

Status Name Step Failures Logs Raw logs
Gradle Tests - JDK 11 Build Failures Logs Raw logs
:heavy_check_mark: Gradle Tests - JDK 11 Windows
:heavy_check_mark: JVM Tests - JDK 11
JVM Tests - JDK 11 Windows Build Failures Logs Raw logs
:heavy_check_mark: JVM Tests - JDK 17
:heavy_check_mark: JVM Tests - JDK 18

Full information is available in the Build summary check run.

Failures

:gear: Gradle Tests - JDK 11 #

- Failing: integration-tests/gradle 

:package: integration-tests/gradle

io.quarkus.gradle.devmode.CompositeBuildWithDependenciesDevModeTest.main line 24 - More details - Source on GitHub

org.awaitility.core.ConditionTimeoutException: Condition with lambda expression in io.quarkus.test.devmode.util.DevModeTestUtils that uses java.util.function.Supplier, java.util.function.Supplierjava.util.concurrent.atomic.AtomicReference, java.util.concurrent.atomic.AtomicReferencejava.lang.String, java.lang.Stringboolean was not fulfilled within 1 minutes.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)

io.quarkus.gradle.devmode.DotEnvQuarkusDevModeConfigurationTest.main line 13 - More details - Source on GitHub

org.awaitility.core.ConditionTimeoutException: Condition with lambda expression in io.quarkus.test.devmode.util.DevModeTestUtils that uses java.util.function.Supplier, java.util.function.Supplierjava.util.concurrent.atomic.AtomicReference, java.util.concurrent.atomic.AtomicReferencejava.lang.String, java.lang.Stringboolean was not fulfilled within 1 minutes.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)

:gear: JVM Tests - JDK 11 Windows #

- Failing: extensions/vertx-http/deployment 
! Skipped: extensions/agroal/deployment extensions/amazon-lambda-http/deployment extensions/amazon-lambda-rest/deployment and 309 more

:package: extensions/vertx-http/deployment

io.quarkus.vertx.http.http2.Http2Test.testHttp2EnabledSsl line 57 - More details - Source on GitHub

java.util.concurrent.ExecutionException: javax.net.ssl.SSLHandshakeException: Failed to create SSL connection
	at java.base/java.util.concurrent.CompletableFuture.reportGet(CompletableFuture.java:395)
	at java.base/java.util.concurrent.CompletableFuture.get(CompletableFuture.java:1999)

quarkus-bot[bot] avatar Aug 10 '22 11:08 quarkus-bot[bot]