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

Switch HTTP client from hackney to finch #758

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 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
6 changes: 3 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ This is the official Sentry SDK for [Sentry].

### Install

To use Sentry in your project, add it as a dependency in your `mix.exs` file. Sentry does not install a JSON library nor HTTP client by itself. Sentry will default to trying to use [Jason] for JSON serialization and [Hackney] for HTTP requests, but can be configured to use other ones. To use the default ones, do:
To use Sentry in your project, add it as a dependency in your `mix.exs` file. Sentry does not install a JSON library nor HTTP client by itself. Sentry will default to trying to use [Jason] for JSON serialization and [Finch] for HTTP requests, but can be configured to use other ones. To use the default ones, do:

```elixir
defp deps do
Expand All @@ -27,7 +27,7 @@ defp deps do

{:sentry, "~> 10.0"},
{:jason, "~> 1.4"},
{:hackney, "~> 1.19"}
{:finch, "~> 0.18"}
]
end
```
Expand Down Expand Up @@ -149,7 +149,7 @@ Licensed under the MIT license, see [`LICENSE`](./LICENSE).

[Sentry]: http://sentry.io/
[Jason]: https://github.com/michalmuskala/jason
[Hackney]: https://github.com/benoitc/hackney
[Finch]: https://github.com/sneako/finch
[Bypass]: https://github.com/PSPDFKit-labs/bypass
[docs]: https://hexdocs.pm/sentry/readme.html
[logger-handlers]: https://www.erlang.org/doc/apps/kernel/logger_chapter#handlers
Expand Down
2 changes: 1 addition & 1 deletion config/config.exs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ if config_env() == :test do
tags: %{},
enable_source_code_context: true,
root_source_code_paths: [File.cwd!()],
hackney_opts: [recv_timeout: 50, pool: :sentry_pool],
finch_opts: [recv_timeout: 50],
send_result: :sync,
send_max_attempts: 1,
dedup_events: false,
Expand Down
2 changes: 1 addition & 1 deletion lib/mix/tasks/sentry.send_test_event.ex
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ defmodule Mix.Tasks.Sentry.SendTestEvent do
end

Mix.shell().info("current environment_name: #{inspect(to_string(Config.environment_name()))}")
Mix.shell().info("hackney_opts: #{inspect(Config.hackney_opts())}\n")
Mix.shell().info("finch_opts: #{inspect(Config.finch_opts())}\n")
savhappy marked this conversation as resolved.
Show resolved Hide resolved
end

