DAOS-7203 control: Add histogram support to Prometheus exporter #5382

mjmac · 2021-04-08T21:54:29Z

Update the Prometheus exporter to support passthrough histograms
from native DAOS telemetry format. Fixes a few bugs and inefficiencies
in the native histogram implementation.

daosbuild1

LGTM. No errors found by checkpatch.

daosbuild1

LGTM. No errors found by checkpatch.

daosbuild1

LGTM. No errors found by checkpatch.

daosbuild1

LGTM. No errors found by checkpatch.

github-actions · 2024-02-17T01:55:06Z

Bug-tracker data:
Ticket title is 'Create standard DAOS dashboards for Grafana'
Status is 'Resolved'
Labels: 'rel20_TB3,rel20_TB4'
https://daosio.atlassian.net/browse/DAOS-7203

github-actions · 2024-02-17T01:55:59Z

Bug-tracker data:
Ticket title is 'Create standard DAOS dashboards for Grafana'
Status is 'Resolved'
Labels: 'rel20_TB3,rel20_TB4'
https://daosio.atlassian.net/browse/DAOS-7203

mjmac · 2024-02-17T02:07:36Z

github-actions · 2024-02-17T04:56:56Z

Functional on EL 9 Test Results (old)

135 tests ±0 130 ✅ - 1 1h 24m 30s ⏱️ - 7m 9s
41 suites ±0 4 💤 ±0
41 files ±0 0 ❌ ±0 1 🔥 +1

For more details on these errors, see this check.

Results for commit 427bb06. ± Comparison against base commit 9894532.

♻️ This comment has been updated with latest results.

github-actions · 2024-02-17T05:08:09Z

Functional on EL 8.8 Test Results (old)

135 tests ±0 130 ✅ - 1 1h 34m 45s ⏱️ + 5m 40s
41 suites ±0 4 💤 ±0
41 files ±0 0 ❌ ±0 1 🔥 +1

For more details on these errors, see this check.

Results for commit 427bb06. ± Comparison against base commit 9894532.

♻️ This comment has been updated with latest results.

github-actions · 2024-02-20T23:19:36Z

Bug-tracker data:
Ticket title is 'Create standard DAOS dashboards for Grafana'
Status is 'Resolved'
Labels: 'rel20_TB3,rel20_TB4'
https://daosio.atlassian.net/browse/DAOS-7203

github-actions · 2024-02-20T23:21:17Z

Bug-tracker data:
Ticket title is 'Create standard DAOS dashboards for Grafana'
Status is 'Resolved'
Labels: 'rel20_TB3,rel20_TB4'
https://daosio.atlassian.net/browse/DAOS-7203

github-actions · 2024-02-21T12:23:55Z

Functional Hardware Large Test Results (old)

64 tests ±0 64 ✅ ±0 28m 16s ⏱️ -26s
14 suites ±0 0 💤 ±0
14 files ±0 0 ❌ ±0

Results for commit 427bb06. ± Comparison against base commit 9894532.

♻️ This comment has been updated with latest results.

github-actions · 2024-02-21T13:33:23Z

Functional Hardware Medium Test Results (old)

130 tests ±0 103 ✅ - 1 2h 12m 10s ⏱️ + 2m 18s
34 suites ±0 26 💤 ±0
34 files ±0 1 ❌ +1

For more details on these failures, see this check.

Results for commit 427bb06. ± Comparison against base commit 9894532.

♻️ This comment has been updated with latest results.

github-actions · 2024-02-21T15:09:54Z

Functional Hardware Medium Verbs Provider Test Results (old)

55 tests ±0 54 ✅ ±0 4h 7m 30s ⏱️ -1s
7 suites ±0 1 💤 ±0
7 files ±0 0 ❌ ±0

Results for commit 427bb06. ± Comparison against base commit 9894532.

♻️ This comment has been updated with latest results.

github-actions · 2024-02-21T21:14:23Z

Bug-tracker data:
Ticket title is 'Create standard DAOS dashboards for Grafana'
Status is 'Resolved'
Labels: 'rel20_TB3,rel20_TB4'
https://daosio.atlassian.net/browse/DAOS-7203

github-actions · 2024-02-21T21:23:50Z

Bug-tracker data:
Ticket title is 'Create standard DAOS dashboards for Grafana'
Status is 'Resolved'
Labels: 'rel20_TB3,rel20_TB4'
https://daosio.atlassian.net/browse/DAOS-7203

github-actions · 2024-05-03T16:00:08Z

Bug-tracker data:
Ticket title is 'Create standard DAOS dashboards for Grafana'
Status is 'Resolved'
Labels: 'rel20_TB3,rel20_TB4'
https://daosio.atlassian.net/browse/DAOS-7203

github-actions · 2024-05-03T16:00:24Z

Ticket title is 'Create standard DAOS dashboards for Grafana'
Status is 'Resolved'
Labels: 'rel20_TB3,rel20_TB4'
https://daosio.atlassian.net/browse/DAOS-7203

daosbuild1 · 2024-05-03T19:00:07Z

Test stage Functional on EL 8.8 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-5382/11/execution/node/1177/log

