Improvements and fixes in the SQL trace listener functionality #25088
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
avpinchuk
reviewed
Aug 12, 2024
| * @return Stack trace element that represents a call to a server component that triggered SQL execution | ||
| */ | ||
| public Optional<StackTraceElement> getCallingApplicationMethod() { | ||
| return Arrays.stream(Thread.currentThread().getStackTrace()) |
There was a problem hiding this comment.
Why not use StackWalker API with the RETAIN_CLASS_REFERENCE option?
There was a problem hiding this comment.
Thank you for the suggestion, I forgot about this API. I redesigned my solution using StackWalker and moved the computation outside of the SQLTraceListener. It means that it will be computed even if the listener doesn't need it. I think it's OK because now the computation is more efficient. It's still computed only if there is at least 1 listener.
Fixed poolName. Added applicationName, moduleName, sqlQuery.
OndroMih
force-pushed
the
ondromih-fix-sql-tracing
branch
from
52b988d
to
6cda551
Compare
…ationMethod - use StackWalker - eagerly find application method and set it to SQLTraceRecord (only if there are some listeners) This should be efficient enough and leads to a simpler design.
And call it from the test.
OndroMih
force-pushed
the
ondromih-fix-sql-tracing
branch
from
f2d136d
to
212540a
Compare
dmatej
reviewed
Aug 19, 2024
nucleus/common/glassfish-api/src/main/java/org/glassfish/api/jdbc/SQLTraceRecord.java
Outdated
Show resolved
Hide resolved
...onnectors-runtime/src/main/java/com/sun/enterprise/connectors/ActiveResourceAdapterImpl.java
Outdated
Show resolved
Hide resolved
dmatej
approved these changes
Aug 19, 2024
avpinchuk
approved these changes
Aug 19, 2024
|
The build is failing because of some failures unrelated to the changes in this PR. |
hs536
approved these changes
Aug 21, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes a few issues and improves the SQL trace listener feature.
Just to remind, the SQL listener feature allows to implement the
SQLTraceListenerfrom GlassFish API, package it into a JAR, drop the JAR to domain/lib directory, and configure a connection pool to trigger this listener when SQL is executed. This allows to execute a custom Java code which gets info about SQL executed by the connection pool.Fixes:
SQLTraceRecordreceived by a listener always returnednullfromgetPoolName()Improvements:
getSqlQuery()method toSQLTraceRecord- returns the executed SQL query if a query was executed (some JDBC driver methods don't execute a query)getCallingApplicationMethod()method toSQLTraceRecord- returns the info about the last method in the application that triggered the queryAdds test that verify the behavior and also demonstrate usage of the SQL trace listener feature.