Skip to content

feat: log through SLF4J and report errors to Sentry - #208

Open
TheMeinerLP wants to merge 1 commit into
build/minestom-extensionsfrom
feat/observability
Open

feat: log through SLF4J and report errors to Sentry#208
TheMeinerLP wants to merge 1 commit into
build/minestom-extensionsfrom
feat/observability

Conversation

@TheMeinerLP

Copy link
Copy Markdown
Contributor

Gestapelt auf #207 (das wiederum auf #206). GitHub zieht die Base automatisch nach, sobald die Vorgänger gemergt sind.

Warum

Titan lief in Produktion faktisch blind:

  1. Kein SLF4J-Binding. slf4j-api kam nur transitiv über Minestom, ein Provider war nie deklariert. Der Fat Jar antwortete auf jeden Log-Aufruf mit No SLF4J providers were found und verwarf ihn.
  2. Minestoms Default-Exception-Handler ist Throwable::printStackTrace. Eine Exception in einem Listener landete roh auf stdout, an SLF4J vorbei.

Zusammen hieß das: ein abstürzender Listener hinterließ einen nackten Stacktrace und sonst nichts.

Logging

  • logback-classic als runtimeOnly in :app und :setup, slf4j-api explizit deklariert
  • eine logback.xml in :common — beide Fat Jars bündeln :common, also eine Konfiguration ohne Kopie
  • TitanObservability.installExceptionHandler() ersetzt Minestoms printStackTrace-Default

Sentry

Optional, nach dem Muster von Sturnus: ohne TITAN_SENTRY_DSN wird Sentry.init nie aufgerufen und der Prozess verhält sich exakt wie heute. Ein Jar bedient Betreiber mit und ohne Sentry-Instanz.

Variable Zweck
TITAN_SENTRY_DSN Aktiviert Reporting. Fehlt sie, passiert nichts.
TITAN_SENTRY_ENVIRONMENT production, beta, … — Default unknown

Die Release-Angabe kommt aus dem Implementation-Version-Manifest-Attribut des Fat Jars (neu in titan.java-conventions), ein Sentry-Issue zeigt also auf einen konkreten Deploy.

Reporting hat genau einen Eingang: den Logback-Appender ab ERROR. Kein zweiter, paralleler Pfad, der doppelt melden oder auseinanderlaufen könnte.

Spieler-Zuordnung

Wie besprochen mit UUID und Name.

EventNodeImpl fängt ab, was ein Listener wirft, und reicht es einen Frame höher an den ExceptionManager — auf demselben Thread. TitanObservability.guard() sitzt in diesem Frame, merkt sich den Spieler des fehlgeschlagenen Events und wirft weiter; der Handler legt ihn als player.uuid / player.name in den MDC, von wo der Sentry-Appender ihn übernimmt.

Kein Kostenpunkt auf dem gesunden Pfad. Aufgezeichnet wird ausschließlich im catch-Block. Ein Listener, der normal zurückkehrt, zahlt ein betretenes try und sonst nichts — relevant, weil zu den 18 gewrappten Listenern PlayerMoveEvent und PlayerPacketEvent gehören.

sendDefaultPii bleibt false: die PII-Defaults des SDK sind Request-Header und IP-Adressen, die über einen Minestom-Absturz nichts aussagen. Die Spieleridentität, die etwas aussagt, wird bewusst gesetzt.

Verifikation am echten Fat Jar

Ohne DSN:

INFO  [main] n.o.t.c.o.TitanObservability - Sentry reporting disabled - TITAN_SENTRY_DSN is not set
INFO  [main] luckperms - Loading configuration...
INFO  [main] luckperms - Successfully enabled. (took 388ms)
ERROR [main] n.o.t.c.o.TitanObservability - Unhandled exception
java.nio.file.NoSuchFileException: worlds

Mit DSN:

INFO  [main] n.o.t.c.o.TitanObservability - Sentry reporting enabled - release 1.14.0, environment smoketest

Vor dieser Änderung gab derselbe Lauf SLF4J(W): No SLF4J providers were found aus und danach einen nackten Stacktrace. Die Release-Angabe 1.14.0 belegt, dass das Manifest-Attribut greift.

