I don't see what this adds. It seems more complex but without gain, removing/replacing reporters can be done just as easily in the existing code.

I also don't see reporting as events. Allowing multiple reportes is fine because of Erlang terms simple lists and list handling, but thinking of them as event handlers feels like going down the wrong path and giving the wrong impression to users.

Even though I wouldn't want to use it, preferring to do this off the node being traced, I can see benefits from a sort of pipeline to pass spans through, but it should be separate from a reporter which is just wanting to get spans out of the system.

I'll have to take a closer look later but that is my initial thoughts.

I understand nomenclatural difference, but my point is that currently we are duplicating work that already has been done. I can extract this to separate library though that will provide gen_sink if you prefer it. Will that be ok?

gen_sink?

Right, I don't think we are duplicating work because ours is simply a function call on an interval. It shouldn't even really be a list, but it didn't really add any complexity to have it be one instead of separate sequence module for the rare case of wanting a sequence of reporters.

List and fact that we allow to dynamically add and remove reporters makes testing so much easier. Additionally it makes configuring reporter in runtime possible as you can do in your application setup:

oc_traces:add_handler(oc_datadog_reporter, [{api_key, os:getenv("DD_TOKEN")}])

And call it a day, with "old way" it is a little bit harder.

@tsloughter additional advantage is that it will provide uniform interface for configuring reporters of metrics and traces. Just this thing is worth of fixes.

And TBH I think that in the end it will make the code simpler, as almost the same modules can be reused for the metrics.

Codecov Report

Merging #136 into master will increase coverage by 1.82%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #136      +/-   ##
==========================================
+ Coverage   79.91%   81.74%   +1.82%     
==========================================
  Files          34       36       +2     
  Lines         722      701      -21     
==========================================
- Hits          577      573       -4     
+ Misses        145      128      -17

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b73a12f...1d985bb. Read the comment docs.

Codecov Report

Merging #136 into master will increase coverage by 1.88%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #136      +/-   ##
==========================================
+ Coverage   79.97%   81.85%   +1.88%     
==========================================
  Files          37       35       -2     
  Lines         814      700     -114     
==========================================
- Hits          651      573      -78     
+ Misses        163      127      -36

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 37d708e...67221ee. Read the comment docs.