deadtrickster / prometheus.ex

Prometheus.io Elixir client
411 stars 68 forks source link

Lack of documentation in `Prometheus.Inject` #25

Open hauleth opened 6 years ago

hauleth commented 6 years ago

I would like to provide one, however it seems quite hard to deduce what is happening there.

deadtrickster commented 6 years ago

This one used to implement decorators, (see tests here https://github.com/deadtrickster/prometheus.ex/blob/master/test/injector_test.exs). For this it needs to walk over respective AST. Most of this file are workarounds for various syntaxes.

hauleth commented 6 years ago

I think it would be better to stick to single form of decorating blocks instead. I would propose to support only function callbacks (0-arity), as these do not need to provide macros and are flexible enough to handle all possible cases. For example current implementation do not support functions with headers. You can test it by yourself with this diff:

diff --git a/test/injector_test.exs b/test/injector_test.exs
index ade3193..d9cdad7 100644
--- a/test/injector_test.exs
+++ b/test/injector_test.exs
@@ -31,6 +31,11 @@ defmodule Prometheus.InjectorTest do
       IO.puts("fun2")
     end

+    def fun_with_header(arg \\ 0)
+
+    def fun_with_header(arg) when is_binary(arg), do: IO.puts("fun_with_header binary")
+    def fun_with_header(_arg), do: IO.puts("fun_with_header other")
+
     Injector.test do
       def fun3() do
         IO.puts("fun3")

Or with private functions:

diff --git a/test/injector_test.exs b/test/injector_test.exs
index ade3193..b18d8a1 100644
--- a/test/injector_test.exs
+++ b/test/injector_test.exs
@@ -39,6 +39,10 @@ defmodule Prometheus.InjectorTest do
           IO.puts(e)
       end
     end
+
+    defp fun_p() do
+      IO.puts("fun_p")
+    end
   end

   def do_dangerous_work(x) do

And second one gives you quite unintuitive message:

== Compilation error in file test/injector_test.exs ==
** (RuntimeError) Mixing defs and other blocks isn't allowed
    (prometheus_ex) lib/prometheus/injector.ex:88: Prometheus.Injector.have_defs/1
    (prometheus_ex) lib/prometheus/injector.ex:41: Prometheus.Injector.inject_/2
    expanding macro: Injector.test/1
    test/injector_test.exs:25: Prometheus.InjectorTest (module)
    (elixir) lib/code.ex:767: Code.require_file/2
    (elixir) lib/kernel/parallel_compiler.ex:209: anonymous fn/4 in Kernel.ParallelCompiler.spawn_workers/6

Because you have used def, just private one, why this one would not work?

So the only form available should be:

Prometheus.Injector.inject(wrapper, fn -> … end)

While I would go with slightly different API, which is:

Prometheus.Injector.inject(callback, [before: &before_cb/0, after: &after_cb/1])

Where after_cb/1 would receive output of the before_cb/0.

Or alternatively remove everything, and implement simple macros/functions to measure behaviours in respective metrics, as this would be IMHO the easiest solution.