Skip to content
This repository has been archived by the owner on Mar 26, 2018. It is now read-only.

Expose blocked connection metrics #23

Merged
merged 3 commits into from
Oct 19, 2016
Merged

Conversation

awh
Copy link
Contributor

@awh awh commented Oct 14, 2016

Fixes #7; depends on weaveworks/weave#2549. See commit messages for implementation details.

# HELP weavenpc_blocked_connections_total Connection attempts blocked by policy controller.
# TYPE weavenpc_blocked_connections_total counter
weavenpc_blocked_connections{dport="80",protocol="tcp"} 1
weavenpc_blocked_connections{dport="53",protocol="udp"} 1

@awh awh added this to the 1.8.0 milestone Oct 14, 2016
@awh awh self-assigned this Oct 14, 2016
@awh awh changed the title Issues/7 blocked connection metrics Expose blocked connection metrics Oct 14, 2016
@awh awh removed their assignment Oct 18, 2016
http.Handle("/metrics", promhttp.Handler())

go func() {
if err := http.ListenAndServe(":8686", nil); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably the port could be passed as a parameter?

@awh awh assigned brb Oct 19, 2016
func gatherMetrics() {
pipe, err := os.Open("/var/log/ulogd.pcap")
if err != nil {
log.Fatal(err)
Copy link
Contributor

Choose a reason for hiding this comment

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

For a debug purpose, I'd add some context to logging messages, e.g. log.Fatal("cannot open /var/log/ulogd.pcap: %s", err).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Turns out it's already contextualised:

2016-10-19 12:29:12.213983 I | open /var/log/ulogd.pcap: no such file or directory

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I meant all occurrences of thelog.Fatal statement.

@brb brb assigned awh and unassigned brb Oct 19, 2016
@awh awh force-pushed the issues/7-blocked-connection-metrics branch 2 times, most recently from 7b89ee4 to 4ad1ea2 Compare October 19, 2016 13:48
Include the ulogd Alpine package in the container image, along with a
configuration file that receives NFLOG messages from group 86 and logs
them in PCAP dump format to a named pipe in a well known location.
Exec the ulogd daemon on weave-npc startup, copying any log output to
our own logs. Exit if the ulogd process terminates for any reason.
@awh awh force-pushed the issues/7-blocked-connection-metrics branch from 4ad1ea2 to 7d2b417 Compare October 19, 2016 14:04
@awh
Copy link
Contributor Author

awh commented Oct 19, 2016

I have added --metrics-addr and contextualised all the log messages - PTAL @brb

@awh awh assigned brb and unassigned awh Oct 19, 2016
@@ -158,3 +172,16 @@ func main() {
signal.Notify(signals, syscall.SIGINT, syscall.SIGTERM)
log.Fatalf("Exiting: %v", <-signals)
}

var RootCmd = &cobra.Command{
Copy link
Contributor

@brb brb Oct 19, 2016

Choose a reason for hiding this comment

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

nitpicking: s/RootCmd/rootCmd ?

@brb brb assigned awh and unassigned brb Oct 19, 2016
@brb
Copy link
Contributor

brb commented Oct 19, 2016

LGTM, just one nitpick.

Read and parse packets from the ulogd named pipe, and serve statistics
via the Prometheus client.
@awh awh force-pushed the issues/7-blocked-connection-metrics branch from 7d2b417 to 57b6621 Compare October 19, 2016 15:17
@awh awh assigned brb and unassigned awh Oct 19, 2016
@awh
Copy link
Contributor Author

awh commented Oct 19, 2016

Nit picked!

@brb brb removed their assignment Oct 19, 2016
@brb brb merged commit 026e057 into master Oct 19, 2016
@awh awh deleted the issues/7-blocked-connection-metrics branch October 19, 2016 15:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants