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

add semaphore state to noobaa endpoint metrics #7384

Merged
merged 1 commit into from
Aug 9, 2023
Merged

add semaphore state to noobaa endpoint metrics #7384

merged 1 commit into from
Aug 9, 2023

Conversation

naveenpaul1
Copy link
Contributor

@naveenpaul1 naveenpaul1 commented Jul 6, 2023

Explain the changes

  1. This code change will add both namespace store and NSFS semaphore state to noobaa endpoint metrics s3-service-monitor

Issues: Fixed #xxx / Gap #xxx

  1. semaphore metrics will give more understanding of semaphore related issues

Testing Instructions:

  1. read/write data to either namespace store or NSFS S3 bucket.
  2. verify Prometheus metrics, noobaa endpoint metrics should have the following metrics
    semaphore_value, semaphore_waiting_value, semaphore_waiting_time, semaphore_waiting_queue
    image
  • Doc added/updated
  • Tests added

Copy link
Contributor

@dannyzaken dannyzaken left a comment

Choose a reason for hiding this comment

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

a few general comments:

  1. is there a reason to share the same metric name for object_io and namespace_fs semaphores? We can have both NSFS and noobaa bucket workloads in the same system. @nimrod-becker I remember something about reducing the number of metrics we send to Prometheus. Is this something to consider?
  2. the metrics contain only one sample of the semaphore values taken only when there is IO. So once IO stops, the value will not be updated, am I right?
  3. I think it can be more consistent with sampling the semaphores values in constant intervals (a function running in the background) and providing an average (e.g., for the last minute).

src/endpoint/endpoint.js Outdated Show resolved Hide resolved
src/endpoint/endpoint.js Show resolved Hide resolved
src/endpoint/endpoint.js Outdated Show resolved Hide resolved
src/sdk/object_io.js Outdated Show resolved Hide resolved
src/sdk/endpoint_stats_collector.js Outdated Show resolved Hide resolved
src/sdk/endpoint_stats_collector.js Outdated Show resolved Hide resolved
config.js Outdated Show resolved Hide resolved
src/endpoint/endpoint.js Outdated Show resolved Hide resolved
src/endpoint/endpoint.js Outdated Show resolved Hide resolved
src/endpoint/endpoint.js Outdated Show resolved Hide resolved
src/endpoint/endpoint.js Outdated Show resolved Hide resolved
src/server/bg_services/semapore_monitor.js Outdated Show resolved Hide resolved
src/server/bg_services/semapore_monitor.js Outdated Show resolved Hide resolved
src/server/bg_services/semapore_monitor.js Outdated Show resolved Hide resolved
src/server/bg_services/semapore_monitor.js Outdated Show resolved Hide resolved
src/server/bg_services/semapore_monitor.js Outdated Show resolved Hide resolved
src/server/bg_services/semapore_monitor.js Outdated Show resolved Hide resolved
src/server/bg_services/semapore_monitor.js Outdated Show resolved Hide resolved
src/server/bg_services/semapore_monitor.js Outdated Show resolved Hide resolved
src/endpoint/endpoint.js Outdated Show resolved Hide resolved
src/sdk/endpoint_stats_collector.js Outdated Show resolved Hide resolved
src/sdk/endpoint_stats_collector.js Outdated Show resolved Hide resolved
Copy link
Contributor

@liranmauda liranmauda left a comment

Choose a reason for hiding this comment

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

Have some comments

config.js Outdated Show resolved Hide resolved
src/endpoint/endpoint.js Show resolved Hide resolved
Signed-off-by: naveenpaul1 <napaul@redhat.com>
@naveenpaul1 naveenpaul1 merged commit 5ed2ab2 into noobaa:master Aug 9, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants