Skip to content

feat(sentry): remove the startup probe #260

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
SyniRon merged 1 commit into from
Sep 13, 2026
Merged

feat(sentry): remove the startup probe #260

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 10 additions & 19 deletions rest/sentry.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,12 @@ package rest
// point calls (the error-writer behind writeError; methodNotAllowed
// reaches it without writeError): one place, every error.
//
// What this file deliberately does NOT reproduce — servers/sentry.go's three
// boot-lifecycle pieces, which the cutover slice (#134) must port before
// deleting that file: sentryDialCheck (warn-only TCP reachability check of
// the DSN ingest host at boot), sentryStartupProbe (one canary event flushed
// through the real transport before any listener opens), and
// flushSentryOnShutdown (the SIGTERM/interrupt flush handler servers.Start
// installs after setupSentry). Dropping them silently would make a
// Sentry-down misconfig invisible at boot and lose every still-buffered
// event on SIGTERM.
// The boot/shutdown lifecycle lives in sentry_boot.go: sentryDialCheck (the
// warn-only TCP reachability check of the DSN ingest host that SetupSentry
// runs at boot) and FlushSentryOnShutdown (the SIGTERM/interrupt flush handler
// servers.Start installs when SetupSentry returns true). Dropping either
// silently would make a Sentry-down misconfig invisible at boot, or lose every
// still-buffered event on SIGTERM.
//
// Tag discipline (PRD #112): events carry the validated key ID and the
// matched route pattern — bearer material NEVER (same rule as the metrics
Expand Down Expand Up @@ -102,18 +99,12 @@ func SetupSentry(release string) bool {
return false
}

// Warn-only reachability pre-check: catches the misconfig class the probe
// below structurally cannot (fast send failures drain the queue and so
// still "flush"). Never changes the enabled/degraded semantics.
// Warn-only reachability pre-check. It never changes the enabled state.
// A refused or unresolvable host warns, and capture stays on because the
// network may heal before the first real error.
sentryDialCheck(dsn)

// Probe failure still returns true — enabled-degraded, not disabled: a
// slow-network false positive must not turn off capture. Do NOT refactor
// this into `return sentryStartupProbe()`.
if sentryStartupProbe() {
Info.Println("Sentry error capture enabled (errors only), release:", release,
"— startup probe flushed (queue drained; delivery not verified — set SENTRY_DEBUG=true to confirm)")
}
Info.Println("Sentry error capture enabled (errors only), release:", release)
return true
}

Expand Down
68 changes: 11 additions & 57 deletions rest/sentry_boot.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
package rest

