feat(core): merge logging changes from gluu4 by yurem · Pull Request #12871 · JanssenProject/jans (original) (raw)

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

Adds a new Boolean configuration flag disableExternalLoggerConfiguration with getters/setters across multiple component AppConfiguration classes, adds corresponding isDisableExternalLoggerConfiguration() implementations in LoggerService classes, and refactors core LoggerService to support JSON/text layouts and external-config toggling.

Changes

Cohort / File(s) Summary
AppConfiguration additions jans-auth-server/model/src/main/java/io/jans/as/model/configuration/AppConfiguration.java, jans-config-api/common/src/main/java/io/jans/configapi/model/configuration/ApiAppConfiguration.java, jans-fido2/model/src/main/java/io/jans/fido2/model/conf/AppConfiguration.java, jans-keycloak-link/model/src/main/java/io/jans/keycloak/link/model/config/AppConfiguration.java, jans-link/model/src/main/java/io/jans/link/model/config/AppConfiguration.java, jans-lock/lock-server/model/src/main/java/io/jans/lock/model/config/AppConfiguration.java, jans-scim/model/src/main/java/io/jans/scim/model/conf/AppConfiguration.java Added private Boolean disableExternalLoggerConfiguration (defaults present in some modules), with getDisableExternalLoggerConfiguration() and setDisableExternalLoggerConfiguration(Boolean) and DocProperty metadata where applicable.
LoggerService implementations jans-auth-server/server/src/main/java/io/jans/as/server/service/logger/LoggerService.java, jans-config-api/server/src/main/java/io/jans/configapi/service/logger/LoggerService.java, jans-fido2/server/src/main/java/io/jans/fido2/service/shared/LoggerService.java, jans-keycloak-link/server/src/main/java/io/jans/keycloak/link/server/service/LoggerService.java, jans-link/server/src/main/java/io/jans/link/server/service/LoggerService.java, jans-lock/lock-server/server/src/main/java/io/jans/lock/server/service/LoggerService.java, jans-scim/service/src/main/java/io/jans/scim/service/logger/LoggerService.java Added overriding public boolean isDisableExternalLoggerConfiguration() returning a null-safe boolean based on the module's AppConfiguration.
Core LoggerService refactor jans-core/service/src/main/java/io/jans/service/logger/LoggerService.java Major refactor: added JSON/text layout constants and tracking (prevLogLoggingLayout), external-config gating (useExternalConfiguration), new/updated methods for applying logger configuration and appenders, improved external config path handling, and new abstract isDisableExternalLoggerConfiguration() method.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Description check ⚠️ Warning The PR description is largely incomplete and missing critical required sections from the template including implementation details, test documentation, and proper structure. Add required sections: implementation details explaining the logging service changes, confirmation of static code analysis, test updates for new functionality, and explicit documentation impact statement in the proper format.

✅ Passed checks (3 passed)

Check name Status Explanation
Title check ✅ Passed The PR title accurately describes the main objective of merging logging changes from gluu4, which aligns with the changes made across all modified files.
Linked Issues check ✅ Passed The PR implements both requirements from issue #12870: adds JSON logging layout support and provides disableExternalLoggerConfiguration property to disable dynamic log level updates across all modules.
Out of Scope Changes check ✅ Passed All changes are focused on adding the disableExternalLoggerConfiguration property and JSON logging layout support across modules, consistently implementing the merge objectives with no unrelated modifications.

📜 Recent review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3226aff and e02283d.

📒 Files selected for processing (6)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.