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

HDDS-11442. Add dashboard for memory consumption metrics #7198

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

len548
Copy link
Contributor

@len548 len548 commented Sep 13, 2024

What changes were proposed in this pull request?

Added Ozone - Memory Consumption Metrics.json to measure memory consumption for the services: OM, SCM, DN and Recon. The following metrics are tracked:

  • Heap usage
  • Non-heap usage
  • GC stats
  • Thread stats
  • JVM load

Also, the dashboard includes these metrics for Ozone:

  • The number of keys
  • The key creation rate

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-11442

How was this patch tested?

Deployed locally.
Screenshot 2024-09-13 at 13 13 09
image
image
image
image

Copy link
Contributor

@tanvipenumudy tanvipenumudy left a comment

Choose a reason for hiding this comment

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

Thank you @len548 for working on the patch, please find a few comments below.

{
"datasource": {
"type": "prometheus",
"uid": "PBFA97CFB590B2093"
Copy link
Contributor

Choose a reason for hiding this comment

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

We could consider skipping the inclusion of the datasource uid to simplify the import process for users.
Ref: grafana/grafana#60769

Copy link
Contributor Author

@len548 len548 Sep 19, 2024

Choose a reason for hiding this comment

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

Agreed. I left the uid because I assumed it wouldn't display any data without uid. Now pushed new change that excludes uids.

{
"datasource": {
"type": "prometheus",
"uid": "PBFA97CFB590B2093"
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as the previous comment, we could skip the datasource uid field in all instances

"timepicker": {},
"timezone": "browser",
"title": "Ozone - Memory Consumption Metrics",
"uid": "fdtf5eoqt93pcc",
Copy link
Contributor

Choose a reason for hiding this comment

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

On similar lines, we could exclude the uid field for the dashboard as well!

"options": {
"mode": "exclude",
"names": [
"{__name__=\"jvm_metrics_threads_new\", context=\"jvm\", hostname=\"ccycloud-1.renkoike.root.comops.site\", instance=\"ccycloud-1.renkoike.root.comops.site:9876\", job=\"ozone\", processname=\"StorageContainerManager\", sessionid=\"null\"}"
Copy link
Contributor

Choose a reason for hiding this comment

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

We may utilize {{hostname}} instead for all instances where we are hardcoding the hostnames (Ref: link).

@len548
Copy link
Contributor Author

len548 commented Sep 23, 2024

@tanvipenumudy @kerneltime Hi, I merged some changes based on the comments and would like you to review it. Thank you for your time!

Copy link
Contributor

@kerneltime kerneltime left a comment

Choose a reason for hiding this comment

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

Minor Nit: The Legend is bloated with too much text and you can clean it up similar to
image

Which fields you want to include is up to you and what you see as useful

},
"disableTextWrap": false,
"editorMode": "builder",
"expr": "jvm_metrics_cpu_jvm_load{instance=\"om:9874\"}",
Copy link
Contributor

Choose a reason for hiding this comment

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

},
"disableTextWrap": false,
"editorMode": "builder",
"expr": "jvm_metrics_mem_heap_used_m{processname=\"OzoneManager\"}",
Copy link
Contributor

Choose a reason for hiding this comment

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

For memory you can select the unit to help plotting the Y axis
image
image

},
"disableTextWrap": false,
"editorMode": "builder",
"expr": "om_metrics_num_keys",
Copy link
Contributor

Choose a reason for hiding this comment

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

You can select the unit as StandardOptions:Misc:Short to see the numbers of millions or billions of files
image

@adoroszlai
Copy link
Contributor

/pending review comments

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Marking this issue as un-mergeable as requested.

Please use /ready comment when it's resolved.

Please note that the PR will be closed after 21 days of inactivity from now. (But can be re-opened anytime later...)

review comments

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

Successfully merging this pull request may close these issues.

5 participants