Merge pull request #437 from jbernardo95/multiple-root-source-code-paths

Updates Sentry.Sources to support multiple source code root paths

Author: mitchellhenke

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"` | |

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"

lib/sentry/config.ex

@@ -77,6 +77,7 @@ 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)
 
@@ -87,6 +88,38 @@ defmodule Sentry.Config do
     end
   end
 
+  # :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)
+
+    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
+
+      not is_nil(path) ->
+        [path]
+
+      true ->
+        raise ArgumentError.exception(":root_source_code_paths must be configured")
+    end
+  end
+
   def source_code_path_pattern do
     get_config(:source_code_path_pattern, default: @default_path_pattern, check_dsn: false)
   end

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:[email protected]/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,19 +65,11 @@ defmodule Sentry.Sources do
   @type source_map :: %{String.t() => file_map}
 
   def load_files do
-    root_path = Config.root_source_code_path()
-    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))
-
-      Map.put(acc, key, value)
-    end)
+    Enum.reduce(
+      Config.root_source_code_paths(),
+      %{},
+      &load_files_for_root_path/2
+    )
   end
 
   @doc """
@@ -125,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

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

test/sources_test.exs

@@ -3,6 +3,51 @@ defmodule Sentry.SourcesTest do
   use Plug.Test
   import Sentry.TestEnvironmentHelper
 
+  describe "load_files/0" do
+    test "loads files" do
+      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"
+      ])
+
+      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
+
+    test "raises error when two files have the same relative path" do
+      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"
+      ])
+
+      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
     correct_context = %{
       "context_line" => "    raise RuntimeError, \"Error\"",

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

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

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

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