defp send_event(opts) do
Expand Down
32 changes: 16 additions & 16 deletions lib/sentry/config.ex
Original file line number Diff line number Diff line change
Expand Up @@ -239,11 +239,11 @@ defmodule Sentry.Config do
client: [
type: :atom,
type_doc: "`t:module/0`",
default: Sentry.HackneyClient,
default: Sentry.FinchClient,
doc: """
A module that implements the `Sentry.HTTPClient`
behaviour. Defaults to `Sentry.HackneyClient`, which uses
[hackney](https://github.com/benoitc/hackney) as the HTTP client.
behaviour. Defaults to `Sentry.FinchClient`, which uses
[finch](https://github.com/sneako/finch) as the HTTP client.
savhappy marked this conversation as resolved.
Show resolved Hide resolved
"""
],
send_max_attempts: [
Expand All @@ -253,30 +253,30 @@ defmodule Sentry.Config do
The maximum number of attempts to send an event to Sentry.
"""
],
hackney_opts: [
Copy link
Collaborator

Choose a reason for hiding this comment

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

We need to keep hackney_opts and deprecate them if we want this to not be a breaking change. NimbleOptions supports deprecating options, check out the docs for that.

This also applies to the options below.

Copy link
Collaborator Author

@savhappy savhappy Jul 30, 2024

Choose a reason for hiding this comment

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

Does this mean we need to keep the option:

    client: [
      type: :atom,
      type_doc: "`t:module/0`",
      default: Sentry.HackneyClient,
      doc: """
      """
    ]

and the module HackneyClient?? @whatyouhide

finch_opts: [
type: :keyword_list,
default: [pool: :sentry_pool],
doc: """
Options to be passed to `hackney`. Only
applied if `:client` is set to `Sentry.HackneyClient`.
Options to be passed to `finch`. Only
applied if `:client` is set to `Sentry.FinchClient`.
"""
],
hackney_pool_timeout: [
finch_pool_timeout: [
type: :timeout,
default: 5000,
doc: """
The maximum time to wait for a
connection to become available. Only applied if `:client` is set to
`Sentry.HackneyClient`.
`Sentry.FinchClient`.
"""
],
hackney_pool_max_connections: [
finch_pool_max_connections: [
type: :pos_integer,
default: 50,
doc: """
The maximum number of
connections to keep in the pool. Only applied if `:client` is set to
`Sentry.HackneyClient`.
`Sentry.FinchClient`.
"""
]
]
Expand Down Expand Up @@ -470,11 +470,11 @@ defmodule Sentry.Config do
@spec environment_name() :: String.t() | nil
def environment_name, do: fetch!(:environment_name)

@spec max_hackney_connections() :: pos_integer()
def max_hackney_connections, do: fetch!(:hackney_pool_max_connections)
@spec max_finch_connections() :: pos_integer()
def max_finch_connections, do: fetch!(:finch_pool_max_connections)

@spec hackney_timeout() :: timeout()
def hackney_timeout, do: fetch!(:hackney_pool_timeout)
@spec finch_timeout() :: timeout()
def finch_timeout, do: fetch!(:finch_pool_timeout)

@spec tags() :: map()
def tags, do: fetch!(:tags)
Expand Down Expand Up @@ -512,8 +512,8 @@ defmodule Sentry.Config do
@spec sample_rate() :: float()
def sample_rate, do: fetch!(:sample_rate)

@spec hackney_opts() :: keyword()
def hackney_opts, do: fetch!(:hackney_opts)
@spec finch_opts() :: keyword()
def finch_opts, do: fetch!(:finch_opts)
Comment on lines +544 to +545
Copy link
Collaborator

Choose a reason for hiding this comment

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

Where do we use these?


@spec before_send() :: (Sentry.Event.t() -> Sentry.Event.t()) | {module(), atom()} | nil
def before_send, do: get(:before_send)
Expand Down
1 change: 0 additions & 1 deletion lib/sentry/envelope.ex
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ defmodule Sentry.Envelope do
end

items_iodata = Enum.map(envelope.items, &item_to_binary(json_library, &1))

{:ok, IO.iodata_to_binary([headers_iodata, items_iodata])}
catch
{:error, _reason} = error -> error
Expand Down
56 changes: 56 additions & 0 deletions lib/sentry/finch_client.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
defmodule Sentry.FinchClient do
@behaviour Sentry.HTTPClient

@moduledoc """
The built-in HTTP client.

This client implements the `Sentry.HTTPClient` behaviour.

It's based on the [finch](https://github.com/sneako/finch) Erlang HTTP client,
savhappy marked this conversation as resolved.
Show resolved Hide resolved
which is an *optional dependency* of this library. If you wish to use another
HTTP client, you'll have to implement your own `Sentry.HTTPClient`. See the
documentation for `Sentry.HTTPClient` for more information.

Finch is built on top of NimblePool. If you need to set other pool configuration options,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you link to nimble_pool's repo here?

see "Pool Configuration Options" in the source code for details on the possible map values.
[finch configuration options](https://github.com/sneako/finch/blob/main/lib/finch.ex)
Copy link
Collaborator

Choose a reason for hiding this comment

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

We shouldn't point people to source code, that can change (especially main). Let's link to Finch for docs

"""
@impl true
def child_spec do
if Code.ensure_loaded?(Finch) do
case Application.ensure_all_started(:finch) do
{:ok, _apps} -> :ok
{:error, reason} -> raise "failed to start the :finch application: #{inspect(reason)}"
end

Finch.child_spec(
name: __MODULE__,
pools: %{
:default => [
size: Sentry.Config.max_finch_connections(),
conn_max_idle_time: Sentry.Config.finch_timeout()
]
}
)
else
raise """
cannot start the :sentry application because the HTTP client is set to \
Sentry.FinchClient (which is the default), but the Finch library is not loaded. \
savhappy marked this conversation as resolved.
Show resolved Hide resolved
Add :finch to your dependencies to fix this.
"""
end
end

@impl true
def post(url, headers, body) do
request = Finch.build(:post, url, headers, body)

case Finch.request(request, __MODULE__) do
{:ok, %Finch.Response{status: status, headers: headers, body: body}} ->
{:ok, status, headers, body}

{:error, error} ->
{:error, error}
end
end
end
56 changes: 0 additions & 56 deletions lib/sentry/hackney_client.ex

This file was deleted.

45 changes: 22 additions & 23 deletions lib/sentry/http_client.ex
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ defmodule Sentry.HTTPClient do
@moduledoc """
A behaviour for HTTP clients that Sentry can use.

The default HTTP client is `Sentry.HackneyClient`.
The default HTTP client is `Sentry.FinchClient`.

To configure a different HTTP client, implement the `Sentry.HTTPClient` behaviour and
change the `:client` configuration:
Expand All @@ -25,46 +25,45 @@ defmodule Sentry.HTTPClient do
## Alternative Clients

Let's look at an example of using an alternative HTTP client. In this example, we'll
use [Finch](https://github.com/sneako/finch), a lightweight HTTP client for Elixir.
use [Hackney](https://github.com/benoitc/hackney), a lightweight HTTP client for Elixir.

First, we need to add Finch to our dependencies:
First, we need to add Hackney to our dependencies:

# In mix.exs
defp deps do
[
# ...
{:finch, "~> 0.16"}
{:hackney, "~> 1.8"}
]
end

Then, we need to define a module that implements the `Sentry.HTTPClient` behaviour:

defmodule MyApp.SentryFinchHTTPClient do
defmodule MyApp.SentryHackneyHTTPClient do
@behaviour Sentry.HTTPClient

@impl true
def child_spec do
Supervisor.child_spec({Finch, name: __MODULE__}, id: __MODULE__)
# @impl true
# def child_spec do
# :hackney_pool.child_spec(
# @hackney_pool_name,
# timeout: Sentry.Config.hackney_timeout(),
# max_connections: Sentry.Config.max_hackney_connections()
# )
# end

# @impl true
# def post(url, headers, body) do
# case :hackney.request(:post, url, headers, body) do
# {:ok, _status, _headers, _body} = result -> result
# {:error, _reason} = error -> error
# end
# end
end

@impl true
def post(url, headers, body) do
request = Finch.build(:post, url, headers, body)

case Finch.request(request, __MODULE__) do
{:ok, %Finch.Response{status: status, headers: headers, body: body}} ->
{:ok, status, headers, body}

{:error, error} ->
{:error, error}
end
end
end

Last, we need to configure Sentry to use our new HTTP client:

config :sentry,
client: MyApp.SentryFinchHTTPClient
client: Sentry.HackneyClient

### Umbrella Apps

Expand Down
6 changes: 3 additions & 3 deletions mix.exs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ defmodule Sentry.Mixfile do
"Plug and Phoenix": [Sentry.PlugCapture, Sentry.PlugContext, Sentry.LiveViewHook],
Loggers: [Sentry.LoggerBackend, Sentry.LoggerHandler],
"Data Structures": [Sentry.Attachment, Sentry.CheckIn],
HTTP: [Sentry.HTTPClient, Sentry.HackneyClient],
HTTP: [Sentry.HTTPClient, Sentry.FinchClient],
Interfaces: [~r/^Sentry\.Interfaces/],
Testing: [Sentry.Test]
],
Expand All @@ -63,7 +63,7 @@ defmodule Sentry.Mixfile do
],
authors: ["Mitchell Henke", "Jason Stiebs", "Andrea Leopardi"]
],
xref: [exclude: [:hackney, :hackney_pool, Plug.Conn, :telemetry]],
xref: [exclude: [Finch, Plug.Conn, :telemetry]],
aliases: [aliases()]
]
end
Expand All @@ -89,7 +89,7 @@ defmodule Sentry.Mixfile do
{:nimble_ownership, "~> 0.3.0"},

# Optional dependencies
{:hackney, "~> 1.8", optional: true},
{:finch, "~> 0.18.0", optional: true},
{:jason, "~> 1.1", optional: true},
{:phoenix, "~> 1.6", optional: true},
{:phoenix_live_view, "~> 0.20", optional: true},
Expand Down
Loading
Loading