Fix missing application logs in Spring Boot services#553
Draft
dushyantk1509 wants to merge 2 commits intolinkedin:mainfrom
Draft
Fix missing application logs in Spring Boot services#553dushyantk1509 wants to merge 2 commits intolinkedin:mainfrom
dushyantk1509 wants to merge 2 commits intolinkedin:mainfrom
Conversation
Iceberg 1.5.x pulls slf4j-api 2.x onto the runtime classpath, but the log4j2 starter still ships the SLF4J 1.x-style log4j-slf4j-impl and Hadoop transitively brings slf4j-log4j12. SLF4J 2.x discovers bindings via ServiceLoader and ignores 1.x bindings, so it silently defaults to NOP - every LoggerFactory.getLogger(...) call in application code was dropped in the docker setup (Spring Boot's own startup lines still appeared because they bypass SLF4J). Exclude the two 1.x bindings in the shared springboot convention and add log4j-slf4j2-impl, which implements SLF4JServiceProvider. Verified end-to-end with './gradlew dockerUp -Precipe=oh-only': creating a table now surfaces OpenHouseInternalCatalog, DefaultStorageSelector, MetricsAspect, and request audit logs that were previously silent. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The previous pin at 2.20.0 made log4j-slf4j2-impl the highest explicit requester for log4j-core, pulling the whole log4j family down from 2.25.4 (the version that was shipping pre-fix, selected via the [2.17.1, 3[ CVE constraint) to 2.20.0. Functionally safe - all critical Log4Shell-class CVEs are fixed in 2.17.1 - but it's a needless 5-minor downgrade. Bumping the pin to 2.25.4 keeps log4j-core at 2.25.4, identical to the pre-fix classpath state. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
17 tasks
Collaborator
|
Thanks for looking at this. #511 overlaps with this PR and extends it a bit further. It addresses same root cause (SLF4J 2.x ignoring the 1.x-style bindings → NOP fallback), applies same core fix (exclude log4j-slf4j-impl / slf4j-log4j12, add log4j-slf4j2-impl). Bit on top of that, #511 also:
Mind taking a look at #511? If it looks good to you, we can merge that one and close this in its favor. |
This file contains hidden or 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
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.
Summary
Application-level logs from the
tables,housetables, andjobsSpring Boot services are silently dropped in the Docker recipes (oh-only,oh-hadoop,oh-hadoop-spark). Only Spring Boot's own banner and a handful of early startup lines reach stdout; anything logged viaLoggerFactory.getLogger(...)in application code (controllers, catalog, metrics aspect, audit, etc.) disappears. This PR restores those logs.Root cause
Iceberg 1.5.x (
com.linkedin.iceberg:iceberg-data:1.5.2.10) pullsslf4j-apiup to 2.x on the runtime classpath. SLF4J 2.x discovers bindings viaServiceLoader(org.slf4j.spi.SLF4JServiceProvider) and ignores the SLF4J 1.x-style bindings that were already packaged:log4j-slf4j-impl:2.13.3— viaspring-boot-starter-log4j2:2.3.4.RELEASEslf4j-log4j12:1.7.25— transitively via HadoopWith no 2.x-compatible provider on the classpath, SLF4J falls back to the NOP logger. The only clue in
docker logsis a handful of lines:Spring Boot's own early startup lines still appear because they bypass SLF4J, which is why the issue isn't obvious at first glance.
Fix
In the shared
buildSrc/src/main/groovy/openhouse.springboot-conventions.gradle(applied by all three Spring Boot services and byiceberg/openhouse/internalcatalog/htscatalog):log4j-slf4j-implslf4j-log4j12org.apache.logging.log4j:log4j-slf4j2-impl:2.25.4, which implementsSLF4JServiceProviderand is discovered by SLF4J 2.x.The version is pinned to
2.25.4to match whatlog4j-corewas already resolving to via the existing CVE-driven[2.17.1, 3[constraint. This keeps the full log4j family at exactly 2.25.4 — no downgrade from the pre-fix classpath.Packaged logging jars (bootJar diff)
Before:
After:
Changes
For all the boxes checked, please include additional details of the changes made in this pull request.
Testing Done
Before: the request returns 201, but
docker logsshows nothing from the request path — noOpenHouseInternalCatalog, noDefaultStorageSelector, noMetricsAspecttimings, no audit or request-payload JSON. Same NOP warning block as above inopenhouse-housetablestoo.After: the same request produces the expected trace, e.g.:
For all the boxes checked, include a detailed description of the testing done for the changes made in this pull request.
Additional Information
For all the boxes checked, include additional details of the changes made in this pull request.