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

refactor: metrics server. Simplify app.nim module #2650

Merged
merged 2 commits into from
Apr 30, 2024

Conversation

Ivansete-status
Copy link
Collaborator

@Ivansete-status Ivansete-status commented Apr 29, 2024

Description

Simplify app.nim and move the "metrics factory" procs to an already existing waku_metrics module

Copy link

github-actions bot commented Apr 29, 2024

You can find the image built from this PR at

quay.io/wakuorg/nwaku-pr:2650-rln-v2

Built from 5929a3d

Copy link

github-actions bot commented Apr 29, 2024

You can find the image built from this PR at

quay.io/wakuorg/nwaku-pr:2650-rln-v1

Built from 5929a3d

Copy link
Contributor

@NagyZoltanPeter NagyZoltanPeter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

return
err("Starting metrics server failed. Continuing in current state:" & $error)

if conf.metricsLogging:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just out of curiosity, is there a mean having metrics server running without metrics logging?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess you can scrape the metrics for monitoring purposes without it polluting the logs?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question @NagyZoltanPeter !
Yes, that could happen indeed. The metrics server is just an http server that delivers current node state, and this can run separately. The log-metrics, is some kind of redundant metrics information that we print regularly in logs. Both are similar but can run separately

Copy link
Contributor

@gabrielmer gabrielmer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazingg, thank you!

@Ivansete-status Ivansete-status merged commit 4a110f6 into master Apr 30, 2024
13 of 15 checks passed
@Ivansete-status Ivansete-status deleted the refactor-metrics-server branch April 30, 2024 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants