Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable node metrics #723

Merged
merged 5 commits into from Sep 14, 2023
Merged

Enable node metrics #723

merged 5 commits into from Sep 14, 2023

Conversation

RolandSherwin
Copy link
Member

@RolandSherwin RolandSherwin commented Sep 12, 2023

Depends on #705

  • Enables collection of node related metrics

Description

Summary generated by Reviewpad on 12 Sep 23 10:03 UTC

This pull request includes several changes across multiple files:

  1. The sn_networking/src/error.rs file introduces new variants to the Error enum related to error handling and network metrics.
  2. The sn_node/src/metrics.rs file adds functionality for collecting and recording metrics for the SAFE Network software.
  3. The sn_node/src/node.rs file includes changes to support the open-metrics feature, refactor network creation, handle network events, and record metrics.
  4. The sn_node/src/api.rs file adds support for metrics using the open-metrics feature, replaces the SwarmDriver implementation, and adds methods for request timeout and concurrency limit.
  5. The sn_node/src/metrics_service.rs file adds a metrics server using the Hyper crate and implements the MetricService struct to handle incoming requests.
  6. The Cargo.toml files include changes related to new dependencies, features, and versions.
  7. Minor changes were made to other files, such as README updates, log marker modifications, and attribute additions/removals.

These changes enhance error handling, network metrics, metrics collection and recording, network creation, API functionality, dependency management, and documentation.

Let me know if you need more information or have any questions.

@reviewpad reviewpad bot requested a review from bochaco September 12, 2023 10:03
@reviewpad reviewpad bot added Large Large sized PR waiting-for-review labels Sep 12, 2023
@reviewpad
Copy link

reviewpad bot commented Sep 12, 2023

Reviewpad Report

‼️ Errors

  • Unconventional commit detected: 'refactor(network): use builder pattern to construct the Network
  • this allows us to enable feature flagged fields to be passed in' (0a49270)
  • Unconventional title detected: 'Enable node metrics ' illegal 'E' character in commit message type: col=00

sn_networking/src/metrics_service.rs Fixed Show fixed Hide fixed
const METRICS_CONTENT_TYPE: &str = "application/openmetrics-text;charset=utf-8;version=1.0.0";

pub(crate) fn run_metrics_server(registry: Registry) {
// Serve on localhost.

Check notice

Code scanning / devskim

Accessing localhost could indicate debug code, or could hinder scaling. Note

Do not leave debug code in production
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();

Check notice

Code scanning / devskim

Accessing localhost could indicate debug code, or could hinder scaling. Note

Do not leave debug code in production
@RolandSherwin RolandSherwin force-pushed the custom_metrics branch 5 times, most recently from c3b0068 to 03590a4 Compare September 12, 2023 16:29
// Serve on localhost.
let addr = ([127, 0, 0, 1], 0).into();

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

Check warning

Code scanning / devskim

An HTTP-based URL without TLS was detected. Warning

Insecure URL
@joshuef joshuef added this pull request to the merge queue Sep 14, 2023
Merged via the queue into maidsafe:main with commit ff534fd Sep 14, 2023
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Large Large sized PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants