update for telemetry 0.4.0 #8
Conversation
src/oc_telemetry.erl
Outdated
| case telemetry:attach(Name, EventName, fun handle_event/4, Options) of | ||
| ok -> {ok, Measure}; | ||
| Error -> Error | ||
| attach(Name, EventName, Measurement, Description, Unit, TagValues) -> |
There was a problem hiding this comment.
I would add attach/5 which would set TagValues to identity function by default. IMHO it would provide nicer API for most common use-case.
There was a problem hiding this comment.
Oh right, wanted to ask about that. I don't get why there are exported attach functions? Wouldn't this only be used for track?
There was a problem hiding this comment.
The idea was to support "manually" adding new listeners without Telemetry.Metrics.
|
Updated. But I'm still not 100% on the API and whether we shouldn't simplify it by removing |
|
@tsloughter AFAIK not all libraries that support |
|
@hauleth oh, I don't think they do. I misspoke. I meant not having the |
tsloughter
force-pushed
the
telemetry-0.4
branch
3 times, most recently
from
0be154b to
18ed2e1
Compare
tsloughter
force-pushed
the
telemetry-0.4
branch
from
b856db9 to
1d6c4bc
Compare
tsloughter
force-pushed
the
telemetry-0.4
branch
from
4efa927 to
7667719
Compare
| aggregation('Elixir.Telemetry.Metrics.Distribution', #{buckets := Buckets}) -> | ||
| {oc_stat_aggregation_distribution, [{buckets, Buckets}]}. | ||
| {oc_stat_aggregation_distribution, [{buckets, Buckets}]}. | ||
|
|
There was a problem hiding this comment.
Might want to handle the new Summary type being passed even if to just log that it's unsupported and continue on.
Still needs some cleanup, like indentation, but wanted to open this before calling it a day.
I also do not like what I had to do on line 70 of
oc_telemetry_SUITEbut I don't know what else to do there :(