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

Move into raven-go? #5

Open
mattrobenolt opened this issue Oct 9, 2015 · 17 comments
Open

Move into raven-go? #5

mattrobenolt opened this issue Oct 9, 2015 · 17 comments

Comments

@mattrobenolt
Copy link

Hey @evalphobia thanks for doing this!

Would you be opposed to moving this into raven-go? We have some other framework support, like Gin, that we're likely going to be moving into raven-go as well.

That way you'd do something like:

import "github.com/getsentry/raven-go/logrus"

Then we'd get documentation going over at https://docs.getsentry.com/hosted/clients/go/integrations/

Thoughts?

@mattrobenolt
Copy link
Author

Not 100% sure how we'd want to do this, but ideally this logrus_sentry.NewSentryHook(YOUR_DSN, ...) installation too since the default client inside raven-go should already exist and we'd leverage that instead.

I'd envision something like:

raven.SetDSN("...")
log := logrus.New()
hook := raven_logrus.NewHook([]logrus.Level{
    logrus.PanicLevel,
    logrus.FatalLevel,
    logrus.ErrorLevel,
})
log.Hooks.Add(hook)

@evalphobia
Copy link
Owner

@mattrobenolt
Sounds good!
If you officially supports sentry library for logrus, it's more stable and be awared by gophers.

@gigaroby @awonak @miloconway
any thoughts?

@miloconway
If you're re-creating the PR in Sirupsen/logrus, I'll wait for it.
or if you need more time, I gonna move this repository before your PR.

/cc @sirupsen FYI

@miloconway
Copy link
Contributor

@evalphobia I'll recreate the diff.

@gigaroby
Copy link
Contributor

@evalphobia no problem :)

@evalphobia
Copy link
Owner

@miloconway @gigaroby
Thanks guys.

@mattrobenolt
When you ready, tell me that?
After that, we have to do...

@stkao05
Copy link
Contributor

stkao05 commented Dec 23, 2015

@evalphobia @mattrobenolt
Would love seeing this happen. If needed, I could help working on that on my fork of raven-go and create PR for you to merge it.

@mzuneska
Copy link

mzuneska commented Aug 8, 2016

Just curious if there was there any progress on this? I don't see any mention of logrus in the raven-go repository.

@mattrobenolt
Copy link
Author

There has not. I'm still open to inclusion here. :) Just need someone to submit a PR.

@mzuneska
Copy link

@mattrobenolt I opened a basic PR in the raven-go repo. It's referenced above.

@mattrobenolt
Copy link
Author

Awesome, I will get to look at that this week. A bit busy at the moment. :) Thanks! <3

@captncraig
Copy link

I would be opposed to this btw. I am using this to send to sentry, but not using any other sentry-specific code in my app. I appreciate that this package is small and lightweight, and prefer it being separate.

@mattrobenolt
Copy link
Author

@captncraig how would this change affect that? Just curious. This package directly depends on raven-go already. There's no way you could use this without that dependency. Merging these repositories would just mean they're under the same roof instead of two dependencies.

@captncraig
Copy link

You are right. I feel silly having said that now, since of course this already depends on raven-go. Sorry I chimed in.

@ellisonleao
Copy link

hey guys, any updates on this?

@macnibblet
Copy link

and we enter into 2018

@evalphobia
Copy link
Owner

WIP getsentry/raven-go#98

@marcosox
Copy link

still WIP I suppose

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants