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-12446. Add a Grafana dashboard for low level RocksDB operations. #7992

Merged
merged 3 commits into from
Mar 18, 2025

Conversation

jojochuang
Copy link
Contributor

What changes were proposed in this pull request?

HDDS-12446. Add a Grafana dashboard for low level RocksDB operations.

Please describe your PR in detail:

  • Add a Grafana dashboard to expose Ozone Manager RocksDB statistics.
  • Some of the metrics require ozone.metastore.rocksdb.statistics ALL or EXCEPT_DETAILED_TIMERS (default is OFF).

What is the link to the Apache JIRA

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

(Please replace this section with the link to the Apache JIRA)

How was this patch tested?

Please see the screenshots:
Screenshot 2025-02-28 at 5 01 19 PM
Screenshot 2025-02-28 at 5 01 34 PM
Screenshot 2025-02-28 at 5 01 49 PM
Screenshot 2025-02-28 at 5 02 02 PM
Screenshot 2025-02-28 at 5 02 12 PM
Screenshot 2025-02-28 at 5 02 22 PM

Change-Id: I8a40918a8e8a4b9d63e2d8253859a84742d3a412
@jojochuang
Copy link
Contributor Author

cc @muskan1012 @Tejaskriya

@kerneltime
Copy link
Contributor

While importing, I had to delete the UID for the data source for the data to show up. Can you try importing to a different Grafana cluster (local dev setup works as well) to filter out the deployment specific json.

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 @jojochuang for the patch, please find a few minor comments

{
"datasource": {
"type": "prometheus",
"uid": "beegq3lpjhvcwf"
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

{
"datasource": {
"type": "prometheus",
"uid": "beegq3lpjhvcwf"
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 for all other instances!

"timepicker": {},
"timezone": "browser",
"title": "OM RocksDB",
"uid": "feegs4uzd60aoe",
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 exclude the dashboard-level uid field as well!

"options": {
"mode": "exclude",
"names": [
"{__name__=\"rocksdb_om_db_db_seek_median\", hostname=\"ccycloud-1.weichiu.root.comops.site\", instance=\"ccycloud-1.weichiu.root.comops.site:9874\", job=\"ozone\"}"
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).

Change-Id: Ibd9635d6fa67bbac5c19a144a3af3eb155b1a705
@jojochuang
Copy link
Contributor Author

Hi Tanvi, thanks for tips. Please check again if this is okay.

Copy link
Contributor

@Tejaskriya Tejaskriya left a comment

Choose a reason for hiding this comment

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

Thanks for working on this @jojochuang , I have left a few comments below regarding excluding hosts for the timeseries graphs

Change-Id: I3359245c3982fa045bad387dd21b15fc94a346b7
@jojochuang
Copy link
Contributor Author

jojochuang commented Mar 11, 2025

Thanks @Tejaskriya for spotting that. Not sure if this is right but I manually removed the overrides in the json output. Re-imported it and looks fine to me.

Copy link
Contributor

@Tejaskriya Tejaskriya left a comment

Choose a reason for hiding this comment

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

Thanks for making the changes, LGTM!

@jojochuang jojochuang merged commit 1a9d9f7 into apache:master Mar 18, 2025
14 checks passed
@jojochuang
Copy link
Contributor Author

Merged. Thanks @Tejaskriya @tanvipenumudy @kerneltime for review!

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.

4 participants