search

getsentry / raven-go

Sentry client in Go
https://sentry.io
BSD 3-Clause "New" or "Revised" License
561 stars 148 forks source link

CaptureError should also have extra field #196

Closed pzhao5 closed 6 years ago

pzhao5 commented 6 years ago

Currently, CaptureError(err error, tags map[string]string, interfaces ...Interface) string.

Instead,it should have: CaptureError(err error, tags map[string]string, extra map[string]string, interfaces ...Interface) string

extra allows us to add more stuff. I could manually create packet struct, but I need to manually create the stack trace, extract info from error, which is very hard to do and I had to dup that logic.

JackWink commented 6 years ago

This issue has been raised before (#125, #111 and even a PR for exactly this issue: #146) but it seems that the concern is breaking the exported API. I think somewhere in these issues there was talk of introducing a new method, CaptureErrorWithExtra to maintain backwards compatibility, but when I took a look, only #168 seemed to have approval approach-wise. It got merged as #190, so extra info can be included by wrapping your error as such: 

raven.CaptureError(raven.WithExtra(err, map[string]interface{}{
    "extra1": "my-extra-data",
    "extra2": 1234,
}), map[string]string{
    "tag1": "tag",
})

If it's acceptable to add a new capture method, it should be an easy PR now that extra data is supported through a public API.

That said, I'd still be in favor of Sentry supporting tagged releases (#119), with the existing codebase being a pre-1.0 release, then cleaning up the Capture API to be consistent (either tags + extra logged in Capture or tags + extra logged as an error wrapper) for a 1.0 release.