Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Replace LogManager.getLogger() with @Slf4j #2036

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Replace LogManager.getLogger() with @Slf4j #2036

wants to merge 2 commits into from

Conversation

ideal3147
Copy link

Issue Number

Purpose

Flexibility – @slf4j is a logging abstraction that allows switching between different logging frameworks (e.g., Log4j2, Logback) without modifying the code.
Future-Proofing – Unlike @log4j, which tightly couples our code to Log4j, @slf4j ensures we can easily switch to another backend if needed.
Less Boilerplate – @slf4j automatically generates the logger instance, removing the need for explicit declarations like LogManager.getLogger().
Best Practices – Many modern Java projects prefer @slf4j as it integrates well with different logging backends and provides better adaptability.

Technical Details

  • Replace all occurrences of private final Logger logger = LogManager.getLogger(); with @slf4j annotation.

@ideal3147 ideal3147 requested a review from a team as a code owner February 23, 2025 06:23
@ideal3147 ideal3147 requested review from nya-elimu, eymaal and alexander-kuruvilla and removed request for a team February 23, 2025 06:23
@codecov Codecov
Copy link

codecov bot commented Feb 23, 2025

Codecov Report

Attention: Patch coverage is 25.40905% with 775 lines in your changes missing coverage. Please review.

Project coverage is 14.52%. Comparing base (2d8231d) to head (b6136ba).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...t/storybook/StoryBookCreateFromEPubController.java 1.88% 52 Missing ⚠️
.../ai/elimu/util/csv/CsvContentExtractionHelper.java 0.00% 50 Missing ⚠️
...n/java/ai/elimu/util/db/DbContentImportHelper.java 0.00% 26 Missing ⚠️
...main/java/ai/elimu/web/SignOnControllerGitHub.java 4.34% 22 Missing ⚠️
.../analytics/LetterLearningEventsRestController.java 5.00% 19 Missing ⚠️
...v2/analytics/WordLearningEventsRestController.java 5.00% 19 Missing ⚠️
...ain/java/ai/elimu/web/SignOnControllerDiscord.java 5.00% 19 Missing ⚠️
.../content/multimedia/audio/AudioEditController.java 5.00% 19 Missing ⚠️
...alytics/StoryBookLearningEventsRestController.java 5.26% 18 Missing ⚠️
.../content/multimedia/image/ImageEditController.java 5.55% 17 Missing ⚠️
... and 121 more
Additional details and impacted files 
@@             Coverage Diff              @@
##               main    #2036      +/-   ##
============================================
+ Coverage     13.15%   14.52%   +1.37%     
- Complexity      301      418     +117     
============================================
  Files           256      256              
  Lines          7049     7162     +113     
  Branches        798      798              
============================================
+ Hits            927     1040     +113     
  Misses         6072     6072              
  Partials         50       50

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jo-elimu
Copy link
Member

jo-elimu commented Feb 23, 2025 •
edited
Loading

@ideal3147 When I run this branch on localhost, I'm no longer seeing any log output in the console. No output beyond the initial Jetty launch:

[INFO] Started ServerConnector@7f8a5a52{HTTP/1.1, (http/1.1)}{0.0.0.0:8080}
[INFO] Started Server@2382b2f{STARTING}[11.0.24,sto=0] @68728ms
[INFO] Automatic redeployment disabled, see 'mvn jetty:help' for more redeployment options

Is this expected behavior? Are you seeing any log output when you run the webapp on your computer?

Note that the Log4j configuration relied on a config file at https://github.com/elimu-ai/webapp/blob/main/src/main/resources/log4j2.xml

And the production servers used a different config file: https://github.com/elimu-ai/webapp/blob/main/src/main/resources/log4j2_PROD.xml

This is so that we could store a daily log file on each server, e.g:

.
├── lang-ENG
│   ├── analytics
│   ├── backups
│   └── logs
├── lang-HIN
│   ├── analytics
│   ├── backups
│   └── logs

# ls -alsh ~/.elimu-ai/lang-HIN/logs/
total 1.7G
 16K drwxr-xr-x 2 root root  12K Feb  2 00:00 .
   0 drwxr-xr-x 5 root root   50 Jan 27 15:41 ..
8.4M -rw-r--r-- 1 root root 8.4M Feb  2 13:40 webapp.log
 19M -rw-r--r-- 1 root root  19M Aug 23  2024 webapp.log.2024-08-22
 49M -rw-r--r-- 1 root root  49M Aug 24  2024 webapp.log.2024-08-23
 41M -rw-r--r-- 1 root root  41M Aug 25 03:56 webapp.log.2024-08-24
.
.
.

With the new SLF4J setup, how can we store log output in a different file per language? For example:

~/.elimu-ai/lang-ENG/logs/webapp.log
~/.elimu-ai/lang-HIN/logs/webapp.log

jo-elimu
jo-elimu requested changes Feb 23, 2025
View reviewed changes
Copy link
Member

@jo-elimu jo-elimu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes look good to me. But we also need to figure out how to handle the existing Log4j configurations:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jo-elimu jo-elimu jo-elimu requested changes

@nya-elimu nya-elimu Awaiting requested review from nya-elimu nya-elimu is a code owner automatically assigned from elimu-ai/codeowners-web

@eymaal eymaal Awaiting requested review from eymaal eymaal is a code owner automatically assigned from elimu-ai/codeowners-web

@alexander-kuruvilla alexander-kuruvilla Awaiting requested review from alexander-kuruvilla alexander-kuruvilla is a code owner automatically assigned from elimu-ai/codeowners-web

@AshishBagdane AshishBagdane Awaiting requested review from AshishBagdane

@Souvik-Cyclic Souvik-Cyclic Awaiting requested review from Souvik-Cyclic

Assignees

@ideal3147 ideal3147

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Replace LogManager.getLogger() with @Slf4j
2 participants
@ideal3147 @jo-elimu