daltonbohning · 2024-05-03T19:08:55Z

FYI - I think the dkey_akey_enum_punch.py failures are being fixed in #14301

daltonbohning · 2024-05-03T19:11:04Z

FYI - I think the dkey_akey_enum_punch.py failures are being fixed in #14301

Actually, sorry. That looks like a different issue

Update the Prometheus exporter to support passthrough histograms from native DAOS telemetry format. Fixes a few bugs and inefficiencies in the native histogram implementation. Features: telemetry Required-githooks: true Change-Id: I7842cc48a107ec0ba0ec93472fb6684db7394d30 Signed-off-by: Michael MacDonald <mjmac@google.com>

daosbuild1 · 2024-11-01T03:32:54Z

Test stage Functional Hardware Medium completed with status UNSTABLE. https://build.hpdd.intel.com/job/daos-stack/job/daos//view/change-requests/job/PR-5382/14/testReport/

mjmac · 2024-11-02T17:25:12Z

Note to reviewers... This is finally in a state where I think it's ready for landing. I reworked the patch a bit. The original version changed the fetch/update bytes counters to histograms in addition to adding all of the logic for exporting via Prometheus. I've reverted that part of it, which should make this patch less risky. Longer term, I would like to revisit that other change, but I'll do that separately so that we can evaluate it in isolation.

kjacque

Nice work. I just had a couple minor comments, which I wouldn't block on.

kjacque · 2024-11-06T01:47:15Z

src/gurt/telemetry.c

@@ -3200,6 +3226,8 @@ d_tm_init_histogram(struct d_tm_node_t *node, char *path, int num_buckets,
 		rc = -DER_NO_SHMEM;
 		goto failure;
 	}
+	histogram->dth_fast_bucketing =
+	    (multiplier == 2 && (initial_width & (initial_width - 1)) == 0);


Would be good to describe this special case in a comment. Is this saying that the initial width is also a power of two?

to me it reads "set dth_fast_bucketing iff mult is 2 and least-significant-bit of initial_width is set, is that correct?

I'll add a comment in a subsequent patch. The idea is easier to see with an example. Say initial_width is 256. As a bit pattern, that value looks like this: 0b100000000. Then, if we subtract 1 from it, we get this pattern: 0b11111111. If we AND those together, we get 0. This only works for numbers that are clean powers of 2.

Even easier to see if we "stack" the two values:

100000000 11111111

When you do add the comment, it would be great to detail not only the meaning of the bitwise math (although maybe that would be most clarified by using a named helper function or macro), but also why fast bucketing only works in the case where everything is a power of two. I think detailing the principle behind it will be most helpful in the future.

kjacque · 2024-11-07T16:01:52Z

src/control/lib/telemetry/promexp/source.go

+			if err != nil {
+				log.Errorf("[%s]: failed to get histogram buckets", baseName)


Would it be useful to log the actual error here too?

kjacque · 2024-11-07T16:04:42Z

src/control/lib/telemetry/promexp/engine_test.go

+			cmpOpts := cmp.Options{
+				cmpopts.SortSlices(func(a, b string) bool {
+					return a < b
+				}),
+			}


Nice! IMO we should be using this in all our unit tests instead of always sorting the values we return in production code. In most cases the order really doesn't matter. To that end, I think it would be nice to add some kind of utility function in the test package for this cmp option.

tanabarr

Nice work! nothing blocking

tanabarr · 2024-11-08T10:20:30Z

src/control/lib/telemetry/duration.go

@@ -25,8 +25,11 @@ import (
 	"time"
 )

+var _ StatsMetric = (*Duration)(nil)
+


nit: missing description comment for exported type

It's not an exported type, it's an assertion that Duration implements StatsMetric.

the comment is referring to "Duration" type below

tanabarr · 2024-11-08T10:22:01Z

src/control/lib/telemetry/histogram.go

+	Values  []uint64
+}
+
+// Histogram provides access to histogram data associated with


nit: unnecessary line break

tanabarr · 2024-11-09T13:02:02Z

src/gurt/telemetry.c

@@ -3200,6 +3226,8 @@ d_tm_init_histogram(struct d_tm_node_t *node, char *path, int num_buckets,
 		rc = -DER_NO_SHMEM;
 		goto failure;
 	}
+	histogram->dth_fast_bucketing =
+	    (multiplier == 2 && (initial_width & (initial_width - 1)) == 0);


to me it reads "set dth_fast_bucketing iff mult is 2 and least-significant-bit of initial_width is set, is that correct?

Update the Prometheus exporter to support passthrough histograms from native DAOS telemetry format. Fixes a few bugs and inefficiencies in the native histogram implementation. Signed-off-by: Michael MacDonald <mjmac@google.com>

