Skip to content

Commit

Permalink
inject-server: Throw an UnsupportedOperationException when Deprecated…
Browse files Browse the repository at this point in the history
…Logging#log is accessed

Problem

The `c.t.inject.server.DeprecatedLogging` trait was introduced to the Finatra
inject TwitterServer trait in order to provide a backward-compatible shim layer
for Finatra services as they may have been setting the util-logging Flags and
complete removal of the util-logging Logging trait would have removed the Flag
definitions which would cause servers to fail startup. The shim layer was
provided such that the Flags would still be defined even though they do nothing.
And the logger instance was redefined to be marked `@deprecated` as a signal
to users that it should not be used. However, users still mistakenly use
it since logging in a Finatra application can be confusing due to the prevalence
of the JUL logging library, `util/util-logging` in the stack. The stack is slowly
moving away from the JUL logging of `util/util-logging` and to the SLF4J-API
but nonetheless it is too nuanced to be obvious to most users right away.

Solution

Make accessing the `log` instance throw an `UnsupportedOperationException`.
This instance should not be used for logging and the `DeprecatedLogging` trait is
slated for removal. Hopefully throwing the exception prevents new incorrect
usages from being introduced such that we can continue to unwind the
removal of the `DeprecatedLogging` trait.

JIRA Issues: CSL-7374

Differential Revision: https://phabricator.twitter.biz/D854365
  • Loading branch information
cacoco authored and jenkins committed Mar 21, 2022
1 parent 404f7ee commit e2f2675
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 1 deletion.
12 changes: 12 additions & 0 deletions CHANGELOG.rst
Expand Up @@ -7,6 +7,17 @@ Note that ``RB_ID=#`` and ``PHAB_ID=#`` correspond to associated message in comm
Unreleased
----------

Runtime Behavior Changes
~~~~~~~~~~~~~~~~~~~~~~~~

* inject-server: Throw an `UnsupportedOperationException` when access to the `c.t.inject.server.DeprecatedLogging#log`
instance is attempted. This is a JUL Logger instance which was provided only as a backward-compatible
shim for Finatra services when the `c.t.server.TwitterServer` framework was moved to the `SLF4J-API`.
The instance was marked `@deprecated` in hopes of alerting users to not use it. We have now updated
it to throw an exception when accessed. Please refer to the Finatra documentation for more information
on using the SLF4J-API for logging with the Finatra framework: https://twitter.github.io/finatra/user-guide/logging/index.html.
``PHAB_ID=D854365``

Added
~~~~~

Expand Down Expand Up @@ -100,6 +111,7 @@ Added

Runtime Behavior Changes
~~~~~~~~~~~~~~~~~~~~~~~~

* http-server: Treat non-constant routes with or without trailing slash as 2 different routes.
For example, "/user/:id" and "/user/:id/" are treated as different routes in Finatra. You can still
use the optional trailing slash `/?` which will indicate Finatra to ignore the trailing slash while
Expand Down
Expand Up @@ -361,7 +361,9 @@ trait TwitterServer
@deprecated("For backwards compatibility of defined flags", "2017-10-06")
private[server] trait DeprecatedLogging extends com.twitter.logging.Logging { self: App =>
@deprecated("For backwards compatibility only.", "2017-10-06")
override lazy val log: com.twitter.logging.Logger = com.twitter.logging.Logger(name)
override lazy val log: com.twitter.logging.Logger =
throw new UnsupportedOperationException(
"Finatra uses the SLF4J-API for logging. This is a JUL logger provided as a backward-compatible shim for legacy reasons and access is thus unsupported for Finatra servers. Please refer to the logging documentation for information on how to log with the Finatra framework: https://twitter.github.io/finatra/user-guide/logging/index.html")

// lint if any com.twitter.logging.Logging flags are set
premain {
Expand Down

0 comments on commit e2f2675

Please sign in to comment.