swarm: add a basic metrics tracer #1973
Conversation
marten-seemann
force-pushed
the
circuitv2-transport-connstate
branch
2 times, most recently
from
3328610
to
52034ff
Compare
marten-seemann
force-pushed
the
swarm-metrics
branch
from
0daaeff
to
28cb275
Compare
|
Here's some preliminary results, obtaining from running a Kubo node with this branch: |
marten-seemann
force-pushed
the
swarm-metrics
branch
from
26a6f97
to
0bf5d37
Compare
marten-seemann
changed the base branch from
circuitv2-transport-connstate
to
master
|
I added the Grafana dashboard. There's probably a lot that can be improved on that front, happy about suggestions (and PRs...). What's really annoying here is that Grafana is too stupid to apply consistent colors on different panels. For example, it makes it soooo much easier to consume if QUIC always has the same color. You can set colors manually via so-called overrides, but those are per panel. So what I did is I defined the overrides for all our transports in one panel, and then copy-pasted a long block of JSON over to the other panels. Apparently that's the only way to do it. 🤢 |
|
I know it's asking a bit, but is there an open grafana dashboard I can see? |
| ) | ||
|
|
||
| func BenchmarkMetricsConnOpen(b *testing.B) { | ||
| b.ReportAllocs() |
There was a problem hiding this comment.
Can we check that this is indeed some low number of allocs?
There was a problem hiding this comment.
In a unit test? Benchmarks are currently not run on CI at all.
| if s.metricsTracer != nil { | ||
| connState := connC.ConnState() | ||
| s.metricsTracer.OpenedConnection(network.DirOutbound, connC.RemotePublicKey(), connState) | ||
| s.metricsTracer.CompletedHandshake(time.Since(start), connState) |
There was a problem hiding this comment.
Do we want to do this on the listen side as well?
There was a problem hiding this comment.
That would require us to put it in the transport. The swarm doesn't know when the handshake of an incoming connection started.
| tags := getStringSlice() | ||
| defer putStringSlice(tags) | ||
| *tags = appendConnectionState(*tags, cs) | ||
| connHandshakeLatency.WithLabelValues(*tags...).Observe(t.Seconds()) |
There was a problem hiding this comment.
It would be great if we could know if this was an early muxer negotiation.
There was a problem hiding this comment.
Could we add this to the ConnectionState struct?
marten-seemann
force-pushed
the
swarm-metrics
branch
from
0bf5d37
to
f2746ea
Compare
marten-seemann
force-pushed
the
swarm-metrics
branch
from
f2746ea
to
efb535f
Compare
|
Merging because I can't wait for this feature! |
Fixes #1910.
Due to our experience with OpenCensus (#1955), this now uses Prometheus. There's a benchmark test that confirms that (by using a
sync.Poolfor the slices containing the labels) we're not allocating at all.