Merge pull request #124 from fishbrain/add-sentry-support

Add support for Sentry error reporting

Author: lhansford

AGENTS.md

@@ -0,0 +1,124 @@
+# AGENTS.md — logging-go
+
+## Overview
+
+Shared logging library for Fishbrain's Go services. Single-package Go module (`package logging`) that wraps [logrus](https://github.com/sirupsen/logrus) with Bugsnag error reporting, Sentry error reporting, Datadog trace correlation, and NSQ log-level bridging.
+
+**Module path**: `github.com/fishbrain/logging-go`
+
+## Commands
+
+| Task  | Command          |
+|-------|------------------|
+| Build | `go build ./...` |
+| Test  | `go test ./...`  |
+
+There is no linter, formatter, or Makefile configured. CI (`go.yml`) runs `go build -v .` only — no test step in CI.
+
+## Project Structure
+
+```
+logging.go        # All library code — types, logger init, entry helpers, Bugsnag/Sentry hooks
+logging_test.go   # All tests
+go.mod / go.sum   # Module definition (Go 1.24+, toolchain 1.26)
+.tool-versions    # asdf version pinning (go 1.26.0)
+```
+
+This is a **single-file library** — everything lives in `logging.go` and `logging_test.go`. No subdirectories, no `cmd/`, no `internal/`.
+
+## Architecture & Key Types
+
+### Global singleton
+
+`Init(LoggingConfig)` initializes the package-level `Log *Logger` variable. It is guarded by a nil check (not a `sync.Once`), so it only runs once. `TestMain` calls `Init(LoggingConfig{})` to set up the singleton before tests run.
+
+### Type hierarchy
+
+- **`Logger`** — wraps `*logrus.Logger`. Provides `WithField`, `WithError`, `WithDDTrace`, `NewEntry`, and `NSQLogger`.
+- **`Entry`** — wraps `*logrus.Entry`. Provides domain-specific field helpers (`WithUser`, `WithEvent`, `WithChannel`, `WithDuration`, etc.) that return `*Entry` for chaining.
+- **`NSQLogger`** — adaptor that implements `Output(int, string) error` so it can be passed to `nsq.SetLogger`.
+- **`bugsnagHook`** — logrus hook that fires on Error/Fatal/Panic levels, forwarding to Bugsnag with metadata.
+- **`sentryHook`** — logrus hook that fires on Error/Fatal/Panic levels, forwarding to Sentry with metadata and extra fields.
+
+### Initialization flow
+
+```
+Init(config) →
+  1. bugsnag.Configure(...)       — sets up Bugsnag client
+  2. bugsnag.OnBeforeNotify(...)  — unwraps *fmt.wrapError to get real error class
+  3. sentry.Init(...)             — sets up Sentry client (if SentryDSN is set and environment matches ErrorNotifyReleaseStages)
+  4. Log = new(true, withSentry, config) — creates Logger with Bugsnag and optionally Sentry hooks attached
+```
+
+## Key Dependencies
+
+| Dependency | Purpose |
+|---|---|
+| `github.com/sirupsen/logrus` | Structured logging (JSON formatter) |
+| `github.com/bugsnag/bugsnag-go/v2` | Error reporting to Bugsnag |
+| `github.com/DataDog/dd-trace-go/v2` | Datadog APM trace/span ID injection |
+| `github.com/getsentry/sentry-go` | Error reporting to Sentry |
+| `github.com/nsqio/go-nsq` | NSQ message queue log-level bridging |
+| `github.com/stretchr/testify` | Test assertions |
+
+## Code Patterns & Conventions
+
+### Fluent entry builder
+
+All `With*` methods return `*Entry` to support chaining:
+
+```go
+Log.WithDDTrace(ctx).WithUser(userID).WithDuration(d).Info("processed request")
+```
+
+When adding new field helpers, follow this pattern: method on `*Entry`, return `*Entry`, delegate to `e.WithField(...)`.
+
+### Error wrapping
+
+Errors passed to `WithError` are wrapped with `bugsnag_errors.New(err, 1)` to capture stack traces. The `1` parameter controls stack frame skipping. The standalone `Errorf` and `ErrorWithStacktrace` functions also use this pattern.
+
+### JSON log output
+
+Logrus is configured with `JSONFormatter` and custom field mapping:
+- `msg` → `message`
+- `func` → `logger.method_name`
+- `file` → `logger.name`
+- `error` key → `error.message`
+- Timestamp format: `RFC3339Nano`
+
+### Log levels
+
+The `LogLevel` config string must be uppercase: `"ERROR"`, `"WARNING"`, `"INFO"`, `"DEBUG"`. Unknown values default to `InfoLevel`.
+
+### NSQ log bridging
+
+`Logger.NSQLogger()` returns an `(NSQLogger, nsq.LogLevel)` tuple for plugging into `nsq.SetLogger`. The `NSQLogger.Output` method parses the 3-character prefix from NSQ log messages to route them to the correct logrus level.
+
+## Testing
+
+- **Framework**: stdlib `testing` + `testify/assert`
+- **Setup**: `TestMain` initializes the global `Log` singleton via `Init(LoggingConfig{})`
+- **Log capture**: Tests use `os.Pipe()` to capture log output by swapping `Log.Out`, then assert on the captured string content
+- **Concurrency test**: `TestConcurrentUseOfEntry` verifies entries are safe for concurrent use across goroutines
+- **Table-driven tests**: `TestGetLogrusLogLevel` uses a table-driven approach with a package-level test data slice
+- **Sentry hook tests**: `TestSentryHookFire`, `TestSentryHookLevels`, `TestNewWithSentry`, and `TestNewWithoutSentry` cover the Sentry hook and its integration into the logger
+- **Release-stage gating tests**: `TestShouldNotify` verifies the `shouldNotify` helper used for conditional Sentry/Bugsnag activation
+
+## Gotchas
+
+1. **No CI test step**: The GitHub Actions workflow builds but does not run tests. Running `go test ./...` locally is essential before pushing.
+2. **Singleton guard is not sync.Once**: `Init` uses `if nil == Log` — safe for single-goroutine init, but not for concurrent callers. In practice this is fine since `Init` is called once at service startup.
+3. **`ioutil.ReadAll` in tests**: Tests use the deprecated `io/ioutil` package. New code should use `io.ReadAll` instead.
+4. **Bugsnag error unwrapping limit**: The `OnBeforeNotify` handler unwraps `*fmt.wrapError` chains up to 11 levels deep, then logs and stops.
+5. **`logrus.ErrorKey` is mutated globally**: `new()` sets `logrus.ErrorKey = "error.message"` as a side effect — this affects all logrus loggers in the process, not just this one.
+6. **Reversed nil check style**: The codebase uses Yoda conditions (`nil == Log`) in the `Init` function.
+7. **`BugsnagNotifyReleaseStages` renamed**: The config field was renamed to `ErrorNotifyReleaseStages` and is now shared between Bugsnag and Sentry for release-stage gating.
+8. **Sentry is conditional**: Sentry is only initialized when `SentryDSN` is non-empty and the current `Environment` is in `ErrorNotifyReleaseStages`. If `sentry.Init` fails, it logs to stderr and proceeds without the Sentry hook.
+
+## Releasing
+
+Create a GitHub Release. The module is imported by other Fishbrain Go services via its module path. Versioning follows Go module semantics (semver tags).
+
+## Ownership
+
+Owned by `@fishbrain/platform-team` (see `CODEOWNERS`). Dependency updates managed by Renovate (see `renovate.json`).

go.mod

@@ -36,6 +36,7 @@ require (
 	github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
 	github.com/dustin/go-humanize v1.0.1 // indirect
 	github.com/ebitengine/purego v0.8.4 // indirect
+	github.com/getsentry/sentry-go v0.43.0 // indirect
 	github.com/go-logr/logr v1.4.3 // indirect
 	github.com/go-logr/stdr v1.2.2 // indirect
 	github.com/go-ole/go-ole v1.3.0 // indirect

go.sum

@@ -60,6 +60,8 @@ github.com/dustin/go-humanize v1.0.1 h1:GzkhY7T5VNhEkwH0PVJgjz+fX1rhBrR7pRT3mDkp
 github.com/dustin/go-humanize v1.0.1/go.mod h1:Mu1zIs6XwVuF/gI1OepvI0qD18qycQx+mFykh5fBlto=
 github.com/ebitengine/purego v0.8.4 h1:CF7LEKg5FFOsASUj0+QwaXf8Ht6TlFxg09+S9wz0omw=
 github.com/ebitengine/purego v0.8.4/go.mod h1:iIjxzd6CiRiOG0UyXP+V1+jWqUXVjPKLAI0mRfJZTmQ=
+github.com/getsentry/sentry-go v0.43.0 h1:XbXLpFicpo8HmBDaInk7dum18G9KSLcjZiyUKS+hLW4=
+github.com/getsentry/sentry-go v0.43.0/go.mod h1:XDotiNZbgf5U8bPDUAfvcFmOnMQQceESxyKaObSssW0=
 github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A=
 github.com/go-logr/logr v1.4.3 h1:CjnDlHq8ikf6E492q6eKboGOC0T8CDaOvkHCIg8idEI=
 github.com/go-logr/logr v1.4.3/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY=

logging.go

@@ -13,6 +13,7 @@ import (
 	"github.com/DataDog/dd-trace-go/v2/ddtrace/tracer"
 	"github.com/bugsnag/bugsnag-go/v2"
 	bugsnag_errors "github.com/bugsnag/bugsnag-go/v2/errors"
+	"github.com/getsentry/sentry-go"
 	nsq "github.com/nsqio/go-nsq"
 	"github.com/sirupsen/logrus"
 )
@@ -26,12 +27,13 @@ var (
 )
 
 type LoggingConfig struct {
-	LogLevel                   string
-	Environment                string
-	AppVersion                 string
-	BugsnagAPIKey              string
-	BugsnagNotifyReleaseStages []string
-	BugsnagProjectPackages     []string
+	LogLevel                 string
+	Environment              string
+	AppVersion               string
+	BugsnagAPIKey            string
+	ErrorNotifyReleaseStages []string
+	BugsnagProjectPackages   []string
+	SentryDSN                string
 }
 
 type Logger struct {
@@ -40,6 +42,8 @@ type Logger struct {
 
 type bugsnagHook struct{}
 
+type sentryHook struct{}
+
 func (l Logger) getNSQLogLevel() nsq.LogLevel {
 	switch l.Level {
 	case logrus.DebugLevel:
@@ -279,7 +283,53 @@ func (b *bugsnagHook) Levels() []logrus.Level {
 	}
 }
 
-func new(withBugsnag bool, config LoggingConfig) *Logger {
+func (s *sentryHook) Fire(entry *logrus.Entry) error {
+	var notifyErr error
+	switch err := entry.Data[logrus.ErrorKey].(type) {
+	case *bugsnag_errors.Error:
+		notifyErr = err
+	case error:
+		if entry.Message != "" {
+			notifyErr = fmt.Errorf("%s: %w", entry.Message, err)
+		} else {
+			notifyErr = err
+		}
+	default:
+		notifyErr = fmt.Errorf("%s", entry.Message)
+	}
+
+	event := sentry.NewEvent()
+	event.Level = sentry.LevelError
+	if entry.Level == logrus.FatalLevel {
+		event.Level = sentry.LevelFatal
+	}
+	event.Message = notifyErr.Error()
+	event.Exception = []sentry.Exception{{
+		Type:  reflect.TypeOf(notifyErr).String(),
+		Value: notifyErr.Error(),
+	}}
+
+	extra := make(map[string]interface{})
+	for key, val := range entry.Data {
+		if key != logrus.ErrorKey {
+			extra[key] = val
+		}
+	}
+	event.Extra = extra
+
+	sentry.CaptureEvent(event)
+	return nil
+}
+
+func (s *sentryHook) Levels() []logrus.Level {
+	return []logrus.Level{
+		logrus.ErrorLevel,
+		logrus.FatalLevel,
+		logrus.PanicLevel,
+	}
+}
+
+func new(withBugsnag bool, withSentry bool, config LoggingConfig) *Logger {
 	log := logrus.New()
 	logrus.ErrorKey = "error.message"
 	log.Formatter = &logrus.JSONFormatter{
@@ -296,18 +346,31 @@ func new(withBugsnag bool, config LoggingConfig) *Logger {
 		log.Hooks.Add(&bugsnagHook{})
 	}
 
+	if withSentry {
+		log.Hooks.Add(&sentryHook{})
+	}
+
 	return &Logger{log}
 }
 
+func shouldNotify(releaseStages []string, environment string) bool {
+	for _, stage := range releaseStages {
+		if stage == environment {
+			return true
+		}
+	}
+	return false
+}
+
 func Init(config LoggingConfig) {
 	if nil == Log {
 		bugsnag.Configure(bugsnag.Configuration{
 			APIKey:              config.BugsnagAPIKey,
 			ReleaseStage:        config.Environment,
 			AppVersion:          config.AppVersion,
-			NotifyReleaseStages: config.BugsnagNotifyReleaseStages,
+			NotifyReleaseStages: config.ErrorNotifyReleaseStages,
 			ProjectPackages:     config.BugsnagProjectPackages,
-			Logger:              stdlog.New(new(false, config).Writer(), "bugsnag: ", 0),
+			Logger:              stdlog.New(new(false, false, config).Writer(), "bugsnag: ", 0),
 		})
 		bugsnag.OnBeforeNotify(
 			func(event *bugsnag.Event, config *bugsnag.Configuration) error {
@@ -334,6 +397,21 @@ func Init(config LoggingConfig) {
 				event.ErrorClass = errClass
 				return nil
 			})
-		Log = new(true, config)
+
+		withSentry := false
+		if config.SentryDSN != "" && shouldNotify(config.ErrorNotifyReleaseStages, config.Environment) {
+			err := sentry.Init(sentry.ClientOptions{
+				Dsn:         config.SentryDSN,
+				Environment: config.Environment,
+				Release:     config.AppVersion,
+			})
+			if err != nil {
+				stdlog.Printf("sentry.Init: %s", err)
+			} else {
+				withSentry = true
+			}
+		}
+
+		Log = new(true, withSentry, config)
 	}
 }