rgw: change section name of rgw_op counters by alimaredia · Pull Request #54623 · ceph/ceph

alimaredia · 2023-11-23T00:34:56Z

The rgw_op section of counter dump/schema becomes:

rgw_op_global for the global op counters
rgw_op_user for the user labeled counters
rgw_op_bucket for the bucket labeled counters

Contribution Guidelines

To sign and title your commits, please refer to Submitting Patches to Ceph.
If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.
When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an x between the brackets: [x]. Spaces and capitalization matter when checking off items this way.

Checklist

Tracker (select at least one)
- References tracker ticket
- Very recent bug; references commit where it was introduced
- New feature (ticket optional)
- Doc update (no ticket needed)
- Code cleanup (no ticket needed)
Component impact
- Affects Dashboard, opened tracker ticket
- Affects Orchestrator, opened tracker ticket
- No impact that needs to be tracked
Documentation (select at least one)
- Updates relevant documentation
- No doc update is appropriate
Tests (select at least one)
- Includes unit test(s)
- Includes integration test(s)
- Includes bug reproducer
- No tests

Show available Jenkins commands

jenkins retest this please
jenkins test classic perf
jenkins test crimson perf
jenkins test signed
jenkins test make check
jenkins test make check arm64
jenkins test submodules
jenkins test dashboard
jenkins test dashboard cephadm
jenkins test api
jenkins test docs
jenkins render docs
jenkins test ceph-volume all
jenkins test ceph-volume tox
jenkins test windows
jenkins test rook e2e

thotz · 2023-11-23T05:05:53Z

doc/radosgw/metrics.rst


