Notify on :error level Logger messages

[rabidpraxis]

We had a customer that expected the Logger.error call to notify Honeybadger. It looks like it would be a trivial change to allow notifying on logger error calls.

My question (@joshuap @sorentwo) is there a reason that we only notify on process crashes?

[sorentwo]

There isn't any reason not to do this now. We already use the error logger to do our reporting. Slight refinement though, we notify on both exceptions (rescue) and crashes (catch).

[yukster]

Hey, just commenting here instead of #315 because this one is still open. I question whether this was a sensible choice. I work on several large and long-running Elixir apps and was surprised to see my HB errors go through the roof after updating a bunch of deps, including the HB client.

I'm going to unhook it from the logger by setting use_logger to false (and hoping that will only turn off the Logger.error calls going to HB) but it seems like assuming calling Logger.error is an exceptional case is a bit presumptuous. We log inter-service communication issues at error level and want to track their volume (in Datadog) but we don't need those in HB. I feel like Honeybadger is a triage system for unexpected errors: something new shows up after a deploy, you make a story (we have the Jira plugin), work up a fix, deploy it, and the error is resolved. But anything that goes into Honeybadger due to a Logger call is obviously never going to be resolved!

Anyway, my two cents... And probably a moot argument if the default for use_logger is false... someone please holler if setting use_logger to false will affect anything other than stopping Logger.error calls from notifying HB!

[joshuap]

@yukster good points.

I think that use_logger: false will also prevent reporting crashes in SASL compliant processes.

It seems like use_logger may have dual responsibilities—I wonder if we should have two config options instead of one? cc @sorentwo @rabidpraxis @TraceyOnim

[yukster]

@joshuap thanks for the fast response! I have to say, I'm a bit confused by the SASL logging docs, especially because it says that SASL logger was deprecated in OTP 21 (I think we're on 23 across the board). Well, I'm going to test this throughly to make sure crashes still go to HB.

[yukster]

So I confirmed that I will only get notifications from the plug integration with use_logger: false... so no error-reporting for GenServers and the like. I think I'm just going to roll back to 0.15.0 for now. And I would love to see a config for logging errors. Maybe I can whip up a PR for it.

[TraceyOnim]

@yukster good points.

I think that use_logger: false will also prevent reporting crashes in SASL compliant processes.

It seems like use_logger may have dual responsibilities—I wonder if we should have two config options instead of one? cc @sorentwo @rabidpraxis @TraceyOnim

@joshuap , how do you mean by two config options? What about using 'ignored_domains option . In such cases error for that domain won't be sent to HB I have tried ignoring [:elixir] domain in my pet project:

ignored_domains: [:elixir]

with this option error logs from GenServer is sent to HB:

{
  "domain" : ["otp"],
  "error_logger" : {
    "report_cb" : "&:gen_server.format_log/1",
    "tag" : "error"
  },
  "gl" : "#PID<0.66.0>",
  "mfa" : [
    "gen_server",
    "error_info",
    7
  ]

Kindly confirm my assumption if its true, does all SASL crashes fall under [:sasl] or [:otp] domain? If yes then I think ignoring only [:elixir] domain might help in such a case

[yukster]

@joshuap how about this?: https://github.com/honeybadger-io/honeybadger-elixir/pull/380

[joshuap]

Kindly confirm my assumption if its true, does all SASL crashes fall under [:sasl] or [:otp] domain? If yes then I think ignoring only [:elixir] domain might help in such a case

I'm not sure about the answer to that question. Might be a good question for the Elixir forums or Slack channel. If yes, I agree that ignoring the domain could be a good option, although I'm also not against adding an explicit configuration option to disable reporting Logger.error, as in #380.

[TraceyOnim]

Kindly confirm my assumption if its true, does all SASL crashes fall under [:sasl] or [:otp] domain? If yes then I think ignoring only [:elixir] domain might help in such a case

I'm not sure about the answer to that question. Might be a good question for the Elixir forums or Slack channel.

I've posted the question on Elixir Forum, I'll follow up with it for responses