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

common/perf_counters: make schema more friendly and update docs #14933

Merged
merged 4 commits into from May 17, 2017

Conversation

Projects
None yet
3 participants
@liewegas
Member

liewegas commented May 3, 2017

I just spoke with a user yesterday who was complaining about the lack of documentation for the metrics. (The docs exist, but they're stale; when I showed him the ideally self-documenting 'perf schema' output it wasn't obvious what was a guage or metric.)

@liewegas liewegas requested a review from branch-predictor May 3, 2017

@liewegas

This comment has been minimized.

Member

liewegas commented May 3, 2017

@branch-predictor Do you mind submitting a patch to the doc file similarly documenting the 'perf historgram (schema|dump)' commands?

@branch-predictor

This comment has been minimized.

Member

branch-predictor commented May 4, 2017

@liewegas sure, on it.

@liewegas

This comment has been minimized.

Member

liewegas commented May 4, 2017

@branch-predictor

This comment has been minimized.

Member

branch-predictor commented May 10, 2017

@liewegas liewegas requested review from tchaikov and badone and removed request for tchaikov May 12, 2017

@badone

@liewegas Sent you a PR for the test failure.

@@ -39,89 +39,121 @@ The ``perf schema`` command dumps a json description of which values are availab
+------+-------------------------------------+

This comment has been minimized.

@badone

badone May 15, 2017

Contributor

Typo in the commit message?

@liewegas

This comment has been minimized.

Member

liewegas commented May 15, 2017

@badone

This comment has been minimized.

Contributor

badone commented May 15, 2017

@liewegas there is a failing test (unittest_perf_counters), see my comment above.

int idx, const char *name,
PerfHistogramCommon::axis_config_d x_axis_config,
PerfHistogramCommon::axis_config_d y_axis_config,
const char *description, const char *nick, int prio)
{
add_impl(idx, name, description, nick, prio,
PERFCOUNTER_U64 | PERFCOUNTER_HISTOGRAM,
add_impl(idx, name, description, nick, prior,

This comment has been minimized.

@badone

badone May 15, 2017

Contributor

Should be prio?

liewegas added some commits May 3, 2017

common/perf_counters: histograms are counters
The histogram is a histogram of counters (not guages), so set the COUNTER
bit.  (This only matters because we expose and document the bits of the
type, for better for for worse.)

Signed-off-by: Sage Weil <sage@redhat.com>
common/perf_counters: make 'perf schema' more friendly
The raw bits are pretty hard to interpret and the documentation isn't
super easy to find.  Expose a string description instead.

Signed-off-by: Sage Weil <sage@redhat.com>
common/perf_counters: rename histogram adder method
Specifically, it's a u64 counter histogram.

Signed-off-by: Sage Weil <sage@redhat.com>
doc/dev/perf_counters: update docs for new friendly properties
Note that histograms are not yet documented.

Signed-off-by: Sage Weil <sage@redhat.com>
@badone

badone approved these changes May 16, 2017

LGTM

@liewegas liewegas merged commit 35834a3 into ceph:master May 17, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment