56quarters / cadence

An extensible Statsd client for Rust
https://docs.rs/cadence/
Apache License 2.0
84 stars 27 forks source link

Expose network-level telemetry #196

Closed mlowicki closed 7 months ago

mlowicki commented 8 months ago

While working with bigger numbers of metrics being emitted it's useful to know things like:

in order to e.g. diagnose issues related agent receiving those being hammered. Currently there isn't anything like this from what I've checked - QueuingMetricSink supports few metrics via WorkerStats but those are related to the queue / channel being used there.

One option would be to have those at the very low layers like UnixWriteAdapter or UdpWriteAdapter but then we would need to have an API to access those:

Any thoughts / ideas?

56quarters commented 8 months ago

While working with bigger numbers of metrics being emitted it's useful to know things like:

  • bytes sent / dropped
  • packets sent / dropped

Where are the "dropped" counters coming from? Implicit when there's an error writing to a socket?

Currently there isn't anything like this from what I've checked - QueuingMetricSink supports few metrics via WorkerStats but those are related to the queue / channel being used there.

io::MultilineWriter keeps some metrics as well but they're mostly for ease of unit testing.

One option would be to have those at the very low layers like UnixWriteAdapter or UdpWriteAdapter but then we would need to have an API to access those:

  • we could extend MetricSink trait with something like stats() to get access those. Wrapping sinks would simply call stats() of the underlying sink.

Adding a new method to MetricSink that returns a SinkMetrics or similar seems pretty reasonable to me. This would likely require adding atomic counters in the various sinks that do the actual writes. My only concern is that we'd have to make it clear that not all sinks emit all metrics (I'm assuming) and this is largely best effort.

  • add APIs only in few places like BufferedUnixMetricSink and UnixWriteAdapter and in a similar manner for UDP Any thoughts / ideas?

Seems like a nice addition to me. Thanks for the issue!

mlowicki commented 8 months ago

While working with bigger numbers of metrics being emitted it's useful to know things like:

  • bytes sent / dropped
  • packets sent / dropped

Where are the "dropped" counters coming from? Implicit when there's an error writing to a socket?

Yea, that was my initial though on it. In theory wrapping sink could have some retry logic (I don't think it's the case currently) so error writing to a socket won't necessary mean that data got dropped on the floor but maybe it would be good enough for start. If sinks would return SinkMetrics then wrapping sink can always implement correct calculation to take retries into account instead of simply returning values from low-level sink like UnixMetricSink or BufferedUnixMetricSink.

Currently there isn't anything like this from what I've checked - QueuingMetricSink supports few metrics via WorkerStats but those are related to the queue / channel being used there.

io::MultilineWriter keeps some metrics as well but they're mostly for ease of unit testing.

One option would be to have those at the very low layers like UnixWriteAdapter or UdpWriteAdapter but then we would need to have an API to access those:

  • we could extend MetricSink trait with something like stats() to get access those. Wrapping sinks would simply call stats() of the underlying sink.

Adding a new method to MetricSink that returns a SinkMetrics or similar seems pretty reasonable to me. This would likely require adding atomic counters in the various sinks that do the actual writes. My only concern is that we'd have to make it clear that not all sinks emit all metrics (I'm assuming) and this is largely best effort.

Right, I can try to prototype it in upcoming days and then we'll see how it looks. Maybe implementing it for few low-level sinks would be just fine and wrapping sink can simply call stats() on the inner sink 🤷 .

  • add APIs only in few places like BufferedUnixMetricSink and UnixWriteAdapter and in a similar manner for UDP Any thoughts / ideas?

Seems like a nice addition to me. Thanks for the issue!

Thanks, let me try to come up with some draft and we can further discuss direction over there.

mlowicki commented 7 months ago

@56quarters initial stub on https://github.com/56quarters/cadence/pull/203. This only exposes bytes / packets sent in some parts. Should be enough to discuss on further direction.

mlowicki commented 7 months ago

Fixed by https://github.com/56quarters/cadence/issues/196.