Skip to content
This repository was archived by the owner on Jun 19, 2020. It is now read-only.

Add Raven (Sentry) appender#31

Open
erickedji wants to merge 4 commits into
joshfire:masterfrom
erickedji:raven-appender
Open

Add Raven (Sentry) appender#31
erickedji wants to merge 4 commits into
joshfire:masterfrom
erickedji:raven-appender

Conversation

@erickedji
Copy link
Copy Markdown

This adds an appender that sends logs to Sentry using the raven-js library. The appender needs a reference to the Raven object provided by raven-js in its configuration.

@tidoust
Copy link
Copy Markdown
Contributor

tidoust commented Jan 28, 2014

Hi @erickedji,

Thanks for the pull request and giving it a shot at integrating Woodman with Sentry. I need to think about it some more but I am worried about a few things in the code you provided:

  1. The reference to the Raven library gets passed as a configuration setting, which explains why you needed to drop the (rather ugly) bit of code that cloned the configuration option in LoggerContext. Cloning the configuration object is needed because the Woodman updates it in place afterwards and it should not update the provided configuration object directly. The Raven-js library should rather be a dependency of the RavenAppender but that may not be directly possible as the library actually has side effects (in the sense that it registers itself to handle uncaught exceptions I think)
  2. Having to pass null as last parameter of the logging function in certain cases strikes me as weird. Developers should not have to know that Sentry is used in the background when they write logs in their code.
  3. The fatal level should be a logging level as any other. It's a bit weird to provide it in the last parameter of the logging function. It's easy to register new levels in Woodman. Not sure if that's needed or if we can live without it, but that's easy to do.

I guess one thing to clarify is the goal of that integration: it does not make a lot of sense IMHO to use raven-js and woodman together in the same app since that mixes two reporting libraries where some of the logs are sent to the Sentry backend directly while others go through Woodman. What makes sense to me is being able to send logs to a Sentry backend. It may not be possible to map all Sentry features to Woodman, but that shouldn't be such a big deal.

Looking at the raven-js library, it seems relatively easy to extract the logic behind the captureMessage function and drop the rest of the code (the ability to capture stack traces is certainly useful but should be added separately, see #6).

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.

2 participants