Skip to content

Added parsing netfilter conntrack statistics from net/stat/nf_conntrack - #254

Merged
pgier merged 1 commit into
prometheus:masterfrom
kozl:conntrack-statistics
Jan 29, 2020
Merged

pgier merged 1 commit into
prometheus:masterfrom
kozl:conntrack-statistics

Conversation

@kozl

@kozl kozl commented Jan 7, 2020

Copy link
Copy Markdown

Added parsing of conntrack statistics from procfs for this PR in node_exporter project: prometheus/node_exporter#1155

@discordianfish please review

@pgier pgier left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks for the contribution! Just had a few minor comments in-line.

Comment thread net_conntrackstat.go Outdated
// A ConntrackStatEntry represents one line from net/stat/nf_conntrack
// and contains netfilter conntrack statistics at one CPU core
type ConntrackStatEntry struct {
CPU int

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is CPU necessary here? Seems like this can be determined by the location in the slice.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, probably this is excessive, removed it.

Comment thread net_conntrackstat.go Outdated
@@ -0,0 +1,154 @@
// Copyright 2019 The Prometheus Authors

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be 2020

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

Comment thread net_conntrackstat_test.go Outdated
@@ -0,0 +1,89 @@
// Copyright 2018 The Prometheus Authors

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be 2020

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@pgier

pgier commented Jan 23, 2020

Copy link
Copy Markdown
Collaborator

Fixes #217

Comment thread net_conntrackstat.go
}

// Parses a ConntrackStatEntry from given array of fields
func parseConntrackStatEntry(fields []string) (*ConntrackStatEntry, error) {

@discordianfish discordianfish Jan 27, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check for length of fields to prevent a runtime panic.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added

@pgier

pgier commented Jan 27, 2020

Copy link
Copy Markdown
Collaborator

Thanks for the update, looks like the new commit doesn't have a signoff, you can either squash it into the first commit, or do a git commit --amend --signoff

@kozl
kozl force-pushed the conntrack-statistics branch from 5089553 to 018e4fe Compare January 27, 2020 23:21
Signed-off-by: Aleksandr Kozlov <avlkozlov@avito.ru>
@kozl
kozl force-pushed the conntrack-statistics branch from 018e4fe to 2285920 Compare January 27, 2020 23:23
@kozl

kozl commented Jan 27, 2020

Copy link
Copy Markdown
Author

Thanks for the update, looks like the new commit doesn't have a signoff, you can either squash it into the first commit, or do a git commit --amend --signoff

👌

@pgier pgier left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@pgier
pgier merged commit 522e93c into prometheus:master Jan 29, 2020
remijouannet pushed a commit to remijouannet/procfs that referenced this pull request Oct 20, 2022
Added parsing netfilter conntrack statistics from net/stat/nf_conntrack
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants