diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a58814..5077486 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,37 @@ # Changelog ## [Unreleased] + +## [0.2.0] + +### Added + +- `ErrAny(v any) []zap.Field` — extracts ECS `error.*` fields from any value, + intended for `recover()` payloads (typed as `any`). Delegates to `Err(err)` + when the value satisfies `error`; falls back to `fmt.Sprint` / + `fmt.Sprintf("%T", v)` for non-error values; returns nil for nil input. +- `Err(err)` now extracts `error.stack_trace` from `github.com/pkg/errors` + errors as well — previously only `samber/oops`-style `StackTrace() []byte` + was supported. + +### Changed + +- `Err(err)` stack-trace extraction is now delegated to an internal + `extractStackTrace` helper that walks the error chain via `errors.As` and + tries the `samber/oops` interface first, then the `pkg/errors` interface. + No public API change. +- `ErrAny(v)` guards against typed-nil error inputs (e.g. `(*MyErr)(nil)` + cast to `error`) — emits `error.message=""` + `error.type` instead of + panicking inside `err.Error()`. + +### Dependencies + +- Add `github.com/pkg/errors v0.9.1` (direct) — required to type-check the + pkg/errors `StackTrace() pkgerrors.StackTrace` interface. + +### Documentation + +- README clarifies the relationship between `ecsf.Err`, `ecsf.ErrorXxx` + single-field helpers, and `zap.Error(err)` under the ecszap encoder; in + particular that `zap.Error` only produces `error.stack_trace` when the + underlying error implements pkg/errors' `StackTracer`. diff --git a/README.md b/README.md index 47ce769..378d998 100644 --- a/README.md +++ b/README.md @@ -84,7 +84,7 @@ See [`example/main.go`](example/main.go) for a runnable end-to-end example. ## Coverage -v0.1.0 covers ~116 helpers across these top-level ECS fieldsets: +v0.2.0 covers ~117 helpers across these top-level ECS fieldsets: | Fieldset | Helpers | Notes | | ------------------------------------ | ------------------ | --------------------------------------------- | @@ -92,7 +92,7 @@ v0.1.0 covers ~116 helpers across these top-level ECS fieldsets: | `host.*` | 9 | top-level only; metrics deferred | | `process.*` | 11 | top-level; io / env_vars / entity_id deferred | | `event.*` | 22 + 4 typed enums | kind / outcome / category / type are typed | -| `error.*` | 5 + `Err()` helper | full top-level | +| `error.*` | 5 + `Err()` / `ErrAny()` | full top-level | | `log.*` (+ origin) | 6 | syslog deferred | | `trace` / `span` / `transaction` ids | 3 | full | | `http.*` | 13 | full | @@ -120,6 +120,33 @@ fields (`numeric_labels.*`, `service.address`, `service.ephemeral_id`, etc.). | [`github.com/elastic/ecs`](https://github.com/elastic/ecs) | Canonical ECS schema definitions and document marshaling for full-document construction. | Different concern. ecsfields is for incremental field-by-field logging in zap, not for building complete ECS documents. | | [`github.com/andrewkroh/go-ecs`](https://github.com/andrewkroh/go-ecs) | ECS schema query and introspection tool. | Different concern (schema introspection at runtime, not log emission). | +### Using ecsfields error helpers vs `zap.Error` + +Under the ecszap encoder, `zap.Error(err)` produces ECS `error.message` +(always) and `error.stack_trace` (only when `err` implements pkg/errors' +`StackTrace() pkgerrors.StackTrace`). Plain `errors.New(...)` does not produce +a stack. You do not have to migrate existing `zap.Error(err)` call sites +purely for ECS compliance. + +That said, `ecsf.Err(err)` does three things `zap.Error(err)` doesn't: + +1. **Always emits `error.type`.** Lets you filter Kibana by Go error class + (`*pq.Error`, `*net.OpError`, ...). `zap.Error` never sets this field. +2. **Encoder-agnostic.** Produces correct ECS keys regardless of encoder. + `zap.Error` only outputs ECS shape under ecszap; with the default JSON + encoder it falls back to a flat `"error":"..."` string. +3. **Composable.** Single-field helpers (`ErrorCode`, `ErrorID`, + `ErrorStackTrace`) and `ErrAny(any)` (for `recover()` values) cover cases + that have no Go `error` to pass to `zap.Error` in the first place. + +`ecsf.Err(err)` extracts `error.stack_trace` via either pkg/errors' +`StackTrace() pkgerrors.StackTrace` or samber/oops' `StackTrace() []byte`, +so any error wrapped by either library carries its stack through. + +Recommended: use `ecsf.Err(err)` for new code or any error you want to +classify in Kibana. Keep existing `zap.Error(err)` call sites if you only +need message + stack_trace and you're committed to the ecszap encoder. + ## License [MIT](LICENSE) diff --git a/docs/ecs-coverage.md b/docs/ecs-coverage.md index 99cadfa..b449e3d 100644 --- a/docs/ecs-coverage.md +++ b/docs/ecs-coverage.md @@ -1,4 +1,4 @@ -# ECS coverage — v0.1.0 +# ECS coverage — v0.2.0 Pinned to **ECS 8.17**. This document tracks which ECS field families are covered, deferred, or out of scope. @@ -12,7 +12,7 @@ covered, deferred, or out of scope. | `host.*` (top-level) | 9 | `HostIP` / `HostMAC` are variadic; `HostUptime` emits seconds | | `process.*` (top-level) | 11 | `ProcessUptime` emits seconds; `ProcessStart` is `time.Time`; endpoint-security subtrees excluded | | `event.*` | 22 | `event.duration` emits **nanoseconds**; `event.original` is bytes; typed enums for `kind`/`outcome`/`category`/`type` | -| `error.*` + `Err()` | 5 + 1 | `Err()` extracts `error.message` / `error.type` always, `error.stack_trace` when source implements `interface{ StackTrace() []byte }` | +| `error.*` + `Err()` / `ErrAny()` | 5 + 2 | `Err(error)` and `ErrAny(any)` both extract `error.message` / `error.type` always, plus `error.stack_trace` when the source implements either `StackTrace() []byte` (samber/oops) or `StackTrace() pkgerrors.StackTrace` (github.com/pkg/errors). `ErrAny` accepts `recover()` payloads (typed as `any`) and delegates to `Err` when the value satisfies `error`. | | `log.*` | 6 | Includes `log.origin.*` | | `trace.id`, `span.id`, `transaction.id` | 3 | APM correlation | | `http.*` | 13 | Bytes are `int64`, status code is `int` | @@ -20,7 +20,7 @@ covered, deferred, or out of scope. | `client.*` (top-level) | 8 | Excludes network-monitoring subtrees | | `server.*` (top-level) | 8 | Mirrors `client.*` | | `user_agent.*` | 4 | `original`, `name`, `version`, `device.name` | -| **Total** | **~116** | | +| **Total** | **~117** | | ## Deferred (additive in future v1.x) diff --git a/go.mod b/go.mod index e8af7fe..ca34414 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,7 @@ module github.com/maxence2997/ecsfields go 1.22.0 require ( + github.com/pkg/errors v0.9.1 github.com/stretchr/testify v1.11.1 go.elastic.co/ecszap v1.0.3 go.uber.org/zap v1.28.0 @@ -10,7 +11,6 @@ require ( require ( github.com/davecgh/go-spew v1.1.1 // indirect - github.com/pkg/errors v0.9.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect go.uber.org/multierr v1.10.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect diff --git a/zap/error.go b/zap/error.go index c62e587..c5c202e 100644 --- a/zap/error.go +++ b/zap/error.go @@ -11,7 +11,9 @@ package zap import ( "errors" "fmt" + "reflect" + pkgerrors "github.com/pkg/errors" "go.uber.org/zap" "go.uber.org/zap/zapcore" ) @@ -32,21 +34,27 @@ func ErrorCode(code string) zapcore.Field { return zap.String("error.code", code // ErrorID emits ECS error.id (a unique identifier for the error instance). func ErrorID(id string) zapcore.Field { return zap.String("error.id", id) } -// stackTracer is the conventional interface for errors that carry a captured stack. -// samber/oops satisfies this interface natively. -type stackTracer interface { +// stackTracerBytes is satisfied by errors that expose a pre-formatted stack +// trace as bytes. samber/oops errors satisfy this interface natively. +type stackTracerBytes interface { StackTrace() []byte } +// stackTracerPCs is satisfied by errors that expose a stack trace as +// pkg/errors-style program counters. github.com/pkg/errors errors (e.g. those +// returned by pkgerrors.New / pkgerrors.Wrap) satisfy this interface natively. +type stackTracerPCs interface { + StackTrace() pkgerrors.StackTrace +} + // Err extracts ECS error.* fields from a Go error. It returns: // // - error.message: always (err.Error()) // - error.type: always (fmt.Sprintf("%T", err)) -// - error.stack_trace: if any error in the chain implements interface{ StackTrace() []byte } -// -// The StackTrace method must have signature: StackTrace() []byte. -// Note: github.com/pkg/errors exposes StackTrace() errors.StackTrace ([]uintptr) -// and does NOT satisfy this interface. Use a wrapper or samber/oops instead. +// - error.stack_trace: if any error in the chain implements one of the +// conventional stack-trace interfaces — checked in this order: +// 1. interface{ StackTrace() []byte } (samber/oops) +// 2. interface{ StackTrace() pkgerrors.StackTrace } (github.com/pkg/errors) // // Err is the only multi-field constructor in the library, provided so callers // do not need any specific zap encoder (e.g. ecszap) to obtain a stack trace. @@ -59,11 +67,87 @@ func Err(err error) []zapcore.Field { ErrorMessage(err.Error()), ErrorType(fmt.Sprintf("%T", err)), } - var st stackTracer - if errors.As(err, &st) { - if stack := st.StackTrace(); len(stack) > 0 { - fields = append(fields, ErrorStackTrace(stack)) - } + if stack := extractStackTrace(err); len(stack) > 0 { + fields = append(fields, ErrorStackTrace(stack)) } return fields } + +// extractStackTrace walks the error chain and returns the first stack trace +// found, in []byte form ready for ErrorStackTrace. Returns nil if no error in +// the chain carries a stack trace. +func extractStackTrace(err error) []byte { + var bytesST stackTracerBytes + if errors.As(err, &bytesST) { + if s := bytesST.StackTrace(); len(s) > 0 { + return s + } + } + var pcsST stackTracerPCs + if errors.As(err, &pcsST) { + if s := pcsST.StackTrace(); len(s) > 0 { + // pkg/errors.StackTrace implements fmt.Formatter; %+v renders each + // frame as "function\n\tfile:line", matching what users expect to + // see in error.stack_trace. + return fmt.Appendf(nil, "%+v", s) + } + } + return nil +} + +// ErrAny extracts ECS error.* fields from any value, intended for cases where +// the input is not statically typed as error — most commonly the result of +// recover() during panic handling. Behavior by input type: +// +// - nil: returns nil (no fields) +// - typed-nil error: error.type emitted, error.message = "" — never +// calls Error() on the typed-nil receiver, which would panic +// - error: delegates to Err(err) — error.stack_trace included if +// the error implements either StackTrace() []byte (samber/oops) or +// StackTrace() pkgerrors.StackTrace (github.com/pkg/errors) +// - other: error.message = fmt.Sprint(v); error.type = fmt.Sprintf("%T", v) +// +// ErrAny intentionally does not call runtime/debug.Stack() itself. To attach +// the panic stack, append ErrorStackTrace(debug.Stack()) at the call site — +// callers may want to skip the cost or use a different stack source. +// +// Typical panic recovery: +// +// defer func() { +// if r := recover(); r != nil { +// fields := ErrAny(r) +// fields = append(fields, ErrorStackTrace(debug.Stack())) +// logger.Error("panic recovered", fields...) +// } +// }() +func ErrAny(v any) []zapcore.Field { + if v == nil { + return nil + } + if err, ok := v.(error); ok { + if isTypedNil(err) { + return []zapcore.Field{ + ErrorMessage(""), + ErrorType(fmt.Sprintf("%T", err)), + } + } + return Err(err) + } + return []zapcore.Field{ + ErrorMessage(fmt.Sprint(v)), + ErrorType(fmt.Sprintf("%T", v)), + } +} + +// isTypedNil reports whether v is non-nil at the interface level but holds a +// nil concrete value (e.g. (*MyErr)(nil) cast to error). Calling methods that +// dereference the receiver on such a value panics, so ErrAny short-circuits +// before invoking err.Error(). +func isTypedNil(v any) bool { + rv := reflect.ValueOf(v) + switch rv.Kind() { + case reflect.Ptr, reflect.Map, reflect.Slice, reflect.Chan, reflect.Func, reflect.Interface: + return rv.IsNil() + } + return false +} diff --git a/zap/error_test.go b/zap/error_test.go index 1f3413c..6665c78 100644 --- a/zap/error_test.go +++ b/zap/error_test.go @@ -7,6 +7,7 @@ import ( "fmt" "testing" + pkgerrors "github.com/pkg/errors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.uber.org/zap/zapcore" @@ -109,3 +110,150 @@ func TestErr_StackTracer_EmitsStackTrace(t *testing.T) { } } } + +func TestErrAny_Nil(t *testing.T) { + assert.Nil(t, ecszap.ErrAny(nil)) +} + +func TestErrAny_Error_DelegatesToErr(t *testing.T) { + err := errors.New("boom") + got := ecszap.ErrAny(err) + require.Len(t, got, 2) + + keys := []string{got[0].Key, got[1].Key} + assert.ElementsMatch(t, []string{"error.message", "error.type"}, keys) + for _, f := range got { + switch f.Key { + case "error.message": + assert.Equal(t, "boom", f.String) + case "error.type": + assert.Equal(t, fmt.Sprintf("%T", err), f.String) + } + } +} + +func TestErrAny_StackTracerError_IncludesStackTrace(t *testing.T) { + err := &fakeStackTracer{msg: "kaboom", stack: []byte("goroutine 1...")} + got := ecszap.ErrAny(err) + require.Len(t, got, 3) + + var keys []string + for _, f := range got { + keys = append(keys, f.Key) + } + assert.ElementsMatch(t, + []string{"error.message", "error.type", "error.stack_trace"}, + keys, + ) +} + +func TestErrAny_String(t *testing.T) { + got := ecszap.ErrAny("oops") + require.Len(t, got, 2) + + for _, f := range got { + switch f.Key { + case "error.message": + assert.Equal(t, "oops", f.String) + case "error.type": + assert.Equal(t, "string", f.String) + default: + t.Fatalf("unexpected key %q", f.Key) + } + } +} + +func TestErrAny_Int(t *testing.T) { + got := ecszap.ErrAny(42) + require.Len(t, got, 2) + + for _, f := range got { + switch f.Key { + case "error.message": + assert.Equal(t, "42", f.String) + case "error.type": + assert.Equal(t, "int", f.String) + default: + t.Fatalf("unexpected key %q", f.Key) + } + } +} + +type panicPayload struct { + Reason string +} + +func TestErrAny_Struct(t *testing.T) { + got := ecszap.ErrAny(panicPayload{Reason: "deadlocked"}) + require.Len(t, got, 2) + + for _, f := range got { + switch f.Key { + case "error.message": + assert.Equal(t, "{deadlocked}", f.String) + case "error.type": + assert.Equal(t, "zap_test.panicPayload", f.String) + default: + t.Fatalf("unexpected key %q", f.Key) + } + } +} + +func TestErr_PkgErrorsStackTracer_EmitsStackTrace(t *testing.T) { + err := pkgerrors.New("boom") + got := ecszap.Err(err) + require.Len(t, got, 3) + + var keys []string + for _, f := range got { + keys = append(keys, f.Key) + } + assert.ElementsMatch(t, + []string{"error.message", "error.type", "error.stack_trace"}, + keys, + ) + + for _, f := range got { + switch f.Key { + case "error.message": + assert.Equal(t, "boom", f.String) + case "error.stack_trace": + assert.Equal(t, zapcore.ByteStringType, f.Type) + enc := zapcore.NewMapObjectEncoder() + f.AddTo(enc) + stack := enc.Fields["error.stack_trace"].(string) + assert.NotEmpty(t, stack) + assert.Contains(t, stack, "TestErr_PkgErrorsStackTracer_EmitsStackTrace", + "stack should reference the test function frame") + } + } +} + +// derefingErr panics on Error() if the receiver is nil — emulating the common +// Go gotcha where panic(typedNilError) is recovered as a non-nil error +// interface holding a nil pointer. +type derefingErr struct{ msg string } + +func (d *derefingErr) Error() string { return d.msg } + +func TestErrAny_TypedNilPointerError_DoesNotPanic(t *testing.T) { + var typedNil *derefingErr + var asInterface error = typedNil + + var got []zapcore.Field + require.NotPanics(t, func() { + got = ecszap.ErrAny(asInterface) + }) + require.Len(t, got, 2) + + for _, f := range got { + switch f.Key { + case "error.message": + assert.Equal(t, "", f.String) + case "error.type": + assert.Equal(t, "*zap_test.derefingErr", f.String) + default: + t.Fatalf("unexpected key %q", f.Key) + } + } +}