… (#15484) Update the Prometheus exporter to support passthrough histograms from native DAOS telemetry format. Fixes a few bugs and inefficiencies in the native histogram implementation. Signed-off-by: Michael MacDonald <mjmac@google.com>

mjmac self-assigned this Apr 8, 2021

daosbuild1 reviewed Apr 8, 2021

View reviewed changes

daosbuild1 reviewed Apr 10, 2021

View reviewed changes

daosbuild1 reviewed Apr 17, 2021

View reviewed changes

mjmac force-pushed the mjmac/DAOS-7203 branch from 209de92 to c6c92f2 Compare April 23, 2021 16:13

daosbuild1 reviewed Apr 23, 2021

View reviewed changes

mjmac force-pushed the mjmac/DAOS-7203 branch from c6c92f2 to 2b06d36 Compare February 17, 2024 01:54

mjmac mentioned this pull request Feb 20, 2024

DAOS-8331 client: add client side metrics #13517

Closed

18 tasks

mjmac force-pushed the mjmac/DAOS-7203 branch from 2b06d36 to 540a8ca Compare February 20, 2024 23:19

mjmac changed the base branch from master to mjmac/agent_prom February 20, 2024 23:20

mjmac force-pushed the mjmac/agent_prom branch from bfc9b03 to 9894532 Compare February 21, 2024 16:12

mjmac force-pushed the mjmac/DAOS-7203 branch from 540a8ca to 1de99d7 Compare February 21, 2024 21:14

mjmac force-pushed the mjmac/DAOS-7203 branch from 1de99d7 to 427bb06 Compare February 21, 2024 21:23

mjmac force-pushed the mjmac/DAOS-7203 branch from 427bb06 to b74fdf0 Compare May 3, 2024 15:53

mjmac changed the base branch from mjmac/agent_prom to master May 3, 2024 15:53

mjmac force-pushed the mjmac/DAOS-7203 branch 2 times, most recently from 5fdafd4 to 5ac09d7 Compare October 31, 2024 19:55

mjmac force-pushed the mjmac/DAOS-7203 branch from 5ac09d7 to 757bddc Compare October 31, 2024 20:12

mjmac marked this pull request as ready for review November 2, 2024 17:22

mjmac requested review from a team as code owners November 2, 2024 17:22

kjacque approved these changes Nov 8, 2024

View reviewed changes

tanabarr approved these changes Nov 9, 2024

View reviewed changes

mjmac merged commit 248bb75 into master Nov 9, 2024
55 checks passed

mjmac deleted the mjmac/DAOS-7203 branch November 9, 2024 15:29

mjmac mentioned this pull request Nov 9, 2024

mjmac/DAOS 7203 google 2.6 #15483

Closed

This was referenced Nov 13, 2024

mjmac/DAOS 16787 google 2.6 #15498

Closed

DAOS-16787 utils: Suppress NLT valgrind false positives (#15478) #15499

Merged

DAOS-16791 control: Add include_fabric_ifaces to agent config (#15470) #15513

Merged

This was referenced Nov 25, 2024

mjmac/DAOS 16355 google 2.6 #15535

Closed

DAOS-16355 client: pydaos.torch module (#15475) #15536

Merged

mjmac mentioned this pull request Dec 2, 2024

DAOS-16822 client: Add mkdir_p variant to dfs_sys API (#15526) #15550

Merged

		if err != nil {
		log.Errorf("[%s]: failed to get histogram buckets", baseName)

DAOS-7203 control: Add histogram support to Prometheus exporter #5382

DAOS-7203 control: Add histogram support to Prometheus exporter #5382

Conversation

mjmac commented Apr 8, 2021 • edited Loading

daosbuild1 left a comment

Choose a reason for hiding this comment

daosbuild1 left a comment

Choose a reason for hiding this comment

daosbuild1 left a comment

Choose a reason for hiding this comment

daosbuild1 left a comment

Choose a reason for hiding this comment

github-actions bot commented Feb 17, 2024

github-actions bot commented Feb 17, 2024

mjmac commented Feb 17, 2024 • edited Loading

github-actions bot commented Feb 17, 2024 • edited Loading

Functional on EL 9 Test Results (old)

github-actions bot commented Feb 17, 2024 • edited Loading

Functional on EL 8.8 Test Results (old)

github-actions bot commented Feb 20, 2024

github-actions bot commented Feb 20, 2024

github-actions bot commented Feb 21, 2024 • edited Loading

Functional Hardware Large Test Results (old)

github-actions bot commented Feb 21, 2024 • edited Loading

Functional Hardware Medium Test Results (old)

github-actions bot commented Feb 21, 2024 • edited Loading

Functional Hardware Medium Verbs Provider Test Results (old)

github-actions bot commented Feb 21, 2024

github-actions bot commented Feb 21, 2024

github-actions bot commented May 3, 2024

github-actions bot commented May 3, 2024

daosbuild1 commented May 3, 2024

daltonbohning commented May 3, 2024

daltonbohning commented May 3, 2024

daosbuild1 commented Nov 1, 2024

mjmac commented Nov 2, 2024

kjacque left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tanabarr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mjmac commented Apr 8, 2021 •

edited

Loading

mjmac commented Feb 17, 2024 •

edited

Loading

github-actions bot commented Feb 17, 2024 •

edited

Loading

github-actions bot commented Feb 17, 2024 •

edited

Loading

github-actions bot commented Feb 21, 2024 •

edited

Loading

github-actions bot commented Feb 21, 2024 •

edited

Loading

github-actions bot commented Feb 21, 2024 •

edited

Loading