lib/connections: Add syncthing_connections_active metric (fixes #9527) #9528
Conversation
DerRockWolf
force-pushed
the
device-connection-metrics
branch
from
5b2c806
to
821332d
Compare
DerRockWolf
force-pushed
the
device-connection-metrics
branch
from
821332d
to
0793ab4
Compare
lib/connections/service.go
Outdated
| @@ -441,6 +441,8 @@ func (s *service) handleHellos(ctx context.Context) error { | |||
| // connections are limited. | |||
| rd, wr := s.limiter.getLimiters(remoteID, c, c.IsLocal()) | |||
|
|
|||
| registerDeviceMetrics(remoteID.String()) | |||
There was a problem hiding this comment.
How about doing this in func (*service) CommitConfiguration(), which is called with the config (incl. list of remote devices) at startup and when the config changes?
There was a problem hiding this comment.
Not 100% sure if I put it to the correct place 😅
I've additionally added a call to remove the metric if the device got deleted from the config. But I just noticed that this probably won't work because the connections are handled at a later point... Where would I best put the metric deletion part?
I could probably first call metricDeviceActiveConnections.DeleteLabelValues(dev.DeviceID.String()) directly followed by registerDeviceMetrics(dev.DeviceID.String()) in the for _, dev := range to.Devices { loop, but that could in theory be racy 🤔
There was a problem hiding this comment.
@calmh have you missed my last comment? The metric deletion probably isn't critical but still worth looking into I think.
And thanks for helping and reviewing so quickly and also for building syncthing ❤️
calmh
changed the title
syncthing_connections_active metric
|
Nice, thank you! |
* main: (45 commits) build: Use Go 1.22.3 at minimum build: Use Go 1.22.3 at minimum gui: Add Hindi (hi) translation template (syncthing#9530) gui, man, authors: Update docs, translations, and contributors lib/connections: Add syncthing_connections_active metric (fixes syncthing#9527) (syncthing#9528) etc: Use 7MiB buffer size (syncthing#9524) gui: Fix Firefox bookmark favicon (fixes syncthing#9506) (syncthing#9507) gui, man, authors: Update docs, translations, and contributors gui, man, authors: Update docs, translations, and contributors gui, man, authors: Update docs, translations, and contributors lib/nat: Don't crash on empty address list (fixes syncthing#9503) (syncthing#9504) lib/db: Drop indexes for outgoing data to force refresh (ref syncthing#9496) (syncthing#9502) gui: Fix missing link to device editor for names with superscript (ref syncthing#9472) (syncthing#9494) lib/db: Hold update lock while taking snapshot (syncthing#9496) build: Update dependencies (syncthing#9497) gui, man, authors: Update docs, translations, and contributors gui, man, authors: Update docs, translations, and contributors Removed no longer relevant Bountysource link (syncthing#9480) lib/api: Missing return after HTTP error lib/api: Extract session store (syncthing#9425) ...
Purpose
Adds a new metric
syncthing_connections_activewhich equals to the amount of active connections per device.Fixes #9527
Testing
I've manually tested it by running syncthing with these changes locally and examining the returned metrics from
/metrics.I've done the following things:
Documentation
https://github.com/syncthing/docs/blob/main/includes/metrics-list.rst needs to be regenerated with find-metrics.go
Authorship
Your name and email will be added automatically to the AUTHORS file
based on the commit metadata.