Missing stack trace when using the logrus integration

[mj3c]

Summary

When using the logrus approach to report Sentry issues (such as in the example), the issues appear on Sentry but they don't have the nicely formatted stack trace, and the JSON event is missing the frames property which I assume is related.

If I try to use the basic example with sentry.CaptureException(err), the stack trace shows up and the JSON object has the frames.

Steps To Reproduce

package main

import (
  "errors"
  "time"

  "github.com/getsentry/sentry-go"
  sentrylogrus "github.com/getsentry/sentry-go/logrus"
  log "github.com/sirupsen/logrus"
)

func main() {
  sentryLevels := []log.Level{log.ErrorLevel, log.FatalLevel, log.PanicLevel}
  sentryHook, _ := sentrylogrus.New(sentryLevels, sentry.ClientOptions{
    Dsn:              "<DSN>",
    Debug:            true,
    AttachStacktrace: true,
  })
  log.AddHook(sentryHook)
  defer sentryHook.Flush(2 * time.Second)

  fakeErr := errors.New("fake error object")
  log.WithError(fakeErr).Error("test")
  // log.WithError(fakeErr).Error(fakeErr) // this does not help either
}

Expected Behavior

This is the Sentry issue in JSON format, it's missing the frames property next to type and value.

    "exception": {
        "values": [
            {
                "type": "error",
                "value": "fake error object"
            }
        ]
    },

Screenshots

What I get:

What I expect to get (as with sentry.CaptureException):

Environment

SDK

• sentry-go version: 0.22.0
• Go version: 1.20
• Using Go Modules? [yes/no] yes

Sentry

• Using hosted Sentry in sentry.io? [yes/no] yes
• Using your own Sentry installation? Version: //
• Anything particular to your environment that could be related to this issue? //

Additional context

