impl: libp2p metrics #705

RolandSherwin · 2023-09-04T14:47:07Z

Depends on #706

Enables libp2p metrics. Example of the metrics collected https://kademlia-exporter.max-inden.de/d/Pfr0Fj6Mk/rust-libp2p?orgId=1&refresh=30s
It is enabled by default for the node.
Can be extended to support more custom metrics.

Description

Summary generated by Reviewpad on 04 Sep 23 14:47 UTC

This pull request includes changes to multiple files. Here is an overview of the changes:

SwarmDriver in sn_networking crate:
The implementation of the SwarmDriver struct and its associated methods have been modified. Changes include:

Importing and using the driver::SwarmDriver trait.
Importing and using the libp2p_metrics::Recorder trait.
Removing the NodeBehaviour struct.
Updating the NodeEvent enum to include additional events.
Updating the SwarmDriver struct to include a network_metrics field of type Recorder.
Updating the handle_msg method to handle request/response messages using the request_response crate.
Updating the handle_kad_event method to handle Kademlia events.

Addition of metrics_service.rs:
A new file called metrics_service.rs has been added. This file implements a metrics service for the SAFE Network Software. It defines a MetricService struct and a MakeMetricService struct. The MetricService struct implements the Service trait from the hyper crate and provides functions for handling HTTP requests. The MakeMetricService struct is responsible for creating instances of the MetricService struct. The file also contains a metrics_server function that runs the metrics service on localhost.
Changes to Cargo.toml:
The Cargo.toml file has been modified to include new dependencies and update existing ones. Changes include:

Adding the dependency libp2p-metrics with version 0.13.1 and features identify and kad.
Adding the dependency prometheus-client with version 0.21.2.
Updating the dependency hyper to version 0.14 and adding features server, tcp, and http1.

Changes to cmd.rs:
The cmd.rs file has been modified to include changes related to imports and function removal. Changes include:

Adding the import statement use crate::driver::SwarmDriver.
Removing the import statement use libp2p::swarm::dial_opts::DialOpts.
Removing the functions dial and dial_with_opts.
Removing some code related to dialing with a peer ID.

Other changes:

In a certain file, changes include removing the import of the rand::Rng module, removing the definitions of specific constants, adjusting the implementation of the Default trait for a struct, removing unused imports, and removing some comments.
Additionally, the file sn_networking/src/msg/mod.rs has been deleted.

Please review these changes to ensure they are correct and meet the requirements of the codebase. Let me know if you need any further clarification!

sn_networking/src/metrics_service.rs

