Skip to content
This repository has been archived by the owner on Aug 29, 2023. It is now read-only.

feat: add metrics #217

Closed
wants to merge 2 commits into from
Closed

Conversation

dapplion
Copy link
Contributor

@dapplion dapplion commented Oct 7, 2022

Adds metrics for Prometheus-like collecting engine:

  • serverStatus: If server is listening or not
  • connections: Current count of active connections from internal net server data structures
  • listenerErrors: Total count of relevant errors by error type
  • socketEvents: Total count of socket events by type

@dapplion dapplion marked this pull request as draft October 7, 2022 11:09
@dapplion dapplion changed the title Add metrics feat: add metrics Oct 7, 2022
@achingbrain
Copy link
Member

These metrics are very useful but they should be added to the built in libp2p metrics object instead of being their own thing

@dapplion
Copy link
Contributor Author

These metrics are very useful but they should be added to the built in libp2p metrics object instead of being their own thing

Why?

@dapplion dapplion marked this pull request as ready for review October 14, 2022 17:13
@achingbrain
Copy link
Member

So there's a central point to collect metrics from for viewing, rather than having to go and ask every component, and so each component submits metrics in the same format following the same interface.

@dapplion
Copy link
Contributor Author

So there's a central point to collect metrics from for viewing, rather than having to go and ask every component, and so each component submits metrics in the same format following the same interface.

How can I do .inc with the built in libp2p metrics object?

@dapplion
Copy link
Contributor Author

dapplion commented Oct 28, 2022

Updated PR to use a "metrics generator" so that consumer only needs to pass something that can create Gauge instances. This follows similar encapsulation principles as built in libp2p metrics object but with no performance penalty.

achingbrain added a commit that referenced this pull request Nov 5, 2022
Uses new metrics interface from libp2p/js-libp2p-interfaces#310
to report useful connection metrics.

Similar to #217 but it adds the listening host/port to the metrics
name to allow multiple TCP listeners to report metrics separately.
@achingbrain achingbrain mentioned this pull request Nov 5, 2022
achingbrain added a commit that referenced this pull request Nov 5, 2022
Uses new metrics interface from libp2p/js-libp2p-interfaces#310 to report useful connection metrics.

Similar to #217 but it adds the listening host/port to the metrics name to allow multiple TCP listeners to report metrics separately.

BREAKING CHANGE: requires metrics interface v4
github-actions bot pushed a commit that referenced this pull request Nov 5, 2022
## [6.0.0](v5.0.2...v6.0.0) (2022-11-05)

### ⚠ BREAKING CHANGES

* requires metrics interface v4

### Features

* add metrics ([#223](#223)) ([c004357](c004357)), closes [#217](#217)
@p-shahi
Copy link
Member

p-shahi commented Nov 8, 2022

depends on libp2p/js-libp2p#1458

@mpetrunic
Copy link
Member

@dapplion Does it make sense to close this one since #223 is merged or?

@p-shahi
Copy link
Member

p-shahi commented Nov 8, 2022

#223 adds the same metrics as this PR: tcp server status, active connections, listener errors, and socket events
However, they were added as counters and not gauges. Not sure if all these are intended to be cumulative (like active connections)

I think we'll want to add a Gauge interface to interface-metrics here and convert some counters to gauges afterwards.

@dapplion
Copy link
Contributor Author

@dapplion Does it make sense to close this one since #223 is merged or?

#223 is flawed will comment there

@dapplion
Copy link
Contributor Author

@dapplion dapplion closed this Nov 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants