From 78dee3c0ba1a241f27e218a5095d270ba3ad209a Mon Sep 17 00:00:00 2001 From: Matt Bostock Date: Tue, 10 Mar 2015 17:45:12 +0000 Subject: [PATCH 1/3] Rename package from logrus_airbrake to airbrake Using underscores in package names in discouraged: https://golang.org/doc/effective_go.html#package-names Given that this package is in a subdirectory of the logrus package, the name `airbrake` should be sufficiently descriptive. --- hooks/airbrake/airbrake.go | 2 +- hooks/airbrake/airbrake_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hooks/airbrake/airbrake.go b/hooks/airbrake/airbrake.go index 75f4db1..9fa108a 100644 --- a/hooks/airbrake/airbrake.go +++ b/hooks/airbrake/airbrake.go @@ -1,4 +1,4 @@ -package logrus_airbrake +package airbrake import ( "github.com/Sirupsen/logrus" diff --git a/hooks/airbrake/airbrake_test.go b/hooks/airbrake/airbrake_test.go index d2fd61d..55df200 100644 --- a/hooks/airbrake/airbrake_test.go +++ b/hooks/airbrake/airbrake_test.go @@ -1,4 +1,4 @@ -package logrus_airbrake +package airbrake import ( "encoding/xml" From 83a820d91e2dcb29f44a317ebf629bff37d3c315 Mon Sep 17 00:00:00 2001 From: Matt Bostock Date: Tue, 10 Mar 2015 17:49:55 +0000 Subject: [PATCH 2/3] Rework the Airbrake hook Rework the Airbrake hook to: a) change the interface so that the Airbrake credentials are stored in an unexported struct, `airbrakeHook`, which is instantiated using the `NewHook()` method b) send log entries where no 'error' field is set to Airbrake, using the `entry.Message` string as the message sent to Airbrake but continue to allow the passing of error types using the 'error' field Update the tests accordingly, assuring that the correct message is received by the Airbrake server. Also update the examples in the README, which would not have worked with the previous implementation of the Airbrake hook. --- README.md | 4 +- examples/hook/hook.go | 7 +- hooks/airbrake/airbrake.go | 56 ++++++------- hooks/airbrake/airbrake_test.go | 138 +++++++++++++++++++++++++------- 4 files changed, 138 insertions(+), 67 deletions(-) diff --git a/README.md b/README.md index e755e7c..c4a59ab 100644 --- a/README.md +++ b/README.md @@ -82,7 +82,7 @@ func init() { // Use the Airbrake hook to report errors that have Error severity or above to // an exception tracker. You can create custom hooks, see the Hooks section. - log.AddHook(&logrus_airbrake.AirbrakeHook{}) + log.AddHook(airbrake.NewHook("https://example.com", "xyz", "development")) // Output to stderr instead of stdout, could also be a file. log.SetOutput(os.Stderr) @@ -211,7 +211,7 @@ import ( ) func init() { - log.AddHook(new(logrus_airbrake.AirbrakeHook)) + log.AddHook(airbrake.NewHook("https://example.com", "xyz", "development")) hook, err := logrus_syslog.NewSyslogHook("udp", "localhost:514", syslog.LOG_INFO, "") if err != nil { diff --git a/examples/hook/hook.go b/examples/hook/hook.go index 42e7a4c..cb5759a 100644 --- a/examples/hook/hook.go +++ b/examples/hook/hook.go @@ -3,21 +3,16 @@ package main import ( "github.com/Sirupsen/logrus" "github.com/Sirupsen/logrus/hooks/airbrake" - "github.com/tobi/airbrake-go" ) var log = logrus.New() func init() { log.Formatter = new(logrus.TextFormatter) // default - log.Hooks.Add(new(logrus_airbrake.AirbrakeHook)) + log.Hooks.Add(airbrake.NewHook("https://example.com", "xyz", "development")) } func main() { - airbrake.Endpoint = "https://exceptions.whatever.com/notifier_api/v2/notices.xml" - airbrake.ApiKey = "whatever" - airbrake.Environment = "production" - log.WithFields(logrus.Fields{ "animal": "walrus", "size": 10, diff --git a/hooks/airbrake/airbrake.go b/hooks/airbrake/airbrake.go index 9fa108a..b0502c3 100644 --- a/hooks/airbrake/airbrake.go +++ b/hooks/airbrake/airbrake.go @@ -1,51 +1,51 @@ package airbrake import ( + "errors" + "fmt" + "github.com/Sirupsen/logrus" "github.com/tobi/airbrake-go" ) // AirbrakeHook to send exceptions to an exception-tracking service compatible -// with the Airbrake API. You must set: -// * airbrake.Endpoint -// * airbrake.ApiKey -// * airbrake.Environment -// -// Before using this hook, to send an error. Entries that trigger an Error, -// Fatal or Panic should now include an "error" field to send to Airbrake. -type AirbrakeHook struct{} +// with the Airbrake API. +type airbrakeHook struct { + APIKey string + Endpoint string + Environment string +} -func (hook *AirbrakeHook) Fire(entry *logrus.Entry) error { - if entry.Data["error"] == nil { - entry.Logger.WithFields(logrus.Fields{ - "source": "airbrake", - "endpoint": airbrake.Endpoint, - }).Warn("Exceptions sent to Airbrake must have an 'error' key with the error") - return nil +func NewHook(endpoint, apiKey, env string) *airbrakeHook { + return &airbrakeHook{ + APIKey: apiKey, + Endpoint: endpoint, + Environment: env, } +} +func (hook *airbrakeHook) Fire(entry *logrus.Entry) error { + airbrake.ApiKey = hook.APIKey + airbrake.Endpoint = hook.Endpoint + airbrake.Environment = hook.Environment + + var notifyErr error err, ok := entry.Data["error"].(error) - if !ok { - entry.Logger.WithFields(logrus.Fields{ - "source": "airbrake", - "endpoint": airbrake.Endpoint, - }).Warn("Exceptions sent to Airbrake must have an `error` key of type `error`") - return nil + if ok { + notifyErr = err + } else { + notifyErr = errors.New(entry.Message) } - airErr := airbrake.Notify(err) + airErr := airbrake.Notify(notifyErr) if airErr != nil { - entry.Logger.WithFields(logrus.Fields{ - "source": "airbrake", - "endpoint": airbrake.Endpoint, - "error": airErr, - }).Warn("Failed to send error to Airbrake") + return fmt.Errorf("Failed to send error to Airbrake: %s", airErr) } return nil } -func (hook *AirbrakeHook) Levels() []logrus.Level { +func (hook *airbrakeHook) Levels() []logrus.Level { return []logrus.Level{ logrus.ErrorLevel, logrus.FatalLevel, diff --git a/hooks/airbrake/airbrake_test.go b/hooks/airbrake/airbrake_test.go index 55df200..058a91e 100644 --- a/hooks/airbrake/airbrake_test.go +++ b/hooks/airbrake/airbrake_test.go @@ -2,26 +2,123 @@ package airbrake import ( "encoding/xml" - "errors" "net/http" "net/http/httptest" "testing" "time" "github.com/Sirupsen/logrus" - "github.com/tobi/airbrake-go" ) type notice struct { - Error struct { - Message string `xml:"message"` - } `xml:"error"` + Error NoticeError `xml:"error"` +} +type NoticeError struct { + Class string `xml:"class"` + Message string `xml:"message"` } -func TestNoticeReceived(t *testing.T) { - msg := make(chan string, 1) - expectedMsg := "foo" +type customErr struct { + msg string +} +func (e *customErr) Error() string { + return e.msg +} + +const ( + testAPIKey = "abcxyz" + testEnv = "development" + expectedClass = "*airbrake.customErr" + expectedMsg = "foo" + unintendedMsg = "Airbrake will not see this string" +) + +var ( + noticeError = make(chan NoticeError, 1) +) + +// TestLogEntryMessageReceived checks if invoking Logrus' log.Error +// method causes an XML payload containing the log entry message is received +// by a HTTP server emulating an Airbrake-compatible endpoint. +func TestLogEntryMessageReceived(t *testing.T) { + log := logrus.New() + ts := startAirbrakeServer(t) + defer ts.Close() + + hook := NewHook(ts.URL, testAPIKey, "production") + log.Hooks.Add(hook) + + log.Error(expectedMsg) + + select { + case received := <-noticeError: + if received.Message != expectedMsg { + t.Errorf("Unexpected message received: %s", received.Message) + } + case <-time.After(time.Second): + t.Error("Timed out; no notice received by Airbrake API") + } +} + +// TestLogEntryMessageReceived confirms that, when passing an error type using +// logrus.Fields, a HTTP server emulating an Airbrake endpoint receives the +// error message returned by the Error() method on the error interface +// rather than the logrus.Entry.Message string. +func TestLogEntryWithErrorReceived(t *testing.T) { + log := logrus.New() + ts := startAirbrakeServer(t) + defer ts.Close() + + hook := NewHook(ts.URL, testAPIKey, "production") + log.Hooks.Add(hook) + + log.WithFields(logrus.Fields{ + "error": &customErr{expectedMsg}, + }).Error(unintendedMsg) + + select { + case received := <-noticeError: + if received.Message != expectedMsg { + t.Errorf("Unexpected message received: %s", received.Message) + } + if received.Class != expectedClass { + t.Errorf("Unexpected error class: %s", received.Class) + } + case <-time.After(time.Second): + t.Error("Timed out; no notice received by Airbrake API") + } +} + +// TestLogEntryWithNonErrorTypeNotReceived confirms that, when passing a +// non-error type using logrus.Fields, a HTTP server emulating an Airbrake +// endpoint receives the logrus.Entry.Message string. +// +// Only error types are supported when setting the 'error' field using +// logrus.WithFields(). +func TestLogEntryWithNonErrorTypeNotReceived(t *testing.T) { + log := logrus.New() + ts := startAirbrakeServer(t) + defer ts.Close() + + hook := NewHook(ts.URL, testAPIKey, "production") + log.Hooks.Add(hook) + + log.WithFields(logrus.Fields{ + "error": expectedMsg, + }).Error(unintendedMsg) + + select { + case received := <-noticeError: + if received.Message != unintendedMsg { + t.Errorf("Unexpected message received: %s", received.Message) + } + case <-time.After(time.Second): + t.Error("Timed out; no notice received by Airbrake API") + } +} + +func startAirbrakeServer(t *testing.T) *httptest.Server { ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { var notice notice if err := xml.NewDecoder(r.Body).Decode(¬ice); err != nil { @@ -29,29 +126,8 @@ func TestNoticeReceived(t *testing.T) { } r.Body.Close() - msg <- notice.Error.Message + noticeError <- notice.Error })) - defer ts.Close() - hook := &AirbrakeHook{} - - airbrake.Environment = "production" - airbrake.Endpoint = ts.URL - airbrake.ApiKey = "foo" - - log := logrus.New() - log.Hooks.Add(hook) - - log.WithFields(logrus.Fields{ - "error": errors.New(expectedMsg), - }).Error("Airbrake will not see this string") - - select { - case received := <-msg: - if received != expectedMsg { - t.Errorf("Unexpected message received: %s", received) - } - case <-time.After(time.Second): - t.Error("Timed out; no notice received by Airbrake API") - } + return ts } From ecc16b3b2a583f3d5308ed4a3a4ce5cf45dbc68b Mon Sep 17 00:00:00 2001 From: Matt Bostock Date: Tue, 10 Mar 2015 18:01:14 +0000 Subject: [PATCH 3/3] Remove outdated version of Airbrake hook It seems unnecessary to duplicate the code (which is now outdated) in the README. Instead, link to the built-in hooks where a user can see the code. --- README.md | 39 ++------------------------------------- 1 file changed, 2 insertions(+), 37 deletions(-) diff --git a/README.md b/README.md index c4a59ab..50faff6 100644 --- a/README.md +++ b/README.md @@ -164,43 +164,8 @@ You can add hooks for logging levels. For example to send errors to an exception tracking service on `Error`, `Fatal` and `Panic`, info to StatsD or log to multiple places simultaneously, e.g. syslog. -```go -// Not the real implementation of the Airbrake hook. Just a simple sample. -import ( - log "github.com/Sirupsen/logrus" -) - -func init() { - log.AddHook(new(AirbrakeHook)) -} - -type AirbrakeHook struct{} - -// `Fire()` takes the entry that the hook is fired for. `entry.Data[]` contains -// the fields for the entry. See the Fields section of the README. -func (hook *AirbrakeHook) Fire(entry *logrus.Entry) error { - err := airbrake.Notify(entry.Data["error"].(error)) - if err != nil { - log.WithFields(log.Fields{ - "source": "airbrake", - "endpoint": airbrake.Endpoint, - }).Info("Failed to send error to Airbrake") - } - - return nil -} - -// `Levels()` returns a slice of `Levels` the hook is fired for. -func (hook *AirbrakeHook) Levels() []log.Level { - return []log.Level{ - log.ErrorLevel, - log.FatalLevel, - log.PanicLevel, - } -} -``` - -Logrus comes with built-in hooks. Add those, or your custom hook, in `init`: +Logrus comes with [built-in hooks](hooks/). Add those, or your custom hook, in +`init`: ```go import (