From 1cb36196cb98538390ebbbafef80a68afbaa631a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C5=81ukasz=20Niemier?= <lukasz@niemier.pl>
Date: Tue, 9 Oct 2018 19:20:04 +0200
Subject: [PATCH] Replace Statix with Prometheus

Prometheus pull method of metrics gathering suits better such
application as it doesn't require gatherer to be configure always. It
also provides metrics in much more "standardized" way as
prometheus_phoenix and prometheus_plugs are quite norm when coming to
the respective library metrics gathering.

There are also predefined Grafana's dashboards that can be found there:

https://github.com/deadtrickster/beam-dashboards
---
 config/config.exs                            |  6 +--
 config/dev.exs                               |  2 +-
 lib/imager.ex                                | 17 ++++----
 lib/imager/application.ex                    |  4 +-
 lib/imager/instrumenter/cache.ex             | 15 +++++++
 lib/imager/instrumenter/processing.ex        | 29 ++++++++++++++
 lib/imager/instrumenter/storage.ex           | 37 +++++++++++++++++
 lib/imager/stats.ex                          | 42 --------------------
 lib/imager/store.ex                          | 14 +++----
 lib/imager_web/controllers/health.ex         |  4 +-
 lib/imager_web/endpoint.ex                   |  7 ++++
 lib/imager_web/instrumenter.ex               | 30 --------------
 lib/imager_web/plug/metrics_exporter.ex      |  3 ++
 lib/imager_web/plug/pipeline_instrumenter.ex |  3 ++
 mix.exs                                      |  9 +++--
 mix.lock                                     |  6 +++
 priv/config.toml.template                    |  5 ---
 test/imager/stats_test.exs                   | 27 -------------
 18 files changed, 125 insertions(+), 135 deletions(-)
 create mode 100644 lib/imager/instrumenter/cache.ex
 create mode 100644 lib/imager/instrumenter/processing.ex
 create mode 100644 lib/imager/instrumenter/storage.ex
 delete mode 100644 lib/imager/stats.ex
 delete mode 100644 lib/imager_web/instrumenter.ex
 create mode 100644 lib/imager_web/plug/metrics_exporter.ex
 create mode 100644 lib/imager_web/plug/pipeline_instrumenter.ex
 delete mode 100644 test/imager/stats_test.exs

diff --git a/config/config.exs b/config/config.exs
index 371cecf..0df4ab8 100644
--- a/config/config.exs
+++ b/config/config.exs
@@ -9,7 +9,7 @@ use Mix.Config
 config :imager, ImagerWeb.Endpoint,
   url: [host: "localhost"],
   render_errors: [view: ImagerWeb.Views.Error, accepts: ~w(json)],
-  instrumenters: [ImagerWeb.Instrumenter]
+  instrumenters: [Prometheus.PhoenixInstrumenter]
 
 # Configures Elixir's Logger
 config :logger, :console,
@@ -21,10 +21,6 @@ config :phoenix, :format_encoders, json: Jason
 config :ex_aws,
   json_codec: Jason
 
-config :vmstats,
-  sink: Imager.Stats,
-  base_key: "erlang"
-
 config :sentry,
   included_environments: [:prod],
   release: Mix.Project.config()[:version],
diff --git a/config/dev.exs b/config/dev.exs
index 9356bc4..93f93e1 100644
--- a/config/dev.exs
+++ b/config/dev.exs
@@ -20,7 +20,7 @@ config :logger, :console, format: "[$level] $message\n"
 config :imager, :stores, %{
   "local" => %{
     store: {Imager.Store.Local, dir: "test/fixtures/"},
-    cache: {Imager.Store.Local, dir: "tmp/cache/"}
+    cache: {Imager.Store.Blackhole, []}
   }
 }
 
diff --git a/lib/imager.ex b/lib/imager.ex
index 70c9389..ad73bd5 100644
--- a/lib/imager.ex
+++ b/lib/imager.ex
@@ -9,9 +9,9 @@ defmodule Imager do
 
   require Logger
 
-  alias Imager.Stats
   alias Imager.Store
   alias Imager.Tool
+  alias Imager.Instrumenter
 
   @type size :: non_neg_integer() | :unknown
   @type mime :: binary()