Tests: 63 gesamt, 0 Fehler — davon 7 neue in :common:

  • gewrappter Listener wird normal durchgereicht, ohne Kontext zu hinterlassen
  • Fehlerfall wirft das Original unverändert weiter (assertSame)
  • Fehler auf einem Player-Event zeichnet UUID + Name auf
  • Kontext wird genau einmal konsumiert — sonst würde der nächste Fehler diesem Spieler angelastet
  • Fehler auf einem Event ohne Spieler zeichnet nichts auf
  • ohne DSN bleibt Sentry.isEnabled() false
  • Release fällt außerhalb eines Jars auf dev zurück statt auf null

Ein Detail, das Erklärung braucht

Der Appender wird mit leerem DSN deklariert:

<options>
    <dsn>${TITAN_SENTRY_DSN:-}</dsn>
</options>

Er wird konstruiert, sobald der erste Logger entsteht — zwangsläufig bevor bootstrap() Sentry.init aufrufen kann. Ein Appender, der zu diesem Zeitpunkt keinen DSN findet, loggt DSN is required auf WARN, und das lässt Logback bei jedem Start seinen kompletten Konfigurations-Status ausgeben (~25 Zeilen). Mit leerem DSN ist der Start sauber; bootstrap() entscheidet ein paar Anweisungen später über die globale SDK-Instanz, ob tatsächlich gesendet wird.

Anmerkung

Commit-Typ ist feat: — anders als #206/#207 erscheint das hier also im Changelog. Das ist beabsichtigt: Betreiber müssen wissen, ab welcher Version TITAN_SENTRY_DSN gesetzt werden kann.

🤖 Generated with Claude Code

https://claude.ai/code/session_01QtbnMbAj6m2q3vrDYFizDr

Titan shipped without an SLF4J binding. slf4j-api arrived transitively
through Minestom, no provider was ever declared, and the fat jars answered
every log call with "No SLF4J providers were found" and dropped it. On top
of that, Minestom's default ExceptionManager handler is
Throwable::printStackTrace, so a listener that threw left a bare stack
trace on stdout and nothing else. A production lobby was effectively
running blind.

Logging
- logback-classic as runtimeOnly in :app and :setup, slf4j-api declared
  explicitly where code compiles against it
- one logback.xml in :common, so both fat jars get the same configuration
  without copying it
- TitanObservability.installExceptionHandler() replaces Minestom's
  printStackTrace default, so exceptions reach the log like everything else

Sentry
- optional, exactly as Sturnus does it: no TITAN_SENTRY_DSN means
  Sentry.init is never called and the process behaves as it does today.
  One jar serves operators with and without a Sentry instance.
- release comes from the jar's Implementation-Version manifest attribute
  (added in titan.java-conventions), so an issue points at a deploy
- environment from TITAN_SENTRY_ENVIRONMENT
- reporting has exactly one entry point, the logback appender at ERROR.
  No second path that could double-report or drift out of sync.

Player attribution
EventNodeImpl catches what a listener throws and hands it to the exception
manager one frame up, on the same thread. TitanObservability.guard() sits
inside that frame and records the failing event's player, which the handler
puts into the MDC as player.uuid / player.name. Recording happens in a
catch block only - a listener that returns normally pays for an entered try
and nothing else, which matters because the guarded set includes
PlayerMoveEvent and PlayerPacketEvent.

sendDefaultPii stays false: the SDK's PII defaults are request headers and
IP addresses, which say nothing about a Minestom crash. The player identity
that does is attached deliberately.

Verified against the real fat jar. Without a DSN:
  INFO  n.o.t.c.o.TitanObservability - Sentry reporting disabled - ...
  INFO  luckperms - Loading configuration...
  ERROR n.o.t.c.o.TitanObservability - Unhandled exception
        java.nio.file.NoSuchFileException: worlds
With TITAN_SENTRY_DSN and TITAN_SENTRY_ENVIRONMENT set:
  INFO  n.o.t.c.o.TitanObservability - Sentry reporting enabled -
        release 1.14.0, environment smoketest
Before this change the same run printed "No SLF4J providers were found"
and a bare stack trace.

The appender is declared with an empty DSN on purpose. It is constructed
when the first logger is created, necessarily before bootstrap() can call
Sentry.init, and an appender that finds no DSN then logs "DSN is required"
at WARN - which makes Logback dump its entire configuration status on every
start.

7 new tests in :common (63 total, 0 failures).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QtbnMbAj6m2q3vrDYFizDr
@TheMeinerLP
TheMeinerLP requested a review from a team as a code owner August 27, 2026 20:25
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

Successfully merging this pull request may close these issues.

1 participant