Skip to content
This repository has been archived by the owner on Aug 30, 2023. It is now read-only.

Logrus hook from logrus_sentry repo #98

Open
wants to merge 32 commits into
base: master
Choose a base branch
from

Conversation

mzuneska
Copy link

@mzuneska mzuneska commented Aug 9, 2016

This is the absolute bare minimum to start to get some feedback I think related to evalphobia/logrus_sentry#5

I didn't update anymore code than was necessary to get all the tests passing.
I didn't bring over the README so some documentation would still need to be added.
I didn't reduce the logrus_sentry API footprint as not to break backwards compatibility. I also figured people who've worked on that repo should have input.


This change is Reviewable

gigaroby and others added 30 commits November 1, 2014 19:55
- Add configuration for stacktrace
- Create stacktrace and append to Sentry packet
- Update tests
Export stackTraceConfiguration struct type
Add special sentry field support.
* format extra data before send to sentry_DSN

convert to string if a field type is:

* error
* fmt.Stringer

if field's type already implements json.Unmarshaler do nothing

* add test for format extra data
- follow pattern of http integration in http.go
- utils which was brought over with logrus_sentry defines the uuid type
- suffix was changed due to file and package change
- line no was reduced by one because raven-go package is no longer
  imported
@mzuneska
Copy link
Author

@mattrobenolt or @evalphobia any feedback on this?

@ellisonleao
Copy link

hey @mattrobenolt any chance on merging this?

@Fank
Copy link

Fank commented Jan 23, 2018

It would be nice to have this in raven

@evalphobia
Copy link

@mzuneska Sorry for late reply 😭

It's good to move evalphobia/logrus_sentry into getsentry 's repository,
but I prefer other repo like getsentry/logrus_sentry. not here getsentry/raven-go.
The reasons are,

  • logrus is not Go's and Sentry's official logger. (although many people use it!)
  • If it's in same repo, we may hard to choose another versions of raven-go and logrus.
  • If it's in same repo, the logrus-hook's tests may affect CI for raven-go's one.
  • segregation of duty
    • raven-go has been mainly maintained by Sentry team. The logrus hook has been mainly maintained by others. So it's easy to add other contributors into the logrus hook's repo.
    • It's easy to tell Pull Requests and Issues from raven-go's or hook's one.

(It's just my opinion, but I think these reasons are part of the reason why sirupsen/logrus contains only syslog hooks now.)

Any thoughts?

@ellisonleao
Copy link

Just out of curiosity, what is the official logger for sentry go projects?

@evalphobia
Copy link

@ellisonleao I think there is no official logger.

I mean logrus is not controllable by Sentry team.
If the logger is controllable by Sentry team, it's okay to include it in the same repo.
Sentry is not only OSS but also a commercial product, so they should not break the official SDK and official SDK should be controllable.

(Maybe it is not so important below, but I'd like to mention it. )
To illustrate what I worried about, imagine the if-situation like below,
If Go updates the version to 1.X and raven-go supports the new version, but the logger does not.
If the logger is maintained by Sentry, they can update the logger and support the new version.
But if the logger is maintained by others, Sentry team cannot update it easily.
(They can create Pull Request, but it may merge soon or ... 3 month later?)

I know the merit/demerit of mono-repo and multi-repo.
In my opinion, it's good to separate it into two repositories, raven-go core and logging hooks to separate the responsibility,

  • getsentry/go-raven
    • Official SDK and small people (only Sentry team?) can be a contributor.
    • If there are critical bugs, Sentry team should fix it. (or use other bug tracking service... 😭 )
  • getsentry/go-raven-hook
    • (the repo name is a temporary one.)
    • Community supported logging hook library and many people (not only Sentry team) can be a contributor.
    • If there are critical bugs, we can fix it. (or use other loggers 😈 )
    • It can have multiple logging hooks in a single repo.
      • (e.g. getsentry/go-raven-hook/logrushook, getsentry/go-raven-hook/zaphook, getsentry/go-raven-hook/gokithook)

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

Successfully merging this pull request may close these issues.

None yet