-
Notifications
You must be signed in to change notification settings - Fork 3k
Logging with Panache: add support for method references of Log methods
#46479
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
Conversation
gastaldi
left a comment
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.
Nice!
core/deployment/src/main/java/io/quarkus/deployment/logging/LoggingWithPanacheProcessor.java
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
dmlloyd
left a comment
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.
LGTM, but let's make sure the test failures are unrelated.
In case the `Log` API is used in a lambda expression, everything already works, because a lambda expression body is compiled into a separate method and the `Log` API is invoked using classic `invokestatic` instruction. However, in case the `Log` API is used as a method reference (`Log::info`), it breaks, because the `LoggingWithPanacheProcessor` fails to transform that usage. This commit fixes that by looking for `invokedynamic` invocations of the `LambdaMetafactory` where the method handle belongs to `Log`. Such instructions are rewritten to delegate to the JBoss Logging instance, as usual.
5e4f1cc to
44b3ce4
Compare
|
Added a couple of comments and streamlined the code: instead of looking for the Also using this opportunity to rerun the failed tests, because those don't seem related on the first sight. If they fail again, I'll investigate properly :-) |
Status for workflow
|
In case the
LogAPI is used in a lambda expression, everything already works, because a lambda expression body is compiled into a separate method and theLogAPI is invoked using classicinvokestaticinstruction.However, in case the
LogAPI is used as a method reference (Log::info), it breaks, because theLoggingWithPanacheProcessorfails to transform that usage. This commit fixes that by looking forinvokedynamicinvocations of theLambdaMetafactorywhere the method handle belongs toLog. Such instructions are rewritten to delegate to the JBoss Logging instance, as usual.