Switch Hackney to Finch

Transition the HTTP client usage from Hackney to Finch to optimize
requests performance.

.changesets/switch-hackney-with-finch.md

@@ -0,0 +1,6 @@
+---
+bump: minor
+type: change
+---
+
+Switch Hackney to Finch as the bundled HTTP client to operate within the integration

lib/appsignal.ex

@@ -48,8 +48,7 @@ defmodule Appsignal do
       {Appsignal.Tracer, []},
       {Appsignal.Monitor, []},
       {Appsignal.Probes, []},
-      {Appsignal.CheckIn.Scheduler, []},
-      :hackney_pool.child_spec(:appsignal_transmitter, [])
+      {Appsignal.CheckIn.Scheduler, []}
     ]
 
     result = Supervisor.start_link(children, strategy: :one_for_one, name: Appsignal.Supervisor)

lib/appsignal/check_in/scheduler.ex

@@ -99,10 +99,10 @@ defmodule Appsignal.CheckIn.Scheduler do
     endpoint = "#{config[:logging_endpoint]}/check_ins/json"
 
     case @transmitter.transmit_and_close(endpoint, {Enum.reverse(events), :ndjson}, config) do
-      {:ok, status_code, _} when status_code in 200..299 ->
+      {:ok, %{status: status_code}, _} when status_code in 200..299 ->
         @integration_logger.trace("Transmitted #{description}")
 
-      {:ok, status_code, _} ->
+      {:ok, %{status: status_code}, _} ->
         @integration_logger.error(
           "Failed to transmit #{description}: status code was #{status_code}"
         )

lib/appsignal/diagnose/report.ex

@@ -11,18 +11,13 @@ defmodule Appsignal.Diagnose.Report do
   @spec send(map(), map()) :: {:ok, String.t()} | {:error, map()}
   def send(config, report) do
     case Transmitter.transmit(config[:diagnose_endpoint], {%{diagnose: report}, :json}, config) do
-      {:ok, 200, _, reference} ->
-        {:ok, body} = :hackney.body(reference)
-        :hackney.close(reference)
-
+      {:ok, %{status: 200, body: body}} ->
         case Jason.decode(body) do
           {:ok, response} -> {:ok, response["token"]}
           {:error, _} -> {:error, %{status_code: 200, body: body}}
         end
 
-      {:ok, status_code, _, reference} ->
-        {:ok, body} = :hackney.body(reference)
-        :hackney.close(reference)
+      {:ok, %{status: status_code, body: body}} ->
         {:error, %{status_code: status_code, body: body}}
 
       {:error, reason} ->

lib/appsignal/transmitter.ex

@@ -4,19 +4,17 @@ defmodule Appsignal.Transmitter do
   require Logger
 
   def request(method, url, headers \\ [], body \\ "") do
-    http_client = Application.get_env(:appsignal, :http_client, :hackney)
-    :application.ensure_all_started(http_client)
+    http_client = Application.get_env(:appsignal, :http_client, Finch)
+    Finch.start_link(name: AppsignalFinch)
 
-    http_client.request(method, url, headers, body, options())
+    method
+    |> http_client.build(url, headers, body)
+    |> http_client.request(AppsignalFinch, options())
   end
 
   def transmit(url, payload_and_format \\ {nil, nil}, config \\ nil)
   def transmit(url, nil, config), do: transmit(url, {nil, nil}, config)
 
-  # This function calls `:hackney.request/5` -- it is the
-  # caller's responsibility to ensure that `:hackney.close/1` is
-  # called afterwards.
-  #
   # If you're not interested in the body, only in the status code
   # and headers, use `transmit_and_close/3` instead.
   def transmit(url, {payload, format}, config) do
@@ -40,8 +38,7 @@ defmodule Appsignal.Transmitter do
 
   def transmit_and_close(url, payload_and_format \\ {nil, nil}, config \\ nil) do
     case transmit(url, payload_and_format, config) do
-      {:ok, status, headers, reference} ->
-        :hackney.close(reference)
+      {:ok, %{status: status, headers: headers}} ->
         {:ok, status, headers}
 
       {:error, reason} ->

lib/appsignal/utils/push_api_key_validator.ex

