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

Make Sentry.Interfaces.Request a struct #611

Merged
merged 1 commit into from
Sep 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions lib/sentry/client.ex
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ defmodule Sentry.Client do
|> update_if_present(:message, &String.slice(&1, 0, @max_message_length))
|> update_if_present(:breadcrumbs, fn bcs -> Enum.map(bcs, &Map.from_struct/1) end)
|> update_if_present(:sdk, &Map.from_struct/1)
|> update_if_present(:request, &(&1 |> Map.from_struct() |> remove_nils()))
|> update_if_present(:extra, &sanitize_non_jsonable_values(&1, json_library))
|> update_if_present(:user, &sanitize_non_jsonable_values(&1, json_library))
|> update_if_present(:tags, &sanitize_non_jsonable_values(&1, json_library))
Expand All @@ -125,6 +126,10 @@ defmodule Sentry.Client do
end)
end

defp remove_nils(map) when is_map(map) do
:maps.filter(fn _key, value -> not is_nil(value) end, map)
end

defp sanitize_non_jsonable_values(map, json_library) do
# We update the existing map instead of building a new one from scratch
# due to performance reasons. See the docs for :maps.map/2.
Expand Down
21 changes: 19 additions & 2 deletions lib/sentry/context.ex
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,23 @@ defmodule Sentry.Context do
optional(atom()) => term()
}

@typedoc """
Request context.

See `set_request_context/1`. This map gets eventually converted
into a `Sentry.Interfaces.Request` struct.
"""
@typedoc since: "9.0.0"
@type request_context() :: %{
optional(:method) => String.t() | nil,
optional(:url) => String.t() | nil,
optional(:query_string) => String.t() | map() | [{String.t(), String.t()}] | nil,
optional(:data) => term(),
optional(:cookies) => String.t() | map() | [{String.t(), String.t()}] | nil,
optional(:headers) => map() | nil,
optional(:env) => map() | nil
}

@typedoc """
Breadcrumb info.

Expand Down Expand Up @@ -154,9 +171,9 @@ defmodule Sentry.Context do
"""
@spec get_all() :: %{
user: user_context(),
request: request_context(),
tags: tags(),
extra: extra(),
request: Interfaces.request(),
breadcrumbs: list()
}
def get_all do
Expand Down Expand Up @@ -296,7 +313,7 @@ defmodule Sentry.Context do
}

"""
@spec set_request_context(Interfaces.request()) :: :ok
@spec set_request_context(request_context()) :: :ok
def set_request_context(request_context) when is_map(request_context) do
set_context(@request_key, request_context)
end
Expand Down
20 changes: 16 additions & 4 deletions lib/sentry/event.ex
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ defmodule Sentry.Event do
contexts: Interfaces.context(),
exception: [Interfaces.Exception.t()],
message: String.t() | nil,
request: Interfaces.request(),
request: Interfaces.Request.t() | nil,
sdk: Interfaces.SDK.t() | nil,
user: Interfaces.user() | nil,

Expand Down Expand Up @@ -113,7 +113,7 @@ defmodule Sentry.Event do
modules: %{},
platform: :elixir,
release: nil,
request: %{},
request: %Interfaces.Request{},
sdk: nil,
server_name: nil,
tags: %{},
Expand Down Expand Up @@ -212,7 +212,7 @@ defmodule Sentry.Event do
@spec create_event([option]) :: t()
when option:
{:user, Interfaces.user()}
| {:request, Interfaces.request()}
| {:request, map()}
| {:extra, Context.extra()}
| {:breadcrumbs, Context.breadcrumb()}
| {:tags, Context.tags()}
Expand Down Expand Up @@ -274,7 +274,7 @@ defmodule Sentry.Event do
modules: Map.new(@deps, &{&1, to_string(Application.spec(&1, :vsn))}),
original_exception: exception,
release: Config.release(),
request: request,
request: coerce_request(request),
sdk: @sdk,
server_name: Config.server_name() || to_string(:net_adm.localhost()),
source: source,
Expand Down Expand Up @@ -321,6 +321,18 @@ defmodule Sentry.Event do
end
end

@request_fields %Interfaces.Request{} |> Map.from_struct() |> Map.keys() |> MapSet.new()

defp coerce_request(request) do
Enum.reduce(request, %Interfaces.Request{}, fn {key, value}, acc ->
if key in @request_fields do
Map.replace!(acc, key, value)
else
raise ArgumentError, "unknown field for the request interface: #{inspect(key)}"
end
end)
end

@doc """
Transforms an exception to a Sentry event.

