Add linux conntrack metrics to host metrics receiver #3370

emilgelman · 2021-06-06T08:03:09Z

Description:
Added support for collecting conntrack metrics (linux only) using the host metrics receiver. The metrics are taken from https://github.com/shirou/gopsutil (already being used in the receiver)

emilgelman · 2021-06-06T08:45:32Z

@mx-psi
Reopened in a new PR to fix the EasyCLA issues.
Please review

mx-psi

I left some comments :)

receiver/hostmetricsreceiver/README.md

receiver/hostmetricsreceiver/internal/scraper/networkscraper/config.go

receiver/hostmetricsreceiver/internal/scraper/networkscraper/factory.go

receiver/hostmetricsreceiver/internal/scraper/networkscraper/network_linux.go

mx-psi · 2021-06-11T11:11:18Z

receiver/hostmetricsreceiver/internal/scraper/networkscraper/network_scraper.go

 }

 // newNetworkScraper creates a set of Network related metrics
 func newNetworkScraper(_ context.Context, cfg *Config) (*scraper, error) {
-	scraper := &scraper{config: cfg, bootTime: host.BootTime, ioCounters: net.IOCounters, connections: net.Connections}
+	scraper := &scraper{config: cfg, bootTime: host.BootTime, ioCounters: net.IOCounters, connections: net.Connections, conntrack: net.FilterCounters}


Should we use FilterCountersWithContext instead, so that we can pass the context in case we want to have cancellation at some point?

Not super convinced that we should do this here (IOCounters also has IOCountersWithContext and we are not using it), so maybe you can just open an issue and we can deal with it in a separate PR.

receiver/hostmetricsreceiver/metadata.yaml

mx-psi · 2021-06-11T11:21:44Z

receiver/hostmetricsreceiver/metadata.yaml

@@ -293,6 +293,22 @@ metrics:
      monotonic: false
    labels: [network.protocol, network.state]

+  system.network.conntrack.count:


I am not knowledgeable enough about macOS or Windows to know if there are equivalents to conntrack in these OSs. It might be a good idea to check this and make the name more general in case we want to add support for other platforms later.

A good way to get feedback on this is to open a PR on the https://github.com/open-telemetry/opentelemetry-specification repository so that other people can chime in.

receiver/hostmetricsreceiver/internal/scraper/networkscraper/network_scraper_test.go

mx-psi · 2021-06-16T10:56:36Z

Unit tests are failing with

--- FAIL: TestGatherMetrics_EndToEnd (0.17s)
    hostmetrics_receiver_test.go:177: 
        	Error Trace:	hostmetrics_receiver_test.go:177
        	            				hostmetrics_receiver_test.go:152
        	            				asm_amd64.s:1371
        	Error:      	Not equal: 
        	            	expected: 26
        	            	actual  : 24
        	Test:       	TestGatherMetrics_EndToEnd
    hostmetrics_receiver_test.go:179: 
        	Error Trace:	hostmetrics_receiver_test.go:179
        	            				hostmetrics_receiver_test.go:152
        	            				asm_amd64.s:1371
        	Error:      	map[string]struct {}{"system.cpu.load_average.15m":struct {}{}, "system.cpu.load_average.1m":struct {}{}, "system.cpu.load_average.5m":struct {}{}, "system.cpu.time":struct {}{}, "system.disk.io":struct {}{}, "system.disk.io_time":struct {}{}, "system.disk.merged":struct {}{}, "system.disk.operation_time":struct {}{}, "system.disk.operations":struct {}{}, "system.disk.pending_operations":struct {}{}, "system.disk.weighted_io_time":struct {}{}, "system.filesystem.inodes.usage":struct {}{}, "system.filesystem.usage":struct {}{}, "system.memory.usage":struct {}{}, "system.network.connections":struct {}{}, "system.network.dropped":struct {}{}, "system.network.errors":struct {}{}, "system.network.io":struct {}{}, "system.network.packets":struct {}{}, "system.paging.faults":struct {}{}, "system.paging.operations":struct {}{}, "system.paging.usage":struct {}{}, "system.processes.count":struct {}{}, "system.processes.created":struct {}{}} does not contain "system.conntrack.count"
        	Test:       	TestGatherMetrics_EndToEnd
    hostmetrics_receiver_test.go:179: 
        	Error Trace:	hostmetrics_receiver_test.go:179
        	            				hostmetrics_receiver_test.go:152
        	            				asm_amd64.s:1371
        	Error:      	map[string]struct {}{"system.cpu.load_average.15m":struct {}{}, "system.cpu.load_average.1m":struct {}{}, "system.cpu.load_average.5m":struct {}{}, "system.cpu.time":struct {}{}, "system.disk.io":struct {}{}, "system.disk.io_time":struct {}{}, "system.disk.merged":struct {}{}, "system.disk.operation_time":struct {}{}, "system.disk.operations":struct {}{}, "system.disk.pending_operations":struct {}{}, "system.disk.weighted_io_time":struct {}{}, "system.filesystem.inodes.usage":struct {}{}, "system.filesystem.usage":struct {}{}, "system.memory.usage":struct {}{}, "system.network.connections":struct {}{}, "system.network.dropped":struct {}{}, "system.network.errors":struct {}{}, "system.network.io":struct {}{}, "system.network.packets":struct {}{}, "system.paging.faults":struct {}{}, "system.paging.operations":struct {}{}, "system.paging.usage":struct {}{}, "system.processes.count":struct {}{}, "system.processes.created":struct {}{}} does not contain "system.conntrack.max"
        	Test:       	TestGatherMetrics_EndToEnd
FAIL

emilgelman · 2021-06-16T11:03:45Z

@mx-psi First of all, thank you for the review.
I've also opened a PR in the specification repo:
open-telemetry/opentelemetry-specification#1761

Still trying to understand why the e2e test fails, running the same docker image with this branch seems to work fine.

emilgelman · 2021-06-16T20:09:15Z

@mx-psi Please review again.
I wasn't able to validate the new conntrack metrics in TestGatherMetrics_EndToEnd
I tried reproducing the CI flow using cimg/go:1.16, and it does work locally. However, it seems like the netfilter files are missing when executed through the CI:
https://app.circleci.com/pipelines/github/open-telemetry/opentelemetry-collector/8159/workflows/4cca0725-3807-4123-821d-17a5faa63eb9/jobs/88360

        	Error:      	Received unexpected error:
        	            	open /proc/sys/net/netfilter/nf_conntrack_count: no such file or directory

Although they exist:

➜  ~ docker run  cimg/go:1.16 ls -la  /proc/sys/net/netfilter/nf_conntrack_count

-r--r--r-- 1 root root 0 Jun 16 20:08 /proc/sys/net/netfilter/nf_conntrack_count
➜  ~ docker run  cimg/go:1.16 ls -la  /proc/sys/net/netfilter/nf_conntrack_max

-rw-r--r-- 1 root root 0 Jun 16 20:08 /proc/sys/net/netfilter/nf_conntrack_max

I believe it happens because the nf_conntrack kernel module isn't loaded on the builder host of CircleCI.

mx-psi

Looks good, just one documentation comment! I think it's fine to not have the conntrack end to end test (but maintainers may disagree with me)

receiver/hostmetricsreceiver/README.md

mx-psi

Looks good to me! Now an approver/maintainer should have a look; remember that the focus is on getting to GA so please be patient; if this doesn't get attention feel free to raise this on the CNCF Slack

github-actions · 2021-06-30T03:16:03Z

This PR was marked stale due to lack of activity. It will be closed in 7 days.

mx-psi · 2021-06-30T07:38:37Z

Not stale but I don't have permissions to remove the label (I hope this comment is enough to mark it as active)

github-actions · 2021-07-12T03:16:45Z

This PR was marked stale due to lack of activity. It will be closed in 7 days.

albertvaka · 2021-07-15T21:31:27Z

Hi! I'm commenting to remove the stale label. mx-psi is on vacation but will be reviewing this when he's back :)

