endpoints-framework should not depend on slf4j-nop

[AdamSantosCA]

Including this in the library's dependencies forces users to exclude if using another binding. See here for more info: https://www.slf4j.org/codes.html#multiple_bindings

[tangiel]

Suggest just excluding the dep in your build file. nop is included because slf4j is meant to be an internal dependency, and we don't want to require most users to include nop to avoid log spam.

[AdamSantosCA]

From the slf4j documentation:

"Embedded components such as libraries or frameworks should not declare a dependency on any SLF4J binding but only depend on slf4j-api. When a library declares a compile-time dependency on a SLF4J binding, it imposes that binding on the end-user, thus negating SLF4J's purpose. When you come across an embedded component declaring a compile-time dependency on any SLF4J binding, please take the time to contact the authors of said component/library and kindly ask them to mend their ways."

[tangiel]

I read the link you provided. If the binding is not imposed on the end user, then the log messages from not having a binding is imposed on the end user. This is not an agreeable user experience either, and we actually explicitly included the dependency in response to complaints that arose because of it.

[AdamSantosCA]

As of slf4j 1.6.0, it defaults to a no-op so that end users should not see logging if they don't provide a binding.

[tangiel]

Ok, I will verify that and remove the dependency in the next version. Thanks!

[tangiel]

On a distribution using 1.6.3, I get this:

SLF4J: Failed to load class "org.slf4j.impl.StaticLoggerBinder".
SLF4J: Defaulting to no-operation (NOP) logger implementation
SLF4J: See http://www.slf4j.org/codes.html#StaticLoggerBinder for further details.
OpenAPI document written to ./openapi.json

[briandealwis]

@tangiel that is the correct message, IMHO: the test app should specify a SLF4j logging backend, not a framework.

[tangiel]

It might be technically correct, but I think it's unreasonable to expect all Endpoints framework users to add a nop dependency to suppress the message. I think it's more reasonable to expect users who actually care about the use of slf4j to exclude the nop dependency. I'm open to other options, but they need to suppress that log message.

[briandealwis]

Instead users post to StackOverflow as nothing is being logged.

https://stackoverflow.com/questions/46307589/slf4j-not-logging-on-a-cloud-endpoints-appengine-application

[tangiel]

The only solution I can think of at the moment that would satisfy all requirements is to shade the Swagger model, which uses slf4j. Then we can include a shaded version of slf4j-nop, and it wouldn't impact the user. However I'm not eager to do this (nor am I sure it would actually work).

[briandealwis]

I suspect shading Swagger's use of SLF4j would mean no SLF4j logging backend can be provided. So we'd always see the org.slf4j.impl.StaticLoggerBinder warning message, and never be able to obtain the logging messages. That would seem to be the worst solution :-)

I believe the correct solution is to modify the README.md — and any other documentation — to add a note to the Migration section that consumers should configure an SLF4j backend, and have the snippet include slf4j-nop.

[tangiel]

@briandealwis swagger-model is an internal implementation detail to Endpoints. The customer should not need to see any of its logs. What I'm suggesting means including a shaded version of slf4j-nop to suppress the message.

Regarding your proposal: Sorry, but I'm not willing to impose the extra work on migrating (and new) users. I'd sooner implement my own Swagger model than do that, because there are already other issues with the model artifact that bother me.

[elharo]

Is swagger-models doing something wrong? Should we report/fix it?

[suzuki-takayuki]

Could you please at least provide one line on README.md telling users to exclude slf4j-nop when they want to use SLF4J with another binding? I didn't know that this conflicted with Stackdriver Logging Logback appender, and I've spent half an hour before finding this issue.