Improve logging when jdbc is shaded #1093
Conversation
Signed-off-by: Vikrant Puppala <[email protected]>
There was a problem hiding this comment.
Pull Request Overview
This PR enhances logging behavior in the JDBC driver by improving package prefix detection when the JAR is shaded (i.e., when packages are relocated during the build process). The changes enable the logger to automatically detect and use the correct package prefix, whether the driver is used in its original or shaded form.
Key Changes:
- Added auto-detection logic to determine the actual package prefix at runtime by inspecting the
JulLoggerclass's package name - Implemented shaded package detection for both main JDBC classes and driver classes
- Added comprehensive test coverage for package prefix detection in both shaded and unshaded scenarios
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/main/java/com/databricks/jdbc/log/JulLogger.java | Added auto-detection logic for package prefixes to handle shaded JARs, including methods to extract shading prefixes and apply them to both JDBC and driver packages |
| src/test/java/com/databricks/jdbc/log/JulLoggerTest.java | Added three new test methods to verify package prefix detection works correctly for both shaded and unshaded JARs, and that logger configuration applies to the correct package hierarchy |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Auto-detect the actual package prefix by using the current class | ||
| // This handles shaded JARs where the package might be relocated | ||
| String actualPackageName = JulLogger.class.getPackage().getName(); | ||
| // Extract the root prefix (e.g., "jdbc.shaded.v1.0.12.OSS.com.databricks.jdbc" |
There was a problem hiding this comment.
The comment shows an example shaded package path but has an inconsistent format with a period at the start. The example should be: shaded.jdbc.v1.0.12.OSS.com.databricks.jdbc or clarify if 'jdbc.' is intentional as part of the shading prefix.
| // Extract the root prefix (e.g., "jdbc.shaded.v1.0.12.OSS.com.databricks.jdbc" | |
| // Extract the root prefix (e.g., "shaded.jdbc.v1.0.12.OSS.com.databricks.jdbc" |
| actualPackageName, packagePrefix)); | ||
|
|
||
| // If the jar is shaded, the prefix should include the shading prefix | ||
| if (actualPackageName.contains(".") && !actualPackageName.startsWith("com.databricks.jdbc")) { |
There was a problem hiding this comment.
The condition actualPackageName.contains(\".\") is redundant since Java package names always contain periods. This check should be removed, leaving only the !actualPackageName.startsWith(\"com.databricks.jdbc\") condition.
| if (actualPackageName.contains(".") && !actualPackageName.startsWith("com.databricks.jdbc")) { | |
| if (!actualPackageName.startsWith("com.databricks.jdbc")) { |
samikshya-db
left a comment
samikshya-db
left a comment
There was a problem hiding this comment.
I have always needed this, thanks.
2 participants
Description
Improve logging when jdbc is shaded
Testing
Unit tests + manually in benchmarking
Additional Notes to the Reviewer