// Sentry boot-lifecycle wiring for the new stack: the three pieces ported from
// Phase 0's servers/sentry.go at the single-listener cutover (#134). SetupSentry
// (sentry.go) calls sentryDialCheck and sentryStartupProbe at boot; the
// composition root installs FlushSentryOnShutdown when capture is enabled.
// Sentry boot-lifecycle wiring for the new stack: the two pieces that survive
// from Phase 0's servers/sentry.go after the single-listener cutover (#134) and
// #259. SetupSentry (sentry.go) calls sentryDialCheck at boot; the composition
// root installs FlushSentryOnShutdown when capture is enabled.
//
// Kept in their own file so the always-on init (sentry.go) and the boot/shutdown
// lifecycle stay legible apart; they share the package's sentryTransport test
Expand All @@ -24,25 +24,20 @@ import (
// events to reach Sentry before the process exits.
const sentryShutdownFlushTimeout = 2 * time.Second

// sentryStartupProbeTimeout bounds the boot-time drain check. A var, not a
// const, only so tests can shrink the window; production never mutates it.
var sentryStartupProbeTimeout = 5 * time.Second

// sentryDialCheckTimeout bounds the boot-time TCP reachability pre-check of
// the DSN ingest host — generous enough for a cold DNS resolve plus a
// the DSN ingest host. Generous enough for a cold DNS resolve plus a
// cross-region handshake, small enough to keep the worst-case boot delay
// acceptable (it stacks with the probe window before any listener opens). A
// acceptable. This is the only network wait before any listener opens. A
// var, not a const, only so tests can shrink the window; production never
// mutates it.
var sentryDialCheckTimeout = 3 * time.Second

// sentryDialCheck is a boot-time, warn-only TCP reachability check of the DSN
// ingest host. It exists because the startup probe cannot see fast send
// failures — DNS errors and refused connections drain the transport queue in
// milliseconds, so the probe's flush still reports success (see
// sentryStartupProbe). A failed dial here names the unreachable host while
// the probe would stay silent. Warn-only by design: the network may heal, and
// a boot-time blip must not disable capture.
// ingest host. sentry.Init does no network I/O, so without this check a wrong
// host, a DNS typo or a closed egress path stays invisible until the first
// real error fails to send. A failed dial names the unreachable host in the
// log before any listener opens. Warn-only by design: the network may heal,
// and a boot-time blip must not disable capture.
func sentryDialCheck(rawDSN string) {
dsn, err := sentry.NewDsn(rawDSN)
if err != nil {
Expand All @@ -58,47 +53,6 @@ func sentryDialCheck(rawDSN string) {
_ = conn.Close()
}

// sentryStartupProbe pushes one canary event through the real transport and
// flushes. sentry.Init does no network I/O, so without this the pipeline is
// first exercised by the first real error.
//
// What a true return PROVES — per the SDK's documented Flush contract (queue
// drained, NOT delivered): the transport finished its send attempts within
// the window. That catches hang-class failures (blackholed egress, connects
// slower than the window) and guarantees one event exercised the full
// pipeline so SENTRY_DEBUG has something to report. What it does NOT prove:
// delivery. The transport worker dequeues on ANY send outcome, so DNS
// failures, refused connections, Sentry-side rejections (bad DSN key → 4xx)
// and rate-limit drops all complete in milliseconds, drain the queue, and
// "flush" successfully — those classes are visible only with
// SENTRY_DEBUG=true (and partially via sentryDialCheck).
func sentryStartupProbe() bool {
// Event hygiene: a fixed fingerprint + info level fold every container
// restart into one low-severity Sentry issue instead of resolve→reopen
// churn; the probe tag makes canaries filterable. Cloned hub so none of
// this leaks into the global scope.
var id *sentry.EventID
hub := sentry.CurrentHub().Clone()
hub.WithScope(func(scope *sentry.Scope) {
scope.SetFingerprint([]string{"sentry-startup-probe"})
scope.SetTag("probe", "true")
scope.SetLevel(sentry.LevelInfo)
id = hub.CaptureMessage("sentry startup probe")
})
if id == nil {
// Nil event ID = the client dropped the event before the transport
// saw it (e.g. a BeforeSend veto) — nothing queued, so a "successful"
// flush below would be vacuous.
Warn.Println("sentry startup probe was dropped client-side — no canary reached the transport (check BeforeSend/sampling)")
return false
}
if !hub.Flush(sentryStartupProbeTimeout) {
Warn.Println("sentry enabled but startup probe did not flush — transport still busy after the window; events may not be reaching Sentry (check DSN/egress, or set SENTRY_DEBUG=true)")
return false
}
return true
}

// FlushSentryOnShutdown installs a signal handler that flushes buffered
// Sentry events before the process exits. The stack has no graceful shutdown
// path (Start blocks on Serve and the process dies by signal); this is the
Expand Down
158 changes: 26 additions & 132 deletions rest/sentry_boot_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,8 @@ package rest

import (
"bytes"
"context"
"net"
"os"
"sync"
"syscall"
"testing"
"time"
Expand All @@ -16,128 +14,27 @@ import (
"github.com/stretchr/testify/require"
)

// drainTransport is an in-memory sentry.Transport with a controllable Flush
// outcome. flushDrains=false simulates a drain timeout (queue still busy when
// the window closes — the hang-class failure mode), NOT a delivery failure:
// per the SDK's Flush contract, fast send failures dequeue and "flush"
// successfully. It complements transportMock (sentry_test.go), whose Flush is
// fixed true — these boot-lifecycle tests need to drive the not-drained path.
type drainTransport struct {
mu sync.Mutex
events []*sentry.Event
flushDrains bool
}

func (t *drainTransport) Configure(sentry.ClientOptions) {}
func (t *drainTransport) Flush(time.Duration) bool { return t.flushDrains }
func (t *drainTransport) FlushWithContext(context.Context) bool { return t.flushDrains }
func (t *drainTransport) Close() {}

func (t *drainTransport) SendEvent(event *sentry.Event) {
t.mu.Lock()
defer t.mu.Unlock()
t.events = append(t.events, event)
}

func (t *drainTransport) Events() []*sentry.Event {
t.mu.Lock()
defer t.mu.Unlock()
return append([]*sentry.Event(nil), t.events...)
}

// bindDrainClient binds a drain-transport Sentry client to the global hub for
// the duration of the test, restoring the unbound (disabled) state afterwards.
func bindDrainClient(t *testing.T, flushDrains bool) *drainTransport {
t.Helper()
transport := &drainTransport{flushDrains: flushDrains}
client, err := sentry.NewClient(sentry.ClientOptions{Transport: transport})
require.NoError(t, err)
sentry.CurrentHub().BindClient(client)
t.Cleanup(func() { sentry.CurrentHub().BindClient(nil) })
return transport
}

func TestSentryStartupProbe_DrainTimeout_WarnsLoudly(t *testing.T) {
bindDrainClient(t, false)

var warnBuf bytes.Buffer
Warn.SetOutput(&warnBuf)
defer Warn.SetOutput(os.Stdout)

ok := sentryStartupProbe()

assert.False(t, ok)
assert.Contains(t, warnBuf.String(), "startup probe did not flush",
"a hang-class transport stall is the one failure mode the drain probe can see — it must be loud")
}

func TestSentryStartupProbe_FlushDrains_NoWarning(t *testing.T) {
transport := bindDrainClient(t, true)

var warnBuf bytes.Buffer
Warn.SetOutput(&warnBuf)
defer Warn.SetOutput(os.Stdout)

ok := sentryStartupProbe()

assert.True(t, ok)
assert.Empty(t, warnBuf.String(), "a drained probe must not warn")
events := transport.Events()
require.Len(t, events, 1, "the probe must push exactly one canary event through the transport")
event := events[0]
assert.Equal(t, "sentry startup probe", event.Message)
assert.Equal(t, []string{"sentry-startup-probe"}, event.Fingerprint,
"a fixed fingerprint folds every container restart into one Sentry issue — no resolve→reopen churn")
assert.Equal(t, "true", event.Tags["probe"], "probe events must be filterable")
assert.Equal(t, sentry.LevelInfo, event.Level, "the canary is informational, never an alertable error")
}

func TestSentryStartupProbe_ClientSideDrop_WarnsAndFails(t *testing.T) {
// A BeforeSend veto makes CaptureMessage return a nil event ID — the
// client dropped the canary before the transport ever saw it, so there is
// nothing to drain and the flush alone would report a false success.
client, err := sentry.NewClient(sentry.ClientOptions{
Transport: &drainTransport{flushDrains: true},
BeforeSend: func(*sentry.Event, *sentry.EventHint) *sentry.Event { return nil },
})
require.NoError(t, err)
sentry.CurrentHub().BindClient(client)
t.Cleanup(func() { sentry.CurrentHub().BindClient(nil) })

var warnBuf bytes.Buffer
Warn.SetOutput(&warnBuf)
defer Warn.SetOutput(os.Stdout)

ok := sentryStartupProbe()

assert.False(t, ok, "a client-side drop means no canary exercised the pipeline — probe failed")
assert.Contains(t, warnBuf.String(), "dropped client-side",
"a canary that never reached the transport must be visible, not a silent flush success")
}

// TestSetupSentry_ProbeStall_WarnsAndWithholdsEnabledLine pins that SetupSentry
// actually INVOKES the startup probe against the client it just configured: a
// drain transport injected through the sentryTransport seam reports
// Flush=not-drained, so the stalled-transport premise holds by construction —
// no socket dial manufactures the stall, and machine load cannot invert the
// outcome (#179: a refused dial is a fast send outcome that drains the queue,
// so the old local-server stall lost the timing race under load). Deleting the
// sentryStartupProbe() call from SetupSentry turns this red (no stall warning,
// the enabled line prints, no canary reaches the transport).
func TestSetupSentry_ProbeStall_WarnsAndWithholdsEnabledLine(t *testing.T) {
transport := &drainTransport{flushDrains: false}
// TestSetupSentry_BootSendsNoEventAndWarnsOnUnreachableHost pins the boot
// contract after #259: with a DSN, SetupSentry initialises the client, runs
// the warn-only dial check, prints the enabled line, and hands the transport
// nothing. Sentry receives no event at boot.
//
// The emptiness check is authoritative, not a race won. With a custom
// Transport the SDK skips its async telemetry processor and hands events to
// the transport on the capturing goroutine (sentry-go client.go, processEvent),
// so a re-added boot event would sit in Events() before SetupSentry returns.
//
// 127.0.0.1:1 refuses instantly, so the dial check resolves no name and opens
// no outbound connection. The test shrinks the dial window anyway so a
// pathological environment cannot stall the suite.
func TestSetupSentry_BootSendsNoEventAndWarnsOnUnreachableHost(t *testing.T) {
transport := &transportMock{}
sentryTransport = transport
t.Cleanup(func() { sentryTransport = nil })

// 127.0.0.1:1 refuses instantly — the suite's deterministic stand-in for
// the dial-check pre-check, which is frozen and irrelevant to the stall
// premise. Shrink its window anyway so a pathological environment cannot
// stall the suite. The probe window itself no longer needs shrinking: the
// stub's Flush reports not-drained immediately, regardless of load.
viper.Set("SENTRY_DSN", "http://public@127.0.0.1:1/1")
t.Cleanup(func() {
viper.Set("SENTRY_DSN", "")
sentry.CurrentHub().BindClient(nil)
sentryTransport = nil
viper.Set("SENTRY_DSN", "")
})
restoreDial := sentryDialCheckTimeout
sentryDialCheckTimeout = 500 * time.Millisecond
Expand All @@ -151,21 +48,18 @@ func TestSetupSentry_ProbeStall_WarnsAndWithholdsEnabledLine(t *testing.T) {

enabled := SetupSentry(testRelease)

assert.True(t, enabled, "a stalled probe means degraded, never disabled — capture stays on")
assert.Contains(t, warnBuf.String(), "startup probe did not flush",
"a transport that cannot drain within the window must be loud at boot")
assert.NotContains(t, infoBuf.String(), "Sentry error capture enabled",
"the success line must be withheld when the probe could not drain")
events := transport.Events()
require.Len(t, events, 1,
"the canary must reach the transport of the client SetupSentry just configured — the probe ran against THAT client, not some pre-bound stub")
assert.Equal(t, "sentry startup probe", events[0].Message)
assert.True(t, enabled, "a refused ingest host means degraded, never disabled. Capture stays on")
assert.Empty(t, transport.Events(),
"boot must hand the transport nothing. Sentry sees no event until the first real error")
assert.Contains(t, infoBuf.String(), "Sentry error capture enabled",
"the enabled line prints whenever init succeeded, whatever the dial check found")
assert.Contains(t, warnBuf.String(), "127.0.0.1:1",
"SetupSentry must still run the dial check, and the warning must name the unreachable host")
}

func TestSentryDialCheck_UnreachableHost_WarnsWithHost(t *testing.T) {
// 127.0.0.1:1 refuses instantly — the deterministic stand-in for the
// wrong-DSN-host misconfig class the startup probe cannot see (fast send
// failures still drain the queue). Shrink the dial window anyway so a
// 127.0.0.1:1 refuses instantly. It is the deterministic stand-in for the
// wrong-DSN-host misconfig class. Shrink the dial window anyway so a
// pathological environment cannot stall the suite.
restore := sentryDialCheckTimeout
sentryDialCheckTimeout = 500 * time.Millisecond
Expand Down
10 changes: 0 additions & 10 deletions rest/sentry_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,15 +103,6 @@ func (t *transportMock) Events() []*sentry.Event {
return append([]*sentry.Event(nil), t.events...)
}

// reset drops any captured events. enableSentry calls it after SetupSentry so
// the boot-time startup-probe canary (#134: SetupSentry now drives the probe)
// does not count toward a test's own event assertions.
func (t *transportMock) reset() {
t.mu.Lock()
defer t.mu.Unlock()
t.events = nil
}

// testRelease is the build-time version stand-in tests pass to SetupSentry.
const testRelease = "v-test-132"

Expand All @@ -133,7 +124,6 @@ func enableSentry(t *testing.T) *transportMock {
viper.Set("SENTRY_DSN", "")
})
require.True(t, SetupSentry(testRelease), "SetupSentry must enable capture when SENTRY_DSN is set")
tr.reset() // discard the boot-time startup-probe canary so tests count only their own events
return tr
}

Expand Down
13 changes: 6 additions & 7 deletions servers/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,13 +147,12 @@ func (server *MicroServer) Start() {
}

// Observability (PRD #112): errors-only Sentry capture, gated on SENTRY_DSN.
// Disabled (local/dev) nothing is initialised — one Info line, no client, no
// signal handler, so shutdown behaves exactly as before. Enabled, this
// BLOCKS boot before any listener opens: the dial pre-check (≤3s on an
// unreachable host) plus the startup-probe flush window (≤5s) — a worst-case
// ~8s delay on a degraded network, by design, so a broken pipeline is
// visible before traffic flows. The shutdown flush handler is installed only
// when capture is enabled.
// Disabled (local/dev) nothing is initialised: one Info line, no client, no
// signal handler, so shutdown behaves exactly as before. Enabled, the dial
// pre-check BLOCKS boot before any listener opens, for up to 3s on an
// unreachable host. That delay is accepted by design so the log names a
// broken pipeline before traffic flows. Start installs the shutdown flush
// handler only when capture is enabled.
if rest.SetupSentry(version) {
rest.FlushSentryOnShutdown()
}
Expand Down