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

[Bug] Maven artifacts should not depend on logging backend #4700

Closed
3 of 6 tasks
ppkarwasz opened this issue Dec 31, 2023 · 3 comments · Fixed by #4719
Closed
3 of 6 tasks

[Bug] Maven artifacts should not depend on logging backend #4700

ppkarwasz opened this issue Dec 31, 2023 · 3 comments · Fixed by #4719
Labels
bug Something isn't working

Comments

@ppkarwasz
Copy link
Contributor

Search before asking

  • I had searched in the issues and found no similar issues.

Environment

Other

EventMesh version

master

What happened

Multiple EventMesh artifacts have a runtime dependency on log4j-core (a Log4j API implementation) and log4j-slf4j-impl (an SLF4J implementation), for example eventmesh-common. These can cause problems to applications that depend on EventMesh artifacts, but use a different logging backend.

Unlike logging APIs (e.g. slf4j-api, log4j-api, commons-logging, etc.) that can coexist within the same application, logging backends and bridges (e.g. logback-classic, log4j-core, log4j-to-slf4j, log4j-slf4j-impl, etc.) are mutually exclusive. A couple of examples:

  • if an application has as runtime dependencies logback-classic and log4j-slf4j-impl (both are implementation of SLF4J) only one of them will work,
  • if an application has as runtime dependencies log4j-to-slf4j (a bridge from Log4j API to SLF4J) and log4j-slf4j-impl (a bridge from SLF4J to Log4j API), both of them will cease to work to prevent an infinite loop.

It is easy to find such examples in practice: an application based both on zookeper and EventMesh, will have both logback-classic and log4j-core on its classpath.

To prevent this kind of conflicts, the following steps should be necessary:

  • all the logging backends and bridges (an incomplete list: logback-classic, log4j-core, log4j-to-slf4j, log4j-slf4j-impl, log4j-to-jul, log4j-jul, log4j-over-slf4j) should be removed from the runtimeClasspath and moved to the testRuntimeOnly configuration,
  • all log4j2.xml configuration files should be removed from the published JARs, so that they not interfere with the configuration provided by applications,
  • the log4j-core, log4j-slf4j-impl and log4j-jul artifacts should be added to the TAR distribution, since it is meant to run as a standalone application.

How to reproduce

To pinpoint the affected artifacts run:

./gradlew dependencies

and check the runtimeClasspath configuration.

Debug logs

No response

Are you willing to submit PR?

  • Yes I am willing to submit a PR!

Code of Conduct
@ppkarwasz ppkarwasz added the bug Something isn't working label Dec 31, 2023
@github-actions GitHub Actions
Copy link
Contributor

Welcome to the Apache EventMesh community!!
We are glad that you are contributing by opening this issue. :D

Please make sure to include all the relevant context.
We will be here shortly.

If you are interested in contributing to our project, please let us know!
You can check out our contributing guide on contributing to EventMesh.

Want to get closer to the community?

WeChat Assistant WeChat Public Account Slack
Join Slack Chat

Mailing Lists:

Name Description Subscribe Unsubscribe Archive
Users User support and questions mailing list Subscribe Unsubscribe Mail Archives
Development Development related discussions Subscribe Unsubscribe Mail Archives
Commits All commits to repositories Subscribe Unsubscribe Mail Archives
Issues Issues or PRs comments and reviews Subscribe Unsubscribe Mail Archives

@Pil0tXia
Copy link
Member

Pil0tXia commented Jan 1, 2024

Thank you for pointing it out! Usually third-party applications will depend on eventmesh-sdk (cf. https://github.com/apache/eventmesh/blob/master/eventmesh-sdks/eventmesh-sdk-java/build.gradle), only slf4j-api dependency is transferred from eventmesh-common.

ppkarwasz added a commit to ppkarwasz/eventmesh that referenced this issue Jan 4, 2024
@ppkarwasz
[ISSUE apache#4700] Remove logging backends from runtime deps
54d7cd0 
This PR removes all the logging backends from the Maven artifacts. As
explained in apache#4700 libraries should only depend on logging APIs, not
logging implementations the same way web applications depend on the
Servlet API, not its implementations.

The logging backends are added to the `dist` folder to be included in
the TAR binary distribution.
ppkarwasz added a commit to ppkarwasz/eventmesh that referenced this issue Jan 4, 2024
@ppkarwasz
[ISSUE apache#4700] Remove logging backends from runtime deps
103daac 
This PR removes all the logging backends from the Maven artifacts. As
explained in apache#4700 libraries should only depend on logging APIs, not
logging implementations the same way web applications depend on the
Servlet API, not its implementations.

The logging backends are added to the `dist` folder to be included in
the TAR binary distribution.
@ppkarwasz ppkarwasz mentioned this issue Jan 4, 2024
Merged
@Pil0tXia
Copy link
Member

We recently encountered this issue when developing EventMesh Dashboard:

image

The EventMesh Dashboard uses Spring Boot's default logback implement, and we have to exclude the implement artifact to avoid SLF4J: Class path contains multiple SLF4J bindings. error.

@Pil0tXia Pil0tXia mentioned this issue Mar 14, 2024
Merged
xwm1992 pushed a commit that referenced this issue Apr 18, 2024
@ppkarwasz
[ISSUE #4700] Remove logging backends from runtime deps (#4719)
1fdb5b2 
* [ISSUE #4700] Remove logging backends from runtime deps

This PR removes all the logging backends from the Maven artifacts. As
explained in #4700 libraries should only depend on logging APIs, not
logging implementations the same way web applications depend on the
Servlet API, not its implementations.

The logging backends are added to the `dist` folder to be included in
the TAR binary distribution.

* Fix licenses

* Fix Logback exclusions

* Fix license check

* Fix `printProjects` according to the review

* Add logging backend to `eventmesh-starter`

* Remove task description

* Fix task dependencies of task `installPlugin`

* Fix `installPlugin` task

* Add comment about exclusions

* Minimize changes to current configuration

This commit minimizes the changes to EventMesh dependencies.

Since a global exclusion is not an effective way to stop propagating
logging backends as **transitive** dependencies we:

* explictly remove logging backends from third-party dependencies that
  include them: RocketMQ, Pulsar, Spring Boot and Zookeeper,
* restore Log4j Core as dependency of `eventmesh-common`,
* exclude Log4j Core as dependency of `eventmesh-sdk-java`.

* Add comments to remove exclusions after upgrade

* Make `installPlugin` independent from `dist`

* Make `copy` tasks easier to understand

* Add `eventmesh-common` to EventMesh OpenConnect deps

* Refactor RocketMQ deps

* Delete `output.dirs`

* Fix typo

* Remove last `outputs.dir`

* Remove dependencies from `installPlugin`

* Add `eventmesh-common` to OpenConnect artifacts
@fabian4 fabian4 mentioned this issue Apr 24, 2024
Closed
@ppkarwasz ppkarwasz mentioned this issue Sep 17, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
2 participants
@ppkarwasz @Pil0tXia