Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move source code maps to ETS #777

Merged
merged 3 commits into from
Aug 28, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
67 changes: 47 additions & 20 deletions lib/sentry/sources.ex
Original file line number Diff line number Diff line change
@@ -1,15 +1,46 @@
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
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)
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 +52,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 +97,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 +131,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
Loading