From 5ca7f873c8a40e96454e0a2b7468d8b56465d08e Mon Sep 17 00:00:00 2001
From: Joao Bernardo <jb.amaro95@gmail.com>
Date: Tue, 6 Oct 2020 23:32:55 +0100
Subject: [PATCH 1/4] Updates Sentry.Sources to support multiple source code
 root paths

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
---
 README.md                                     |  4 +--
 config/config.exs                             |  2 +-
 lib/sentry/config.ex                          | 24 +++++++++++++----
 lib/sentry/sources.ex                         | 27 ++++++++++---------
 test/sources_test.exs                         | 26 ++++++++++++++++++
 .../apps/app_a/lib/module_a.ex                |  5 ++++
 .../apps/app_b/lib/module_b.ex                |  5 ++++
 7 files changed, 73 insertions(+), 20 deletions(-)
 create mode 100644 test/support/example-umbrella-app/apps/app_a/lib/module_a.ex
 create mode 100644 test/support/example-umbrella-app/apps/app_b/lib/module_b.ex

diff --git a/README.md b/README.md
index fb56f7be..d3f1a5cc 100644
--- a/README.md
+++ b/README.md
@@ -139,7 +139,7 @@ config :sentry,
   environment_name: Mix.env(),
   included_environments: [:prod],
   enable_source_code_context: true,
