Updates Sentry.Sources to support multiple source code root paths

[jbernardo95]

Before the changes in this commit Sentry.Sources only looked for source
code files in one root path, the :root_source_code_path.

With the changes in this commit this module will now be able to look for
source code in multiple paths. The paths can be configured in the
:root_source_code_paths config key.

This change is specially important for umbrella applications [1], that
have their code nested in the apps path. If we don't specify the root
path for each individual application (which isn't possible without the
changes in this commit), and instead just use File.cwd!() as the root
path, the source code files of each umbrella app will be loaded with
apps/<app name> prefix in their name. This is a problem because these
names will never match the file names in the stacktrace, which means
that Sentry won't be able to attach source code context.

To avoid a configuration breaking change, the changes in this commit
still support the :root_source_code_path configuration key, but "soft
deprecate" it by removing it from documentation. This config key should
be fully deprecated in the next major Sentry release.

[1] https://elixir-lang.org/getting-started/mix-otp/dependencies-and-umbrella-projects.html

[jbernardo95]

@mitchellhenke would appreciate a review when you have the chance 😊

[mitchellhenke]

Thanks for opening a PR!

The sticky issue that's prevented similar solutions is how to handle collisions.

apps/app_a/lib/module_a.ex and apps/app_b/lib/module_a.ex would both resolve to lib/module_a.ex, and could lead to some surprising results in the Sentry interface. Raising an error is one option.

In terms of the option deprecation, what do you think of raising an error on configuring both, and potentially emitting a warning for the deprecated option?

Also as a reminder to myself that should this get merged, the documentation on the Sentry site will need updating as well 🙂

[jbernardo95]

The sticky issue that's prevented similar solutions is how to handle collisions.

apps/app_a/lib/module_a.ex and apps/app_b/lib/module_a.ex would both resolve to lib/module_a.ex, and could lead to some surprising results in the Sentry interface. Raising an error is one option.

Very good point 🤔 I think that this should very rare, so raising an error if this happens makes sense to me. We can suggest users to rename files if this is the case.

In terms of the option deprecation, what do you think of raising an error on configuring both, and potentially emitting a warning for the deprecated option?

100% agree!

[jbernardo95]

@mitchellhenke pushed a new commit (that I'll squash before merging) addressing your suggestions. Let me know what you think!

[jbernardo95]

@mitchellhenke quick reminder on this.

[mitchellhenke]

Apologies for the delay, I've had some more time to think about this and left a couple comments.

I'm happy to merge and pick this up if you're unable to work on the comments I've added.

Thanks again for the work to improve Sentry 🙂

[mitchellhenke]

For the deprecation strategy, I think it would be preferable to keep the old method with the same behavior, but mark it as @deprecated?

I'm guessing its usage is rare, but I'd rather not remove it until the next major release.

[jbernardo95]

@mitchellhenke do you mean keeping both the new and the old versions? See if the changes in 52d32cf is what you had in mind.

[mitchellhenke]

Yep, exactly!

[mitchellhenke]

@jbernardo95 thank you!! 💖 💖

[jbernardo95]

Thank you @mitchellhenke! ❤️

(btw I hadn't rebased my commits, so a few !fixup commits were merged to master)

[jbernardo95]

@mitchellhenke btw do you have any plans to release v8.0.3 soon?

[mitchellhenke]

@jbernardo95 sorry for the delay, just released 8.0.3 to Hex 🙂

[jbernardo95]

Awesome! Thank you 🙌

[jbernardo95]

@mitchellhenke I just realised that the README is now out of date here.

[mitchellhenke]

@jbernardo95 whoops, thank you! Removed in dac9c3e