@@ -9,7 +9,7 @@ defmodule Appsignal.Utils.PushApiKeyValidator do
       {:ok, 200, _} -> :ok
       {:ok, 401, _} -> {:error, :invalid}
       {:ok, status_code, _} -> {:error, status_code}
-      {:error, reason} -> {:error, reason}
+      {:error, %{reason: reason}} -> {:error, reason}
     end
   end
 end

mix.exs

@@ -103,12 +103,6 @@ defmodule Appsignal.Mixfile do
     system_version = System.version()
     otp_version = System.otp_release()
 
-    hackney_version =
-      case otp_version >= "21" do
-        true -> "~> 1.6 and <= 1.21.0"
-        false -> "1.18.1"
-      end
-
     decorator_version =
       case Version.compare(system_version, "1.5.0") do
         :lt -> "~> 1.2.3"
@@ -140,9 +134,10 @@ defmodule Appsignal.Mixfile do
       end
 
     [
+      {:certifi, "~> 2.14"},
       {:decimal, "~> 2.0"},
       {:benchee, "~> 1.0", only: :bench},
-      {:hackney, hackney_version},
+      {:finch, "~> 0.18.0"},
       {:jason, "~> 1.0"},
       {:decorator, decorator_version},
       {:plug, plug_version, only: [:test, :test_no_nif]},

mix_helpers.exs

@@ -144,8 +144,7 @@ defmodule Mix.Appsignal.Helper do
 
       false ->
         Mix.shell().info("Downloading agent release")
-        :application.unset_env(:hackney, :mod_metrics)
-        :application.ensure_all_started(:hackney)
+        Finch.start_link(name: AppsignalFinchDownload)
 
         case do_download_file!(filename, local_filename, Appsignal.Agent.mirrors()) do
           {:ok, url} ->
@@ -210,17 +209,18 @@ defmodule Mix.Appsignal.Helper do
   end
 
   defp do_download_file!(url, local_filename) do
-    case :hackney.request(:get, url, [], "", download_options()) do
-      {:ok, 200, _, reference} ->
-        case :hackney.body(reference) do
-          {:ok, body} -> File.write(local_filename, body)
-          {:error, reason} -> {:error, reason}
-        end
+    request =
+      Finch.build(:get, url, [], "")
+      |> Finch.request(AppsignalFinchDownload, download_options())
+
+    case request do
+      {:ok, %{status: 200, body: body}} ->
+        File.write(local_filename, body)
 
       response ->
         message = """
         - URL: #{url}
-        - Error (hackney response):
+        - Error (Finch response):
         #{inspect(response)}
         """
 

test/appsignal/diagnose/report_test.exs

@@ -66,7 +66,7 @@ defmodule Mix.Tasks.Appsignal.Diagnose.ReportTest do
     end
 
     test "sends the diagnostics report to AppSignal and returns an error" do
-      assert send() == {:error, %{body: "woops", status_code: 500}}
+      assert send() == {:error, %{status_code: 500, body: "woops"}}
     end
   end
 

test/appsignal/transmitter_test.exs

@@ -5,7 +5,7 @@ defmodule Appsignal.TransmitterTest do
   import ExUnit.CaptureLog
 
   setup do
-    Application.put_env(:appsignal, :http_client, FakeHackney)
+    Application.put_env(:appsignal, :http_client, FakeFinch)
 
     on_exit(fn ->
       Application.delete_env(:appsignal, :http_client)

test/appsignal/utils/push_api_key_validator_test.exs

@@ -64,7 +64,7 @@ defmodule Appsignal.Utils.PushApiKeyValidatorTest do
     end
 
     test "returns an error", %{config: config} do
-      assert PushApiKeyValidator.validate(config) == {:error, :econnrefused}
+      assert PushApiKeyValidator.validate(config) == {:error, :econn_refused}
     end
   end
 end

test/support/fake_finch.ex

@@ -0,0 +1,9 @@
+defmodule FakeFinch do
+  def build(method, url, headers, body) do
+    [method, url, headers, body]
+  end
+
+  def request([method, url, headers, body], _, options) do
+    [method, url, headers, body, options]
+  end
+end