Added virtual memory scraper to hostmetrics receiver #989

james-bebbington · 2020-05-19T08:01:57Z

Link to tracking Issue:
#847

Description:
Added virtual memory scraper to the hostmetricsreceiver, which has significanly differnet logic for Windows & Linux.

On windows, scrape pagefile usage (a.k.a. swap usage) metric via a system call and paging rate (major only) metrics via performance counters.
On non-windows, get swap usage, paging & page fault rates from gopsutil.

Screenshot of Metrics on Windows:

codecov-commenter · 2020-05-19T10:13:32Z

Codecov Report

Merging #989 into master will increase coverage by 0.10%.
The diff coverage is 98.24%.

@@            Coverage Diff             @@
##           master     #989      +/-   ##
==========================================
+ Coverage   85.63%   85.73%   +0.10%     
==========================================
  Files         189      192       +3     
  Lines       13167    13275     +108     
==========================================
+ Hits        11275    11381     +106     
- Misses       1439     1441       +2     
  Partials      453      453

Impacted Files	Coverage Δ
...rtualmemoryscraper/virtualmemory_scraper_others.go	`97.43% <97.43%> (ø)`
receiver/hostmetricsreceiver/factory.go	`73.58% <100.00%> (+0.50%)`	⬆️
...r/internal/scraper/virtualmemoryscraper/factory.go	`100.00% <100.00%> (ø)`
...er/virtualmemoryscraper/virtualmemory_constants.go	`100.00% <100.00%> (ø)`
service/service.go	`49.09% <0.00%> (-0.73%)`	⬇️
translator/internaldata/resource_to_oc.go	`76.47% <0.00%> (+2.94%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 118e5d4...d6940e5. Read the comment docs.

receiver/hostmetricsreceiver/internal/scraper/virtualmemoryscraper/virtualmemory_constants.go

james-bebbington · 2020-05-21T07:29:30Z

@bogdandrutu @jrcamp

I made some pretty significant changes as per Jay's comments if you could take another look:

Changed terminology from "page file" -> "swap" for better consistency across Windows/Linux
Calculate page file usage using a syscall instead of performance counter
Derive paging (swap) usage cumulative values from paging per second performance counter instead of reporting per second values

...r/hostmetricsreceiver/internal/scraper/virtualmemoryscraper/virtualmemory_scraper_windows.go

james-bebbington · 2020-05-21T07:38:52Z

receiver/hostmetricsreceiver/internal/scraper/virtualmemoryscraper/pagefile_windows.go

+var (
+	modPsapi           = windows.NewLazySystemDLL("psapi.dll")
+	procEnumPageFilesW = modPsapi.NewProc("EnumPageFilesW")
+)


Ideally everything in this file will get upstreamed to gopsutil. I'll create a PR in that repo as well and update here if that gets merged. Added note to #973.

james-bebbington · 2020-05-21T09:37:26Z

Regarding coverage, is it possible to override the new "codecov/patch" gate in some cases?

The problem here is the generated cover report is only based on Linux, and this PR pretty much exclusively adds code & tests for Windows, with a few lines of scaffolding for Linux. It would seem a bit overkill to hack this PR to increase coverage for that (see the codecode diff).

Coverage of the virtualmemoryscraper package on Windows is:

> go-acc .
ok      go.opentelemetry.io/collector/receiver/hostmetricsreceiver/internal/scraper/virtualmemoryscraper        0.392s  coverage: 95.3

Edit: I've added Linux support for virtual memory metrics in this PR, so this comment isn't relevant anymore

jrcamp

Looks good!

receiver/hostmetricsreceiver/internal/scraper/virtualmemoryscraper/pagefile_windows.go

...r/hostmetricsreceiver/internal/scraper/virtualmemoryscraper/virtualmemory_scraper_windows.go

james-bebbington · 2020-05-25T07:25:41Z

...er/hostmetricsreceiver/internal/scraper/virtualmemoryscraper/virtualmemory_scraper_others.go

+	idps := metric.Int64DataPoints()
+	idps.Resize(1)
+	initializePageFaultDataPoint(idps.At(0), startTime, minorTypeLabelValue, int64(swap.PgFault))
+	// TODO add swap.PgMajFault once available in gopsutil


shirou/gopsutil#882

james-bebbington · 2020-05-25T07:26:44Z

...r/hostmetricsreceiver/internal/scraper/virtualmemoryscraper/virtualmemory_scraper_windows.go

+
+// Initialize
+func (s *scraper) Initialize(_ context.Context) error {
+	s.startTime = pdata.TimestampUnixNano(uint64(time.Now().UnixNano()))


Note since we are deriving the counter from per second values, the counter essentially starts from when we take the first reading, thus why I set this to the current time here. I believe that's correct?

james-bebbington · 2020-05-25T09:43:13Z

I did one more pass to this PR adding support for Linux metrics (as a second commit). Note the comment above about coverage isn't relevant anymore since I made this change.

@jrcamp - If you could pls take a look at the Linux changes, in particular:

I'm not 100% confident about where I'm getting the swap usage data from (although this seems to match what collectd does)
I changed the metric names/formats slightly from the original docs:
- Combined host/swap/paging & host/memory/paging into host/swap/paging with major/minor labels respectively
- Renamed host/memory/page_faults to host/swap/page_faults (I feel like it could be named either, but like the cleanliness of having all the virtual memory metrics labelled under swap ... but happy to be convinced otherwise)
I haven't added the host/memory/pages metric. Not too sure how important this one is, but it doesn't appear to be currently exposed by gopsutil

jrcamp · 2020-05-26T14:55:33Z

I'm not 100% confident about where I'm getting the swap usage data from (although this seems to match what collectd does)

Looks correct to me based on kernel docs.

Actually their SwapFree description is confusing. It makes it sound like something different but looking at a Linux VM it appears to be what it sounds like from the name (unused swap space).

The rest sounds good!

jrcamp · 2020-05-27T17:52:43Z

@bogdandrutu ptal

bogdandrutu · 2020-05-28T14:06:52Z

receiver/hostmetricsreceiver/internal/scraper/loadscraper/load_scraper_windows_test.go

@@ -39,7 +39,7 @@ func TestStartSampling(t *testing.T) {
 	// override the processor queue length perf counter with a mock
 	// that will ensure a positive value is returned
 	assert.IsType(t, &pdh.PerfCounter{}, samplerInstance.processorQueueLengthCounter)
-	samplerInstance.processorQueueLengthCounter = pdh.NewMockPerfCounter(100)
+	samplerInstance.processorQueueLengthCounter = pdh.NewMockPerfCounter(100.0)


In the future better to send changes unrelated to the main PR separately.

) * Initial commit of host metrics virtual memory scraper for windows * Added non-windows virtual memory scraper implementation

* Change default Sampler to ParentOrElse(AlwaysOn) * add note to changelog

james-bebbington requested review from bogdandrutu, dmitryax, flands, owais, pjanotti, rghetia, songy23 and tigrannajaryan as code owners May 19, 2020 08:01

james-bebbington changed the title ~~Initial commit of host metrics virtual memory scraper for windows~~ Added virtual memory scraper to hostmetrics receiver (Windows only for now) May 19, 2020

james-bebbington force-pushed the hostmetrics-virtualmemory branch 3 times, most recently from a936a5b to 408d495 Compare May 19, 2020 10:04

jrcamp reviewed May 19, 2020

View reviewed changes

tigrannajaryan added this to In progress in Collector via automation May 19, 2020

tigrannajaryan assigned jrcamp May 20, 2020

james-bebbington force-pushed the hostmetrics-virtualmemory branch 3 times, most recently from a6ca503 to ff8690f Compare May 21, 2020 07:25

james-bebbington commented May 21, 2020

View reviewed changes

...r/hostmetricsreceiver/internal/scraper/virtualmemoryscraper/virtualmemory_scraper_windows.go Show resolved Hide resolved

james-bebbington commented May 21, 2020

View reviewed changes

james-bebbington force-pushed the hostmetrics-virtualmemory branch from ff8690f to 1972a0d Compare May 21, 2020 09:17

james-bebbington force-pushed the hostmetrics-virtualmemory branch from 1972a0d to b3fca5c Compare May 21, 2020 12:22

jrcamp reviewed May 21, 2020

View reviewed changes

receiver/hostmetricsreceiver/internal/scraper/virtualmemoryscraper/pagefile_windows.go Outdated Show resolved Hide resolved

...r/hostmetricsreceiver/internal/scraper/virtualmemoryscraper/virtualmemory_scraper_windows.go Show resolved Hide resolved

james-bebbington force-pushed the hostmetrics-virtualmemory branch 4 times, most recently from f9ec8bc to aca3553 Compare May 25, 2020 07:24

james-bebbington commented May 25, 2020

View reviewed changes

james-bebbington changed the title ~~Added virtual memory scraper to hostmetrics receiver (Windows only for now)~~ Added virtual memory scraper to hostmetrics receiver May 25, 2020

james-bebbington force-pushed the hostmetrics-virtualmemory branch 4 times, most recently from 02aad64 to 9d70736 Compare May 25, 2020 07:51

james-bebbington added 2 commits May 26, 2020 17:15

Initial commit of host metrics virtual memory scraper for windows

a813d8a

Added non-windows virtual memory scraper implementation

d6940e5

james-bebbington force-pushed the hostmetrics-virtualmemory branch from 9d70736 to d6940e5 Compare May 26, 2020 07:16

jrcamp approved these changes May 26, 2020

View reviewed changes

jrcamp removed their assignment May 27, 2020

bogdandrutu reviewed May 28, 2020

View reviewed changes

Collector automation moved this from In progress to Reviewer approved May 28, 2020

bogdandrutu approved these changes May 28, 2020

View reviewed changes

bogdandrutu merged commit 7219fb1 into open-telemetry:master May 28, 2020

Collector automation moved this from Reviewer approved to Done May 28, 2020

flands added this to the Beta 0.4 milestone Jun 4, 2020

MovieStoreGuy pushed a commit to atlassian-forks/opentelemetry-collector that referenced this pull request Nov 11, 2021

Change default Sampler to ParentOrElse(AlwaysOn) (open-telemetry#989)

1299210

* Change default Sampler to ParentOrElse(AlwaysOn) * add note to changelog

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added virtual memory scraper to hostmetrics receiver #989

Added virtual memory scraper to hostmetrics receiver #989

james-bebbington commented May 19, 2020 •

edited

codecov-commenter commented May 19, 2020 •

edited by codecov bot

james-bebbington commented May 21, 2020 •

edited

james-bebbington May 21, 2020 •

edited

james-bebbington commented May 21, 2020 •

edited

jrcamp left a comment

james-bebbington May 25, 2020

james-bebbington May 25, 2020

james-bebbington commented May 25, 2020

jrcamp commented May 26, 2020

jrcamp commented May 27, 2020

bogdandrutu May 28, 2020

Added virtual memory scraper to hostmetrics receiver #989

Added virtual memory scraper to hostmetrics receiver #989

Conversation

james-bebbington commented May 19, 2020 • edited

codecov-commenter commented May 19, 2020 • edited by codecov bot

Codecov Report

james-bebbington commented May 21, 2020 • edited

james-bebbington May 21, 2020 • edited

Choose a reason for hiding this comment

james-bebbington commented May 21, 2020 • edited

jrcamp left a comment

Choose a reason for hiding this comment

james-bebbington May 25, 2020

Choose a reason for hiding this comment

james-bebbington May 25, 2020

Choose a reason for hiding this comment

james-bebbington commented May 25, 2020

jrcamp commented May 26, 2020

jrcamp commented May 27, 2020

bogdandrutu May 28, 2020

Choose a reason for hiding this comment

james-bebbington commented May 19, 2020 •

edited

codecov-commenter commented May 19, 2020 •

edited by codecov bot

james-bebbington commented May 21, 2020 •

edited

james-bebbington May 21, 2020 •

edited

james-bebbington commented May 21, 2020 •

edited