@@ -24,7 +24,7 @@ defmodule Imager do
   def process(store, file_name, commands, opts \\ [])
 
   def process(%{store: store}, file_name, [], opts) do
-    Stats.increment("imager.passthrough", 1, tags: Stats.tags())
+    Instrumenter.Cache.passthrough(store)
 
     Store.retrieve(store, file_name, opts)
   end
@@ -32,17 +32,14 @@ defmodule Imager do
   def process(%{store: store, cache: cache}, file_name, commands, opts) do
     {mime, result_name} = Tool.result(file_name, commands)
     args = Tool.build_command(commands)
-    tags = Stats.tags(~w(
-      store:#{elem(store, 0)}
-      cache:#{elem(cache, 0)}
-    ))
+
+    Instrumenter.Processing.command(commands)
 
     Logger.metadata(input: file_name, commands: inspect(commands))
 
     with :error <- Store.retrieve(cache, result_name, opts),
          {:ok, {_, _, stream}} <- Store.retrieve(store, file_name, opts) do
       Logger.debug("Start processing")
-      Stats.increment("imager.process.started", 1, tags: tags)
 
       process =
         Porcelain.spawn(executable(), args,
@@ -53,7 +50,7 @@ defmodule Imager do
 
       case Porcelain.Process.await(process) do
         {:ok, %{status: 0}} ->
-          Stats.increment("imager.process.succeeded", 1, tags: tags)
+          Instrumenter.Processing.succeeded(store)
 
           stream =
             process.out
@@ -62,7 +59,7 @@ defmodule Imager do
           {:ok, {:unknown, mime, stream}}
 
         _ ->
-          Stats.increment("imager.process.failed", 1, tags: tags)
+          Instrumenter.Processing.failed(store)
           Logger.error(process.err)
 
           :failed
@@ -70,7 +67,7 @@ defmodule Imager do
     else
       {:ok, {_, _, _}} = result ->
         Logger.debug("Cache hit")
-        Stats.increment("imager.process.cache_hit", 1, tags: tags)
+        Instrumenter.Cache.cache_hit(cache)
 
         result
 
diff --git a/lib/imager/application.ex b/lib/imager/application.ex
index e9d8af3..bfcc26a 100644
--- a/lib/imager/application.ex
+++ b/lib/imager/application.ex
@@ -3,6 +3,8 @@ defmodule Imager.Application do
 
   use Application
 
+  require Prometheus.Registry
+
   def start(_type, _args) do
     children = [
       ImagerWeb.Endpoint
@@ -14,8 +16,8 @@ defmodule Imager.Application do
       Application.get_env(:imager, :sentry_dsn)
     )
 
-    Imager.Stats.start()
     {:ok, _} = Logger.add_backend(Sentry.LoggerBackend)
+    Prometheus.Registry.register_collector(:prometheus_process_collector)
 
     opts = [strategy: :one_for_one, name: Imager.Supervisor]
     Supervisor.start_link(children, opts)
diff --git a/lib/imager/instrumenter/cache.ex b/lib/imager/instrumenter/cache.ex
new file mode 100644
index 0000000..8369c56
--- /dev/null
+++ b/lib/imager/instrumenter/cache.ex
@@ -0,0 +1,15 @@
+defmodule Imager.Instrumenter.Cache do
+  use Prometheus.Metric
+
+  @counter [
+    name: :cache_hits_total,
+    help: "Count of cached images hits",
+    labels: [:type, :store]
+  ]
+
+  def passthrough({store, _}),
+    do: Counter.inc(name: :cache_hits_total, labels: [:passthrough, store])
+
+  def cache_hit({store, _}),
+    do: Counter.inc(name: :cache_hits_total, labels: [:cache, store])
+end
diff --git a/lib/imager/instrumenter/processing.ex b/lib/imager/instrumenter/processing.ex
new file mode 100644
index 0000000..0cd0790
--- /dev/null
+++ b/lib/imager/instrumenter/processing.ex
@@ -0,0 +1,29 @@
+defmodule Imager.Instrumenter.Processing do
+  use Prometheus.Metric
+
+  @counter [
+    name: :processed_total,
+    help: "Count of processed images",
+    labels: [:status, :store]
+  ]
+  @counter [
+    name: :process_commands_total,
+    help: "Processing commands defined",
+    labels: [:command, :argument]
+  ]
+
+  def succeeded({store, _}),
+    do: Counter.inc(name: :processed_total, labels: ["ok", store])
+
+  def failed({store, _}),
+    do: Counter.inc(name: :processed_total, labels: ["failed", store])
+
+  def command({command, argument}) do
+    Counter.inc(
+      name: :process_commands_total,
+      labels: [command, argument]
+    )
+  end
+
+  def command(list) when is_list(list), do: Enum.each(list, &command/1)
+end
diff --git a/lib/imager/instrumenter/storage.ex b/lib/imager/instrumenter/storage.ex
new file mode 100644
index 0000000..4c5fc6c
--- /dev/null
+++ b/lib/imager/instrumenter/storage.ex
@@ -0,0 +1,37 @@
+defmodule Imager.Instrumenter.Storage do
+  use Prometheus.Metric
+
+  @counter [
+    name: :store_retrieved_total,
+    help: "Count of retrieves from the given store",
+    labels: [:store]
+  ]
+  @counter [
+    name: :store_saved_total,
+    help: "Count of saves into the given store",
+    labels: [:store]
+  ]
+
+  @summary [
+    name: :store_retrieved_bytes,
+    help: "Bytes retrieved from the store",
+    labels: [:store]
+  ]
+  @summary [
+    name: :store_saved_bytes,
+    help: "Saved bytes to given store",
+    labels: [:store]
+  ]
+
+  def retrieved(stream, store) do
+    Counter.inc(name: :store_retrieved_total, labels: [store])
+
+    Stream.each(stream, &Summary.observe([name: :store_retrieved_bytes, labels: [store]], byte_size(&1)))
+  end
+
+  def saved(stream, store) do
+    Counter.inc(name: :store_saved_total, labels: [store])
+
+    Stream.each(stream, &Summary.observe([name: :store_saved_bytes, labels: [store]], byte_size(&1)))
+  end
+end
diff --git a/lib/imager/stats.ex b/lib/imager/stats.ex
deleted file mode 100644
index 33f0924..0000000
--- a/lib/imager/stats.ex
+++ /dev/null
@@ -1,42 +0,0 @@
-defmodule Imager.Stats do
-  @moduledoc """
-  StatsD statistics gatherer.
-  """
-
-  @behaviour :vmstats_sink
-
-  use Statix, runtime_config: true
-
-  def start do
-    if config = Application.get_env(:imager, :stats) do
-      Application.put_env(:statix, __MODULE__,
-        host: Map.fetch!(config, :host),
-        port: Map.fetch!(config, :port)
-      )
-    end
-
-    connect()
-    {:ok, _} = Application.ensure_all_started(:vmstats, :permanent)
-
-    :ok
-  end
-
-  def collect(type, key, value) do
-    do_collect(
-      type,
-      List.to_string(key),
-      value,
-      tags()
-    )
-  end
-
-  def tags(tags \\ []) when is_list(tags) do
-    ["app:imager", "version:#{version()}" | tags]
-  end
-
-  defp do_collect(:counter, key, value, opts), do: increment(key, value, opts)
-  defp do_collect(:gauge, key, value, opts), do: gauge(key, value, opts)
-  defp do_collect(:timing, key, value, opts), do: histogram(key, value, opts)
-
-  def version, do: :imager |> Application.spec(:vsn) |> List.to_string()
-end
diff --git a/lib/imager/store.ex b/lib/imager/store.ex
index 0b7e677..b4c0f58 100644
--- a/lib/imager/store.ex
+++ b/lib/imager/store.ex
@@ -4,7 +4,7 @@ defmodule Imager.Store do
   in application.
   """
 
-  alias Imager.Stats
+  alias Imager.Instrumenter
 
   @type size :: non_neg_integer()
   @type stream :: Enumerable.t()
@@ -31,13 +31,9 @@ defmodule Imager.Store do
   @spec retrieve(store, binary, keyword) ::
           {:ok, {size, mime, stream}} | :error
   def retrieve({store, glob_opts}, path, options) do
-    Stats.increment(
-      "imager.store.retrieve",
-      1,
-      Stats.tags(~w(module:#{store}))
-    )
-
-    store.retrieve(path, Keyword.merge(glob_opts, options))
+    with {:ok, {size, mime, stream}} <-
+           store.retrieve(path, Keyword.merge(glob_opts, options)),
+         do: {:ok, {size, mime, Instrumenter.Storage.retrieved(stream, store)}}
   end
 
   @doc """
@@ -45,7 +41,7 @@ defmodule Imager.Store do
   """
   @spec store(stream, store, mime, binary, keyword) :: stream
   def store(stream, {store, glob_opts}, mime, path, options) do
-    Stats.increment("imager.store.store", 1, Stats.tags(~w(module:#{store})))
     store.store(path, mime, stream, Keyword.merge(glob_opts, options))
+    |> Instrumenter.Storage.saved(store)
   end
 end
diff --git a/lib/imager_web/controllers/health.ex b/lib/imager_web/controllers/health.ex
index 705e8ff..d34e892 100644
--- a/lib/imager_web/controllers/health.ex
+++ b/lib/imager_web/controllers/health.ex
@@ -6,8 +6,10 @@ defmodule ImagerWeb.Controllers.Health do
   def get(conn, _params) do
     json(conn, %{
       status: "pass",
-      version: Imager.Stats.version(),
+      version: version(),
       description: @description
     })
   end
+
+  defp version, do: Application.spec(:imager, :vsn) |> List.to_string()
 end
diff --git a/lib/imager_web/endpoint.ex b/lib/imager_web/endpoint.ex
index 64b7ad6..a68c63f 100644
--- a/lib/imager_web/endpoint.ex
+++ b/lib/imager_web/endpoint.ex
@@ -1,7 +1,11 @@
 defmodule ImagerWeb.Endpoint do
   use Phoenix.Endpoint, otp_app: :imager
+  # use Prometheus.PlugPipelineInstrumenter
   use Sentry.Phoenix.Endpoint
 
+  plug(ImagerWeb.Plug.PipelineInstrumenter)
+  plug(ImagerWeb.Plug.MetricsExporter)
+
   # Code reloading can be explicitly enabled under the
   # :code_reloader configuration of your endpoint.
   if code_reloading? do
@@ -27,6 +31,9 @@ defmodule ImagerWeb.Endpoint do
   configuration should be loaded from the system environment.
   """
   def init(_key, config) do
+    ImagerWeb.Plug.PipelineInstrumenter.setup()
+    ImagerWeb.Plug.MetricsExporter.setup()
+
     port =
       System.get_env("PORT") || Application.get_env(:imager, :port) ||
         raise "expected the PORT environment variable to be set"
diff --git a/lib/imager_web/instrumenter.ex b/lib/imager_web/instrumenter.ex
deleted file mode 100644
index e8b412b..0000000
--- a/lib/imager_web/instrumenter.ex
+++ /dev/null
@@ -1,30 +0,0 @@
-defmodule ImagerWeb.Instrumenter do
-  @moduledoc """
-  Instrument application.
-
-  This module gathers statistics about running application and sends them
-  to StatsD compatible server.
-  """
-
-  def phoenix_controller_call(:start, _, %{conn: conn}) do
-    Imager.Stats.tags(~w[
-      host:#{conn.host}
-      method:#{conn.method}
-      scheme:#{conn.scheme}
-      controller:#{module_name(conn.private[:phoenix_controller])}
-      action:#{conn.private[:phoenix_action]}
-      format:#{conn.private[:phoenix_format] || "unknown"}
-    ])
-  end
-
-  def phoenix_controller_call(:stop, time, tags) do
-    Imager.Stats.histogram("phoenix.request.duration", time, tags: tags)
-  end
-
-  defp module_name(nil), do: nil
-  defp module_name("Elixir." <> name), do: name
-  defp module_name(name) when is_bitstring(name), do: name
-
-  defp module_name(name) when is_atom(name),
-    do: module_name(Atom.to_string(name))
-end
diff --git a/lib/imager_web/plug/metrics_exporter.ex b/lib/imager_web/plug/metrics_exporter.ex
new file mode 100644
index 0000000..032c8eb
--- /dev/null
+++ b/lib/imager_web/plug/metrics_exporter.ex
@@ -0,0 +1,3 @@
+defmodule ImagerWeb.Plug.MetricsExporter do
+  use Prometheus.PlugExporter
+end
diff --git a/lib/imager_web/plug/pipeline_instrumenter.ex b/lib/imager_web/plug/pipeline_instrumenter.ex
new file mode 100644
index 0000000..1348dd1
--- /dev/null
+++ b/lib/imager_web/plug/pipeline_instrumenter.ex
@@ -0,0 +1,3 @@
+defmodule ImagerWeb.Plug.PipelineInstrumenter do
+  use Prometheus.PlugPipelineInstrumenter
+end
diff --git a/mix.exs b/mix.exs
index f7d4f10..d3a4514 100644
--- a/mix.exs
+++ b/mix.exs
@@ -17,7 +17,6 @@ defmodule Imager.Mixfile do
       compilers: [:phoenix] ++ Mix.compilers(),
       start_permanent: Mix.env() == :prod,
       deps: deps(),
-      dialyzer: [plt_add_apps: [:vmstats]],
       test_coverage: [tool: ExCoveralls],
       preferred_cli_env: [
         coveralls: :test,
@@ -36,7 +35,7 @@ defmodule Imager.Mixfile do
     [
       mod: {Imager.Application, []},
       extra_applications: [:logger, :inets, :runtime_tools],
-      included_applications: [:vmstats]
+      included_applications: []
     ]
   end
 
@@ -53,13 +52,15 @@ defmodule Imager.Mixfile do
       {:phoenix_pubsub, "~> 1.0"},
       {:cowboy, "~> 1.0"},
       {:distillery, "~> 2.0"},
-      {:statix, ">= 0.0.0"},
+      {:prometheus_ex, "~> 3.0"},
+      {:prometheus_phoenix, "~> 1.2.0"},
+      {:prometheus_plugs, "~> 1.1.1"},
+      {:prometheus_process_collector, github: "deadtrickster/prometheus_process_collector", override: true, manager: :rebar3},
       {:ex_aws, "~> 2.0"},
       {:ex_aws_s3, "~> 2.0"},
       {:hackney, "~> 1.9"},
       {:sweet_xml, "~> 0.6"},
       {:jason, ">= 0.0.0"},
-      {:vmstats, "~> 2.2", runtime: false},
       {:sentry, "~> 7.0"},
       {:porcelain, "~> 2.0.3"},
       {:jose, "~> 1.8"},
diff --git a/mix.lock b/mix.lock
index e4020d5..969e243 100644
--- a/mix.lock
+++ b/mix.lock
@@ -1,4 +1,5 @@
 %{
+  "accept": {:hex, :accept, "0.3.3", "548ebb6fb2e8b0d170e75bb6123aea6ceecb0189bb1231eeadf52eac08384a97", [:rebar3], [], "hexpm"},
   "artificery": {:hex, :artificery, "0.2.6", "f602909757263f7897130cbd006b0e40514a541b148d366ad65b89236b93497a", [:mix], [], "hexpm"},
   "base64url": {:hex, :base64url, "0.0.1", "36a90125f5948e3afd7be97662a1504b934dd5dac78451ca6e9abf85a10286be", [:rebar], [], "hexpm"},
   "bunt": {:hex, :bunt, "0.2.0", "951c6e801e8b1d2cbe58ebbd3e616a869061ddadcc4863d0a2182541acae9a38", [:mix], [], "hexpm"},
@@ -30,6 +31,11 @@
   "poison": {:hex, :poison, "3.1.0", "d9eb636610e096f86f25d9a46f35a9facac35609a7591b3be3326e99a0484665", [:mix], [], "hexpm"},
   "poolboy": {:hex, :poolboy, "1.5.1", "6b46163901cfd0a1b43d692657ed9d7e599853b3b21b95ae5ae0a777cf9b6ca8", [:rebar], [], "hexpm"},
   "porcelain": {:hex, :porcelain, "2.0.3", "2d77b17d1f21fed875b8c5ecba72a01533db2013bd2e5e62c6d286c029150fdc", [:mix], [], "hexpm"},
+  "prometheus": {:hex, :prometheus, "4.1.0", "3bb851df031c204d1c94bf55fff2ecc9ab834f0236e64c080c9d5945b48d428d", [:mix, :rebar3], [], "hexpm"},
+  "prometheus_ex": {:hex, :prometheus_ex, "3.0.3", "5d722263bb1f7a9b1d02554de42e61ea672b4e3c07c3f74e23ce35ab5e111cfa", [:mix], [{:prometheus, "~> 4.0", [hex: :prometheus, repo: "hexpm", optional: false]}], "hexpm"},
+  "prometheus_phoenix": {:hex, :prometheus_phoenix, "1.2.1", "964a74dfbc055f781d3a75631e06ce3816a2913976d1df7830283aa3118a797a", [:mix], [{:phoenix, "~> 1.3", [hex: :phoenix, repo: "hexpm", optional: false]}, {:prometheus_ex, "~> 1.3 or ~> 2.0 or ~> 3.0", [hex: :prometheus_ex, repo: "hexpm", optional: false]}], "hexpm"},
+  "prometheus_plugs": {:hex, :prometheus_plugs, "1.1.5", "25933d48f8af3a5941dd7b621c889749894d8a1082a6ff7c67cc99dec26377c5", [:mix], [{:accept, "~> 0.1", [hex: :accept, repo: "hexpm", optional: false]}, {:plug, "~> 1.0", [hex: :plug, repo: "hexpm", optional: false]}, {:prometheus_ex, "~> 1.1 or ~> 2.0 or ~> 3.0", [hex: :prometheus_ex, repo: "hexpm", optional: false]}, {:prometheus_process_collector, "~> 1.1", [hex: :prometheus_process_collector, repo: "hexpm", optional: true]}], "hexpm"},
+  "prometheus_process_collector": {:git, "https://github.com/deadtrickster/prometheus_process_collector.git", "fa11b50f2f81d4e3bed70298fcde54e30670e195", []},
   "ranch": {:hex, :ranch, "1.3.2", "e4965a144dc9fbe70e5c077c65e73c57165416a901bd02ea899cfd95aa890986", [:rebar3], [], "hexpm"},
   "sentry": {:hex, :sentry, "7.0.0", "12421006c29d4fd6dbd822316868e542f49c934ba9bf18862304147779d4d0fc", [:mix], [{:hackney, "~> 1.8 or 1.6.5", [hex: :hackney, repo: "hexpm", optional: false]}, {:jason, "~> 1.1", [hex: :jason, repo: "hexpm", optional: true]}, {:phoenix, "~> 1.3", [hex: :phoenix, repo: "hexpm", optional: true]}, {:plug, "~> 1.6", [hex: :plug, repo: "hexpm", optional: true]}], "hexpm"},
   "ssl_verify_fun": {:hex, :ssl_verify_fun, "1.1.4", "f0eafff810d2041e93f915ef59899c923f4568f4585904d010387ed74988e77b", [:make, :mix, :rebar3], [], "hexpm"},
diff --git a/priv/config.toml.template b/priv/config.toml.template
index 3dfffa7..bfbcc93 100644
--- a/priv/config.toml.template
+++ b/priv/config.toml.template
@@ -8,11 +8,6 @@
 #
 # Sentry DSN key used to report errors
 # sentry_dsn = ""
-#
-# StatsD configuration
-# [stats]
-#       host = "localhost" # host send stats to
-#       port = 8125 # UDP port to send stats to
 
 # # Store definition
 #
diff --git a/test/imager/stats_test.exs b/test/imager/stats_test.exs
deleted file mode 100644
index 14c65c6..0000000
--- a/test/imager/stats_test.exs
+++ /dev/null
@@ -1,27 +0,0 @@
-defmodule Imager.StatsTest do
-  use ExUnit.Case, async: true
-  use ExUnitProperties
-
-  property "tags contain all provided entries" do
-    check all tags <- list_of(string(:alphanumeric)) do
-      generated = Imager.Stats.tags(tags)
-
-      for tag <- tags do
-        assert tag in generated
-      end
-    end
-  end
-
-  property "tags contains default tags" do
-    check all tags <- list_of(string(:alphanumeric)) do
-      generated = Imager.Stats.tags(tags)
-
-      assert "app:imager" in generated
-
-      assert Enum.find(generated, fn
-               "version:" <> _ -> true
-               _ -> false
-             end)
-    end
-  end
-end