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

Should not shade apache commons logging #714

Closed
OskarKjellin opened this issue Mar 5, 2024 · 6 comments
Closed

Should not shade apache commons logging #714

OskarKjellin opened this issue Mar 5, 2024 · 6 comments
Assignees

Comments

@OskarKjellin
Copy link

The current approach of shading all dependencies also means that when the apache http client is shaded, the logging it uses (apache commons logging) also becomes shaded.
When this is shaded it's not possible to use the commons logging to slf4j bridge, meaning that our logs get filled with:

log4j:WARN No appenders could be found for logger (net.snowflake.client.jdbc.internal.apache.http.client.protocol.RequestAddCookies).

And we don't get any logs from apache http client.

commons-logging:commons-logging should probably be excluded from shading just as org.slf4j:slf4j-api is

@sfc-gh-lsembera
Copy link
Contributor

sfc-gh-lsembera commented Mar 6, 2024

HI @OskarKjellin, could you define an appender in your log4j configuration for net.snowflake.client.jdbc.internal? Alternatively, we are also releasing an unshaded version of the SDK. They are typically released in a few days after the shaded release. 2.1.0-unshaded should be released in a few days.

@OskarKjellin
Copy link
Author

I could, but I want to bridge it to slf4j, not add an appender.
We'd like to keep using the shaded because otherwise we'll have a ton of classpath issues for the other classes.

Is there a reason why one logging facade (slf4j) is not shaded while another one is (commons-logging)? This mostly seems like an oversight to me, not the wanted behavior

@sfc-gh-lsembera
Copy link
Contributor

I checked SDK dependencies, and it neither directly nor transitively depends on commons-logging, so it is also not shading it. The class net.snowflake.client.jdbc.internal.apache.http.client.protocol.RequestAddCookies is shaded into the Snowflake JDBC driver, so even if you used the unshaded SDK, it wouldn't help in this case.

I am still not sure if I understand the issue, though. You mention you are bridging commons-logging to slf4j-api (via jcl-over-slf4j.jar?), but the error says log4j:WARN No appenders could be found for logger..... Log4j is your implementation of slf4j-api? Doesn't it mean that the log message got bridged correctly, if log4j is complaining here?

@OskarKjellin
Copy link
Author

Hmm, so this issue should be with the JDBC driver?

Log4j is not our implementation. The snowflake jdbc driver also includes a shaded version of log4j :(

@sfc-gh-lsembera
Copy link
Contributor

sfc-gh-lsembera commented Mar 11, 2024

Yeah, please open this issue in the JDBC driver repo: https://github.com/snowflakedb/snowflake-jdbc/issues

@OskarKjellin
Copy link
Author

Hmm, maybe the jdbc driver doesn't include a shaded version of log4j. Seems to be something else, but still shouldn't shade commons-logging imo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants