beam-telemetry / telemetry

Dynamic dispatching library for metrics and instrumentations.
https://hexdocs.pm/telemetry
Apache License 2.0
872 stars 66 forks source link

small qol update to :telemetry.attach/4 docs #126

Open ethangunderson opened 1 year ago

ethangunderson commented 1 year ago

This has come up a few times for us. It's not always obvious that the function supplied to attach must be /4. When supplying something else you get an exception message that also doesn't reveal the problem.

iex(2)> :telemetry.attach("test", [:test, :event], fn -> IO.inspect("Hello") end, nil)
** (FunctionClauseError) no function clause matching in :telemetry.attach_many/4

    The following arguments were given to :telemetry.attach_many/4:
        # 1
        "test"

        # 2
        [[:test, :event]]

        # 3
        #Function<43.3316493/0 in :erl_eval.expr/6>

        # 4
        nil

    (telemetry 1.2.1) /Users/egunderson/Library/Caches/mix/installs/elixir-1.14.2-erts-13.1.2/e123f7ef1d039f8d998d1a6a03fb9b81/deps/telemetry/src/telemetry.erl:106: :telemetry.attach_many/4
    iex:2: (file)

I'm hoping that adding a slightly more explicit explanation of the function argument in function doc will help some people avoid this confusion in the future.

whatyouhide commented 1 year ago

I think it might be worth improving the error message too. In Elixir, we've often done this by removing the arity from the is_function check. In this case, that is, use is_function(Fun) instead of is_function(Fun, 4). With this, when you try to call the function with a wrong arity, the error message is a lot easier to debug 😉

ethangunderson commented 1 year ago

@whatyouhide I like that suggestion. Do you want me to make that change in this PR?