+
+    tokio::spawn(async move {
+        let server = Server::bind(&addr).serve(MakeMetricService::new(registry));
+        info!("Metrics server on http://{}/metrics", server.local_addr());


sn_networking/src/metrics_service.rs

+const METRICS_CONTENT_TYPE: &str = "application/openmetrics-text;charset=utf-8;version=1.0.0";
+
+pub(crate) fn metrics_server(registry: Registry) {
+    // Serve on localhost.


sn_networking/src/driver.rs

+                });
+            }
+            NodeRecordStoreConfig {
+                max_value_bytes: MAX_PACKET_SIZE, // TODO, does this need to be _less_ than MAX_PACKET_SIZE


sn_networking/src/driver.rs

+
+/// What is the largest packet to send over the network.
+/// Records larger than this will be rejected.
+// TODO: revisit once utxo is in


reviewpad · 2023-09-04T16:42:36Z

Reviewpad Report

‼️ Errors

Unconventional commit detected: 'feat(network): feature gate libp2p metrics

this is enabled by default for a node though' (c1e7941)

Unconventional title detected: 'impl: libp2p metrics' illegal 'i' character in commit message type: col=00

sn_networking/src/metrics_service.rs

+    fn respond_with_404_not_found(&mut self) -> Response<String> {
+        let mut resp = Response::default();
+        *resp.status_mut() = StatusCode::NOT_FOUND;
+        *resp.body_mut() = "Not found try localhost:[port]/metrics".to_string();


sn_networking/src/metrics_service.rs

sn_networking/src/metrics_server.rs

joshuef · 2023-09-08T08:26:15Z

How do we access these metrics? Do we need tostart a custom server for this? does each node run a server to access metrics?

joshuef

I think metrics here confuses with out logging metrics output. I'd say we want a clearer name for this feature.

libp2p-metrics, network-metrics ? something-else ?

joshuef · 2023-09-09T05:48:24Z

I'm also torn on this being on by default atm...

RolandSherwin · 2023-09-11T06:56:39Z

How do we access these metrics? Do we need tostart a custom server for this? does each node run a server to access metrics?

Yes, each node runs a separate hyper server on a random port. This returns the OpenMetrics response type that can be fed into grafana / other tools.

Also, I will push a change to update the feature name and remove it from the default features list.

- this is enabled by default for a node though

…es list

reviewpad bot added the Large Large sized PR label Sep 4, 2023

RolandSherwin changed the title ~~libp2p metrics~~ impl: libp2p metrics Sep 4, 2023

github-advanced-security bot found potential problems Sep 4, 2023

View reviewed changes

RolandSherwin force-pushed the network_metrics branch from fab265e to 757c0ce Compare September 4, 2023 16:41

RolandSherwin marked this pull request as ready for review September 4, 2023 16:42

reviewpad bot added the waiting-for-review label Sep 4, 2023

reviewpad bot requested a review from bochaco September 4, 2023 16:42

RolandSherwin force-pushed the network_metrics branch from 757c0ce to d772972 Compare September 5, 2023 07:25

github-advanced-security bot found potential problems Sep 5, 2023

View reviewed changes

sn_networking/src/metrics_service.rs Fixed Show fixed Hide fixed

RolandSherwin force-pushed the network_metrics branch from d772972 to 786dda3 Compare September 5, 2023 07:30

github-advanced-security bot found potential problems Sep 5, 2023

View reviewed changes

RolandSherwin force-pushed the network_metrics branch from a87b203 to 44e2a85 Compare September 5, 2023 08:13

github-advanced-security bot found potential problems Sep 5, 2023

View reviewed changes

sn_networking/src/metrics_service.rs Fixed Show fixed Hide fixed

sn_networking/src/metrics_service.rs Fixed Show fixed Hide fixed

RolandSherwin force-pushed the network_metrics branch from 44e2a85 to 2e02237 Compare September 5, 2023 08:18

github-advanced-security bot found potential problems Sep 5, 2023

View reviewed changes

sn_networking/src/metrics_server.rs Fixed Show fixed Hide fixed

sn_networking/src/metrics_server.rs Fixed Show fixed Hide fixed

RolandSherwin force-pushed the network_metrics branch 2 times, most recently from 4a3dacb to 9079f6d Compare September 5, 2023 11:31

reviewpad bot added Medium Medium sized PR and removed Large Large sized PR labels Sep 5, 2023

RolandSherwin force-pushed the network_metrics branch 2 times, most recently from d80cf29 to 9207811 Compare September 6, 2023 10:17

joshuef requested changes Sep 9, 2023

View reviewed changes

RolandSherwin force-pushed the network_metrics branch from 9207811 to d84c4f1 Compare September 11, 2023 07:14

RolandSherwin requested a review from joshuef September 11, 2023 07:15

RolandSherwin added 2 commits September 12, 2023 11:00

feat(network): implement libp2p metrics

87c06f3

feat(network): feature gate libp2p metrics

c1e7941

- this is enabled by default for a node though

RolandSherwin added 3 commits September 12, 2023 11:00

chore(network): remove unwraps inside metrics server

b0087a6

chore(metrics): rename network metrics and remove from default featur…

263faf4

…es list

chore(docs): add docs about network metrics

06471da

RolandSherwin force-pushed the network_metrics branch from 6d30e10 to 06471da Compare September 12, 2023 05:30

RolandSherwin mentioned this pull request Sep 12, 2023

Enable node metrics #723

Merged

joshuef approved these changes Sep 12, 2023

View reviewed changes

joshuef added this pull request to the merge queue Sep 12, 2023

Merged via the queue into maidsafe:main with commit dc58921 Sep 12, 2023
24 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

impl: libp2p metrics #705

impl: libp2p metrics #705

RolandSherwin commented Sep 4, 2023 •

edited

reviewpad bot commented Sep 4, 2023 •

edited

joshuef commented Sep 8, 2023

joshuef left a comment

joshuef commented Sep 9, 2023

RolandSherwin commented Sep 11, 2023

impl: libp2p metrics #705

impl: libp2p metrics #705

Conversation

RolandSherwin commented Sep 4, 2023 • edited

Description

Summary generated by Reviewpad on 04 Sep 23 14:47 UTC

reviewpad bot commented Sep 4, 2023 • edited

joshuef commented Sep 8, 2023

joshuef left a comment

Choose a reason for hiding this comment

joshuef commented Sep 9, 2023

RolandSherwin commented Sep 11, 2023

RolandSherwin commented Sep 4, 2023 •

edited

reviewpad bot commented Sep 4, 2023 •

edited