Merge pull request #2 from Recruitee/add-multiple-collectors

Add multiple collectors
Recruitee · Dec 20, 2022 · a87f552 · a87f552
2 parents 9b0995b + 369ee74
commit a87f552
Show file tree

Hide file tree

Showing 6 changed files with 103 additions and 85 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,13 @@
+## v0.2.0 [2022-12-16]
+
+* Enhancements:
+  * Ability to define multiple callback "collector" functions.
+
+* Breaking changes:
+  * Modified configuration syntax to facilitate multiple callback "collector" functions.
+    Migration to the new syntax: replace old `:collect_fun` configuration key with new `:collectors`
+    syntax, see documentation for details.
+
 ## v0.1.1 [2022-03-31]
 
 Available on hex.pm.

diff --git a/README.md b/README.md
@@ -6,18 +6,17 @@
 Basic instrumentation library to intercept and collect Plug pipeline connection parameters for
 further reporting, monitoring or analysis with user provided function.
 
-PlugCollect is inspired by other similar libraries:
+PlugCollect is inspired by other libraries:
 * [Appsignal.Plug](https://github.com/appsignal/appsignal-elixir-plug),
 * [Sentry.PlugCapture](https://github.com/getsentry/sentry-elixir).
 
-
 ## Installation
 Add `plug_collect` to your application dependencies list in `mix.exs`:
 ```elixir
 #mix.exs
 def deps do
   [
-    {:plug_collect, "~> 0.1.1"}
+    {:plug_collect, "~> 0.2.0"}
   ]
 end
 ```
@@ -27,13 +26,13 @@ Add `use PlugCollect` to your application's Phoenix endpoint or router, for exam
 ```elixir
 #endpoint.ex
 defmodule MyAppWeb.Endpoint do
-  use PlugCollect, collect_fun: &MyApp.Monitor.my_collect/2
+  use PlugCollect, collectors: [&MyApp.Monitor.my_collect/2]
   use Phoenix.Endpoint, otp_app: :my_app
   # ...
 ```
 
 Callback function `MyApp.Monitor.my_collect/2` will be invoked on each request.
-Example `:collect_fun` callback implementation:
+Example `my_collect/2` callback implementation:
 ```elixir
 defmodule MyApp.Monitor do
   def my_collect(_status, %Plug.Conn{assigns: assigns} = _conn),

diff --git a/lib/plug_collect.ex b/lib/plug_collect.ex
@@ -1,47 +1,52 @@
 defmodule PlugCollect do
   @moduledoc """
   Basic instrumentation library to intercept and collect Plug pipeline connection parameters for
-  further reporting, monitoring or analysis with user provided callback function.
+  further reporting, monitoring or analysis with user provided callback functions.
 
-  Functionality provided by PlugCollect is very similar to one offered by
-  `Plug.Conn.register_before_send/2` function.
-  `register_before_send/2` registers a callback to be invoked before the response is sent.
-  When using `register_before_send/2`, user defined callback will not be invoked if response
-  processing pipeline raises.
+  PlugCollect functionality is similar to this provided by `Plug.Conn.register_before_send/2`
+  or `Plug.ErrorHandler`.
 
-  PlugCollect similarly to `register_before_send/2` registers a callback to be invoked before
-  sending response, however callback function is always executed, even if response pipeline fails.
+  PlugCollect registers a user defined callback "collectors" functions that will be applied before
+  sending request response.
+  Callback functions are applied always, even if request response pipeline fails.
 
   ### Usage
-  Add `use PlugCollect` to your application's Phoenix endpoint or router, for example:
+  Add `use PlugCollect` to your application's Phoenix endpoint or router with your callback
+  functions defined with a required `:collectors` key as a list, for example:
   ```elixir
   #router.ex
   defmodule MyAppWeb.Router do
     use MyAppWeb, :router
-    use PlugCollect, collect_fun: &MyApp.MyModule.my_collect/2
+    use PlugCollect,
+      collectors: [
+        &MyApp.MyModule.my_collect1/2,
+        {MyApp.MyModule, :my_collect2}
+      ]
     # ...
   end
   ```
 
-  Using `PlugCollect` **requires** `:collect_fun` parameter specifying a user defined
-  callback function with two arity.
-
-  Callback function defined by `:collect_fun` key has following characteristics:
-  1. As a first argument function will receive an atom `:ok` or `:error` describing request
+  Callback functions defined with a `:collectors` list have following characteristics:
+  1. As a first argument callback function will receive an atom `:ok` or `:error` describing request
      pipeline processing status.
-     `:ok` atom means that intercepted request was processed successfully by Plug pipeline.
+     `:ok` atom means that intercepted request was processed successfully by a Plug pipeline.
      `:error` means that during request processing, pipeline raised, exited or did throw an error.
   2. As a second argument function will receive a `Plug.Conn` struct with current pipeline
-     connection.
-  3. Callback function is invoked on each Plug request, after processing entire Plug pipeline
-     defined below `use PlugCollect` statement.
-  4. Function is executed even if code declared after `use PlugCollect` raises, exits or throws an
+     connection. `Plug.Conn` struct is normalized using approach similar to this implemented in a
+     `Plug.ErrorHandler`.
+  3. Callback functions are applied after processing entire Plug pipeline defined below
+     `use PlugCollect` statement.
+  4. Functions are executed even if code declared after `use PlugCollect` raises, exits or throws an
      error.
-  5. Callback function is executed synchronously. It should not contain any blocking or costly IO
-     operations, as it would delay or block sending request response to the user.
-  6. Callback function result is ignored.
+  5. Callback functions are executed synchronously. They should not contain any blocking or costly IO
+     operations, as it would delay or block sending request response to the user. Async processing
+     can be easily achieved by using for example `spawn/1` in a collector function body.
+  6. Callback functions results are ignored.
+  7. Callback functions are executed in the order in which they appear in the `:collectors` list.
+  8. Callback functions can be provided using anonymous function reference syntax or with
+     `{module, function}` tuple.
 
-  Example `:collect_fun` implementation:
+  Example collector function implementation:
   ```elixir
   defmodule MyApp.MyModule do
     def my_collect(:ok, %Plug.Conn{assigns: assigns} = _conn),
@@ -53,31 +58,45 @@ defmodule PlugCollect do
 
   defmacro __using__(opts) do
     quote bind_quoted: [opts: opts] do
-      @collect_fun Keyword.fetch!(opts, :collect_fun)
+      @before_compile PlugCollect
+      @collectors Keyword.fetch!(opts, :collectors)
+    end
+  end
+
+  defmacro __before_compile__(_) do
+    quote do
+      defoverridable call: 2
 
       def call(conn, opts) do
         super(conn, opts)
       rescue
         error ->
-          apply(@collect_fun, [:error, prepare_conn(error, conn)])
+          apply_collectors(:error, normalize_conn(error, conn))
           :erlang.raise(:error, error, __STACKTRACE__)
       catch
         kind, reason ->
-          apply(@collect_fun, [:error, prepare_conn(reason, conn)])
+          apply_collectors(:error, normalize_conn(reason, conn))
           :erlang.raise(kind, reason, __STACKTRACE__)
       else
         conn ->
-          apply(@collect_fun, [:ok, conn])
+          apply_collectors(:ok, conn)
           conn
       end
 
-      def prepare_conn(%{conn: conn, plug_status: status}, _conn) when is_integer(status),
-        do: Map.put(conn, :status, status)
+      defp apply_collectors(status, conn) do
+        Enum.each(@collectors, fn collector ->
+          case collector do
+            c when is_function(c, 2) -> apply(c, [status, conn])
+            {m, f} -> apply(m, f, [status, conn])
+          end
+        end)
+      end
 
-      def prepare_conn(%{conn: conn}, _conn), do: conn
-      def prepare_conn(_err, %Plug.Conn{} = conn), do: conn
+      defp normalize_conn(%{conn: conn, plug_status: status}, _conn),
+        do: Plug.Conn.put_status(conn, status)
 
-      defoverridable call: 2
+      defp normalize_conn(%{conn: conn}, _conn), do: conn
+      defp normalize_conn(_err, %Plug.Conn{} = conn), do: conn
     end
   end
 end
diff --git a/mix.exs b/mix.exs
@@ -2,7 +2,7 @@ defmodule PlugCollect.MixProject do
   use Mix.Project
 
   @source_url "https://github.com/Recruitee/plug_collect"
-  @version "0.1.1"
+  @version "0.2.0"
 
   def project do
     [
@@ -22,7 +22,7 @@ defmodule PlugCollect.MixProject do
 
   defp deps() do
     [
-      {:ex_doc, "~> 0.28.0", only: :dev, runtime: false},
+      {:ex_doc, "~> 0.29.0", only: :dev, runtime: false},
       {:plug, ">= 1.1.0"}
     ]
   end

diff --git a/mix.lock b/mix.lock
@@ -1,12 +1,12 @@
 %{
-  "earmark_parser": {:hex, :earmark_parser, "1.4.24", "344f8d2a558691d3fcdef3f9400157d7c4b3b8e58ee5063297e9ae593e8326d9", [:mix], [], "hexpm", "1f6451b0116dd270449c8f5b30289940ee9c0a39154c783283a08e55af82ea34"},
-  "ex_doc": {:hex, :ex_doc, "0.28.2", "e031c7d1a9fc40959da7bf89e2dc269ddc5de631f9bd0e326cbddf7d8085a9da", [:mix], [{:earmark_parser, "~> 1.4.19", [hex: :earmark_parser, repo: "hexpm", optional: false]}, {:makeup_elixir, "~> 0.14", [hex: :makeup_elixir, repo: "hexpm", optional: false]}, {:makeup_erlang, "~> 0.1", [hex: :makeup_erlang, repo: "hexpm", optional: false]}], "hexpm", "51ee866993ffbd0e41c084a7677c570d0fc50cb85c6b5e76f8d936d9587fa719"},
+  "earmark_parser": {:hex, :earmark_parser, "1.4.29", "149d50dcb3a93d9f3d6f3ecf18c918fb5a2d3c001b5d3305c926cddfbd33355b", [:mix], [], "hexpm", "4902af1b3eb139016aed210888748db8070b8125c2342ce3dcae4f38dcc63503"},
+  "ex_doc": {:hex, :ex_doc, "0.29.1", "b1c652fa5f92ee9cf15c75271168027f92039b3877094290a75abcaac82a9f77", [:mix], [{:earmark_parser, "~> 1.4.19", [hex: :earmark_parser, repo: "hexpm", optional: false]}, {:makeup_elixir, "~> 0.14", [hex: :makeup_elixir, repo: "hexpm", optional: false]}, {:makeup_erlang, "~> 0.1", [hex: :makeup_erlang, repo: "hexpm", optional: false]}], "hexpm", "b7745fa6374a36daf484e2a2012274950e084815b936b1319aeebcf7809574f6"},
   "makeup": {:hex, :makeup, "1.1.0", "6b67c8bc2882a6b6a445859952a602afc1a41c2e08379ca057c0f525366fc3ca", [:mix], [{:nimble_parsec, "~> 1.2.2 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "0a45ed501f4a8897f580eabf99a2e5234ea3e75a4373c8a52824f6e873be57a6"},
   "makeup_elixir": {:hex, :makeup_elixir, "0.16.0", "f8c570a0d33f8039513fbccaf7108c5d750f47d8defd44088371191b76492b0b", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}, {:nimble_parsec, "~> 1.2.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "28b2cbdc13960a46ae9a8858c4bebdec3c9a6d7b4b9e7f4ed1502f8159f338e7"},
   "makeup_erlang": {:hex, :makeup_erlang, "0.1.1", "3fcb7f09eb9d98dc4d208f49cc955a34218fc41ff6b84df7c75b3e6e533cc65f", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}], "hexpm", "174d0809e98a4ef0b3309256cbf97101c6ec01c4ab0b23e926a9e17df2077cbb"},
-  "mime": {:hex, :mime, "2.0.2", "0b9e1a4c840eafb68d820b0e2158ef5c49385d17fb36855ac6e7e087d4b1dcc5", [:mix], [], "hexpm", "e6a3f76b4c277739e36c2e21a2c640778ba4c3846189d5ab19f97f126df5f9b7"},
+  "mime": {:hex, :mime, "2.0.3", "3676436d3d1f7b81b5a2d2bd8405f412c677558c81b1c92be58c00562bb59095", [:mix], [], "hexpm", "27a30bf0db44d25eecba73755acf4068cbfe26a4372f9eb3e4ea3a45956bff6b"},
   "nimble_parsec": {:hex, :nimble_parsec, "1.2.3", "244836e6e3f1200c7f30cb56733fd808744eca61fd182f731eac4af635cc6d0b", [:mix], [], "hexpm", "c8d789e39b9131acf7b99291e93dae60ab48ef14a7ee9d58c6964f59efb570b0"},
-  "plug": {:hex, :plug, "1.13.4", "addb6e125347226e3b11489e23d22a60f7ab74786befb86c14f94fb5f23ca9a4", [:mix], [{:mime, "~> 1.0 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:plug_crypto, "~> 1.1.1 or ~> 1.2", [hex: :plug_crypto, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4.3 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "06114c1f2a334212fe3ae567dbb3b1d29fd492c1a09783d52f3d489c1a6f4cf2"},
-  "plug_crypto": {:hex, :plug_crypto, "1.2.2", "05654514ac717ff3a1843204b424477d9e60c143406aa94daf2274fdd280794d", [:mix], [], "hexpm", "87631c7ad914a5a445f0a3809f99b079113ae4ed4b867348dd9eec288cecb6db"},
-  "telemetry": {:hex, :telemetry, "1.0.0", "0f453a102cdf13d506b7c0ab158324c337c41f1cc7548f0bc0e130bbf0ae9452", [:rebar3], [], "hexpm", "73bc09fa59b4a0284efb4624335583c528e07ec9ae76aca96ea0673850aec57a"},
+  "plug": {:hex, :plug, "1.14.0", "ba4f558468f69cbd9f6b356d25443d0b796fbdc887e03fa89001384a9cac638f", [:mix], [{:mime, "~> 1.0 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:plug_crypto, "~> 1.1.1 or ~> 1.2", [hex: :plug_crypto, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4.3 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "bf020432c7d4feb7b3af16a0c2701455cbbbb95e5b6866132cb09eb0c29adc14"},
+  "plug_crypto": {:hex, :plug_crypto, "1.2.3", "8f77d13aeb32bfd9e654cb68f0af517b371fb34c56c9f2b58fe3df1235c1251a", [:mix], [], "hexpm", "b5672099c6ad5c202c45f5a403f21a3411247f164e4a8fab056e5cd8a290f4a2"},
+  "telemetry": {:hex, :telemetry, "1.1.0", "a589817034a27eab11144ad24d5c0f9fab1f58173274b1e9bae7074af9cbee51", [:rebar3], [], "hexpm", "b727b2a1f75614774cff2d7565b64d0dfa5bd52ba517f16543e6fc7efcc0df48"},
 }
diff --git a/test/plug_collect_test.exs b/test/plug_collect_test.exs
@@ -1,7 +1,7 @@
 defmodule PlugCollectTest do
   defmodule CollectRouter do
     use Plug.Router
-    use PlugCollect, collect_fun: &__MODULE__.collect_request/2
+    use PlugCollect, collectors: [&__MODULE__.collect_anonymous/2, {__MODULE__, :collect_mf}]
 
     plug(:match)
     plug(:dispatch)
@@ -39,97 +39,87 @@ defmodule PlugCollectTest do
       _ = String.to_integer("one")
     end
 
-    def collect_request(:ok, _conn), do: :persistent_term.put(:on_success, true)
-    def collect_request(:error, _conn), do: :persistent_term.put(:on_error, true)
+    def collect_anonymous(:ok, _conn), do: :persistent_term.put(:on_ok_anonymous, true)
+    def collect_anonymous(:error, _conn), do: :persistent_term.put(:on_error_anonymous, true)
+    def collect_mf(:ok, _conn), do: :persistent_term.put(:on_ok_mf, true)
+    def collect_mf(:error, _conn), do: :persistent_term.put(:on_error_mf, true)
   end
 
   use ExUnit.Case, async: false
   use Plug.Test
 
-  describe ":collect_fun callback function is always invoked before sending response" do
+  describe "collectors are always applied before sending response" do
     setup do
-      :persistent_term.put(:on_success, false)
-      :persistent_term.put(:on_error, false)
+      :persistent_term.put(:on_ok_anonymous, false)
+      :persistent_term.put(:on_error_anonymous, false)
+      :persistent_term.put(:on_ok_mf, false)
+      :persistent_term.put(:on_error_mf, false)
     end
 
     test "200" do
-      refute :persistent_term.get(:on_success)
-      refute :persistent_term.get(:on_error)
       conn = :get |> conn("/200") |> CollectRouter.call([])
-      assert :persistent_term.get(:on_success)
-      refute :persistent_term.get(:on_error)
       assert conn.state == :sent
       assert conn.status == 200
       assert conn.resp_body == "200"
+
+      assert ok_request()
     end
 
     test "404" do
-      refute :persistent_term.get(:on_success)
-      refute :persistent_term.get(:on_error)
       conn = :get |> conn("/404") |> CollectRouter.call([])
-      assert :persistent_term.get(:on_success)
-      refute :persistent_term.get(:on_error)
       assert conn.state == :sent
       assert conn.status == 404
       assert conn.resp_body == "404"
+
+      assert ok_request()
     end
 
     test "raise" do
-      refute :persistent_term.get(:on_success)
-      refute :persistent_term.get(:on_error)
-
       assert_raise Plug.Conn.WrapperError, "** (RuntimeError) Error", fn ->
         :get |> conn("/raise") |> CollectRouter.call([])
       end
 
-      refute :persistent_term.get(:on_success)
-      assert :persistent_term.get(:on_error)
+      assert error_request()
     end
 
     test "throw" do
-      refute :persistent_term.get(:on_success)
-      refute :persistent_term.get(:on_error)
-
       assert :get |> conn("/throw") |> CollectRouter.call([]) |> catch_throw() == :throw_error
 
-      refute :persistent_term.get(:on_success)
-      assert :persistent_term.get(:on_error)
+      assert error_request()
     end
 
     test "exit" do
-      refute :persistent_term.get(:on_success)
-      refute :persistent_term.get(:on_error)
-
       assert :get |> conn("/exit") |> CollectRouter.call([]) |> catch_exit() == :exit_error
 
-      refute :persistent_term.get(:on_success)
-      assert :persistent_term.get(:on_error)
+      assert error_request()
     end
 
     test "bad_request" do
-      refute :persistent_term.get(:on_success)
-      refute :persistent_term.get(:on_error)
-
       assert_raise Plug.Conn.WrapperError,
                    "** (Plug.BadRequestError) could not process the request due to client error",
                    fn ->
                      :get |> conn("/bad_request") |> CollectRouter.call([])
                    end
 
-      refute :persistent_term.get(:on_success)
-      assert :persistent_term.get(:on_error)
+      assert error_request()
     end
 
     test "badarg" do
-      refute :persistent_term.get(:on_success)
-      refute :persistent_term.get(:on_error)
-
       assert_raise Plug.Conn.WrapperError, fn ->
         :get |> conn("/badarg") |> CollectRouter.call([])
       end
 
-      refute :persistent_term.get(:on_success)
-      assert :persistent_term.get(:on_error)
+      assert error_request()
     end
   end
+
+  defp ok_request() do
+    :persistent_term.get(:on_ok_anonymous) and :persistent_term.get(:on_ok_mf) and
+      not (:persistent_term.get(:on_error_anonymous) and :persistent_term.get(:on_error_mf))
+  end
+
+  defp error_request() do
+    not (:persistent_term.get(:on_ok_anonymous) and :persistent_term.get(:on_ok_mf)) and
+      (:persistent_term.get(:on_error_anonymous) and :persistent_term.get(:on_error_mf))
+  end
 end