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.

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

[Refactor][Enhancement] Client logger module refactor and enhancement with SPI to load adapter #11956

Closed
6 tasks done
KomachiSion opened this issue Apr 12, 2024 · 0 comments
Closed
6 tasks done
Assignees
Labels
area/Client Related to Nacos Client SDK kind/refactor
Milestone

Comments

@KomachiSion
Copy link
Collaborator

KomachiSion commented Apr 12, 2024

Is your feature request related to a problem? Please describe.

Nacos client is capable of logging using different frameworks such as Logback or Log4j2 based on the user's choice. Nacos mainly handles sidecar traffic, not participating in the main business logic, and has some disaster recovery asynchronous tasks within the client. Therefore, the client's logs are expected to be printed to a designated location (defaulting to ${user.home}/logs/nacos/*) instead of mixing with the user's business logs.

In most cases, this functionality works correctly, but in some scenarios, such as when used with Spring Cloud Alibaba, logs often end up in the business logs. This issue occurs because the Nacos client only loads the logging configuration once by default, while Spring Cloud reconfigures logging multiple times during startup, which can overwrite the Nacos client's log configuration. Despite joint efforts with the Spring Cloud Alibaba community to address this issue, changes in the Spring Cloud framework often bring the problem back in new versions.

To resolve this, Nacos client version 2.1.0 adapted the Logback listener context, which allows Nacos log configurations to be automatically re-added to the context upon a reload of Logback's context. However, this adjustment relies on the versions and configurations of the logging framework itself, leading to potential incompatibility issues, such as with version 1.4 of Logback, which is not backward compatible with some of the previous inner APIs. To address this, the community created a logback-adapter project under the nacos-group to adapt to Logback versions 1.4.0 and above.

While this method is not very flexible and requires users to include an additional adapter.

Describe the solution you'd like
I am considering restructuring and redesigning the related parts of the Nacos client to enhance flexibility and convenience. The main changes are as follows:

  1. Integrate the adapter as an SPI (Service Provider Interface).
  2. Specify corresponding logging frameworks and versions for each adapter, set the logging framework to optional and provided scope in the POM, and include it in the main Nacos repository.
  3. Based on the Slf4j SPI logger implementation, select the appropriate adapter to monitor the context, or perform periodic checks (if the framework itself does not offer context monitoring) and re-add Nacos log configuration if it gets overwritten.
  4. The nacos-client will include all adapter implementations by default. Since the logging frameworks are set as optional and provided scope, there won't be any loading conflicts because it uses Slf4j's SPI to select which adapter to use.

Describe alternatives you've considered
The expected outcome of the redesign is:

  1. Users only need to incorporate their application framework without the need to add an additional adapter.
  2. Proactively monitor the logging context to determine if it contains Nacos' configuration, and if needed, load it back into the context; this reduces reliance on external implementations like Spring Cloud Alibaba.
  3. Provide quick adaptability to new versions and logging frameworks without affecting the adaptability of older versions.

Additional context

TODO List:

  • Add the new adapter SPI.
  • Refactor current Logback implementation in nacos-client to adapter SPI.
  • Move nacos-group/logback-adapter to nacos repo and set it as a module. (logback 1.3 upper need jdk11, client supported from jdk 1.8, if move to nacos repo, there is build problem)
  • Release nacos-group/logback-adapter with new version and new SPI, and nacos-client depend logback-adapter directly.
  • Refactor current log4j2 implementation in nacos-client to adapter SPI.
  • Make log4j2 adapter support auto listen context and auto re-add nacos log configuration.
@KomachiSion KomachiSion added kind/refactor area/Client Related to Nacos Client SDK labels Apr 12, 2024
@KomachiSion KomachiSion added this to the 2.4.0 milestone Apr 12, 2024
@KomachiSion KomachiSion self-assigned this Apr 12, 2024
KomachiSion added a commit that referenced this issue Apr 16, 2024
…ogger adapter. (#11964)

* Add NacosLoggingAdapter

* Refactor Logback Logging adapter to Nacos logging adapter.

* Refactor log4j2 Logging adapter to Nacos logging adapter.

* Remove useless abstract class

* Add and Adapt log4j2 impl to SPI.

* Remove useless method in NacosLogbackConfigurator

* Fix IT ClassNotFound problem
KomachiSion added a commit that referenced this issue Apr 23, 2024
…11996)

* Move some basic logging classes into common module.

* Move logback 12 version adapter to single module.

* For checkstyle.

* Enhance logger adapter pom.

* Refactor the adapter initialization logic to avoid spi loader error.

* Move log4j2 logger adapter to single module.

* Fix the ut cover and create useless dir.
syshenyao pushed a commit to syshenyao/nacos that referenced this issue Apr 25, 2024
…dule. (alibaba#11996)

* Move some basic logging classes into common module.

* Move logback 12 version adapter to single module.

* For checkstyle.

* Enhance logger adapter pom.

* Refactor the adapter initialization logic to avoid spi loader error.

* Move log4j2 logger adapter to single module.

* Fix the ut cover and create useless dir.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/Client Related to Nacos Client SDK kind/refactor
Projects
None yet
Development

No branches or pull requests

1 participant