Fix metrics (#28)

* Add a module for exporting cache metrics (#27)

* add a module for exporting cache metrics

* add metrics to measure cache call, delete, and put duration

* remove the counter metric for cache hit

* remove unnecessary telemetry execute action

* remove unused metadata field

---------

Co-authored-by: alvaro <[email protected]>

* chore: add more error metrics and fix metric defs

* chore: add credo config

---------

Co-authored-by: Alvaro Callero <[email protected]>
Co-authored-by: alvaro <[email protected]>

Author: 3 people

.credo.exs

@@ -0,0 +1,143 @@
+allowed_imports = [
+  [:Absinthe],
+  [:ChannelCase],
+  [:DataCase],
+  [:EctoEnum],
+  [:Ecto],
+  [:ExUnit, :CaptureLog],
+  [:ExUnit],
+  [:Mix],
+  [:Plug],
+  [:Router, :Helpers],
+  [:Telemetry, :Metrics]
+]
+
+%{
+  configs: [
+    %{
+      name: "default",
+      files: %{
+        included: [
+          "lib/",
+          "src/",
+          "test/",
+          "web/",
+          "apps/*/lib/",
+          "apps/*/src/",
+          "apps/*/test/",
+          "apps/*/web/"
+        ],
+        excluded: [~r"_build/", ~r"deps/"]
+      },
+      plugins: [],
+      strict: true,
+      parse_timeout: 10000,
+      color: true,
+      checks: [
+
+        # BlitzCredoChecks
+
+        {BlitzCredoChecks.SetWarningsAsErrorsInTest, false},
+        {BlitzCredoChecks.DocsBeforeSpecs, []},
+        {BlitzCredoChecks.DoctestIndent, []},
+        {BlitzCredoChecks.NoAsyncFalse, []},
+        {BlitzCredoChecks.NoDSLParentheses, []},
+        {BlitzCredoChecks.NoIsBitstring, []},
+        {BlitzCredoChecks.StrictComparison, []},
+        {BlitzCredoChecks.UseStream, []},
+        {BlitzCredoChecks.LowercaseTestNames, []},
+        {BlitzCredoChecks.ImproperImport, allowed_modules: allowed_imports},
+
+        # Consistency Checks
+        {Credo.Check.Consistency.ExceptionNames, []},
+        {Credo.Check.Consistency.LineEndings, []},
+        {Credo.Check.Consistency.ParameterPatternMatching, []},
+        {Credo.Check.Consistency.SpaceAroundOperators, []},
+        {Credo.Check.Consistency.SpaceInParentheses, []},
+        {Credo.Check.Consistency.TabsOrSpaces, []},
+
+        # Design Checks
+        {Credo.Check.Design.AliasUsage, false},
+
+        # No outstanding TODOs
+        {Credo.Check.Design.TagTODO, []},
+        {Credo.Check.Design.TagFIXME, []},
+
+        # # Readability Checks
+        {Credo.Check.Readability.AliasOrder, false},
+        {Credo.Check.Readability.FunctionNames, []},
+        {Credo.Check.Readability.LargeNumbers, []},
+        {Credo.Check.Readability.MaxLineLength, [max_length: 120]},
+        {Credo.Check.Readability.ModuleAttributeNames, []},
+        {Credo.Check.Readability.ModuleDoc, false},
+        {Credo.Check.Readability.ModuleNames, []},
+        {Credo.Check.Readability.ParenthesesInCondition, []},
+        {Credo.Check.Readability.ParenthesesOnZeroArityDefs, []},
+        {Credo.Check.Readability.PredicateFunctionNames, []},
+        {Credo.Check.Readability.PreferImplicitTry, []},
+        {Credo.Check.Readability.RedundantBlankLines, false},
+        {Credo.Check.Readability.Semicolons, []},
+        {Credo.Check.Readability.SpaceAfterCommas, false},
+        {Credo.Check.Readability.StringSigils, []},
+        {Credo.Check.Readability.TrailingBlankLine, false},
+        {Credo.Check.Readability.TrailingWhiteSpace, false},
+        {Credo.Check.Readability.UnnecessaryAliasExpansion, []},
+        {Credo.Check.Readability.VariableNames, []},
+        #
+        # Refactoring Opportunities
+        {Credo.Check.Refactor.CondStatements, []},
+        {Credo.Check.Refactor.CyclomaticComplexity, false},
+        {Credo.Check.Refactor.FunctionArity, []},
+        {Credo.Check.Refactor.LongQuoteBlocks, false},
+        {Credo.Check.Refactor.MapInto, false},
+        {Credo.Check.Refactor.MatchInCondition, []},
+        {Credo.Check.Refactor.NegatedConditionsInUnless, []},
+        {Credo.Check.Refactor.NegatedConditionsWithElse, []},
+        {Credo.Check.Refactor.Nesting, false},
+        {Credo.Check.Refactor.UnlessWithElse, []},
+        {Credo.Check.Refactor.WithClauses, []},
+
+        # Warnings
+        {Credo.Check.Warning.BoolOperationOnSameValues, []},
+        {Credo.Check.Warning.ExpensiveEmptyEnumCheck, []},
+        {Credo.Check.Warning.IExPry, []},
+        {Credo.Check.Warning.IoInspect, []},
+        {Credo.Check.Warning.LazyLogging, false},
+        {Credo.Check.Warning.MixEnv, false},
+        {Credo.Check.Warning.OperationOnSameValues, []},
+        {Credo.Check.Warning.OperationWithConstantResult, []},
+        {Credo.Check.Warning.RaiseInsideRescue, []},
+        {Credo.Check.Warning.UnusedEnumOperation, []},
+        {Credo.Check.Warning.UnusedFileOperation, []},
+        {Credo.Check.Warning.UnusedKeywordOperation, []},
+        {Credo.Check.Warning.UnusedListOperation, []},
+        {Credo.Check.Warning.UnusedPathOperation, []},
+        {Credo.Check.Warning.UnusedRegexOperation, []},
+        {Credo.Check.Warning.UnusedStringOperation, []},
+        {Credo.Check.Warning.UnusedTupleOperation, []},
+        {Credo.Check.Warning.UnsafeExec, []},
+
+        # Controversial and experimental checks
+        {Credo.Check.Readability.StrictModuleLayout, false},
+        {Credo.Check.Consistency.MultiAliasImportRequireUse, false},
+        {Credo.Check.Consistency.UnusedVariableNames, false},
+        {Credo.Check.Design.DuplicatedCode, false},
+        {Credo.Check.Readability.AliasAs, false},
+        {Credo.Check.Readability.MultiAlias, false},
+        {Credo.Check.Readability.Specs, false},
+        {Credo.Check.Readability.SinglePipe, []},
+        {Credo.Check.Readability.WithCustomTaggedTuple, []},
+        {Credo.Check.Refactor.ABCSize, false},
+        {Credo.Check.Refactor.AppendSingleItem, false},
+        {Credo.Check.Refactor.DoubleBooleanNegation, false},
+        {Credo.Check.Refactor.ModuleDependencies, false},
+        {Credo.Check.Refactor.NegatedIsNil, false},
+        {Credo.Check.Refactor.PipeChainStart, []},
+        {Credo.Check.Refactor.VariableRebinding, false},
+        {Credo.Check.Warning.LeakyEnvironment, false},
+        {Credo.Check.Warning.MapGetUnsafePass, false},
+        {Credo.Check.Warning.UnsafeToAtom, false}
+      ]
+    }
+  ]
+}

lib/cache.ex

@@ -10,7 +10,9 @@ defmodule Cache do
 
   @callback opts_definition() :: Keyword.t()
 
-  @callback start_link(cache_opts :: Keyword.t()) :: {:ok, pid()} | {:error, {:already_started, pid()} | {:shutdown, term()} | term()} | :ignore
+  @callback start_link(
+    cache_opts :: Keyword.t()
+  ) :: {:ok, pid()} | {:error, {:already_started, pid()} | {:shutdown, term()} | term()} | :ignore
 
   @callback put(cache_name :: atom, key :: atom | String.t(), ttl :: pos_integer, value :: any) ::
               :ok | ErrorMessage.t()
@@ -112,22 +114,75 @@ defmodule Cache do
         value = Cache.TermEncoder.encode(value, @compression_level)
         key = maybe_sandbox_key(key)
 
-        @cache_adapter.put(@cache_name, key, ttl, value, adapter_options())
+        :telemetry.span(
+          [:elixir_cache, :cache, :put],
+          %{cache_name: @cache_name},
+          fn ->
+            result = with {:error, error} = e <- @cache_adapter.put(@cache_name, key, ttl, value, adapter_options()) do
+              :telemetry.execute([:elixir_cache, :cache, :put, :error], %{count: 1}, %{
+                cache_name: @cache_name,
+                error: error
+              })
+
+              e
+            end
+
+            {result, %{cache_name: @cache_name}}
+          end
+        )
       end
 
       def get(key) do
         key = maybe_sandbox_key(key)
 
-        with {:ok, value} when not is_nil(value) <-
-               @cache_adapter.get(@cache_name, key, adapter_options()) do
-          {:ok, Cache.TermEncoder.decode(value)}
-        end
+        :telemetry.span(
+          [:elixir_cache, :cache, :get],
+          %{cache_name: @cache_name},
+          fn ->
+            result =
+              case @cache_adapter.get(@cache_name, key, adapter_options()) do
+                {:ok, nil} = res ->
+                  :telemetry.execute([:elixir_cache, :cache, :get, :miss], %{count: 1}, %{
+                    cache_name: @cache_name
+                  })
+
+                  res
+
+                {:ok, value} -> {:ok, Cache.TermEncoder.decode(value)}
+
+                {:error, error} = e ->
+                  :telemetry.execute([:elixir_cache, :cache, :get, :error], %{count: 1}, %{
+                    cache_name: @cache_name,
+                    error: error
+                  })
+
+                  e
+              end
+
+            {result, %{cache_name: @cache_name}}
+          end
+        )
       end
 
       def delete(key) do
         key = maybe_sandbox_key(key)
 
-        @cache_adapter.delete(@cache_name, key, adapter_options())
+        :telemetry.span(
+          [:elixir_cache, :cache, :delete],
+          %{cache_name: @cache_name},
+          fn ->
+            result = with {:error, error} = e <- @cache_adapter.delete(@cache_name, key, adapter_options()) do
+              :telemetry.execute([:elixir_cache, :cache, :delete, :error], %{count: 1}, %{
+                cache_name: @cache_name,
+                error: error
+              })
+
+              e
+            end
+
+            {result, %{cache_name: @cache_name}}
+          end
+        )
       end
 
       def get_or_create(key, fnc) do

lib/cache/metrics.ex

@@ -0,0 +1,114 @@
+if Code.ensure_loaded?(PrometheusTelemetry) do
+  defmodule Cache.Metrics do
+    @moduledoc """
+    Add the following metrics for elixir_cache:
+
+    Metrics included:
+      - `elixir_cache.cache.get.count`
+      - `elixir_cache.cache.get.miss.count`
+      - `elixir_cache.cache.get.error.count`
+      - `elixir_cache.cache.put.count`
+      - `elixir_cache.cache.delete.count`
+      - `elixir_cache.cache.get.duration.millisecond`
+    """
+
+    import Telemetry.Metrics, only: [counter: 2, distribution: 2]
+
+    @duration_unit {:native, :millisecond}
+    @buckets PrometheusTelemetry.Config.default_millisecond_buckets()
+
+    def metrics do
+      [
+        counter("elixir_cache.cache.get.count",
+          event_name: [:elixir_cache, :cache, :get, :start],
+          measurement: :count,
+          description: "Total cache calls",
+          tags: [:cache_name]
+        ),
+
+        counter("elixir_cache.cache.get.miss.count",
+          event_name: [:elixir_cache, :cache, :get, :miss],
+          measurement: :count,
+          description: "Cache miss count",
+          tags: [:cache_name]
+        ),
+
+        counter("elixir_cache.cache.get.error.count",
+          event_name: [:elixir_cache, :cache, :get, :error],
+          measurement: :count,
+          description: "Cache error count",
+          tags: [:cache_name, :error],
+          tag_values: &extract_error_metadata/1
+        ),
+
+        counter("elixir_cache.cache.put.count",
+          event_name: [:elixir_cache, :cache, :put],
+          measurement: :count,
+          description: "Cache put count",
+          tags: [:cache_name]
+        ),
+
+        counter("elixir_cache.cache.put.error.count",
+          event_name: [:elixir_cache, :cache, :put, :error],
+          measurement: :count,
+          description: "Cache error count",
+          tags: [:cache_name, :error],
+          tag_values: &extract_error_metadata/1
+        ),
+
+        counter("elixir_cache.cache.delete.count",
+          event_name: [:elixir_cache, :cache, :delete],
+          measurement: :count,
+          description: "Cache delete count",
+          tags: [:cache_name]
+        ),
+
+        counter("elixir_cache.cache.delete.error.count",
+          event_name: [:elixir_cache, :cache, :delete, :error],
+          measurement: :count,
+          description: "Cache error count",
+          tags: [:cache_name, :error],
+          tag_values: &extract_error_metadata/1
+        ),
+
+        distribution("elixir_cache.cache.get.duration.millisecond",
+          event_name: [:elixir_cache, :cache, :get, :stop],
+          measurement: :duration,
+          description: "Time taken for cache get",
+          tags: [:cache_name],
+          unit: @duration_unit,
+          reporter_options: [buckets: @buckets]
+        ),
+
+        distribution("elixir_cache.cache.put.duration.millisecond",
+          event_name: [:elixir_cache, :cache, :put, :stop],
+          measurement: :duration,
+          description: "Time taken for cache put",
+          tags: [:cache_name],
+          unit: @duration_unit,
+          reporter_options: [buckets: @buckets]
+        ),
+
+        distribution("elixir_cache.cache.delete.duration.millisecond",
+          event_name: [:elixir_cache, :cache, :delete, :stop],
+          measurement: :duration,
+          description: "Time taken for cache delete",
+          tags: [:cache_name],
+          unit: @duration_unit,
+          reporter_options: [buckets: @buckets]
+        )
+      ]
+    end
+
+    defp extract_error_metadata(metadata) do
+      metadata
+      |> Map.take([:error, :cache_name])
+      |> Map.update(:error, "unknown", fn
+        %ErrorMessage{code: code, error: error} -> "code}: #{error}"
+        reason when is_atom(reason) -> to_string(reason)
+        reason when is_binary(reason) -> reason
+        _ -> "unknown"
+      end)
+    end
+  end
+end