spandex-project / spandex_phoenix

Phoenix Instrumentation tracer
MIT License
82 stars 29 forks source link

Mark 500s as errors #24

Closed jared-mackey closed 4 years ago

jared-mackey commented 4 years ago

Errors that cause a 500 but do not bubble all the way up to the handler do not get marked as errors on datadog. An example is an Absinthe resolver that causes an exception. Absinthe handles the exception and sets the status as 500 and returns normally. The datadog code here does not set the 500 as an error.

zachdaniel commented 4 years ago

I don't have access to a datadog account with data right now (I will again soon), but I thought they treated error http responses as errors internally or something along those lines. We don't currently set it to an error for a 500 status code, as you say. If datadog isn't doing it, then we'll want to make a configuration that lets you make some status codes map to errors, like error_status_codes: [500]. Maybe more expressive than that, but thats the gist.

bforchhammer commented 4 years ago

I managed to resolve this via the following customize_metadata hook:


  defp customize_metadata(%Plug.Conn{} = conn) do
    conn
    |> SpandexPhoenix.default_metadata()
    |> check_response_status(conn)
  end

  defmodule StatusError do
    defexception message: "HTTP status not in success range"

    def create(status) do
      %__MODULE__{message: "HTTP status #{status} is not in 200..399 range"}
    end
  end

  defp check_response_status(span_opts, %Plug.Conn{status: status}) do
    if status in 200..399 do
      span_opts
    else
      Keyword.put(span_opts, :error, error?: true, exception: StatusError.create(status))
    end
  end

I decided to use an exception to give a bit more context to the error messages on datadog; only setting error?: true without context can lead to confusion in the datadog UI in my experience 😉

zachdaniel commented 4 years ago

I think that we can close the issue since there is a decent way to do it using existing tooling. PRs welcome to make it more convenient.