status: export metrics about MemStats into timeseries #119819

lyang24 · 2024-03-01T18:49:15Z

This commit exposes 5 metrics into cockroachdb's RuntimeStatSampler timeseries. The added metrics are MemStackSysBytes, HeapFragmentBytes, HeapReservedBytes, HeapReleasedBytes, TotalAlloc. These metrics are derived from rumtime/metrics.

Fixes: #96717

Relase note: None

blathers-crl · 2024-03-01T18:49:19Z

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

Thank you for contributing to CockroachDB. Please ensure you have followed the guidelines for creating a PR.

Before a member of our team reviews your PR, I have some potential action items for you:

Please ensure your git commit message contains a release note.
When CI has completed, please ensure no errors have appeared.

I have added a few people who may be able to assist in reviewing:

@1lann (author of obs: export metrics about MemStats into timeseries #96717)

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.}

cockroach-teamcity · 2024-03-01T18:49:23Z

This change is

blathers-crl · 2024-03-12T21:42:06Z

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

Thank you for updating your pull request.

Before a member of our team reviews your PR, I have some potential action items for you:

Please ensure your git commit message contains a release note.
When CI has completed, please ensure no errors have appeared.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.}

blathers-crl · 2024-03-13T04:22:42Z

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

Thank you for updating your pull request.

Before a member of our team reviews your PR, I have some potential action items for you:

Please ensure your git commit message contains a release note.
When CI has completed, please ensure no errors have appeared.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.}

blathers-crl · 2024-03-13T07:51:33Z

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

Thank you for updating your pull request.

Before a member of our team reviews your PR, I have some potential action items for you:

Please ensure your git commit message contains a release note.
When CI has completed, please ensure no errors have appeared.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.}

blathers-crl · 2024-03-14T20:57:01Z

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

Thank you for updating your pull request.

Before a member of our team reviews your PR, I have some potential action items for you:

Please ensure your git commit message contains a release note.
When CI has completed, please ensure no errors have appeared.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.}

nvanbenschoten

Reviewed all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @1lann, @lyang24, and @xinhaoz)

pkg/server/status/runtime.go line 160 at r1 (raw file):

		Unit:        metric.Unit_BYTES,
	}
	metaMemStackSysBytes = metric.Metadata{

These metrics are all coming from the go runtime. Let's make the following changes:

move them up below metaGoTotalBytes
rename them to start with metaGo
rename the metrics to start with sys.go.
rename the gauges to start with Go
move the corresponding metric.Gauges up below GoTotalBytes.
move the corresponding metric.Gauge construction in up NewRuntimeStatSampler up below GoTotalBytes: ....
move the corresponding metric.Gauge.Update calls up

pkg/server/status/runtime.go line 184 at r1 (raw file):

		Unit:        metric.Unit_BYTES,
	}
	metaTotalAlloc = metric.Metadata{

metaGoTotalAllocBytes

Same thing with the gauge, TotalAllocBytes.

pkg/server/status/runtime.go line 371 at r1 (raw file):

// Stack memory allocated by the underlying operating system.
// In non-cgo programs this metric is currently zero. This may
// change in the future.In cgo programs this metric includes

nit: missing space after future.

pkg/server/status/runtime.go line 874 at r1 (raw file):

	rsr.TotalMemBytes.Update(totalMem)
	rsr.MemStackSysBytes.Update(int64(osStackBytes))
	rsr.HeapFragmentBytes.Update(int64(rsr.goRuntimeSampler.uint64(runtimeMetricHeapFragmentBytes)))

We're already fetching these first three from the goRuntimeSampler above. Should we pull them out into variables?

blathers-crl · 2024-03-15T16:58:52Z

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

Thank you for updating your pull request.

Before a member of our team reviews your PR, I have some potential action items for you:

Please ensure your git commit message contains a release note.
When CI has completed, please ensure no errors have appeared.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.}

blathers-crl · 2024-03-15T17:01:09Z

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

Thank you for updating your pull request.

Before a member of our team reviews your PR, I have some potential action items for you:

Please ensure your git commit message contains a release note.
When CI has completed, please ensure no errors have appeared.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.}

lyang24

Thank you for review, I have addressed the comments please take another look.

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @1lann and @xinhaoz)

blathers-crl · 2024-03-15T17:04:08Z

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

Thank you for updating your pull request.

Before a member of our team reviews your PR, I have some potential action items for you:

Please ensure your git commit message contains a release note.
When CI has completed, please ensure no errors have appeared.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.}

nvanbenschoten

once the remaining comments are resolved.

Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @1lann, @lyang24, and @xinhaoz)

pkg/server/status/runtime.go line 184 at r1 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

metaGoTotalAllocBytes

Same thing with the gauge, TotalAllocBytes.

Looks like this comment was missed.

