Skip to content

Commit a73a295

Browse files
cacocojenkins
authored andcommitted
inject-app: Remove SLF4J-API logging bridges as dependencies
Problem It is incorrect to blindly specify the SLF4J-API logging bridges on the inject-app library of the framework at it causes trouble for usages that cannot use Logback as an SLF4J-API logging implementation. Solution Remove the SLF4J-API logging bridges as dependencies. Users should provide the correct bridges as needed for their chosen SLF4J-API logging implementation. Note that the `TwitterServer` logging libraries will bring the correct bridges for the given logging implementation so for users that include a dependency on one of those libraries, there is nothing else that needs to be done. Result Less conflicting logging implementations. JIRA Issues: CSL-9669 Differential Revision: https://phabricator.twitter.biz/D854393
1 parent f9a3bb2 commit a73a295

3 files changed

Lines changed: 13 additions & 33 deletions

File tree

CHANGELOG.rst

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,19 @@ Unreleased
1010
Runtime Behavior Changes
1111
~~~~~~~~~~~~~~~~~~~~~~~~
1212

13+
* inject-app: Remove the SLF4J-API logging bridges as dependencies. These were originally
14+
added as the framework was expressly opionated that users should use Logback as an SLF4J-API
15+
implementation, however specifying the bridges on the Finatra inject/inject-app library
16+
causes many issues with code that must use a different SLF4J-API logging implementation
17+
but still uses the Finatra framework. Users should note that if they do not include these
18+
bridges in some other manner that they may lose logging if they have any libraries which
19+
log with one of the formerly bridged implementations. Also note that servers using a
20+
`TwitterServer` logging implementation to support `dynamically changing log levels <https://twitter.github.io/twitter-server/Features.html#dynamically-change-log-levels>`__ will get the proper bridges as dependencies.
21+
``PHAB_ID=D854393``
22+
23+
Runtime Behavior Changes
24+
~~~~~~~~~~~~~~~~~~~~~~~~
25+
1326
* inject-server: Throw an `UnsupportedOperationException` when access to the `c.t.inject.server.DeprecatedLogging#log`
1427
instance is attempted. This is a JUL Logger instance which was provided only as a backward-compatible
1528
shim for Finatra services when the `c.t.server.TwitterServer` framework was moved to the `SLF4J-API`.

build.sbt

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -491,13 +491,6 @@ lazy val injectApp = (project in file("inject/inject-app"))
491491
"com.twitter" %% "util-core" % versions.twLibVersion,
492492
"com.twitter" %% "util-slf4j-jul-bridge" % versions.twLibVersion,
493493
"org.slf4j" % "slf4j-api" % versions.slf4j,
494-
// -------- BEGIN: slf4j-api logging bridges -------------------------------
495-
// Add the slf4j-api logging bridges to ensure that any dependents
496-
// of the library have bridges on their classpath at runtime.
497-
"org.slf4j" % "jcl-over-slf4j" % versions.slf4j,
498-
"org.slf4j" % "jul-to-slf4j" % versions.slf4j,
499-
"org.slf4j" % "log4j-over-slf4j" % versions.slf4j,
500-
// -------- END: slf4j-api logging bridges ---------------------------------
501494
"org.slf4j" % "slf4j-simple" % versions.slf4j % "test-internal"
502495
),
503496
ScoverageKeys.coverageExcludedPackages := "<empty>;.*TypeConverter.*",

inject/inject-app/src/main/scala/com/twitter/inject/app/BUILD

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,3 @@
1-
# These are defined here and not on inject-slf4j to allow
2-
# for usage of the inject-slf4j library in environments
3-
# that don't want to bridge these impls. since the framework
4-
# recommendation is to use Logback for application logging
5-
# we add these here.
6-
target(
7-
name = "logging-bridges",
8-
tags = [
9-
"bazel-compatible",
10-
"logging_impl_check_whitelisted_target",
11-
],
12-
dependencies = [
13-
# -------- BEGIN: slf4j-api logging bridges -------------------------------
14-
# Add the slf4j-api logging bridges to ensure that any dependents
15-
# of the library have bridges on their classpath at runtime.
16-
"3rdparty/jvm/org/slf4j:jcl-over-slf4j",
17-
"3rdparty/jvm/org/slf4j:jul-to-slf4j",
18-
"3rdparty/jvm/org/slf4j:log4j-over-slf4j",
19-
# -------- END: slf4j-api logging bridges ---------------------------------
20-
],
21-
)
22-
231
scala_library(
242
sources = ["*.scala"],
253
compiler_option_sets = ["fatal_warnings"],
@@ -31,10 +9,6 @@ scala_library(
319
strict_deps = True,
3210
tags = ["bazel-compatible"],
3311
dependencies = [
34-
scoped(
35-
":logging-bridges",
36-
scope = "runtime",
37-
),
3812
"3rdparty/jvm/com/google/inject:guice",
3913
"3rdparty/jvm/org/slf4j:slf4j-api",
4014
"finatra/inject/inject-app/src/main/java/com/twitter/inject/annotations",

0 commit comments

Comments
 (0)