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.

Sign up for GitHub

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

Jump to bottom

Metrics in base_node should be opt-out by default #5787

Closed
SWvheerden opened this issue Sep 19, 2023 · 0 comments · Fixed by #6073
Closed

Metrics in base_node should be opt-out by default #5787

SWvheerden opened this issue Sep 19, 2023 · 0 comments · Fixed by #6073
Assignees
@brianp
Labels
P-high-risk Process - High risk

Comments

@SWvheerden
Copy link
Collaborator

Metrics in the base node should be an optional compile-in. This should not be an always-enabled option and should be opt-in.
The library used Prometheus is full of unwrap code and features no error handles.
@SWvheerden SWvheerden added the release-blocker Something that needs to be fixed before a release can be made label Sep 19, 2023
@SWvheerden SWvheerden added P-high-risk Process - High risk and removed release-blocker Something that needs to be fixed before a release can be made labels Oct 31, 2023
@brianp brianp self-assigned this Jan 8, 2024
@brianp brianp mentioned this issue Jan 11, 2024
Merged
4 tasks
SWvheerden pushed a commit that referenced this issue Feb 8, 2024
@brianp
feat: turn off node metrics by default (#6073)
5ed661c 
Description
---
This make the node metrics opt-in instead of opt-out at compile time.

It is strange we need to leave metrics on in the integration tests. This
is mostly because clippy is often run with `--all-features` which means
metrics would be on in the node, and then required in the integration
configuration.

Closes: #5787 

Motivation and Context
---
Mostly the metrics library has a chance of failure (panics) and it's
generally better (and more private) if we just have metrics off by
default.

How Has This Been Tested?
---
CI

Breaking Changes
---

- [x] None
- [ ] Requires data directory on base node to be deleted
- [ ] Requires hard fork
- [ ] Other - Please specify
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@brianp brianp

Labels
P-high-risk Process - High risk
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: turn off node metrics by default brianp/tari
2 participants
@brianp @SWvheerden