Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
.crush
26 changes: 8 additions & 18 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

## 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.
Shared logging library for Fishbrain's Go services. Single-package Go module (`package logging`) that wraps [logrus](https://github.com/sirupsen/logrus) with Sentry error reporting, Datadog trace correlation, and NSQ log-level bridging.

**Module path**: `github.com/fishbrain/logging-go`

Expand All @@ -18,7 +18,7 @@ There is no linter, formatter, or Makefile configured. CI (`go.yml`) runs `go bu
## Project Structure

```
logging.go # All library code — types, logger init, entry helpers, Bugsnag/Sentry hooks
logging.go # All library code — types, logger init, entry helpers, Sentry hook
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)
Expand All @@ -37,25 +37,21 @@ This is a **single-file library** — everything lives in `logging.go` and `logg
- **`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
1. sentry.Init(...) — sets up Sentry client (if SentryDSN is set and environment matches ErrorNotifyReleaseStages)
2. Log = new(withSentry, config) — creates Logger with optionally Sentry hook 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 |
Expand All @@ -73,10 +69,6 @@ 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:
Expand All @@ -102,18 +94,16 @@ The `LogLevel` config string must be uppercase: `"ERROR"`, `"WARNING"`, `"INFO"`
- **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
- **Release-stage gating tests**: `TestShouldNotify` verifies the `shouldNotify` helper used for conditional Sentry 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.
4. **`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.
5. **Reversed nil check style**: The codebase uses Yoda conditions (`nil == Log`) in the `Init` function.
6. **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

Expand Down
3 changes: 0 additions & 3 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ toolchain go1.26.1

require (
github.com/DataDog/dd-trace-go/v2 v2.6.0
github.com/bugsnag/bugsnag-go/v2 v2.6.3
github.com/getsentry/sentry-go v0.43.0
github.com/nsqio/go-nsq v1.1.0
github.com/sirupsen/logrus v1.9.4
Expand All @@ -30,7 +29,6 @@ require (
github.com/DataDog/go-tuf v1.1.1-0.5.2 // indirect
github.com/DataDog/sketches-go v1.4.7 // indirect
github.com/Microsoft/go-winio v0.6.2 // indirect
github.com/bugsnag/panicwrap v1.3.4 // indirect
github.com/cenkalti/backoff/v5 v5.0.3 // indirect
github.com/cespare/xxhash/v2 v2.3.0 // indirect
github.com/cihub/seelog v0.0.0-20170130134532-f561c5e57575 // indirect
Expand All @@ -45,7 +43,6 @@ require (
github.com/google/uuid v1.6.0 // indirect
github.com/hashicorp/go-version v1.7.0 // indirect
github.com/json-iterator/go v1.1.12 // indirect
github.com/kardianos/osext v0.0.0-20190222173326-2bc1f35cddc0 // indirect
github.com/klauspost/compress v1.18.0 // indirect
github.com/klauspost/cpuid/v2 v2.2.3 // indirect
github.com/linkdata/deadlock v0.5.5 // indirect
Expand Down
12 changes: 4 additions & 8 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,6 @@ github.com/DataDog/sketches-go v1.4.7/go.mod h1:eAmQ/EBmtSO+nQp7IZMZVRPT4BQTmIc5
github.com/Microsoft/go-winio v0.5.0/go.mod h1:JPGBdM1cNvN/6ISo+n8V5iA4v8pBzdOpzfwIujj1a84=
github.com/Microsoft/go-winio v0.6.2 h1:F2VQgta7ecxGYO8k3ZZz3RS8fVIXVxONVUPlNERoyfY=
github.com/Microsoft/go-winio v0.6.2/go.mod h1:yd8OoFMLzJbo9gZq8j5qaps8bJ9aShtEA8Ipt1oGCvU=
github.com/bitly/go-simplejson v0.5.1 h1:xgwPbetQScXt1gh9BmoJ6j9JMr3TElvuIyjR8pgdoow=
github.com/bitly/go-simplejson v0.5.1/go.mod h1:YOPVLzCfwK14b4Sff3oP1AmGhI9T9Vsg84etUnlyp+Q=
github.com/bugsnag/bugsnag-go/v2 v2.6.3 h1:RQ30/n7OS+fz04WeV0gZ1Wq5RP+R6MD6QOohGCKI1g8=
github.com/bugsnag/bugsnag-go/v2 v2.6.3/go.mod h1:S9njhE7l6XCiKycOZ2zp0x1zoEE5nL3HjROCSsKc/3c=
github.com/bugsnag/panicwrap v1.3.4 h1:A6sXFtDGsgU/4BLf5JT0o5uYg3EeKgGx3Sfs+/uk3pU=
github.com/bugsnag/panicwrap v1.3.4/go.mod h1:D/8v3kj0zr8ZAKg1AQ6crr+5VwKN5eIywRkfhyM/+dE=
github.com/cenkalti/backoff/v5 v5.0.3 h1:ZN+IMa753KfX5hd8vVaMixjnqRZ3y8CuJKRKj1xcsSM=
github.com/cenkalti/backoff/v5 v5.0.3/go.mod h1:rkhZdG3JZukswDf7f0cwqPNk4K0sa+F97BxZthm/crw=
github.com/cespare/xxhash/v2 v2.1.1/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs=
Expand All @@ -62,6 +56,8 @@ github.com/ebitengine/purego v0.8.4 h1:CF7LEKg5FFOsASUj0+QwaXf8Ht6TlFxg09+S9wz0o
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-errors/errors v1.4.2 h1:J6MZopCL4uSllY1OfXM374weqZFFItUbrImctkmUxIA=
github.com/go-errors/errors v1.4.2/go.mod h1:sIVyrIiJhuEF+Pj9Ebtd6P/rEYROXFi3BopGUQ5a5Og=
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=
Expand Down Expand Up @@ -91,8 +87,6 @@ github.com/hashicorp/go-version v1.7.0 h1:5tqGy27NaOTB8yJKUZELlFAS/LTKJkrmONwQKe
github.com/hashicorp/go-version v1.7.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA=
github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnrnM=
github.com/json-iterator/go v1.1.12/go.mod h1:e30LSqwooZae/UwlEbR2852Gd8hjQvJoHmT4TnhNGBo=
github.com/kardianos/osext v0.0.0-20190222173326-2bc1f35cddc0 h1:iQTw/8FWTuc7uiaSepXwyf3o52HaUYcV+Tu66S3F5GA=
github.com/kardianos/osext v0.0.0-20190222173326-2bc1f35cddc0/go.mod h1:1NbS8ALrpOvjt0rHPNLyCIeMtbizbir8U//inJ+zuB8=
github.com/klauspost/compress v1.18.0 h1:c/Cqfb0r+Yi+JtIEq73FWXVkRonBlf0CRNYc8Zttxdo=
github.com/klauspost/compress v1.18.0/go.mod h1:2Pp+KzxcywXVXMr50+X0Q/Lsb43OQHYWRCY2AiWywWQ=
github.com/klauspost/cpuid/v2 v2.2.3 h1:sxCkb+qR91z4vsqw4vGGZlDgPz3G7gjaLyK3V8y70BU=
Expand Down Expand Up @@ -125,6 +119,8 @@ github.com/petermattis/goid v0.0.0-20250813065127-a731cc31b4fe h1:vHpqOnPlnkba8i
github.com/petermattis/goid v0.0.0-20250813065127-a731cc31b4fe/go.mod h1:pxMtw7cyUw6B2bRH0ZBANSPg+AoSud1I1iyJHI69jH4=
github.com/philhofer/fwd v1.2.0 h1:e6DnBTl7vGY+Gz322/ASL4Gyp1FspeMvx1RNDoToZuM=
github.com/philhofer/fwd v1.2.0/go.mod h1:RqIHx9QI14HlwKwm98g9Re5prTQ6LdeRQn+gXJFxsJM=
github.com/pingcap/errors v0.11.4 h1:lFuQV/oaUMGcD2tqt+01ROSmJs75VG1ToEOkZIZ4nE4=
github.com/pingcap/errors v0.11.4/go.mod h1:Oi8TUi2kEtXXLMJk9l1cGmz20kV3TaQ0usTwv5KuLY8=
github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10 h1:GFCKgmp0tecUJ0sJuv4pzYCqS9+RGSn52M3FUwPs+uo=
Expand Down
135 changes: 37 additions & 98 deletions logging.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ import (
"time"

"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"
Expand All @@ -30,20 +28,29 @@ type LoggingConfig struct {
LogLevel string
Environment string
AppVersion string
BugsnagAPIKey string
ErrorNotifyReleaseStages []string
BugsnagProjectPackages []string
SentryDSN string
}

type Logger struct {
*logrus.Logger
}

type bugsnagHook struct{}

type sentryHook struct{}

type errorWithStacktrace struct {
err error
stacktrace *sentry.Stacktrace
}

func (e *errorWithStacktrace) Error() string {
return e.err.Error()
}

func (e *errorWithStacktrace) Unwrap() error {
return e.err
}

func (l Logger) getNSQLogLevel() nsq.LogLevel {
switch l.Level {
case logrus.DebugLevel:
Expand Down Expand Up @@ -111,7 +118,7 @@ func (l *Logger) WithDDTrace(ctx context.Context) *Entry {
}

func (l *Logger) WithError(err error) *Entry {
return l.NewEntry().WithError(bugsnag_errors.New(err, 1))
return l.NewEntry().WithError(err)
}

func (e *Entry) WithField(field string, value interface{}) *Entry {
Expand Down Expand Up @@ -200,14 +207,13 @@ func (e *Entry) WithChannel(channel string) *Entry {
}

func (e *Entry) WithError(err error) *Entry {
return &Entry{e.Entry.WithError(bugsnag_errors.New(err, 1))}
return &Entry{e.Entry.WithError(err)}
}

func (e *Entry) WithDDTrace(ctx context.Context) *Entry {
var traceID, spanID uint64
span, ok := tracer.SpanFromContext(ctx)
if ok {
// there was a span in the context
traceID, spanID = span.Context().TraceIDLower(), span.Context().SpanID()
return &Entry{e.Entry.WithFields(logrus.Fields{
"dd.trace_id": traceID,
Expand All @@ -217,12 +223,19 @@ func (e *Entry) WithDDTrace(ctx context.Context) *Entry {
return e
}

func Errorf(format string, a ...interface{}) *bugsnag_errors.Error {
return bugsnag_errors.New(fmt.Errorf(format, a...), 1)
func Errorf(format string, a ...interface{}) error {
return fmt.Errorf(format, a...)
}

func ErrorWithStacktrace(err error) *bugsnag_errors.Error {
return bugsnag_errors.New(err, 1)
func ErrorWithStacktrace(err error) error {
st := sentry.NewStacktrace()
if st != nil && len(st.Frames) > 0 {
st.Frames = st.Frames[:len(st.Frames)-1]
}
return &errorWithStacktrace{
err: err,
stacktrace: st,
}
}

func getLogrusLogLevel(level string) logrus.Level {
Expand All @@ -242,52 +255,9 @@ func getLogrusLogLevel(level string) logrus.Level {
return loglevel
}

func (b *bugsnagHook) 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)
}

metadata := bugsnag.MetaData{}
metadata["metadata"] = make(map[string]interface{})
for key, val := range entry.Data {
if key != logrus.ErrorKey {
metadata["metadata"][key] = val
}
}

skipStackFrames := 4
errWithStack := bugsnag_errors.New(notifyErr, skipStackFrames)
bugsnagErr := bugsnag.Notify(errWithStack, metadata)
if bugsnagErr != nil {
return bugsnagErr
}

return nil
}

func (b *bugsnagHook) Levels() []logrus.Level {
return []logrus.Level{
logrus.ErrorLevel,
logrus.FatalLevel,
logrus.PanicLevel,
}
}

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)
Expand All @@ -304,9 +274,16 @@ func (s *sentryHook) Fire(entry *logrus.Entry) error {
event.Level = sentry.LevelFatal
}
event.Message = notifyErr.Error()
var stacktrace *sentry.Stacktrace
var errSt *errorWithStacktrace
if errors.As(notifyErr, &errSt) {
stacktrace = errSt.stacktrace
}

event.Exception = []sentry.Exception{{
Type: reflect.TypeOf(notifyErr).String(),
Value: notifyErr.Error(),
Type: reflect.TypeOf(notifyErr).String(),
Value: notifyErr.Error(),
Stacktrace: stacktrace,
}}

extra := make(map[string]interface{})
Expand All @@ -329,7 +306,7 @@ func (s *sentryHook) Levels() []logrus.Level {
}
}

func new(withBugsnag bool, withSentry bool, config LoggingConfig) *Logger {
func new(withSentry bool, config LoggingConfig) *Logger {
log := logrus.New()
logrus.ErrorKey = "error.message"
log.Formatter = &logrus.JSONFormatter{
Expand All @@ -342,10 +319,6 @@ func new(withBugsnag bool, withSentry bool, config LoggingConfig) *Logger {
}
log.Level = getLogrusLogLevel(config.LogLevel)

if withBugsnag {
log.Hooks.Add(&bugsnagHook{})
}

if withSentry {
log.Hooks.Add(&sentryHook{})
}
Expand All @@ -364,40 +337,6 @@ func shouldNotify(releaseStages []string, environment string) bool {

func Init(config LoggingConfig) {
if nil == Log {
bugsnag.Configure(bugsnag.Configuration{
APIKey: config.BugsnagAPIKey,
ReleaseStage: config.Environment,
AppVersion: config.AppVersion,
NotifyReleaseStages: config.ErrorNotifyReleaseStages,
ProjectPackages: config.BugsnagProjectPackages,
Logger: stdlog.New(new(false, false, config).Writer(), "bugsnag: ", 0),
})
bugsnag.OnBeforeNotify(
func(event *bugsnag.Event, config *bugsnag.Configuration) error {
errClass := event.ErrorClass
count := 0
wrappedError := event.Error.Err
for {
if errClass != "*fmt.wrapError" {
break
}

wrappedError = errors.Unwrap(wrappedError)
if wrappedError != nil {
errClass = reflect.TypeOf(wrappedError).String()
} else {
break
}
count++
if count >= 11 {
stdlog.Printf("Failed to unwrap error %s %s %+v", event.ErrorClass, errClass, event.Error)
break
}
}
event.ErrorClass = errClass
return nil
})

withSentry := false
if config.SentryDSN != "" && shouldNotify(config.ErrorNotifyReleaseStages, config.Environment) {
err := sentry.Init(sentry.ClientOptions{
Expand All @@ -412,6 +351,6 @@ func Init(config LoggingConfig) {
}
}

Log = new(true, withSentry, config)
Log = new(withSentry, config)
}
}
Loading