Skip to content

Commit

Permalink
Move source code maps to ETS (#777)
Browse files Browse the repository at this point in the history
  • Loading branch information
whatyouhide authored Aug 28, 2024
1 parent e40e9a8 commit 6a971dd
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 46 deletions.
4 changes: 2 additions & 2 deletions lib/sentry/application.ex
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ defmodule Sentry.Application do

use Application

alias Sentry.{Config, Sources}
alias Sentry.Config

@impl true
def start(_type, _opts) do
Expand All @@ -24,6 +24,7 @@ defmodule Sentry.Application do
children =
[
{Registry, keys: :unique, name: Sentry.Transport.SenderRegistry},
Sentry.Sources,
Sentry.Dedupe,
{Sentry.Integrations.CheckInIDMappings,
[
Expand All @@ -35,7 +36,6 @@ defmodule Sentry.Application do
[Sentry.Transport.SenderPool]

cache_loaded_applications()
_ = Sources.load_source_code_map_if_present()

with {:ok, pid} <-
Supervisor.start_link(children, strategy: :one_for_one, name: Sentry.Supervisor) do
Expand Down
5 changes: 3 additions & 2 deletions lib/sentry/event.ex
Original file line number Diff line number Diff line change
Expand Up @@ -551,8 +551,9 @@ defmodule Sentry.Event do
not Config.enable_source_code_context?() ->
frame

source_map = Sources.get_source_code_map_from_persistent_term() ->
{pre_context, context, post_context} = Sources.get_source_context(source_map, file, line)
source_map_for_file = Sources.get_lines_for_file(file) ->
{pre_context, context, post_context} =
Sources.get_source_context(source_map_for_file, line)

%Interfaces.Stacktrace.Frame{
frame
Expand Down
70 changes: 50 additions & 20 deletions lib/sentry/sources.ex
Original file line number Diff line number Diff line change
@@ -1,15 +1,49 @@
defmodule Sentry.Sources do
@moduledoc false

use GenServer

alias Sentry.Config

@type source_map_for_file :: %{
optional(line_no :: pos_integer()) => line_contents :: String.t()
}

@type source_map :: %{
optional(String.t()) => %{
(line_no :: pos_integer()) => line_contents :: String.t()
}
optional(String.t()) => source_map_for_file()
}

@source_code_map_key {:sentry, :source_code_map}
## GenServer

@table __MODULE__

@spec start_link(keyword()) :: GenServer.on_start()
def start_link([] = _) do
GenServer.start_link(__MODULE__, nil, name: __MODULE__)
end

@impl true
def init(nil) do
_ = :ets.new(@table, [:public, :named_table, read_concurrency: true])
{:ok, :no_state, {:continue, :load_source_code_map}}
end

@impl true
def handle_continue(:load_source_code_map, state) do
:ok =
with {:loaded, source_map} <- load_source_code_map_if_present() do
Enum.each(source_map, fn {path, lines_map} ->
:ets.insert(@table, {path, lines_map})
end)
else
_error -> :ok
end

{:noreply, state}
end

## Other functions

@compression_level if Mix.env() == :test, do: 0, else: 9

# Default argument is here for testing.
Expand All @@ -21,7 +55,6 @@ defmodule Sentry.Sources do

with {:ok, contents} <- File.read(path),
{:ok, source_map} <- decode_source_code_map(contents) do
:persistent_term.put(@source_code_map_key, source_map)
{:loaded, source_map}
else
{:error, :binary_to_term} ->
Expand Down Expand Up @@ -67,11 +100,6 @@ defmodule Sentry.Sources do
end
end

@spec get_source_code_map_from_persistent_term() :: source_map() | nil
def get_source_code_map_from_persistent_term do
:persistent_term.get(@source_code_map_key, nil)
end

@spec load_files(keyword()) :: {:ok, source_map()} | {:error, message :: String.t()}
def load_files(config \\ []) when is_list(config) do
config = Sentry.Config.validate!(config)
Expand Down Expand Up @@ -106,23 +134,25 @@ defmodule Sentry.Sources do
source_map -> {:ok, source_map}
end

@spec get_source_context(source_map(), String.t() | nil, pos_integer() | nil) ::
{[String.t()], String.t() | nil, [String.t()]}
def get_source_context(%{} = files, file_name, line_number) do
context_lines = Config.context_lines()

case Map.fetch(files, file_name) do
:error -> {[], nil, []}
{:ok, file} -> get_source_context_for_file(file, line_number, context_lines)
@spec get_lines_for_file(Path.t()) :: map() | nil
def get_lines_for_file(file) do
case :ets.lookup(@table, file) do
[{^file, lines}] -> lines
[] -> nil
end
end

defp get_source_context_for_file(file, line_number, context_lines) do
@spec get_source_context(source_map_for_file(), pos_integer() | nil) ::
{[String.t()], String.t() | nil, [String.t()]}
def get_source_context(source_map_for_file, line_number)
when is_map(source_map_for_file) and (is_integer(line_number) or is_nil(line_number)) do
context_lines = Config.context_lines()

context_line_indices = 0..(2 * context_lines)

Enum.reduce(context_line_indices, {[], nil, []}, fn i, {pre_context, context, post_context} ->
context_line_number = line_number - context_lines + i
source = Map.get(file, context_line_number)
source = Map.get(source_map_for_file, context_line_number)

cond do
context_line_number == line_number && source ->
Expand Down
4 changes: 4 additions & 0 deletions lib/sentry/transport/sender.ex
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ defmodule Sentry.Transport.Sender do

@impl GenServer
def init([]) do
if function_exported?(Process, :set_label, 1) do
apply(Process, :set_label, [__MODULE__])
end

{:ok, %__MODULE__{}}
end

Expand Down
32 changes: 10 additions & 22 deletions test/sources_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -82,45 +82,33 @@ defmodule Sentry.SourcesTest do
end

@tag :tmp_dir
test "stores the source code map in :persistent_term if valid", %{tmp_dir: tmp_dir} do
test "reads the source code map from the file", %{tmp_dir: tmp_dir} do
encoded_map = Sources.encode_source_code_map(%{"foo.ex" => %{}})
path = Path.join(tmp_dir, "valid.map")
File.write!(path, encoded_map)
assert {:loaded, map} = Sources.load_source_code_map_if_present(path)
assert map == %{"foo.ex" => %{}}
assert :persistent_term.get({:sentry, :source_code_map}) == %{"foo.ex" => %{}}
after
:persistent_term.erase({:sentry, :source_code_map})
end
end

describe "get_source_context/3" do
describe "get_source_context/2" do
test "returns the correct context" do
map = %{
"foo.ex" => %{
1 => "defmodule Foo do",
2 => " def bar do",
3 => " \"bar\"",
4 => " end",
5 => "end"
},
"bar.ex" => %{
1 => "defmodule Bar do",
2 => " def baz do",
3 => " \"baz\"",
4 => " end",
5 => "end"
}
1 => "defmodule Foo do",
2 => " def bar do",
3 => " \"bar\"",
4 => " end",
5 => "end"
}

assert {pre, context, post} = Sources.get_source_context(map, "foo.ex", 4)
assert {pre, context, post} = Sources.get_source_context(map, 4)
assert pre == ["defmodule Foo do", " def bar do", " \"bar\""]
assert context == " end"
assert post == ["end"]
end

test "works if the file doesn't exist" do
assert {[], nil, []} = Sources.get_source_context(%{}, "foo.ex", 4)
test "works if the line number doesn't exist" do
assert {[], nil, []} = Sources.get_source_context(%{}, 4)
end
end
end

0 comments on commit 6a971dd

Please sign in to comment.