datahub icon indicating copy to clipboard operation
datahub copied to clipboard

fix(logging): Remove lombok as source of slf4j-api

Open david-leifker opened this issue 2 years ago • 3 comments

Remove lombok as source of slf4j-api, convert to compileOnly where possible

Checklist

  • [ ] The PR conforms to DataHub's Contributing Guideline (particularly Commit Message Format)
  • [ ] Links to related issues (if applicable)
  • [ ] Tests for the changes have been added/updated (if applicable)
  • [ ] Docs related to the changes have been added/updated (if applicable). If a new feature has been added a Usage Guide has been added for the same.
  • [ ] For any breaking change/potential downtime/deprecation/big changes an entry has been made in Updating DataHub

david-leifker avatar Dec 02 '22 18:12 david-leifker

Unit Test Results (build & test)

621 tests  ±0   617 :heavy_check_mark: ±0   15m 41s :stopwatch: -6s 157 suites ±0       4 :zzz: ±0  157 files   ±0       0 :x: ±0 

Results for commit e1c6f0b3. ± Comparison against base commit 4876fdd0.

:recycle: This comment has been updated with latest results.

github-actions[bot] avatar Dec 02 '22 20:12 github-actions[bot]

Unit Test Results (metadata ingestion)

       8 files         8 suites   58m 23s :stopwatch:    766 tests    764 :heavy_check_mark: 2 :zzz: 0 :x: 1 534 runs  1 529 :heavy_check_mark: 5 :zzz: 0 :x:

Results for commit e1c6f0b3.

:recycle: This comment has been updated with latest results.

github-actions[bot] avatar Dec 02 '22 20:12 github-actions[bot]

Looks like tests are failing, probably need a testCompile dep on logback

RyanHolstien avatar Dec 02 '22 21:12 RyanHolstien

  1. removal of htrace, removed shaded log4j impl, which caused an slf4 forwarder to point to log4j forwarder (infinite loop)
  2. applied project wide transitive exclusions to directly pick versions and impl (logback)
  3. some upstream dependencies depend on the log4j-core, not just log4j-api, requires log4j-impl to compile
  4. slf4j unlike log4j doesn’t tolerate alt impls (or complains) or different impl versions, especially major ones
  5. lombok includes an slf4j-api version and we are incorrectly setting compile instead of compileOnly
  6. most of our modules are libraries and therefore should only bring in slf4j-api and not include an impl (exceptions frontend, war, consumers, etc). this leads to some runtimeOnly dependencies for library tests

david-leifker avatar Dec 05 '22 13:12 david-leifker