Better in app module allow list default

[solnic]

This adds otp_apps config option which is then used to auto-discover module names for the in_app_module_allow_list option.

Closes #800

[sl0thentr0py]

so if I understand correctly, the user will still have to supply these values?
Is there no way to just detect a project root and just use that so that it works ootb?
We really want a good ootb experience here, that's kinda important.

[sl0thentr0py]

we can merge this but let's also please make the ootb experience possible

[solnic]

so if I understand correctly, the user will still have to supply these values?

No, the user would only have to provide names of applications that should be considered when auto-discovering the modules. For example if you're building an app called Blog and there's a corresponding :blog OTP app, then you just configure it in otp_apps: [:blog] and every module from this app will be added to in_app_module_allow_list setting.

[sl0thentr0py]

yes but that's still manual, why cannot we just detect a project root like other SDKs? we want good default ootb stacktraces

[solnic]

@sl0thentr0py due to Elixir limitations, it cannot provide us with this information in a released Elixir app, so it does work in dev env, but not released prod.

[sl0thentr0py]

okay but from here
#800 (comment)

If yes, then we get all the apps from the umbrella and do what's described below, otherwise just do it for the root app.

all the apps are already in the mix config, why do we ask for them again in a separate config?

[solnic]

okay but from here #800 (comment)

If yes, then we get all the apps from the umbrella and do what's described below, otherwise just do it for the root app.

all the apps are already in the mix config, why do we ask for them again in a separate config?

It's explained here #854 (comment)

[whatyouhide]

We need to mention the new option in :in_app_module_allow_list too

[Miradorn]

Hey, just tried this new option and I really want to love it, but it might have one problem (maybe special for us): previously the list was something like 12 entries for us due to the "greedy" nature of it, using the new option the list now has ~25k entries needing something like 1.5 mb. I'm wondering if it would be reasonable to generate the modules also in a more "prefix" way to avoid this problem?

[whatyouhide]

@Miradorn can't you override the current option ([]) and keep the old option around?

[Miradorn]

Sorry I wasn't clear enough. I wanted to migrate to the new options since it seems a lot clearer to me (and also avoids possibly forgetting to add new module namespaces when they get added in the future).
There is no problem when I'm not using the new option, I'd just prefer to migrate to it :D

[whatyouhide]

@Miradorn your use case sounds like you might be better off with the old prefix-based option to me?

[Miradorn]

@whatyouhide Hi again and sorry for picking this up again, I just couldn't stop thinking about this since something seemed wrong to me:

I think there might be actually a somewhat easy optimization in here from which everybody might benefit:
Sort the list of modules and only keep the shortest Prefixes that are part of the list. Or stated the other way: throwing away all modules away that are a submodule of a module in the list. In our case that reduces the list of modules from ~12500 entries to 800. Since the matching algorithm is greedy it should change exactly nothing semantic wise, it might improve performance (only scanning the small list) and have at least a small memory imapct :D

If you agree i could prepare an MR including the logic, probably something like (thanks to chatgpt :D):
If you say the optimization is not worth it I'll leave it at that. Thanks again!

defmodule ModulePrefixFilter do
  def filter_shortest_prefixes(modules) do
    modules
    |> Enum.sort_by(&module_name_to_string/1)
    |> filter_prefixes([])
  end

  defp filter_prefixes([], acc), do: Enum.reverse(acc)

  defp filter_prefixes([head | tail], acc) do
    head_str = module_name_to_string(head)
    if Enum.any?(acc, fn existing_module ->
         existing_module_str = module_name_to_string(existing_module)
         String.starts_with?(head_str, existing_module_str)
       end) do
      filter_prefixes(tail, acc)
    else
      filter_prefixes(tail, [head | acc])
    end
  end

  defp module_name_to_string(module) when is_atom(module) do
    module
    |> Atom.to_string()
    |> String.replace(~r{\.}, ".")
  end
end