pkg/server/status/runtime.go line 69 at r2 (raw file):

	metaGoMemStackSysBytes = metric.Metadata{
		Name:        "sys.go.stack.systembytes",
		Help:        "Stack memories obtained from the OS.",

s/memories/memory/

pkg/server/status/runtime.go line 75 at r2 (raw file):

	metaGoHeapFragmentBytes = metric.Metadata{
		Name:        "sys.go.heap.heapfragmentbytes",
		Help:        "Total heap fragmentation bytes, derived from bytes in in-use spans subtracts bytes allocated",

s/subtracts/minus/g

blathers-crl · 2024-03-15T17:25:12Z

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

Thank you for updating your pull request.

Before a member of our team reviews your PR, I have some potential action items for you:

Please ensure your git commit message contains a release note.
When CI has completed, please ensure no errors have appeared.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.}

This commit exposes 5 metrics into cockroachdb's RuntimeStatSampler timeseries. The added metrics are MemStackSysBytes, HeapFragmentBytes, HeapReservedBytes, HeapReleasedBytes, TotalAlloc. These metrics are derived from rumtime/metrics. Fixes: cockroachdb#96717 Relase note: None

blathers-crl · 2024-03-15T17:27:47Z

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

Thank you for updating your pull request.

Before a member of our team reviews your PR, I have some potential action items for you:

Please ensure your git commit message contains a release note.
When CI has completed, please ensure no errors have appeared.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.}

lyang24

Appreciated the reviews!

Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @1lann, @nvanbenschoten, and @xinhaoz)

pkg/server/status/runtime.go line 184 at r1 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

Looks like this comment was missed.

indeed thanks for catching

lyang24 · 2024-03-15T19:43:06Z

bors r+

craig · 2024-03-15T20:37:22Z

Build succeeded:

blathers-crl bot added O-community Originated from the community X-blathers-triaged blathers was able to find an owner labels Mar 1, 2024

blathers-crl bot requested a review from 1lann March 1, 2024 18:49

lyang24 force-pushed the go_memstats2 branch from cfbfc14 to c26465c Compare March 12, 2024 21:42

lyang24 force-pushed the go_memstats2 branch from c26465c to 659eb15 Compare March 13, 2024 04:22

lyang24 requested a review from nvanbenschoten March 13, 2024 04:27

lyang24 force-pushed the go_memstats2 branch from 659eb15 to 7a1f96f Compare March 13, 2024 07:51

lyang24 marked this pull request as ready for review March 13, 2024 07:53

lyang24 requested a review from a team March 13, 2024 07:53

lyang24 requested a review from a team as a code owner March 13, 2024 07:53

lyang24 requested review from xinhaoz and removed request for a team March 13, 2024 07:53

lyang24 force-pushed the go_memstats2 branch from 7a1f96f to f86f758 Compare March 14, 2024 20:56

nvanbenschoten reviewed Mar 15, 2024

View reviewed changes

lyang24 force-pushed the go_memstats2 branch from f86f758 to 1d85398 Compare March 15, 2024 16:58

lyang24 force-pushed the go_memstats2 branch from 1d85398 to 006fb7c Compare March 15, 2024 17:01

lyang24 commented Mar 15, 2024

View reviewed changes

lyang24 force-pushed the go_memstats2 branch from 006fb7c to 6602d67 Compare March 15, 2024 17:04

lyang24 requested a review from nvanbenschoten March 15, 2024 17:04

nvanbenschoten approved these changes Mar 15, 2024

View reviewed changes

lyang24 force-pushed the go_memstats2 branch from 6602d67 to 181e322 Compare March 15, 2024 17:25

lyang24 force-pushed the go_memstats2 branch from 181e322 to 13066e1 Compare March 15, 2024 17:27

lyang24 commented Mar 15, 2024

View reviewed changes

craig bot merged commit ae00dcb into cockroachdb:master Mar 15, 2024
17 of 22 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

status: export metrics about MemStats into timeseries #119819

status: export metrics about MemStats into timeseries #119819

lyang24 commented Mar 1, 2024

blathers-crl bot commented Mar 1, 2024

cockroach-teamcity commented Mar 1, 2024

blathers-crl bot commented Mar 12, 2024

blathers-crl bot commented Mar 13, 2024

blathers-crl bot commented Mar 13, 2024

blathers-crl bot commented Mar 14, 2024

nvanbenschoten left a comment

blathers-crl bot commented Mar 15, 2024

blathers-crl bot commented Mar 15, 2024

lyang24 left a comment

blathers-crl bot commented Mar 15, 2024

nvanbenschoten left a comment

blathers-crl bot commented Mar 15, 2024

blathers-crl bot commented Mar 15, 2024

lyang24 left a comment

lyang24 commented Mar 15, 2024

craig bot commented Mar 15, 2024

status: export metrics about MemStats into timeseries #119819

status: export metrics about MemStats into timeseries #119819

Conversation

lyang24 commented Mar 1, 2024

blathers-crl bot commented Mar 1, 2024

cockroach-teamcity commented Mar 1, 2024

blathers-crl bot commented Mar 12, 2024

blathers-crl bot commented Mar 13, 2024

blathers-crl bot commented Mar 13, 2024

blathers-crl bot commented Mar 14, 2024

nvanbenschoten left a comment

Choose a reason for hiding this comment

blathers-crl bot commented Mar 15, 2024

blathers-crl bot commented Mar 15, 2024

lyang24 left a comment

Choose a reason for hiding this comment

blathers-crl bot commented Mar 15, 2024

nvanbenschoten left a comment

Choose a reason for hiding this comment

blathers-crl bot commented Mar 15, 2024

blathers-crl bot commented Mar 15, 2024

lyang24 left a comment

Choose a reason for hiding this comment

lyang24 commented Mar 15, 2024

craig bot commented Mar 15, 2024