Adjust inspect options for stacktrace arguments for increased visibility.

[archseer]

Hi! We've been using this or a while since the regular inspect would cut off our stack traces if they were too long.

[mitchellhenke]

@archseer thanks for opening this!

I am hesitant to switch this to the default, as some users may unexpectedly end up sending (or fail to send) large events. Similarly, I don't know that changing how structs appear is beneficial enough to deviate from the defaults.

Ideally, it would be possible to be flexible around this, and allow configuration, but I do have concerns about adding too many configuration options as well 🙂

What do you think?

[aghyad]

@archseer Thank you for opening this PR. I had the same problem, and I opened an issue about it before realizing there's already a PR.

@mitchellhenke I understand your concerns. What do you think about adding those as optional opts passed from any function that calls Event.create_event/1?

For instance:

  @spec args_from_stacktrace(Exception.stacktrace(), Keyword.t()) :: map()
  def args_from_stacktrace([{_m, _f, a, _} | _], opts \\ [])

  def args_from_stacktrace([{_m, _f, a, _} | _], []) when is_list(a) do
    Enum.with_index(a)
    |> Enum.into(%{}, fn {arg, index} ->
      {"arg#{index}", inspect(arg)}
    end)
  end

  def args_from_stacktrace([{_m, _f, a, _} | _], opts) when is_list(a) do
    Enum.with_index(a)
    |> Enum.into(%{}, fn {arg, index} ->
      {"arg#{index}", inspect(arg, opts)}
    end)
  end

  def args_from_stacktrace(_, _), do: %{}

  ## So it can be used like this:
  args_from_stacktrace([line], limit: :infinity, pretty: true, structs: false, width: 0)
  ## or like this:
  args_from_stacktrace([line]

[mitchellhenke]

@aghyad that is a good option. It would mean that events from Sentry.Plug and Sentry.LoggerBackend would always use the defaults, which may be ok?

[aghyad]

@mitchellhenke I was hoping to have these options enabled at a level closer to the public interface of this lib. That includes Sentry.Plug and Sentry.LoggerBackend

[aghyad]

However, on a second thought, I think spending the man power to make such an update that optionally allows for infinity inspecting is probably not the best approach on the long run.

@mitchellhenke what bad side-effects you're assuming will happen if the default setting for inspect/2 changed from :limited to :infinity?

[mitchellhenke]

@aghyad The Sentry server will reject events that are too large, which would cause a user to miss an event that previously would have truncated some event details.

To help guide any potential implementation, what is the typical size range of the terms that are larger than the default of 50 in your applications?

[archseer]

What's the maximum byte limit set by Sentry? We could at least increase the truncation limit somewhere closer to the Sentry limit. Specifically, setting :limit, to :infinity, but setting :printable_limit to a higher value.

:printable_limit - limits the number of bytes that are printed for strings and charlists. Defaults to 4096. If you don't want to limit the number of items to a particular number, use :infinity.

[mitchellhenke]

@archseer unfortunately, :limit and :printable_limit are separate limits for list-y types and binaries/charlists, and setting either to :infinity creates the same risk. Calling inspect on a long list with a lower :printable_limit will still output all of the long list. Example:

IO.inspect(Enum.to_list(1..20), printable_limit: 5, limit: :infinity)
# prints [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20]

[mitchellhenke]

Hey, wanted to check back in on this issue. Guidance on what types of situations (e.g. large web requests, large GenServer state, etc.) lead to expand what is inspected to :infinity vs. a smaller limit, and the typical lengths involved would help me towards a solution.

I'd really like to get to something that makes it work better for y'all without risking the size limits.

[archseer]

We have GenServers that can contain large nested state and/or crashes related to printing out Ecto models. The default limit is way too low (it's useful for output in iex, but not for debugging) and ended up truncating the relevant parts of the struct. As such I'd like to set the limit to as close to Sentry payload limits as possible.

[mitchellhenke]

Thanks for the additional context 🙂

It would be unfortunately relatively complex to know ahead of time what things to what extent things can be allowed to be larger, as other parts of the event will vary in size as well, and they all get encoded at once.

I think it makes the most sense to find a way to allow configuring inspect's options. I'll be playing around with some potential solutions today.

[mitchellhenke]

I've put some time into looking into this today, but I think it makes the most sense to always attempt to send the max.

Each argument is limited to 512 characters, so allowing config of :infinity isn't necessarily helpful.

I've opened a PR to do that here: #340

[mitchellhenke]

I've merged #340 to fix this, so I'm going to close this PR. If you are able to try out the master branch and report any issues here, that would be super, otherwise I plan to publish a release sometime this week.

Thank you again for taking the time to open this issue and discuss it with me 🙂