Skip to content

App frames #177

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Jun 29, 2017
Merged

App frames #177

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3381d83
in-app frames whitelist
mitchellhenke Jun 27, 2017
5887edf
docs
mitchellhenke Jun 27, 2017
7a039e8
update bypass
mitchellhenke Jun 28, 2017
15b0e50
update ex_doc
mitchellhenke Jun 28, 2017
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: 3 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ config :sentry,
| `before_send_event` | False | | |
| `after_send_event` | False | | |
| `sample_rate` | False | 1.0 | |
| `in_app_module_whitelist` | False | `[]` | |
| `enable_source_code_context` | True | | |
| `root_source_code_path` | Required if `enable_source_code_context` is enabled | | Should generally be set to `File.cwd!`|
| `context_lines` | False | 3 | |
Expand All @@ -92,7 +93,8 @@ config :sentry,
tags: %{
env: "production"
},
hackney_opts: [pool: :my_pool]
hackney_opts: [pool: :my_pool],
in_app_module_whitelist: [MyApp]
```

The `environment_name` and `included_environments` work together to determine
Expand Down
4 changes: 4 additions & 0 deletions docs/config.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,10 @@ Optional settings

The sampling factor to apply to events. A value of 0.0 will deny sending any events, and a value of 1.0 will send 100% of events.

.. describe:: in_app_module_whitelist

Expects a list of modules that is used to distinguish among stacktrace frames that belong to your app and ones that are part of libraries or core Elixir. This is used to better display the significant part of stacktraces. The logic is greedy, so if your app's root module is ``MyApp`` and your setting is ``[MyApp]``, that module as well as any submodules like ``MyApp.Submodule`` would be considered part of your app. Defaults to ``[]``.

.. describe:: context_lines

The number of lines of source code before and after the line that caused the exception to be included. Defaults to ``3``.
Expand Down
27 changes: 27 additions & 0 deletions lib/sentry/event.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@ defmodule Sentry.Event do
@moduledoc """
Provides an Event Struct as well as transformation of Logger
entries into Sentry Events.


### Configuration

* `:in_app_module_whitelist` - Expects a list of modules that is used to distinguish among stacktrace frames that belong to your app and ones that are part of libraries or core Elixir. This is used to better display the significant part of stacktraces. The logic is greedy, so if your app's root module is `MyApp` and your setting is `[MyApp]`, that module as well as any submodules like `MyApp.Submodule` would be considered part of your app. Defaults to `[]`.

"""

defstruct event_id: nil,
Expand Down Expand Up @@ -154,6 +160,7 @@ defmodule Sentry.Event do

@spec stacktrace_to_frames(Exception.stacktrace) :: [map]
def stacktrace_to_frames(stacktrace) do
in_app_module_whitelist = Application.get_env(:sentry, :in_app_module_whitelist, [])
stacktrace
|> Enum.map(fn(line) ->
{mod, function, arity, location} = line
Expand All @@ -167,6 +174,7 @@ defmodule Sentry.Event do
function: Exception.format_mfa(mod, function, arity),
module: mod,
lineno: line_number,
in_app: is_in_app?(mod, in_app_module_whitelist),
}
|> put_source_context(file, line_number)
end)
Expand All @@ -191,4 +199,23 @@ defmodule Sentry.Event do

defp arity_to_integer(arity) when is_list(arity), do: Enum.count(arity)
defp arity_to_integer(arity) when is_integer(arity), do: arity

defp is_in_app?(nil, _in_app_whitelist), do: false
defp is_in_app?(_, []), do: false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this right? Shouldn't it default to true because we want to send everything?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the part that is difficult to explain/document for me.

It sends up the entire stacktrace regardless, but certain lines in it are marked as being part of the app or not. If no modules are configured as being "part of the app", then we default to false.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also wondering if instead of doing all that work we could rely on filepath like ruby.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right but would it hide all of the lines? Should we default to nil in that case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the case that no lines are marked as "in_app", it just shows everything with no option to filter by "in_app".

File path is probably a no go, as it doesn't give us full path. Example:

{Poison.Encoder.Any, :encode, 2, [file: 'lib/poison/encoder.ex', line: 383]}
{Poison, :encode!, 2, [file: 'lib/poison.ex', line: 41]}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jeregrine ping

defp is_in_app?(module, in_app_module_whitelist) do
split_modules = module_split(module)

Enum.any?(in_app_module_whitelist, fn(module) ->
whitelisted_split_modules = module_split(module)

count = Enum.count(whitelisted_split_modules)
Enum.take(split_modules, count) == whitelisted_split_modules
end)
end

defp module_split(module) when is_binary(module) do
String.split(module, ".")
|> Enum.reject(&(&1 == "Elixir"))
end
defp module_split(module), do: module_split(String.Chars.to_string(module))
end
61 changes: 55 additions & 6 deletions test/event_test.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,12 @@ defmodule Sentry.EventTest do
assert event.message == "(UndefinedFunctionError) function Sentry.Event.not_a_function/0 is undefined or private"
assert is_binary(event.server_name)
assert event.stacktrace == %{frames: Enum.reverse([
%{filename: nil, function: "Sentry.Event.not_a_function/0", lineno: nil, module: Sentry.Event, context_line: nil, post_context: [], pre_context: []},
%{filename: "test/event_test.exs", function: "Sentry.EventTest.event_generated_by_exception/1", lineno: 8, module: Sentry.EventTest, context_line: nil, post_context: [], pre_context: []},
%{filename: "test/event_test.exs", function: "Sentry.EventTest.\"test parses error exception\"/1", lineno: 15, module: Sentry.EventTest, context_line: nil, post_context: [], pre_context: []},
%{filename: "lib/ex_unit/runner.ex", function: "ExUnit.Runner.exec_test/1", lineno: 302, module: ExUnit.Runner, context_line: nil, post_context: [], pre_context: []},
%{filename: "timer.erl", function: ":timer.tc/1", lineno: 166, module: :timer, context_line: nil, post_context: [], pre_context: []},
%{filename: "lib/ex_unit/runner.ex", function: "anonymous fn/3 in ExUnit.Runner.spawn_test/3", lineno: 250, module: ExUnit.Runner, context_line: nil, post_context: [], pre_context: []}])
%{filename: nil, function: "Sentry.Event.not_a_function/0", lineno: nil, module: Sentry.Event, context_line: nil, post_context: [], pre_context: [], in_app: false},
%{filename: "test/event_test.exs", function: "Sentry.EventTest.event_generated_by_exception/1", lineno: 8, module: Sentry.EventTest, context_line: nil, post_context: [], pre_context: [], in_app: false},
%{filename: "test/event_test.exs", function: "Sentry.EventTest.\"test parses error exception\"/1", lineno: 15, module: Sentry.EventTest, context_line: nil, post_context: [], pre_context: [], in_app: false},
%{filename: "lib/ex_unit/runner.ex", function: "ExUnit.Runner.exec_test/1", lineno: 302, module: ExUnit.Runner, context_line: nil, post_context: [], pre_context: [], in_app: false},
%{filename: "timer.erl", function: ":timer.tc/1", lineno: 166, module: :timer, context_line: nil, post_context: [], pre_context: [], in_app: false},
%{filename: "lib/ex_unit/runner.ex", function: "anonymous fn/3 in ExUnit.Runner.spawn_test/3", lineno: 250, module: ExUnit.Runner, context_line: nil, post_context: [], pre_context: [], in_app: false}])
}
assert event.tags == %{}
assert event.timestamp =~ ~r/\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}/
Expand Down Expand Up @@ -70,9 +70,58 @@ defmodule Sentry.EventTest do
event = Sentry.Event.transform_exception(exception, [fingerprint: ["hello", "world"]])
assert event.fingerprint == ["hello", "world"]
end

test "not sending fingerprint when unset" do
exception = RuntimeError.exception("error")
event = Sentry.Event.transform_exception(exception, [])
assert event.fingerprint == ["{{ default }}"]
end

test "sets app_frame to true when configured" do
modify_env(:sentry, in_app_module_whitelist: [Sentry, :random, Sentry.Submodule])
exception = RuntimeError.exception("error")
event = Sentry.Event.transform_exception(exception, [stacktrace: [{Elixir.Sentry.Fun, :method, 2, []}, {Elixir.Sentry, :other_method, 4, []},
{:other_module, :a_method, 8, []}, {:random, :uniform, 0, []},
{Sentry.Submodule.Fun, :this_method, 0, []}]])
assert %{frames: [
%{
module: Sentry.Submodule.Fun,
function: "Sentry.Submodule.Fun.this_method/0",
in_app: true,
filename: nil, lineno: nil,
context_line: nil, post_context: [], pre_context: []
},
%{
module: :random,
function: ":random.uniform/0",
in_app: true,
filename: nil, lineno: nil,
context_line: nil, post_context: [], pre_context: []
},
%{
module: :other_module,
function: ":other_module.a_method/8",
in_app: false,
filename: nil, lineno: nil,
context_line: nil, post_context: [], pre_context: []
},
%{
module: Sentry,
function: "Sentry.other_method/4",
in_app: true,
filename: nil, lineno: nil,
context_line: nil, post_context: [], pre_context: []
},
%{
filename: nil,
function: "Sentry.Fun.method/2",
module: Sentry.Fun,
lineno: nil,
in_app: true,
context_line: nil,
post_context: [],
pre_context: []
},
]} == event.stacktrace
end
end
1 change: 1 addition & 0 deletions test/logger_test.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ defmodule Sentry.LoggerTest do
assert List.first(json["exception"])["value"] == "** (exit) :function_clause"
assert List.last(json["stacktrace"]["frames"]) == %{"filename" => "lib/calendar.ex",
"function" => "NaiveDateTime.from_erl/2",
"in_app" => false,
"lineno" => 1214,
"module" => "Elixir.NaiveDateTime",
"context_line" => nil,
Expand Down