mx-psi · 2021-07-27T07:20:21Z

@emilgelman can you update the PR with the new Number types as was done in #3710?

codeboten

The code looks good, I added some suggestions to change the code from IntSum to Sum as per @mx-psi's recommendation, please take a look.

codeboten · 2021-08-04T15:45:20Z

receiver/hostmetricsreceiver/internal/scraper/conntrackscraper/conntrack_scraper_test.go

+
+func assertNetworkConntrackMetricValid(t *testing.T, metric pdata.Metric, descriptor pdata.Metric) {
+	internal.AssertDescriptorEqual(t, descriptor, metric)
+	assert.Equal(t, 1, metric.IntSum().DataPoints().Len())


Suggested change

assert.Equal(t, 1, metric.IntSum().DataPoints().Len())

assert.Equal(t, 1, metric.Sum().DataPoints().Len())

codeboten · 2021-08-04T15:45:46Z

receiver/hostmetricsreceiver/internal/scraper/conntrackscraper/conntrack_scraper_linux.go

+func initializeNetworkConntrackMetric(metric pdata.Metric, metricIntf metadata.MetricIntf, now pdata.Timestamp, value int64) {
+	metricIntf.Init(metric)
+
+	idps := metric.IntSum().DataPoints()


Suggested change

idps := metric.IntSum().DataPoints()

idps := metric.Sum().DataPoints()

codeboten · 2021-08-04T15:46:13Z

receiver/hostmetricsreceiver/internal/scraper/conntrackscraper/conntrack_scraper_linux.go

+func initializeNetworkConntrackDataPoint(dataPoint pdata.IntDataPoint, now pdata.Timestamp, value int64) {
+	dataPoint.SetTimestamp(now)
+	dataPoint.SetValue(value)


Suggested change

func initializeNetworkConntrackDataPoint(dataPoint pdata.IntDataPoint, now pdata.Timestamp, value int64) {

dataPoint.SetTimestamp(now)

dataPoint.SetValue(value)

func initializeNetworkConntrackDataPoint(dataPoint pdata.NumberDataPoint, now pdata.Timestamp, value int64) {

dataPoint.SetTimestamp(now)

dataPoint.SetIntVal(value)

codeboten · 2021-08-04T15:47:52Z

receiver/hostmetricsreceiver/metadata.yaml

+    description: Number of currently allocated flow entries.
+    unit: "{flow entries}"
+    data:
+      type: int sum


Suggested change

type: int sum

type: sum

codeboten · 2021-08-04T15:48:01Z

receiver/hostmetricsreceiver/metadata.yaml

+    description: Size of connection tracking table.
+    unit: "{flow entries}"
+    data:
+      type: int sum


Suggested change

type: int sum

type: sum

codeboten · 2021-08-04T15:49:48Z

receiver/hostmetricsreceiver/internal/metadata/generated_metrics.go

@@ -59,6 +59,8 @@ type metricStruct struct {
 	ProcessDiskIo               MetricIntf


This will need to be regenerated after the suggestions are merged to change from IntSum to Sum

github-actions · 2021-08-12T03:15:57Z

This PR was marked stale due to lack of activity. It will be closed in 7 days.

github-actions · 2021-08-23T03:15:51Z

This PR was marked stale due to lack of activity. It will be closed in 7 days.

bogdandrutu · 2021-08-24T15:06:58Z

@emilgelman sorry, unfortunately this needs to be moved to contrib, since we moved the hostmetricsreceiver there.

Add linux conntrack metrics to host metrics receiver

b984ea0

emilgelman requested a review from a team as a code owner June 6, 2021 08:03

emilgelman requested a review from dmitryax June 6, 2021 08:03

mx-psi reviewed Jun 11, 2021

View reviewed changes

code review comments

3d5428a

alolita added the area:receiver label Jun 15, 2021

emilgelman mentioned this pull request Jun 15, 2021

Add Network conntrack metrics open-telemetry/opentelemetry-specification#1761

Closed

emilgelman added 4 commits June 15, 2021 15:07

move conntrack metrics to a seperate scraper

407b328

cleanup

099c3c1

Merge branch 'main' into linux_conntrack_metrics

5504647

cleanup

8247289

emilgelman added 8 commits June 16, 2021 14:26

fix test

0c6639f

fix test

18300db

fix test

4794650

fix test

b598fd1

debug test

7d6bbcf

debug e2e test

06177a0

debug e2e test

03bb0be

remove conntrack from e2e test

635de31

mx-psi reviewed Jun 22, 2021

View reviewed changes

receiver/hostmetricsreceiver/README.md Outdated Show resolved Hide resolved

emilgelman added 3 commits June 22, 2021 18:59

update readme

60bef2a

Merge branch 'main' into linux_conntrack_metrics

41a4a52

fix go.sum

fab5584

mx-psi approved these changes Jun 22, 2021

View reviewed changes

github-actions bot added the Stale label Jun 30, 2021

github-actions bot removed the Stale label Jul 1, 2021

emilgelman mentioned this pull request Jul 4, 2021

add linux conntrack metrics open-telemetry/opentelemetry-specification#1789

Closed

rename conntrack metrics based on specification suggestion

3b380d2

github-actions bot added the Stale label Jul 12, 2021

github-actions bot removed the Stale label Jul 16, 2021

codeboten reviewed Aug 4, 2021

View reviewed changes

github-actions bot added the Stale label Aug 12, 2021

bogdandrutu removed the Stale label Aug 16, 2021

github-actions bot added the Stale label Aug 23, 2021

bogdandrutu removed the Stale label Aug 24, 2021

bogdandrutu closed this Aug 24, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add linux conntrack metrics to host metrics receiver #3370

Add linux conntrack metrics to host metrics receiver #3370

emilgelman commented Jun 6, 2021

emilgelman commented Jun 6, 2021

mx-psi left a comment

mx-psi Jun 11, 2021

mx-psi Jun 11, 2021

mx-psi commented Jun 16, 2021

emilgelman commented Jun 16, 2021

emilgelman commented Jun 16, 2021 •

edited

mx-psi left a comment

mx-psi left a comment

github-actions bot commented Jun 30, 2021

mx-psi commented Jun 30, 2021

github-actions bot commented Jul 12, 2021

albertvaka commented Jul 15, 2021

mx-psi commented Jul 27, 2021

codeboten left a comment

codeboten Aug 4, 2021

codeboten Aug 4, 2021

codeboten Aug 4, 2021

codeboten Aug 4, 2021

codeboten Aug 4, 2021

codeboten Aug 4, 2021

github-actions bot commented Aug 12, 2021

github-actions bot commented Aug 23, 2021

bogdandrutu commented Aug 24, 2021

	assert.Equal(t, 1, metric.IntSum().DataPoints().Len())
	assert.Equal(t, 1, metric.Sum().DataPoints().Len())

	idps := metric.IntSum().DataPoints()
	idps := metric.Sum().DataPoints()

		@@ -59,6 +59,8 @@ type metricStruct struct {
		ProcessDiskIo MetricIntf

Add linux conntrack metrics to host metrics receiver #3370

Add linux conntrack metrics to host metrics receiver #3370

Conversation

emilgelman commented Jun 6, 2021

emilgelman commented Jun 6, 2021

mx-psi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mx-psi commented Jun 16, 2021

emilgelman commented Jun 16, 2021

emilgelman commented Jun 16, 2021 • edited

mx-psi left a comment

Choose a reason for hiding this comment

mx-psi left a comment

Choose a reason for hiding this comment

github-actions bot commented Jun 30, 2021

mx-psi commented Jun 30, 2021

github-actions bot commented Jul 12, 2021

albertvaka commented Jul 15, 2021

mx-psi commented Jul 27, 2021

codeboten 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

github-actions bot commented Aug 12, 2021

github-actions bot commented Aug 23, 2021

bogdandrutu commented Aug 24, 2021

emilgelman commented Jun 16, 2021 •

edited