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

hooks: add airbrake hook #17

Merged
merged 1 commit into from Apr 23, 2014
Merged

hooks: add airbrake hook #17

merged 1 commit into from Apr 23, 2014

Conversation

sirupsen
Copy link
Owner

This adds an Airbrake default hook. I would've liked to have put this in its own directory inside logrus, but I'm getting a cycle dependency warning. It sucks if everyone has to install the Airbrake library because of this..

@snormore @burke @aybatme can you guys take a look?

@sirupsen
Copy link
Owner Author

Oops, typoed @aybabtme

@aybabtme
Copy link
Collaborator

Hey, just saw that. I'll have a look tomorrow.

@burke
Copy link
Contributor

burke commented Apr 14, 2014

👍

@sirupsen
Copy link
Owner Author

Did you guys have an idea on how to move this out of the main install so not everyone has to install the Airbrake library for example?

@Soulou
Copy link

Soulou commented Apr 15, 2014

The first solution I can see is build tags

aibrake_hook.go

// +build airbrake

package logrus

[...]

Then

  • go get -tags airbrake
  • go build -tags airbrake

@sirupsen
Copy link
Owner Author

I didn't know about build tags, that's cool! I might go ahead with that :-)

@aybabtme
Copy link
Collaborator

Why not make the airbrake hook a separate package? It seems odd to be baked into the library.

@aybabtme
Copy link
Collaborator

Like, move it into some folder (or another repo of common hooks, or where ever else not in the same namespace):

% pwd
$GOPATH/github.com/Sirupsen/logrus/hooks/airbrake
% head airbrake.go
package airbrake

import (...) 

type AirbrakeHook struct {...}

// implement logrus.Hook

func Hook(authStuff ...) *AirbrakeHook {
    return &AirbrakeHook{...}
}

Then:

log := logrus.New()
log.Hooks.Add(airbrake.Hook(...))

On a side note, I dislike build tags.

@Soulou
Copy link

Soulou commented Apr 15, 2014

I think I prefer @aybabtme solution (Even if I've personnaly given the other one ;-) ) It's cleaner to separate things and more than that, using build tags is a bit annoying, when you take a golang project, usually you want to "go get" it, and nothing else.

I event would have done something like having a hooks directory in which you put other packages like airbrake, rollbar, or sentry. Managing another repository for each hooks can look "heavy" but it's what's happening with different projects (example https://github.com/martini-contrib)

@sirupsen
Copy link
Owner Author

Yeah, a different repo with subfolders sounds good to me. I like that separation of concerns. I dislike having a repo for each one of them. I'll go ahead with that :-)

@aybabtme
Copy link
Collaborator

👍

@sirupsen
Copy link
Owner Author

It's in the logrus-hooks repo, I'll update the readme laters

hooks: Add airbrake hook

examples: move around to avoid overloading main

airbarke move around

remove tex.tgo

readme: cleanup

readme: try something different
sirupsen added a commit that referenced this pull request Apr 23, 2014
@sirupsen sirupsen merged commit 67dfc7a into master Apr 23, 2014
@sirupsen sirupsen deleted the airbrake branch April 23, 2014 23:56
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

Successfully merging this pull request may close these issues.

None yet

4 participants