-    "rgw_op": [
+The counters in the ``rgw_op_global`` section reflect the totals of each op metric for a given Ceph Object Gateway.
+The counters in the ``rgw_op_user`` and ``rgw_op_bucket`` sections are labeled counters of op metrics for a user or bucket respectively.


Suggested change

The counters in the ``rgw_op_user`` and ``rgw_op_bucket`` sections are labeled counters of op metrics for a user or bucket respectively.

The counters in the ``rgw_op_user`` and ``rgw_op_bucket`` sections are labelled counters of op metrics for a user or bucket respectively.

Looks like both spellings are correct, it's just a matter of British vs American English: https://www.grammarly.com/blog/labeled-labelled/

@zdover23 do you have an opinion on this spelling?

exactly, we should try to lean into one spelling dialect

FWIW labelled looks wrong to me, and we've tended toward Murican English in the past for docs, so I vote to leave it as it is.

Even though I live in the Commonwealth, I think that American English is in ascendancy. "labelled".

avanthakkar · 2023-11-29T11:03:25Z

Following points came upon discussion with @cloudbehl. @cloudbehl please feel free to add anything I may have missed.

- Lowercase for all labels (User, Bucket)
- Redundant "bucket" for list/del OPs  metrics name like ceph_rgw_op_bucket_list_buckets_lat_count, 
ceph_rgw_op_bucket_list_buckets_lat_sum
ceph_rgw_op_bucket_del_bucket_lat_count
- Also avoid Plural names for  list_buckets metrics (labels: user, bucket, global)
ceph_rgw_op_bucket_list_buckets_lat_count ```

alimaredia · 2023-11-29T18:03:30Z

Following points came upon discussion with @cloudbehl. @cloudbehl please feel free to add anything I may have missed.

- Lowercase for all labels (User, Bucket)
- Redundant "bucket" for list/del OPs  metrics name like ceph_rgw_op_bucket_list_buckets_lat_count, 
ceph_rgw_op_bucket_list_buckets_lat_sum
ceph_rgw_op_bucket_del_bucket_lat_count
- Also avoid Plural names for  list_buckets metrics (labels: user, bucket, global)
ceph_rgw_op_bucket_list_buckets_lat_count ```

I'll go ahead and make the labels lowercase. For the other two suggestions, it's sounds like you have an issue with the names of the perf counters within the RGW.

In the example you gave list_buckets_* and del_bucket_* correspond to the operations for listing of all buckets (not the objects in a specific bucket) and deletion of a bucket.

I don't think it makes sense to change these perf counters internally just because the metrics sent to Prometheus has the word "bucket" twice. Do you have a better name for those perf counters?

cloudbehl · 2023-11-30T06:24:48Z

@alimaredia Thanks for making the change. The naming convention and the metrics, looks good.

Related to the global metrics, do we want to call it global explicitly or can we just use the regular name rgw_op_ for global metrics?

cbodley · 2023-11-30T14:52:30Z

Related to the global metrics, do we want to call it global explicitly or can we just use the regular name rgw_op_ for global metrics?

i don't care for the 'global' part either - it only makes sense if you know there are per-bucket/per-user versions in contrast

what do you think about the names rgw_op, rgw_op_per_user, rgw_op_per_bucket?

alimaredia · 2023-12-12T21:01:57Z

@cloudbehl @avanthakkar updated with name changes

cloudbehl

LGTM, minor nits in docs for the name change.

cloudbehl · 2023-12-13T10:13:26Z

doc/radosgw/metrics.rst

+The sections are ``rgw_op_global``, ``rgw_op_user``, and ``rgw_op_bucket``.

-    "rgw_op": [
+The counters in the ``rgw_op_global`` section reflect the totals of each op metric for a given Ceph Object Gateway.


Suggested change

The counters in the ``rgw_op_global`` section reflect the totals of each op metric for a given Ceph Object Gateway.

The counters in the ``rgw_op`` section reflect the totals of each op metric for a given Ceph Object Gateway.

cloudbehl · 2023-12-13T10:13:39Z

doc/radosgw/metrics.rst

+
+To view op metrics in the Ceph Object Gateway go to the ``rgw_op`` sections of the output of the ``counter dump`` command::
+
+    "rgw_op_global": [


Suggested change

"rgw_op_global": [

"rgw_op": [

cloudbehl · 2023-12-13T10:13:53Z

doc/radosgw/metrics.rst

+The sections are ``rgw_op_global``, ``rgw_op_user``, and ``rgw_op_bucket``.

-    "rgw_op": [
+The counters in the ``rgw_op_global`` section reflect the totals of each op metric for a given Ceph Object Gateway.


Suggested change

The counters in the ``rgw_op_global`` section reflect the totals of each op metric for a given Ceph Object Gateway.

The counters in the ``rgw_op`` section reflect the totals of each op metric for a given Ceph Object Gateway.

cloudbehl

The PR looks good to me.

avanthakkar · 2024-01-05T10:34:40Z

jenkins test api

alimaredia · 2024-01-11T19:08:10Z

@cbodley can I get approval on this PR?

alimaredia · 2024-01-11T19:08:25Z

jenkins test api

alimaredia · 2024-01-12T19:51:00Z

@avanthakkar @cloudbehl for reference could you add some of the reasoning behind the motivation for this PR that you saw when you were building dashboards? Specifically why each section should have a consistent number of labels instead of varying labels.

cloudbehl · 2024-01-23T08:11:54Z

@avanthakkar @cloudbehl for reference could you add some of the reasoning behind the motivation for this PR that you saw when you were building dashboards? Specifically why each section should have a consistent number of labels instead of varying labels.

Hi Ali, We have two major issues with that.

cardinality : Single metric having multiple labels and that increases the cardinality and creates the performance bottleneck and scale issue. As in our case, we were having same 1 metric name that provides global counters, then user counters and bucket counters. When we were adding user metrics then we were adding the user label and when we were having bucket metric then we were having the bucket label and when no label is added then its global. So having such a scenario, increases metrics complexity. So its better to have 1 metric for one kind of data.
Filtering issues: This is the major issue. Having single metric provide every data makes things complex while plotting the data in grafana charts and adding filters based on specific type like user or bucket. Like in our case I have to use regex in our case to make sue if the metric value has bucket label then this is a bucket chart, if the metric value has user label then plot user charts and if none are there then its global. Plus, Grafana provides ways to define variables based on metrics which is not possible to be done if the labels are non-consistent.

alimaredia · 2024-01-24T22:28:55Z

@cbodley anything left to do here you see? should I run this through the verify suite in teuthology?

The rgw_op section of `counter dump/schema` becomes: - rgw_op_global for the global op counters - rgw_op_per_user for the user labeled counters - rgw_op_per_bucket for the bucket labeled counters Signed-off-by: Ali Maredia <amaredia@redhat.com>

Signed-off-by: Ali Maredia <amaredia@redhat.com>

alimaredia · 2024-02-01T13:15:39Z

@cbodley this branched passed a minimal teuthology run: https://pulpito.ceph.com/amaredia-2024-02-01_05:08:51-rgw:verify-wip-rgw-op-metrics-section-rename-distro-default-smithi/

Is this enough or should I do a bigger one?

cbodley · 2024-02-02T19:01:19Z

@cbodley this branched passed a minimal teuthology run: https://pulpito.ceph.com/amaredia-2024-02-01_05:08:51-rgw:verify-wip-rgw-op-metrics-section-rename-distro-default-smithi/

Is this enough or should I do a bigger one?

please run the full rgw suite

alimaredia · 2024-02-05T18:46:49Z

@cbodley here is a larger run of the rgw suite: https://pulpito.ceph.com/amaredia-2024-02-03_20:08:31-rgw-wip-rgw-op-metrics-section-rename-distro-default-smithi/

alimaredia requested review from a team as code owners November 23, 2023 00:34

github-actions bot added documentation rgw labels Nov 23, 2023

thotz reviewed Nov 23, 2023

View reviewed changes

alimaredia force-pushed the wip-rgw-op-metrics-section-rename branch from 606a48c to a59d7d2 Compare December 12, 2023 21:01

cloudbehl reviewed Dec 13, 2023

View reviewed changes

alimaredia force-pushed the wip-rgw-op-metrics-section-rename branch from a59d7d2 to 02029cd Compare December 13, 2023 16:13

cloudbehl approved these changes Jan 4, 2024

View reviewed changes

alimaredia requested a review from cbodley January 4, 2024 14:23

avanthakkar approved these changes Jan 5, 2024

View reviewed changes

cbodley approved these changes Jan 24, 2024

View reviewed changes

cloudbehl mentioned this pull request Jan 25, 2024

mgr/dashboard: Fixing RGW graph panels #55314

Merged

14 tasks

alimaredia added 2 commits January 31, 2024 17:18

rgw: change topic label to lowercase

6a0960b

Signed-off-by: Ali Maredia <amaredia@redhat.com>

alimaredia force-pushed the wip-rgw-op-metrics-section-rename branch from 02029cd to 6a0960b Compare January 31, 2024 22:18

alimaredia merged commit 6576c81 into ceph:main Feb 7, 2024

cloudbehl mentioned this pull request Feb 27, 2024

monitoring/ceph-mixin: Cleanup of variables, queries and tests (to fix showMultiCluster=True) #55495

Merged

14 tasks

avanthakkar mentioned this pull request May 28, 2024

doc/monitoring: update rgw metrics names #57739

Merged

14 tasks

	The counters in the ``rgw_op_user`` and ``rgw_op_bucket`` sections are labeled counters of op metrics for a user or bucket respectively.
	The counters in the ``rgw_op_user`` and ``rgw_op_bucket`` sections are labelled counters of op metrics for a user or bucket respectively.

	The counters in the ``rgw_op_global`` section reflect the totals of each op metric for a given Ceph Object Gateway.
	The counters in the ``rgw_op`` section reflect the totals of each op metric for a given Ceph Object Gateway.


		To view op metrics in the Ceph Object Gateway go to the ``rgw_op`` sections of the output of the ``counter dump`` command::

		"rgw_op_global": [

Conversation

alimaredia commented Nov 23, 2023

Contribution Guidelines

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mattbenjamin Jan 31, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

avanthakkar commented Nov 29, 2023

Uh oh!

alimaredia commented Nov 29, 2023

Uh oh!

cloudbehl commented Nov 30, 2023

Uh oh!

cbodley commented Nov 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alimaredia commented Dec 12, 2023

Uh oh!

cloudbehl left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cloudbehl left a comment

Choose a reason for hiding this comment

Uh oh!

avanthakkar commented Jan 5, 2024

Uh oh!

alimaredia commented Jan 11, 2024

Uh oh!

alimaredia commented Jan 11, 2024

Uh oh!

alimaredia commented Jan 12, 2024

Uh oh!

cloudbehl commented Jan 23, 2024

Uh oh!

alimaredia commented Jan 24, 2024

Uh oh!

alimaredia commented Feb 1, 2024

Uh oh!

cbodley commented Feb 2, 2024

Uh oh!

alimaredia commented Feb 5, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

mattbenjamin Jan 31, 2024 •

edited

Loading

cbodley commented Nov 30, 2023 •

edited

Loading