search

snowflakedb / snowflake-ingest-java

Java SDK for the Snowflake Ingest Service -
http://www.snowflake.net
Apache License 2.0
72 stars 57 forks source link

Should not shade apache commons logging #714

Closed OskarKjellin closed 8 months ago

OskarKjellin commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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 commented 8 months ago

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 commented 8 months ago

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

OskarKjellin commented 8 months ago

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