sentry-go icon indicating copy to clipboard operation
sentry-go copied to clipboard

Use source location instead of error struct type for issue list

Open yonderblue opened this issue 3 years ago • 5 comments

Issue

Most errors in Go that make their way into Sentry will be underneath structs like errors.errorString, project structs like myCustomError, or in different error package structs like xerrors.wrapError. Using this name for the Type (https://github.com/getsentry/sentry-go/blob/master/client.go#L397), which are listed in the Sentry Issue UI, is not terribly useful because they frequently represent errors that have nothing to do with each other.

Potential solution

Perhaps the package and function name of the lowest wrapped error, and/or the line number for the "Type" would be more useful. This way all the issues listed are likely the real issues at hand. Plus the events count would then be accurate.

yonderblue avatar Nov 30 '20 13:11 yonderblue

We have the same issue. Value is more important than Type, so we do this hack:

BeforeSend: func(event *sentry.Event, hint *sentry.EventHint) *sentry.Event {
	for idx, exception := range event.Exception {
		exception.Type, exception.Value = exception.Value, exception.Type
		event.Exception[idx] = exception
	}
	return event
}

horejsek avatar Dec 01 '20 11:12 horejsek

@horejsek 👍 that is a nice way without changing the sentry code. I think i'll change what I was using (https://github.com/yonderblue/sentry-go/commit/5dce33ae0499ae497e7f8a51750fee7ffe1f80fe#diff-4b667feae66c9d46b21b9ecc19e8958cf4472d162ce0a47ac3e8386af8bbd8cfR406) to that, except still using the module+func, since I find the value can have things formatted in it that prevent any grouping at all in the issue list, although I see how that could be desired too.

yonderblue avatar Dec 02 '20 15:12 yonderblue

@yonderblue your issue statement is legit. We've observed this Type x Value with some other languages too. See https://github.com/getsentry/sentry/issues/17837.

If we flip the values in the SDK for everyone, we potentially create confusion as we subvert the semantics of the fields. All the way through the ingestion pipeline till the error surfaces in sentry.io, or an email notification, etc, all systems will see the flipped fields to a point that you can't be sure if event.value is the value or the type (would depend on SDK language & version).

On the other hand, in the meantime, users can opt-in and flip like @horejsek suggested, or even customize it to whatever makes best sense in their project. The tools at your disposal are BeforeSend (applies to all errors), Scope.AddEventProcessor (applies to a single scope), Event.Fingerprint (to control grouping, see below).

I concede that sometimes the type is too generic to be useful, but sometimes it is very clear and desirable. I have not found yet a single alternative that works consistently better for all Go projects.

I would be happy to try out some of your suggestions if you can provide examples, either in the form of open source code that I can reproduce errors or links to errors in sentry.io.

Alternatively, we can focus our efforts on the issue I linked above and make the UI more configurable. For example, a different direction to take is to allow configuring the "title" and "subtitle" of issues.

the line number for the "Type"

AFAIK we historically don't do that because the line number can change between versions of your code and that would affect the default grouping rules.

If you pass a custom Event.Fingerprint, it will be used for grouping and then you can have whatever you want in the type/value fields.

https://docs.sentry.io/product/sentry-basics/guides/grouping-and-fingerprints/

rhcarvalho avatar Dec 03 '20 11:12 rhcarvalho

Alternatively, we can focus our efforts on the issue I linked above and make the UI more configurable. For example, a different direction to take is to allow configuring the "title" and "subtitle" of issues.

This would be very good, indeed!

horejsek avatar Dec 03 '20 11:12 horejsek

@rhcarvalho How does sentry group errors under one issue? Is there any documentation/code I can read?

In some cases, I've noticed that two errors of the same type errors.errorString, but different values were grouped under the same issue (with the event count incremented), and in some cases, it creates separate issues.

makalaaneesh avatar Apr 27 '21 05:04 makalaaneesh

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

github-actions[bot] avatar Dec 07 '22 09:12 github-actions[bot]

@makalaaneesh I recognise how long ago you posted this but wanted to note that the grouping is done by Fingerprinting (see https://docs.sentry.io/product/data-management-settings/event-grouping/). You can customise Fingerprint Rules within your Sentry project but from my experience, this isn't sufficient for Go projects given the limited fingerprint matchers and variables.

I ended up up using SDK Fingerprinting to customise the fingerprint to denote the particular project (main module from https://pkg.go.dev/runtime/debug#BuildInfo) that is executing, followed by the exception's value, and lastly the exception's type. The value has more worth since most of my errors are *fmt.wrapError types and effectively irrelevant; likewise, since all errors are captured through a central library (which happens outside of a generic wrapper function), the stack trace seen at the time of calling sentry.CaptureException is also effectively irrelevant.

Coupling this strategy with inverting the value/type in the event itself, like @horejsek's suggestion, splits up errors in the Sentry UI and makes them more readable, with the value taking precedence.

davidjb avatar Mar 06 '24 07:03 davidjb