From what I managed to find while debugging, the problem comes from the fact that when using Sentry with logrus, the ExtractStackTrace function exits (which is expected I assume with Go's standard errors) and the NewStackTrace function is never called, but it is called when using plain sentry.CaptureException even though I am using Go's standard errors. Any particular reason why the behaviour is different? Am I missing something to have the stack trace frames generated when using Sentry with logrus?

[yiuiua]

Hi mj3c. nice to meet you.

Go's standard errors don't come with a stack by themselves.
You will need to use some third-party error packages to build error in order to get stack information.

https://github.com/pkg/errors
https://github.com/pingcap/errors
https://github.com/go-errors/errors

You can take a look at this function extractReflectedStacktraceMethod. It's used to parse the error and get the stack.
This unit test TestExtractStacktrace gives you a better visualization of the stack information carried by the error

I hope my answer is helpful.

[mj3c]

Hi @yiuiua, thank you!

That is also what I've gathered from looking at the source code, that a 3rd party library for errors is needed.

Though if you look at the examples/screenshots I've provided, which are both done using Go's standard errors, you can see that the stack trace is present when the issue is reported using sentry.CaptureException. From what I've found, the stack trace is built in that case using the runtime package, inside the NewStackTrace() function. Can this same procedure not be done when using the Sentry integration with logrus? Why is the stack only missing in that case, but not missing when done with sentry.CaptureException?

[cleptric]

Would you be up opening a PR that adds stack traces to our logrus integration?
I think this is more of a missing feature than a bug.

[yiuiua]

Hi @yiuiua, thank you!

That is also what I've gathered from looking at the source code, that a 3rd party library for errors is needed.

Though if you look at the examples/screenshots I've provided, which are both done using Go's standard errors, you can see that the stack trace is present when the issue is reported using sentry.CaptureException. From what I've found, the stack trace is built in that case using the runtime package, inside the NewStackTrace() function. Can this same procedure not be done when using the Sentry integration with logrus? Why is the stack only missing in that case, but not missing when done with sentry.CaptureException?

@mj3c Yay, you're right, i overlooked your examples/screenshots, and I apologize.
logrus integration here the function(*Hook.exceptions) only handles the 3rd party error library (carrying the stack), ignoring the go standard error (missing stack). adding a default stack trace like the one here is, in my opinion, the fastest way to optimize, and there are some modifications that can be added to make it more robust.

func (h *Hook) exceptions(err error) []sentry.Exception {
	if err == nil {
		return nil
	}

	if !h.hub.Client().Options().AttachStacktrace {
		return []sentry.Exception{{
			Type:  "error",
			Value: err.Error(),
		}}
	}
	excs := []sentry.Exception{}
	var last *sentry.Exception
	for ; err != nil; err = errors.Unwrap(err) {
		exc := sentry.Exception{
			Type:       "error",
			Value:      err.Error(),
			Stacktrace: sentry.ExtractStacktrace(err),
		}
		if last != nil && exc.Value == last.Value {
			if last.Stacktrace == nil {
				last.Stacktrace = exc.Stacktrace
				continue
			}
			if exc.Stacktrace == nil {
				continue
			}
		}
		excs = append(excs, exc)
		last = &excs[len(excs)-1]
	}

	// Add a trace of the current stack to the most recent error in a chain if
	// it doesn't have a stack trace yet.
	// see https://github.com/getsentry/sentry-go/blob/master/interfaces.go#L366-L372
	if excs[0].Stacktrace == nil {
		excs[0].Stacktrace = sentry.NewStacktrace()
	}

	// reverse
	for i, j := 0, len(excs)-1; i < j; i, j = i+1, j-1 {
		excs[i], excs[j] = excs[j], excs[i]
	}
	return excs
}

[mj3c]

Hey @yiuiua, that's exactly the solution I came up with too. Also, if you test that, you will notice that all errors will be sent with type "error" since it is hardcoded there for some reason.

Here's a patch that I think works okay:

diff --git a/logrus/logrusentry.go b/logrus/logrusentry.go
index 0a3a722..fea68ff 100644
--- a/logrus/logrusentry.go
+++ b/logrus/logrusentry.go
@@ -4,6 +4,7 @@ package sentrylogrus
 import (
        "errors"
        "net/http"
+       "reflect"
        "time"
 
        sentry "github.com/getsentry/sentry-go"
@@ -184,7 +185,7 @@ func (h *Hook) entryToEvent(l *logrus.Entry) *sentry.Event {
 func (h *Hook) exceptions(err error) []sentry.Exception {
        if !h.hub.Client().Options().AttachStacktrace {
                return []sentry.Exception{{
-                       Type:  "error",
+                       Type:  reflect.TypeOf(err).String(),
                        Value: err.Error(),
                }}
        }
@@ -192,7 +193,7 @@ func (h *Hook) exceptions(err error) []sentry.Exception {
        var last *sentry.Exception
        for ; err != nil; err = errors.Unwrap(err) {
                exc := sentry.Exception{
-                       Type:       "error",
+                       Type:       reflect.TypeOf(err).String(),
                        Value:      err.Error(),
                        Stacktrace: sentry.ExtractStacktrace(err),
                }
@@ -208,6 +209,11 @@ func (h *Hook) exceptions(err error) []sentry.Exception {
                excs = append(excs, exc)
                last = &excs[len(excs)-1]
        }
+
+       if excs[0].Stacktrace == nil {
+               excs[0].Stacktrace = sentry.NewStacktrace()
+       }
+
        // reverse
        for i, j := 0, len(excs)-1; i < j; i, j = i+1, j-1 {
                excs[i], excs[j] = excs[j], excs[i]

However this would require updating some tests as well, and there may be more to this change as I did not have time to really test it and create a PR. In the meantime we ended up creating a custom logger that has the same interface as logrus but also utilizes sentry.CaptureException in certain calls.

[rozanecm]

Hi @yiuiua ! I noticed there's been an open PR for this for a while. Could you provide any updates on its progress?

Thank you in advance for your time and contributions.