I don't want to get too DDD but, as I raised on Twitter:
Not sure about the names “labels” and “instrumentation”. Turned out I strongly expected “metadata” and “events”, both to match the language in :telemetry. I’d seek that change before taking the dependency.
Then, later:
I was wrong about the MyApp.Events convention. MyApp.Telemetry clearly says what it does, and without an alias clash with either :telemetry or Notion.
Pausing to explore the impact: we'd rename the example module to MyApp.Telemetry, do the same in any tests, and broadly replace labels with metadata. I'd also go to town with the documentation, but that's just me.
@coryodaniel said:
I was flip flopping on this. I ultimately end up shipping instrumentation to Prometheus so “labels” is the term I’m used to. Was planning to add some Prometheus.ex integration...
I think the two most important contexts for Notion are Elixir and :telemetry. I'd put the responsibility of translating from :telemetry to Prometheus in another package… and that package would use "label" a whole lot.
Back to Notion, though, and the idea that a little polish and sticking to the terminology from :telemetry would give Notion the best shot at being the de-facto official package for making :telemetry easier to deal with in Elixir:
I think any common atoms at the beginning of an t:event_name/0 should be called an t:event_prefix/0. So, we'd rename the name keyword argument to the Notion macro to event_prefix, have it take either an atom or a list of atoms to be more forgiving to the users, rename the macro's name/0 to event_prefix/0, and have it return a list of atoms. We'd then be able to list our handlers with MyApp.Telemetry.event_prefix() |> :telemetry.list_handlers(). That's clean enough, I'm not sure I'd bother wrapping it as MyApp.Telemetry.list_handlers/0.
Have I missed any other mismatches with :telemetry? I figure I might flush out more as I look through the code to solve some other problems, but do let me know if you think of any.
Zooming back out, if the idea Notion could be the de-facto Elixir wrapper for :telemetry is ridiculous or unwanted, all this is moot. Is it moot? :)
I don't want to get too DDD but, as I raised on Twitter:
Then, later:
Pausing to explore the impact: we'd rename the example module to
MyApp.Telemetry, do the same in any tests, and broadly replacelabelswithmetadata. I'd also go to town with the documentation, but that's just me.@coryodaniel said:
I think the two most important contexts for Notion are Elixir and
:telemetry. I'd put the responsibility of translating from:telemetryto Prometheus in another package… and that package would use "label" a whole lot.Back to Notion, though, and the idea that a little polish and sticking to the terminology from
:telemetrywould give Notion the best shot at being the de-facto official package for making:telemetryeasier to deal with in Elixir:I think any common atoms at the beginning of an
t:event_name/0should be called ant:event_prefix/0. So, we'd rename thenamekeyword argument to theNotionmacro toevent_prefix, have it take either an atom or a list of atoms to be more forgiving to the users, rename the macro'sname/0toevent_prefix/0, and have it return a list of atoms. We'd then be able to list our handlers withMyApp.Telemetry.event_prefix() |> :telemetry.list_handlers(). That's clean enough, I'm not sure I'd bother wrapping it asMyApp.Telemetry.list_handlers/0.Have I missed any other mismatches with
:telemetry? I figure I might flush out more as I look through the code to solve some other problems, but do let me know if you think of any.Zooming back out, if the idea Notion could be the de-facto Elixir wrapper for
:telemetryis ridiculous or unwanted, all this is moot. Is it moot? :)