Integrate with :telemetry

[rkallos]

:telemetry seems to be picking up steam as the way forward for libraries to emit telemetry for applications. This largely replaces your use of ?METRICS calls with matching calls to the new shackle_telemetry module.

Also, a bunch of links in the docs got updated when I regenerated them. It might be worth looking into generating them with rebar3_ex_doc so they turn into pretty docs on hexdocs.pm.

[lpgauth]

Did I quick review and it looks good. Once thing I noticed is that the dep wasn't added to the 'rebar.config.script' file which TBH I don't remember what it was used for...

What else could I do to make shackle more elixir friendly? mix config file and a shackle.ex file to expose the main APIs?

[lpgauth]

Oh, I think the rebar.config.script was for rebar2 compatibility. I think I can clean this up now.

[rkallos]

What else could I do to make shackle more elixir friendly?

It's an interesting question! I haven't given it much thought because I haven't really been using shackle directly; I've been using buoy. I suspect I'd have a much clearer picture if I write a shackle client in Elixir.

I think one big improvement might be having docs generated and uploaded to hexdocs, using an approach similar to :telemetry. The repo has a basic mix.exs file, and I suspect that, when publishing to hex, mix docs is the task that builds the documentation that winds up on Hexdocs.

EDIT: I found these instructions for releasing :telemetry; it uses rebar3_ex_doc, which is distributed as a rebar3 plugin and as a standalone escript

[rkallos]

I got rebar3_ex_doc working. However, there's a bit of ugliness. There's a section for each module where defined types are listed. Since a bunch of modules have -include("shackle.hrl")., there are a bunch of duplicate definitions for those types. It also appears as though jumping to source for those type definitions doesn't work either.

I think it would be pretty straightforward to move type definitions into the modules where they're used the most.

[rkallos]

I cleaned up a lot of the duplicated types in this branch (based on top of this PR branch). The resulting generated documentation is much cleaner, and linking back to definitions and whatnot is much nicer.

[lpgauth]

LGTM.

[x0id]

With all respect to telemetry devs, I think this PR is a step back.

1. The original hooks are based on the foil library which is faster than ets lookups (used internally by telemetry to get a list of handlers).
2. The telemetry calls, if needed, might be implemented via original hooks with no need to introduce an extra dependency on the library which not everybody is interested in.

[lpgauth]

@x0id Hey, I'll talk with @rkallos and discuss what we should do. I agree that this implementation is a bit slower because of the ETS calls.

Moving forward I see two options:

1. Switch back to hooks, but also ship a telemetry module with Shackle so it can be easily enabled.
2. Fork/open a PR to have telemetry use foil.

[x0id]

Both are great options, but still, I prefer the first variant, because the second introduces an extra dependency on the telemetry.
(Not sure if this is bad though, telemetry has a good chance to become mainstream and even be integrated into OTP at some point.)

[rkallos]

In #133, I removed telemetry as a dependency, and instead reintroduced shackle_hooks, but with a calling convention that is identical to telemetry's: (EventName, Measurements, Metadata).

This removes the overhead of the telemetry ETS table lookups, but for now, it does construct many small maps for the Measurements and Metadata terms. I imagine it would be possible to avoid creating those maps with a macro that performs the foil lookup before the call to shackle_hooks:event/3, but it should probably be benchmarked.

@x0id please let me know what you think of my proposed changes 😃

[x0id]

@rkallos, ideally I'd implement everything via macros.
1st conditional layer is compile-time (macros) will enable/disable hooks and maybe even select one of the predefined sets of hooks. For example, we could have abstract foil-based hooks (similar to the original), or we can have telemetry calls (with ets-based selectors), or custom, provided by a client.
Lazy ops as much as we can - never calculate size/length in advance, don't create any map/record/etc wrappers in advance, only when static/dynamic selector resolved to a real event handler.
Ideally, telemetry might be enhanced with the foil-based selector, but this is another story.