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

NSFS | NC |endpoint metrics should aggregated fork #7562

Merged

naveenpaul1 merged 1 commit into noobaa:master from naveenpaul1:nsfs_metrics

Dec 8, 2023

Contributor

naveenpaul1 commented Nov 6, 2023 •

edited

Explain the changes

Currently the endpoint_stats_collector just prints the metrics to log in standalone mode (no rpc_client).
In addition, when combined with --forks, the metrics port does not work as expected and will just load balance each request to a different fork.
NSFS Certificate doc updated

Issues: Fixed #xxx / Gap #xxx

NSFS | NC | endpoint metrics should be exposed on prometheus and aggregated to parent of forks #7482

Testing Instructions:

start nsfs with multiple forks and Prometheus port
Hit Prometheus url and verify the values

Doc added/updated
Tests added

naveenpaul1 requested review from guymguym, dannyzaken, romayalon and tangledbytes

November 6, 2023 13:48

pull-request-size bot added the size/M label

naveenpaul1 force-pushed the nsfs_metrics branch 3 times, most recently from 3c0028d to 0a08da7 Compare

November 6, 2023 15:48

naveenpaul1 requested a review from liranmauda

November 7, 2023 04:16

naveenpaul1 force-pushed the nsfs_metrics branch 2 times, most recently from 9c11c0f to 751472b Compare

November 7, 2023 07:00

romayalon reviewed

View reviewed changes

src/endpoint/endpoint.js Show resolved Hide resolved

src/server/analytic_services/prometheus_reports/base_prometheus_report.js Show resolved Hide resolved

src/sdk/endpoint_stats_collector.js Show resolved Hide resolved

tangledbytes reviewed

View reviewed changes

docs/non_containerized_NSFS.md Outdated Show resolved Hide resolved

naveenpaul1 force-pushed the nsfs_metrics branch from 751472b to 11efdab Compare

November 8, 2023 05:47

pull-request-size bot added size/L and removed size/M labels

naveenpaul1 force-pushed the nsfs_metrics branch 5 times, most recently from e4a1922 to 510b72f Compare

November 13, 2023 11:50

Contributor

nimrod-becker commented Nov 14, 2023

@liranmauda @dannyzaken can you please review?

tangledbytes mentioned this pull request

[NC | NSFS] Fix NSFS prometheus write bytes #7602

Merged

2 tasks

romayalon reviewed

View reviewed changes

src/util/fork_utils.js Show resolved Hide resolved

src/server/system_services/stats_aggregator.js Outdated Show resolved Hide resolved

src/server/system_services/stats_aggregator.js Outdated Show resolved Hide resolved

docs/non_containerized_NSFS.md Outdated Show resolved Hide resolved

src/endpoint/endpoint.js Show resolved Hide resolved

src/server/analytic_services/prometheus_reporting.js Show resolved Hide resolved

naveenpaul1 force-pushed the nsfs_metrics branch 6 times, most recently from 40d58be to 0138baa Compare

November 28, 2023 17:10

naveenpaul1 force-pushed the nsfs_metrics branch 3 times, most recently from fa315be to 616021a Compare

November 30, 2023 05:42

dannyzaken reviewed

View reviewed changes

src/cmd/manage_nsfs.js Outdated

                       print_account_usage();
                       return;
                   }

Contributor

dannyzaken Dec 5, 2023

remove

dannyzaken approved these changes

View reviewed changes

Contributor

dannyzaken left a comment

Approving the PR.
since it's pretty large, I think we need more testing before merging, mainly to make sure that it does not affect prometheus reporting in Openshift\K8s

naveenpaul1 force-pushed the nsfs_metrics branch 2 times, most recently from 4302979 to 8d222c8 Compare

December 8, 2023 12:44

Contributor Author

naveenpaul1 commented Dec 8, 2023

Verified the Prometheus metrics in openshift cluster.


          NSFS | NC |endpoint metrics should aggregated fork

868ff1e

Signed-off-by: naveenpaul1 <napaul@redhat.com>

naveenpaul1 force-pushed the nsfs_metrics branch from 8d222c8 to 868ff1e Compare

December 8, 2023 12:52

naveenpaul1 merged commit 13878f4 into noobaa:master

8 of 9 checks passed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

romayalon romayalon left review comments

tangledbytes tangledbytes left review comments

dannyzaken dannyzaken approved these changes

guymguym Awaiting requested review from guymguym

liranmauda Awaiting requested review from liranmauda