-  root_source_code_path: File.cwd!()
+  root_source_code_paths: [File.cwd!()]
 ```
 
 The `environment_name` and `included_environments` work together to determine
@@ -169,7 +169,7 @@ The full range of options is the following:
 | `in_app_module_whitelist` | False | `[]` | |
 | `report_deps` | False | True | Will attempt to load Mix dependencies at compile time to report alongside events |
 | `enable_source_code_context` | False | False | |
-| `root_source_code_path` | Required if `enable_source_code_context` is enabled | | Should generally be set to `File.cwd!()`|
+| `root_source_code_paths` | Required if `enable_source_code_context` is enabled | | Should usually be set to `[File.cwd!()]`. For umbrella applications you should list all your applications paths in this list (e.g. `["#{File.cwd!()}/apps/app_1", "#{File.cwd!()}/apps/app_2"]`. |
 | `context_lines` | False  | 3 | |
 | `source_code_exclude_patterns` | False  | `[~r"/_build/", ~r"/deps/", ~r"/priv/"]` | |
 | `source_code_path_pattern` | False  | `"**/*.ex"` | |
diff --git a/config/config.exs b/config/config.exs
index 9843744c..b2ae7374 100644
--- a/config/config.exs
+++ b/config/config.exs
@@ -5,6 +5,6 @@ config :sentry,
   environment_name: :dev,
   tags: %{},
   enable_source_code_context: true,
-  root_source_code_path: File.cwd!()
+  root_source_code_paths: [File.cwd!()]
 
 import_config "#{Mix.env()}.exs"
diff --git a/lib/sentry/config.ex b/lib/sentry/config.ex
index e7fdf963..556bb321 100644
--- a/lib/sentry/config.ex
+++ b/lib/sentry/config.ex
@@ -77,13 +77,27 @@ defmodule Sentry.Config do
     get_config(:enable_source_code_context, default: false, check_dsn: false)
   end
 
-  def root_source_code_path do
+  # :root_source_code_path (single path) was replaced by :root_source_code_paths (list of
+  # paths).
+  #
+  # In order for this to not be a breaking change we still accept the old
+  # :root_source_code_path as a fallback.
+  #
+  # We should deprecate this the :root_source_code_path completely in the next major
+  # release.
+  def root_source_code_paths do
+    paths = get_config(:root_source_code_paths)
     path = get_config(:root_source_code_path)
 
-    if path do
-      path
-    else
-      raise ArgumentError.exception(":root_source_code_path must be configured")
+    cond do
+      not is_nil(paths) ->
+        paths
+
+      not is_nil(path) ->
+        [path]
+
+      true ->
+        raise ArgumentError.exception(":root_source_code_paths must be configured")
     end
   end
 
diff --git a/lib/sentry/sources.ex b/lib/sentry/sources.ex
index ba96b34c..9b1407b8 100644
--- a/lib/sentry/sources.ex
+++ b/lib/sentry/sources.ex
@@ -8,13 +8,14 @@ defmodule Sentry.Sources do
 
   ### Configuration
   There is configuration required to set up this functionality.  The options
-  include `:enable_source_code_context`, `:root_source_code_path`, `:context_lines`,
+  include `:enable_source_code_context`, `:root_source_code_paths`, `:context_lines`,
   `:source_code_exclude_patterns`, and `:source_code_path_pattern`.
 
   * `:enable_source_code_context` - when `true`, enables reporting source code
     alongside exceptions.
-  * `:root_source_code_path` - The path from which to start recursively reading files from.
-    Should usually be set to `File.cwd!()`.
+  * `:root_source_code_paths` - List of paths from which to start recursively reading files from.
+    Should usually be set to `[File.cwd!()]`. For umbrella applications you should list all your
+    applications paths in this list (e.g. `["#{File.cwd!()}/apps/app_1", "#{File.cwd!()}/apps/app_2"]`.
   * `:context_lines` - The number of lines of source code before and after the line that
     caused the exception to be included.  Defaults to `3`.
   * `:source_code_exclude_patterns` - a list of Regex expressions used to exclude file paths that
@@ -28,7 +29,7 @@ defmodule Sentry.Sources do
       config :sentry,
         dsn: "https://public:secret@app.getsentry.com/1",
         enable_source_code_context: true,
-        root_source_code_path: File.cwd!(),
+        root_source_code_path: [File.cwd!()],
         context_lines: 5
 
   ### Source code storage
@@ -64,18 +65,20 @@ defmodule Sentry.Sources do
   @type source_map :: %{String.t() => file_map}
 
   def load_files do
-    root_path = Config.root_source_code_path()
+    root_paths = Config.root_source_code_paths()
     path_pattern = Config.source_code_path_pattern()
     exclude_patterns = Config.source_code_exclude_patterns()
 
-    Path.join(root_path, path_pattern)
-    |> Path.wildcard()
-    |> exclude_files(exclude_patterns)
-    |> Enum.reduce(%{}, fn path, acc ->
-      key = Path.relative_to(path, root_path)
-      value = source_to_lines(File.read!(path))
+    Enum.reduce(root_paths, %{}, fn root_path, acc1 ->
+      Path.join(root_path, path_pattern)
+      |> Path.wildcard()
+      |> exclude_files(exclude_patterns)
+      |> Enum.reduce(acc1, fn path, acc2 ->
+        key = Path.relative_to(path, root_path)
+        value = source_to_lines(File.read!(path))
 
-      Map.put(acc, key, value)
+        Map.put(acc2, key, value)
+      end)
     end)
   end
 
diff --git a/test/sources_test.exs b/test/sources_test.exs
index 0e48e736..e0098742 100644
--- a/test/sources_test.exs
+++ b/test/sources_test.exs
@@ -3,6 +3,32 @@ defmodule Sentry.SourcesTest do
   use Plug.Test
   import Sentry.TestEnvironmentHelper
 
+  describe "load_files/0" do
+    test "loads files" do
+      Application.put_env(:sentry, :root_source_code_paths, [
+        File.cwd!() <> "/test/support/example-umbrella-app/apps/app_a",
+        File.cwd!() <> "/test/support/example-umbrella-app/apps/app_b"
+      ])
+
+      assert %{
+               "lib/module_a.ex" => %{
+                 1 => "defmodule ModuleA do",
+                 2 => "  def test do",
+                 3 => "    \"test a\"",
+                 4 => "  end",
+                 5 => "end"
+               },
+               "lib/module_b.ex" => %{
+                 1 => "defmodule ModuleB do",
+                 2 => "  def test do",
+                 3 => "    \"test b\"",
+                 4 => "  end",
+                 5 => "end"
+               }
+             } = Sentry.Sources.load_files()
+    end
+  end
+
   test "exception makes call to Sentry API" do
     correct_context = %{
       "context_line" => "    raise RuntimeError, \"Error\"",
diff --git a/test/support/example-umbrella-app/apps/app_a/lib/module_a.ex b/test/support/example-umbrella-app/apps/app_a/lib/module_a.ex
new file mode 100644
index 00000000..9c3fd6ca
--- /dev/null
+++ b/test/support/example-umbrella-app/apps/app_a/lib/module_a.ex
@@ -0,0 +1,5 @@
+defmodule ModuleA do
+  def test do
+    "test a"
+  end
+end
diff --git a/test/support/example-umbrella-app/apps/app_b/lib/module_b.ex b/test/support/example-umbrella-app/apps/app_b/lib/module_b.ex
new file mode 100644
index 00000000..771a6e26
--- /dev/null
+++ b/test/support/example-umbrella-app/apps/app_b/lib/module_b.ex
@@ -0,0 +1,5 @@
+defmodule ModuleB do
+  def test do
+    "test b"
+  end
+end

From 32383dd899f59ab98557e4c469d6d0e87088b9e7 Mon Sep 17 00:00:00 2001
From: Joao Bernardo <jb.amaro95@gmail.com>
Date: Sat, 10 Oct 2020 13:42:06 +0100
Subject: [PATCH 2/4] fixup! Updates Sentry.Sources to support multiple source
 code root paths

---
 lib/sentry/config.ex                          |  8 +++
 lib/sentry/sources.ex                         | 51 +++++++++++++------
 test/config_test.exs                          | 42 +++++++++++++++
 test/sources_test.exs                         | 19 +++++++
 .../apps/app_a/lib/module_a.ex                |  5 ++
 .../apps/app_b/lib/module_a.ex                |  5 ++
 6 files changed, 115 insertions(+), 15 deletions(-)
 create mode 100644 test/support/example-umbrella-app-with-conflict/apps/app_a/lib/module_a.ex
 create mode 100644 test/support/example-umbrella-app-with-conflict/apps/app_b/lib/module_a.ex

diff --git a/lib/sentry/config.ex b/lib/sentry/config.ex
index 556bb321..d212efb1 100644
--- a/lib/sentry/config.ex
+++ b/lib/sentry/config.ex
@@ -90,6 +90,14 @@ defmodule Sentry.Config do
     path = get_config(:root_source_code_path)
 
     cond do
+      not is_nil(path) and not is_nil(paths) ->
+        raise ArgumentError, """
+        :root_source_code_path and :root_source_code_paths can't be configured at the \
+        same time.
+
+        :root_source_code_path is deprecated. Set :root_source_code_paths instead.
+        """
+
       not is_nil(paths) ->
         paths
 
diff --git a/lib/sentry/sources.ex b/lib/sentry/sources.ex
index 9b1407b8..e3265476 100644
--- a/lib/sentry/sources.ex
+++ b/lib/sentry/sources.ex
@@ -65,21 +65,11 @@ defmodule Sentry.Sources do
   @type source_map :: %{String.t() => file_map}
 
   def load_files do
-    root_paths = Config.root_source_code_paths()
-    path_pattern = Config.source_code_path_pattern()
-    exclude_patterns = Config.source_code_exclude_patterns()
-
-    Enum.reduce(root_paths, %{}, fn root_path, acc1 ->
-      Path.join(root_path, path_pattern)
-      |> Path.wildcard()
-      |> exclude_files(exclude_patterns)
-      |> Enum.reduce(acc1, fn path, acc2 ->
-        key = Path.relative_to(path, root_path)
-        value = source_to_lines(File.read!(path))
-
-        Map.put(acc2, key, value)
-      end)
-    end)
+    Enum.reduce(
+      Config.root_source_code_paths(),
+      %{},
+      &load_files_for_root_path/2
+    )
   end
 
   @doc """
@@ -128,6 +118,37 @@ defmodule Sentry.Sources do
     end)
   end
 
+  defp load_files_for_root_path(root_path, files) do
+    root_path
+    |> find_files_for_root_path()
+    |> Enum.reduce(files, fn path, acc ->
+      key = Path.relative_to(path, root_path)
+
+      if Map.has_key?(acc, key) do
+        raise RuntimeError, """
+        Found two source files in different source root paths with the same relative \
+        path: #{key}
+
+        This means that both source files would be reported to Sentry as the same \
+        file. Please rename one of them to avoid this.
+        """
+      else
+        value = source_to_lines(File.read!(path))
+
+        Map.put(acc, key, value)
+      end
+    end)
+  end
+
+  defp find_files_for_root_path(root_path) do
+    path_pattern = Config.source_code_path_pattern()
+    exclude_patterns = Config.source_code_exclude_patterns()
+
+    Path.join(root_path, path_pattern)
+    |> Path.wildcard()
+    |> exclude_files(exclude_patterns)
+  end
+
   defp exclude_files(file_names, []), do: file_names
 
   defp exclude_files(file_names, [exclude_pattern | rest]) do
diff --git a/test/config_test.exs b/test/config_test.exs
index 8e63ca5e..44df9ff1 100644
--- a/test/config_test.exs
+++ b/test/config_test.exs
@@ -72,4 +72,46 @@ defmodule Sentry.ConfigTest do
       assert ["test", "dev"] == Config.included_environments()
     end
   end
+
+  describe "root_source_code_paths" do
+    test "raises error if :root_source_code_path and :root_source_code_paths are set" do
+      modify_env(:sentry, root_source_code_path: "/test")
+      modify_env(:sentry, root_source_code_paths: ["/test"])
+
+      expected_error_message = """
+      :root_source_code_path and :root_source_code_paths can't be configured at the \
+      same time.
+
+      :root_source_code_path is deprecated. Set :root_source_code_paths instead.
+      """
+
+      assert_raise ArgumentError, expected_error_message, fn ->
+        Config.root_source_code_paths()
+      end
+    end
+
+    test "raises error if :root_source_code_path and :root_source_code_paths are not set" do
+      delete_env(:sentry, [:root_source_code_path, :root_source_code_paths])
+
+      expected_error_message = ":root_source_code_paths must be configured"
+
+      assert_raise ArgumentError, expected_error_message, fn ->
+        Config.root_source_code_paths()
+      end
+    end
+
+    test "returns :root_source_code_path if it's set" do
+      modify_env(:sentry, root_source_code_path: "/test")
+      modify_env(:sentry, root_source_code_paths: nil)
+
+      assert Config.root_source_code_paths() == ["/test"]
+    end
+
+    test "returns :root_source_code_paths if it's set" do
+      modify_env(:sentry, root_source_code_path: nil)
+      modify_env(:sentry, root_source_code_paths: ["/test"])
+
+      assert Config.root_source_code_paths() == ["/test"]
+    end
+  end
 end
diff --git a/test/sources_test.exs b/test/sources_test.exs
index e0098742..07e0240d 100644
--- a/test/sources_test.exs
+++ b/test/sources_test.exs
@@ -27,6 +27,25 @@ defmodule Sentry.SourcesTest do
                }
              } = Sentry.Sources.load_files()
     end
+
+    test "raises error when two files have the same relative path" do
+      Application.put_env(:sentry, :root_source_code_paths, [
+        File.cwd!() <> "/test/support/example-umbrella-app-with-conflict/apps/app_a",
+        File.cwd!() <> "/test/support/example-umbrella-app-with-conflict/apps/app_b"
+      ])
+
+      expected_error_message = """
+      Found two source files in different source root paths with the same relative \
+      path: lib/module_a.ex
+
+      This means that both source files would be reported to Sentry as the same \
+      file. Please rename one of them to avoid this.
+      """
+
+      assert_raise RuntimeError, expected_error_message, fn ->
+        Sentry.Sources.load_files()
+      end
+    end
   end
 
   test "exception makes call to Sentry API" do
diff --git a/test/support/example-umbrella-app-with-conflict/apps/app_a/lib/module_a.ex b/test/support/example-umbrella-app-with-conflict/apps/app_a/lib/module_a.ex
new file mode 100644
index 00000000..9c3fd6ca
--- /dev/null
+++ b/test/support/example-umbrella-app-with-conflict/apps/app_a/lib/module_a.ex
@@ -0,0 +1,5 @@
+defmodule ModuleA do
+  def test do
+    "test a"
+  end
+end
diff --git a/test/support/example-umbrella-app-with-conflict/apps/app_b/lib/module_a.ex b/test/support/example-umbrella-app-with-conflict/apps/app_b/lib/module_a.ex
new file mode 100644
index 00000000..771a6e26
--- /dev/null
+++ b/test/support/example-umbrella-app-with-conflict/apps/app_b/lib/module_a.ex
@@ -0,0 +1,5 @@
+defmodule ModuleB do
+  def test do
+    "test b"
+  end
+end

From efc26fa54d3b05de3f6e17db0587c1504283c58c Mon Sep 17 00:00:00 2001
From: Joao Bernardo <jb.amaro95@gmail.com>
Date: Wed, 28 Oct 2020 23:28:51 +0000
Subject: [PATCH 3/4] fixup! Updates Sentry.Sources to support multiple source
 code root paths

---
 test/sources_test.exs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/sources_test.exs b/test/sources_test.exs
index 07e0240d..005536cc 100644
--- a/test/sources_test.exs
+++ b/test/sources_test.exs
@@ -5,7 +5,7 @@ defmodule Sentry.SourcesTest do
 
   describe "load_files/0" do
     test "loads files" do
-      Application.put_env(:sentry, :root_source_code_paths, [
+      modify_env(:sentry, root_source_code_paths: [
         File.cwd!() <> "/test/support/example-umbrella-app/apps/app_a",
         File.cwd!() <> "/test/support/example-umbrella-app/apps/app_b"
       ])
@@ -29,7 +29,7 @@ defmodule Sentry.SourcesTest do
     end
 
     test "raises error when two files have the same relative path" do
-      Application.put_env(:sentry, :root_source_code_paths, [
+      modify_env(:sentry, root_source_code_paths: [
         File.cwd!() <> "/test/support/example-umbrella-app-with-conflict/apps/app_a",
         File.cwd!() <> "/test/support/example-umbrella-app-with-conflict/apps/app_b"
       ])

From 52d32cfb787d0a94c15228450f407917c57e1335 Mon Sep 17 00:00:00 2001
From: Joao Bernardo <jb.amaro95@gmail.com>
Date: Wed, 28 Oct 2020 23:28:57 +0000
Subject: [PATCH 4/4] fixup! Updates Sentry.Sources to support multiple source
 code root paths

---
 lib/sentry/config.ex | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/lib/sentry/config.ex b/lib/sentry/config.ex
index d212efb1..1bb33386 100644
--- a/lib/sentry/config.ex
+++ b/lib/sentry/config.ex
@@ -77,6 +77,17 @@ defmodule Sentry.Config do
     get_config(:enable_source_code_context, default: false, check_dsn: false)
   end
 
+  @deprecated "Use root_source_code_paths/0 instead"
+  def root_source_code_path do
+    path = get_config(:root_source_code_path)
+
+    if path do
+      path
+    else
+      raise ArgumentError.exception(":root_source_code_path must be configured")
+    end
+  end
+
   # :root_source_code_path (single path) was replaced by :root_source_code_paths (list of
   # paths).
   #