Skip to content

Commit

Permalink
Updates Sentry.Sources to support multiple source code root paths
Browse files Browse the repository at this point in the history
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
  • Loading branch information
jbernardo95 committed Oct 6, 2020
1 parent b3a92b7 commit c92750d
Show file tree
Hide file tree
Showing 5 changed files with 96 additions and 20 deletions.
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"` | |
Expand Down
2 changes: 1 addition & 1 deletion config/config.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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"
24 changes: 19 additions & 5 deletions lib/sentry/config.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
27 changes: 15 additions & 12 deletions lib/sentry/sources.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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

Expand Down
59 changes: 59 additions & 0 deletions test/sources_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,65 @@ defmodule Sentry.SourcesTest do
use Plug.Test
import Sentry.TestEnvironmentHelper

describe "load_files/0" do
test "loads files" do
tmp_dir = System.tmp_dir!()

File.mkdir(tmp_dir <> "apps")
File.mkdir(tmp_dir <> "apps/app_a")
File.mkdir(tmp_dir <> "apps/app_a/lib")
File.mkdir(tmp_dir <> "apps")
File.mkdir(tmp_dir <> "apps/app_b")
File.mkdir(tmp_dir <> "apps/app_b/lib")

File.write!(
tmp_dir <> "apps/app_a/lib/module_a.ex",
"""
defmodule ModuleA do
def test do
"test a"
end
end
"""
)

File.write!(
tmp_dir <> "apps/app_b/lib/module_b.ex",
"""
defmodule ModuleB do
def test do
"test b"
end
end
"""
)

Application.put_env(:sentry, :root_source_code_paths, [
tmp_dir <> "apps/app_a",
tmp_dir <> "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()

File.rm_rf!(tmp_dir <> "apps")
end
end

test "exception makes call to Sentry API" do
correct_context = %{
"context_line" => " raise RuntimeError, \"Error\"",
Expand Down

0 comments on commit c92750d

Please sign in to comment.