beam-telemetry / telemetry_metrics_statsd

Telemetry.Metrics reporter for StatsD-compatible metric servers
https://hexdocs.pm/telemetry_metrics_statsd
MIT License
75 stars 44 forks source link

Allow to configure a pool of sockets #41

Closed juanperi closed 4 years ago

juanperi commented 4 years ago

Current implementation uses a single gen server to route every published metric. With this new one, the genserver does not route them anymore, and there is a pool of sockets that can do the work of shipping the metrics.

My original approach at this was to follow the same approach as https://andrealeopardi.com/posts/process-pools-with-elixirs-registry/ But I realized that I was introducing sweeping changes, completely changing the suprevision tree and most of the wiring.

So i went back to the drawing board, and tried to accomplish the same without all that. I did that using a named :ets table Clients reading can directly go to the :ets. In case of errors, I'm still using the genserver to serialize the requests so the port can be re-created a single time

As things stand, this change is backwards compatible with the existing implementation.

Closes #33

juanperi commented 4 years ago

I just noticed based on the test errors that liveness requires elixir 1.7, but test matrix uses 1.6. Do you want to bump the version constraints?

juanperi commented 4 years ago

cleanups addressed. only outstanding question is about the usage of a named ets table or not

arkgil commented 4 years ago

@epilgrim I think we should generate a unique ETS name per reporter process, save that in the event handler, and use it to get the UDP sockets.

When you do this, can you add a test that checks if multiple reporters can be started?

juanperi commented 4 years ago

sure, will do it. But unlikely I'll be able to push it today.

juanperi commented 4 years ago

changes done. Now there is no harcoded ets name

arkgil commented 4 years ago

Great, thanks! I updated Elixir versions on CI on master, can you please rebase on that?

juanperi commented 4 years ago

rebased!