Expand Down
47 changes: 31 additions & 16 deletions lib/sentry/interfaces.ex
Original file line number Diff line number Diff line change
Expand Up @@ -33,22 +33,6 @@ defmodule Sentry.Interfaces do
optional(atom()) => term()
}

@typedoc """
The **request** interface.

See <https://develop.sentry.dev/sdk/event-payloads/request>.
"""
@typedoc since: "9.0.0"
@type request() :: %{
optional(:method) => String.t() | nil,
optional(:url) => String.t() | nil,
optional(:query_string) => String.t() | map() | [{String.t(), String.t()}] | nil,
optional(:data) => term(),
optional(:cookies) => String.t() | map() | [{String.t(), String.t()}] | nil,
optional(:headers) => map() | nil,
optional(:env) => map() | nil
}

@typedoc """
The generic **context** interface.

Expand All @@ -57,6 +41,37 @@ defmodule Sentry.Interfaces do
@typedoc since: "9.0.0"
@type context() :: map()

defmodule Request do
@moduledoc """
The struct for the **request** interface.

See <https://develop.sentry.dev/sdk/event-payloads/request>.
"""

@moduledoc since: "9.0.0"

@typedoc since: "9.0.0"
@type t() :: %__MODULE__{
method: String.t() | nil,
url: String.t() | nil,
query_string: String.t() | map() | [{String.t(), String.t()}] | nil,
data: term(),
cookies: String.t() | map() | [{String.t(), String.t()}] | nil,
headers: map() | nil,
env: map() | nil
}

defstruct [
:method,
:url,
:query_string,
:data,
:cookies,
:headers,
:env
]
end

defmodule SDK do
@moduledoc """
The struct for the **SDK** interface.
Expand Down
4 changes: 2 additions & 2 deletions test/context_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ defmodule Sentry.ContextTest do

import Sentry.TestEnvironmentHelper

alias Sentry.{Context, Event}
alias Sentry.{Context, Event, Interfaces}

doctest Context, except: [add_breadcrumb: 1]

Expand Down Expand Up @@ -64,7 +64,7 @@ defmodule Sentry.ContextTest do

event = Event.create_event(request: %{method: "GET"})

assert event.request == %{url: "https://wow", method: "GET"}
assert event.request == %Interfaces.Request{url: "https://wow", method: "GET"}
end

test "passing in extra context as option overrides Context" do
Expand Down
4 changes: 2 additions & 2 deletions test/envelope_test.exs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
defmodule Sentry.EnvelopeTest do
use ExUnit.Case, async: true

alias Sentry.{Envelope, Event}
alias Sentry.{Envelope, Event, Interfaces}

describe "from_binary/1" do
test "parses envelope with empty headers" do
Expand Down Expand Up @@ -62,7 +62,7 @@ defmodule Sentry.EnvelopeTest do
original_exception: nil,
platform: :elixir,
release: nil,
request: %{},
request: %Interfaces.Request{},
server_name: "john-linux",
tags: %{},
timestamp: "2021-10-09T03:53:22",
Expand Down
8 changes: 7 additions & 1 deletion test/event_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ defmodule Sentry.EventTest do

assert event.user == %{id: 2, username: "foo", email: "foo@example.com"}

assert event.request == %{
assert event.request == %Interfaces.Request{
method: "POST",
url: "https://a.com",
data: "yes"
Expand Down Expand Up @@ -259,6 +259,12 @@ defmodule Sentry.EventTest do
assert event.source == :plug
assert event.original_exception == exception
end

test "raises if the :request option contains non-request fields" do
assert_raise ArgumentError, ~r/unknown field for the request interface: :bad_key/, fn ->
Event.create_event(request: %{method: "GET", bad_key: :indeed})
end
end
end

test "respects the max_breadcrumbs configuration" do
Expand Down