Perf counters cache + Labeled RGW Op counters #53003
Conversation
This commit contains the following features: - a perf counters cache per CephContext that acts as a wrapper around perf counters for storing and modifying labeled perf counters per CephContext - instrumentation of the rgw with labeled perf counters for the major rgw ops Signed-off-by: Ali Maredia <amaredia@redhat.com>
Signed-off-by: Ali Maredia <amaredia@redhat.com>
- combined frontend counters together - use const string& to cache calls - remove eviction - SetupCounters key is now string_view Signed-off-by: Ali Maredia <amaredia@redhat.com>
Signed-off-by: Ali Maredia <amaredia@redhat.com>
Signed-off-by: Ali Maredia <amaredia@redhat.com>
Signed-off-by: Ali Maredia <amaredia@redhat.com>
Signed-off-by: Ali Maredia <amaredia@redhat.com>
| }, | ||
| "bar": { | ||
| "type": 1, | ||
| "metric_type": "gauge", |
There was a problem hiding this comment.
This means we're getting rid of this counter from schema as well?
There was a problem hiding this comment.
Can you explain in detail why we need the schemas for counters that aren't being dumped? Could you build and run this branch and show me what happens?
Are the schemas being stored in Prometheus too? Making the change to add the send the schemas but not the counters would be a pretty easy change.
There was a problem hiding this comment.
@idryomov Do you have any thoughts on filtering out values from a labeled counter that haven't been initialized yet from the counter dump command? I'm doing this ensure prometheus isn't filled with unitialized data in labeled counter.
Signed-off-by: Ali Maredia <amaredia@redhat.com>
Signed-off-by: Ali Maredia <amaredia@redhat.com>
Signed-off-by: Ali Maredia <amaredia@redhat.com>
Signed-off-by: Ali Maredia <amaredia@redhat.com>
Signed-off-by: Ali Maredia <amaredia@redhat.com>
Signed-off-by: Ali Maredia <amaredia@redhat.com>
Signed-off-by: Ali Maredia <amaredia@redhat.com>
…rusive_lru Signed-off-by: Ali Maredia <amaredia@redhat.com>
Signed-off-by: Ali Maredia <amaredia@redhat.com>
Signed-off-by: Ali Maredia <amaredia@redhat.com>
| add_double_or_int_metric(builder, sum_value, name + "_sum", description + " total", | ||
| metric_type, labels); | ||
| json_value avg_value = perf_values.as_object()["avgtime"]; | ||
| add_double_or_int_metric(builder, avg_value, name + "_avg", description + " average", |
There was a problem hiding this comment.
@cbodley @idryomov This change reflects that were weren't sending the avgtime metric to prometheus from our time based counters that were being dumped via counter dump.
@avanthakkar's rationale for not including these was that the average could be computed in grafana/prometheus from the total time, and the total number of entries also dump in a time based counters.
As tech leads do you have any strong opinion on including or excluding these from what the exporter is sending to Prometheus? I'm inclined to remove it just to send out less data, but I don't feel strongly either way.
Signed-off-by: Ali Maredia <amaredia@redhat.com>
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. "pacific"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.
Checklist
Show available Jenkins commands
jenkins retest this pleasejenkins test classic perfjenkins test crimson perfjenkins test signedjenkins test make checkjenkins test make check arm64jenkins test submodulesjenkins test dashboardjenkins test dashboard cephadmjenkins test apijenkins test docsjenkins render docsjenkins test ceph-volume alljenkins test ceph-volume toxjenkins test windows