rocketmq icon indicating copy to clipboard operation
rocketmq copied to clipboard

rocketmq-tools should not depend on logback-classic

Open travisdowns opened this issue 3 years ago • 8 comments

rocketmq-tools depends on logback-classic, which includes a StaticLoggerBinder class which binds slf4j to logback.

This binding shouldn't be made by a library since only one binding can exist in the entire application, so any application that uses multiple libraries may have inconsistent bindings. The binding should be chosen only by the final application (i.e., the person implementing main()).

This, for example, causes warnings in openmessaging/benchmark which have chosen log4j binding, but the logback binding included from rocketmq causes a conflict.

It rocketmq-tools depended on logback-core instead, it would not include the slf4j binding which is one way to solve this problem.

travisdowns avatar Oct 18 '22 16:10 travisdowns

rocketmq-tool is also a 'final' application, used in mqadmin command. It might be the reason why logback-classic was imported.

caigy avatar Oct 19 '22 02:10 caigy

@travisdowns I think you are right. As a library, rocketmq-tools should not introduce the facade and the implementation at the same time.

I will start a RIP about replacing current logging module by a shaded logback recently, which may also solve this problem, you could view the related code from https://github.com/aliyun-mq/rocketmq-logging.

aaron-ai avatar Oct 21 '22 02:10 aaron-ai

rocketmq-tool is also a 'final' application, used in mqadmin command. It might be the reason why logback-classic was imported.

Ah, thanks for that detail. So there is a main() method in the .jar?

I am not sure how to resolve that situation, that a given .jar is intended to be used both as a library to be consumed by other applications, and in some contexts an application itself. Maybe one way is to have an artifact with all the code except main(), which would be the library artifact pulled in my projects that depend on it, then tools-cli or whatever which would depend on that library and basically be a 1-liner with a main() that calls into the shared tools lib.

travisdowns avatar Oct 24 '22 16:10 travisdowns

@travisdowns I think you are right. As a library, rocketmq-tools should not introduce the facade and the implementation at the same time.

One question: does the current rocket-mq use the slf4j API implementation or does it use any logback API directly?

I ask because the usual solution to this problem (when the offending library can't be fixed or will only be fixed in a subsequent version) is to exclude the logback-classic jar like so in the consuming project:

    <exclusions>
        <exclusion>
            <groupId>ch.qos.logback</groupId>
            <artifactId>logback-classic</artifactId>
        </exclusion>
    </exclusions>

I could apply this fix in openmessaging benchmark, but I want to check that rocketmq does not use logback-classic API directly since if so this would break that (since logback-classic will not be available at runtime).

travisdowns avatar Oct 24 '22 16:10 travisdowns

@aaron-ai any idea?

travisdowns avatar Oct 31 '22 23:10 travisdowns

This issue is stale because it has been open for 365 days with no activity. It will be closed in 3 days if no further activity occurs.

github-actions[bot] avatar Nov 01 '23 00:11 github-actions[bot]

further activity

travisdowns avatar Nov 01 '23 02:11 travisdowns

@aaron-ai - the repo you mentioned as been archived. Not sure if there is any plan to move this forward?

travisdowns avatar Jan 30 '24 21:01 travisdowns