Sentry releases are broken due to no ordering

AudriusButkevicius · July 14, 2019, 7:04pm

I’ve created all releases that were not there, and associated commits with them. The things that were created prior me running thing have an older creation date.

AudriusButkevicius · July 14, 2019, 8:11pm

There, fixed the dates too, so marking stuff should be ok now.

imsodin · July 22, 2019, 6:36pm

Now the debian releases start interfering too… https://sentry.syncthing.net/syncthing/syncthing/releases/v0.14.43-ds1/

Maybe it’s possible to disable the “auto-creation” of releases? Thus only create the releases manually through CI so we don’t need to worry about every possible case (I guess every panic on a debug build would introduce a version too).

AudriusButkevicius · July 22, 2019, 6:58pm

I’ve just deleted the release.

I don’t think it’s possible to prevent release creation, and we should make sure we disable reporting on non-prod builds (or perhaps even builds that don’t match our build tags, solving the debian issue).

calmh · July 23, 2019, 8:09am

I think it’s much better to accept the reports and mentally deal with the occasional duplicate.

imsodin · July 23, 2019, 8:35am

We could maybe also put some version parsing into the reporting (strip trailing weirdness, potentially add another boolean attribute to specify that it’s not a “clean” build).

It’s not the duplicates that bug me, it’s the regressions, i.e. inability to mark something as solved in version x.y.z. For example the fmut deadlock did not yet come up in 1.2.1-rc.x, so I marked it as solved there. If it comes up again, we get correctly notified (yeiii), but now we get notified that it regressed in 0.14.43-ds1 (or something like that).

calmh · July 23, 2019, 10:06am

Right. So there’s the version parsing stuff:

github.com

syncthing/syncthing/blob/6f08162376ab59d93b91bbc05119074fa2f44fe0/cmd/stcrashreceiver/sentry.go#L153-L204


// syncthing v1.1.4-rc.1+30-g6aaae618-dirty-crashrep "Erbium Earthworm" (go1.12.5 darwin-amd64) jb@kvin.kastelo.net 2019-05-23 16:08:14 UTC
var longVersionRE = regexp.MustCompile(`syncthing\s+(v[^\s]+)\s+"([^"]+)"\s\(([^\s]+)\s+([^-]+)-([^)]+)\)\s+([^\s]+)`)


type version struct {
	version  string // "v1.1.4-rc.1+30-g6aaae618-dirty-crashrep"
	tag      string // "v1.1.4-rc.1"
	commit   string // "6aaae618", blank when absent
	codename string // "Erbium Earthworm"
	runtime  string // "go1.12.5"
	goos     string // "darwin"
	goarch   string // "amd64"
	builder  string // "jb@kvin.kastelo.net"
}


func (v version) environment() string {
	if v.commit != "" {
		return "Development"
	}
	if strings.Contains(v.tag, "-rc.") {
		return "Candidate"

This file has been truncated. show original

We don’t want to outright strip dashes and stuff though, because 1.1.4-rc.1 is different from 1.1.4, and indeed 0.14.43-ds1 is probably quite different from 0.14.43…

Personally I think marking stuff as resolved etc in Sentry is a fool’s errand, which is why I’m not super worried about this. My workflow is to look at the latest screen of disasters and see if there’s anything new/interesting in a recent release, and if so link it to a GitHub issue…

imsodin · July 23, 2019, 12:10pm

I already see myself being convinced by that in a not too distant future, but I am not quite there yet

Raven/sentry has three “sections” related to releases: Release, version and tag. What do you think about setting Release, which presumably creates the releases in the sentry interface, to “our official release” that closest resembles the version from the panic trace. The original version will still be preserved in the version section.

AudriusButkevicius · July 23, 2019, 12:13pm

Sounds good. Lets strip the debian nonsense. There is also a dist section which we could indicate the channel/builder

calmh · July 27, 2019, 6:12am

We filled up the crash reporting volume tonight (10 GiB). I’ve increased the size so it’s cool, and also made the crash reporter store the reports in a compressed format (they are repetitive text so they compress well).

Regardless, we should have a retention policy. I’m adding a cron find | xargs rm for files older than six months, for now. If we haven’t fixed the bug by then I’m pretty sure the crash backtrace is pretty much moot anyway.

(This doesn’t affect Sentry at all and I have no idea how it handles retention or database growth.)