Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update from Raven to new Sentry SDK #162

Closed
Mr0grog opened this issue Nov 11, 2018 · 10 comments
Closed

Update from Raven to new Sentry SDK #162

Mr0grog opened this issue Nov 11, 2018 · 10 comments

Comments

@Mr0grog
Copy link
Member

Mr0grog commented Nov 11, 2018

At some point in the last few months, Sentry deprecated all their “Raven” SDKs and switched to a whole new series of “Sentry SDKs.” (Not sure how I missed this, but I really feel like there wasn’t any major, clear notice.) We should update, since docs for old Raven SDK are now hard to find, increasing the likelihood we misconfigure something.

@stale
Copy link

stale bot commented May 10, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in seven days if no further activity occurs. If it should not be closed, please comment! Thank you for your contributions.

@stale stale bot added the stale label May 10, 2019
@Mr0grog
Copy link
Member Author

Mr0grog commented May 10, 2019

Gah, I can’t believe I never got this done! This is only getting more important as the old SDK gets more and more out of date.

@c3ho
Copy link
Contributor

c3ho commented Oct 3, 2019

Hi, I'm a first time contributor and want to work on this issue. Is it possible to work on this issue?

@Mr0grog
Copy link
Member Author

Mr0grog commented Oct 4, 2019

@c3ho Absolutely, go ahead!

@c3ho
Copy link
Contributor

c3ho commented Oct 5, 2019

Thanks!

I installed the new SDK, however I think there are a few things I have questions about.

In the legacy:
image

This is the new configuration method:
image

  1. Right now in the sentry-error.js file const Raven, the variable is also exported for use. Did you want to use the Raven variable for now and change it in another PR?

  2. The new config method requires the whole thing to be an object, the old one on line 51 of sentry-error.js

exports.setup = function (options, dsn = process.env['SENTRY_DSN']) {
  if (dsn) {
    Raven.config(dsn, Object.assign({}, defaultOptions, options)).install();
  }
  return exports;
}

the following is what I have for the new one

exports.setup = function (options, dsn = process.env['SENTRY_DSN']) {
  if (dsn) {
    var dsnObj = { dsn: dsn };
    Raven.init(Object.assign({}, dsnObj, defaultOptions, options));
  }
  return exports;
}
  1. Do you have any tips on how I would test the changes to make sure the current changes are not breaking anything? I tried following the instructions in the readme, however I keep getting a web page that looks like this :

issue

@Mr0grog
Copy link
Member Author

Mr0grog commented Oct 7, 2019

Right now in the sentry-error.js file const Raven, the variable is also exported for use. Did you want to use the Raven variable for now and change it in another PR?

It’s not actually used, so you should just change it now.

the following is what I have for the new one

That works fine, but it might be nicer to just have the signature be setup(options) now. I’m not super concerned about it, either way.

I tried following the instructions in the readme, however I keep getting a web page that looks like this

Where do you wind up with a web page? The scrape-versionista script is a CLI tool that generates JSON or CSV files.

@c3ho
Copy link
Contributor

c3ho commented Oct 10, 2019

Right now in the sentry-error.js file const Raven, the variable is also exported for use. Did you want to use the Raven variable for now and change it in another PR?

It’s not actually used, so you should just change it now.

Got it, changed all that

That works fine, but it might be nicer to just have the signature be setup(options) now. I’m not super concerned about it, either way.

You mean to

exports.setup = function (options){
  Sentry.init(Object.assign({}, defaultOptions, options});
  return exports;
}

I was under the impression from the documentation a dsn is required for init.

Where do you wind up with a web page? The scrape-versionista script is a CLI tool that generates JSON or CSV files.

Sorry, that was my mistake, I followed the process and made an account on versionista(with a site to follow) as well as sentry(to get a dsn)

I still do get an error though, no csv, but an email with the same issue I got from CLI

Screen Shot 2019-10-10 at 12 24 41 AM

Lastly, sorry for all the back and forth!

@Mr0grog
Copy link
Member Author

Mr0grog commented Oct 10, 2019

I was under the impression from the documentation a dsn is required for init.

If it’s not provided, it’ll read it from the SENTRY_DSN environment variable (which is how we use it), and if that’s not set, everything is fine, but it makes any Sentry method calls into no-ops (which is super useful, because you don’t have to guard your Sentry calls with things like if (isDevMode) { sentry.captureMessage(...); }).

I still do get an error though, no csv, but an email with the same issue I got from CLI

You’ll need to rebase on the master branch to get the fix that was in #244.

@c3ho
Copy link
Contributor

c3ho commented Oct 11, 2019

Thank you, that fixed everything!

@c3ho
Copy link
Contributor

c3ho commented Oct 11, 2019

I've submitted a PR for the issue #162

c3ho added a commit to c3ho/web-monitoring-versionista-scraper that referenced this issue Oct 11, 2019
c3ho added a commit to c3ho/web-monitoring-versionista-scraper that referenced this issue Oct 12, 2019
Web Monitoring automation moved this from Ready to Done! Oct 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Web Monitoring
  
Done!
Development

No branches or pull requests

2 participants