cockroachdb / errors

Go error library with error portability over the network
Apache License 2.0
2.08k stars 66 forks source link

Switch to sentry-go when building the reports #20

Closed yuzefovich closed 4 years ago

yuzefovich commented 4 years ago

Previously, we were using getsentry/raven-go to populate the reports for Sentry. However, that tool has become obsolete. This commit switches to using getsentry/sentry-go.

There are some differences in the API, and I made the following changes:


This change is Reviewable

claassistantio commented 4 years ago

CLA assistant check
All committers have signed the CLA.

knz commented 4 years ago

Thank you!

For clarity, can you copy-paste a screenshot of a sentry report generated with the new code?

cc @tooolbox - I wasn't expected we'd invest in this so soon, but here you are.

yuzefovich commented 4 years ago

I ran the test with DSN specified as cockroachdb project, and here is what I got:

screencapture-sentry-io-organizations-cockroach-labs-issues-1402512507-events-ec150053681548f6b2ea2fc75c994614-2019-12-22-16_38_56

(The link to the event)

knz commented 4 years ago

Thank you this looks good overall. However I'd like to see how this impacts the reporting of SQL errors. I would recommend to operate as follows:

  1. make sure your new code is in your $GOPATH
  2. take your cockroachdb work dir and rm -rf vendor/github.com/cockroachdb/errors
  3. build binary
  4. make it report a SQL error (e.g. crdb_internal.force_error('XX000', 'woops')
  5. see how the sentry report looks like.

Is the SQL statement included? Is it clear from the sentry message that the error was generated with force_error?

If yes then this is good to go.

yuzefovich commented 4 years ago

Thanks for the pointers!

I made some modifications to cockroach to use sentry-go as well, and here is the report screencapture-sentry-io-organizations-cockroach-labs-issues-1404688926-2019-12-24-09_54_54

(and the link)

I think it looks good, right?

knz commented 4 years ago

Oh yeah that looks awesome. LGTM!

knz commented 4 years ago

After you merge this we need to bump the version because this is a backward-incompatible API change. You can add a tag for v1.3.

yuzefovich commented 4 years ago

TFTR!

I'll push a new tag momentarily.