heroku / rollrus

Logrus Rollbar Hook

 

What

Rollrus is what happens when Logrus meets Rollbar.

Install the hook into a Logrus logger to report logged messages to Rollbar. By default, only messages with the Error, Fatal, or Panic level are reported.

Panic and Fatal errors are reported synchronously to help ensure that logs are delivered before the process exits. All other messages are delivered in the background, and may be dropped if the queue is full.

If the error includes a StackTrace, that StackTrace is reported to rollbar.

Usage

Examples available in the tests or on GoDoc.

rollrus's Issues

Use official go client

https://github.com/rollbar/rollbar-go and couple it with rollbar/rollbar-go#3 (comment)

Ignore well-known error fields when reporting custom fields

I think it's better to ignore all of the well-known error fields when reporting the custom fields, since they are/should be already taken care of by rollrus in some other fashion (e.g. by putting the cause error in the title, like rollrus already does).

Or, at the very least, there needs to be some detection whether a custom field is an error, and if it is, just send the message (an error with a stack trace from github.com/pkg/errors currently gets sent with the whole stack trace, which is unnecessary since the stack trace is already taken care of separately).

Able to change the level reported?

Is there a way to override the Levels() to make it report Warn or other levels?

https://github.com/heroku/rollrus/blob/master/rollrus.go#L77

Repo dead/lacking maintainer?

Seems like this repo is dead/lacking a maintainer?

This package clobbers your existing formatter

Hey. I just found this today and it's perfect for what I need, except:

log.SetFormatter(&log.TextFormatter{DisableTimestamp: true})

Why is this in here exactly? I'm using log.JsonFormatter so I can easily parse my logs on-disk (which contain all sorts of other stuff). I was surprised when a hook actually overwrote this!

Add a test for a Causer that returns nil

See #30

License

Could you add a license for rollrus?

extractError() will discard stack if root cause doesn't have a stack trace attached

Since extractError() only looks at the root cause and discards other errors in the error stack (https://github.com/heroku/rollrus/blob/master/rollrus.go#L270-L275), if there are multiple wrapped errors and the root cause does not have a stacktrace, it will discard the stacktraces from the other errors. I propose that it uses the deepest stack it can find:

func getDeepestStackTrace(err error) errors.StackTrace {
	type errorCauser interface {
		Cause() error
	}
	type stackTracer interface {
		StackTrace() errors.StackTrace
	}

	var deepestStackTrace errors.StackTrace
	for {
		if tracer, ok := err.(stackTracer); ok {
			deepestStackTrace = tracer.StackTrace()
		}

		if causer, ok := err.(errorCauser); ok {
			err = causer.Cause()
		} else {
			break
		}
	}
	return deepestStackTrace
}

Error message is discarded when embedding an error

Hi,

whenever WithError is used to log a message, the cause is taken from the embedded error and the logged message gets completely lost.

Example:

package main

import (
	"github.com/heroku/rollrus"
	log "github.com/sirupsen/logrus"
	"errors"
)

func main() {
	log.SetLevel(log.ErrorLevel)
	rollbarHook := rollrus.NewHook("bc", "test")
	log.AddHook(rollbarHook)

	log.WithError(errors.New("[Testing] inner message")).Error("[Testing] outer message")
}

This is what appears on rollbar:

 Traceback (most recent call last):
File "github.com/heroku/rollrus/rollrus.go" line 191 in rollrus.(*Hook).report
File "github.com/heroku/rollrus/rollrus.go" line 170 in rollrus.(*Hook).Fire
File "github.com/sirupsen/logrus/hooks.go" line 28 in logrus.LevelHooks.Fire
File "github.com/sirupsen/logrus/entry.go" line 98 in logrus.Entry.log
File "github.com/sirupsen/logrus/entry.go" line 159 in logrus.(*Entry).Error
File "rollrustest.go" line 15 in main.main

{699508d8}: [Testing] inner message

The "outer message" does not appear anywhere.

I traced down the problem to extractError, which takes the cause from the err field if it is present.

In my opinion, if there is an entry.Message, that should be used as cause, and the err field's cause should be logged in a separate field.

I can prepare a PR if needed.

Thanks!

Ability to set person fields

This would be really helpful.

https://godoc.org/github.com/rollbar/rollbar-go#SetPerson

We currently do something like this with sentry and it's logrus hook:

https://github.com/evalphobia/logrus_sentry#special-fields

This hook could use the same special fields user_name, user_email and user_id and pass them into SetPerson().

This could also be optional using WithPersonFieldsFunc OptionFunc.

Cause's implementation of interface Causer can return nil

The method Cause of the interface Causer (github.com/pkg/errors) can returns nil. cf. implementation and documentation

https://github.com/pkg/errors/blob/937e8c55285fe9d0c5c2cf16e40e71c65e4b4c6c/errors.go#L269-L282

Function extractError does not check the return value of the call to errors.Cause. In this specific case it leads to a Panic in rollrus package

rollrus/rollrus.go

Line 270 in dc55b37

cause := errors.Cause(err)

Build fails

Hi,

Recent change in errors package causes this package to fail: pkg/errors#183

Don't require development dependencies

If deps are vendored for various reasons (i.e. cache), a lot of unnecessary code (golang-ci, go-critic) are pulled and put into git.

Suggested solution(s):

• install under a temporary folder and use the binary
• pull the binary directly

Panic in WithError and hash of unhashable type internal.PGError

https://github.com/heroku/rollrus/blob/master/rollrus.go#L182

go-pg threw internal.PGError, we tried to log it WithError(err) and the logger panicked the worker. This definitely shouldn't happen.

The fix should be easy, though I am not 100 % sure how to do it right now.

Migrate to go modules?

If this is of interest (which I believe it should be), I can submit a PR since I forked rollrus today and made the (tiny) changes to adopt go modules.

New Trace log level in logrus

Hello, I'm just opening this issue to let you know, the next release of logrus (v1.2.0) will add a new trace level named Trace below Debug.
Here is the PR for reference sirupsen/logrus#844
You may want to take into account this new level.

Take measures to prevent cred leaks

Run stuff through github.com/heroku/x/scrub or the like before submitting.