From 2fc896b45a48381373a5cd834cc24af1db8f6a2e Mon Sep 17 00:00:00 2001 From: Patrick Jahns Date: Sun, 26 Apr 2020 22:20:06 +0200 Subject: [PATCH 1/2] test: improved tests Added another scenario where values are unexpected, to validate that this will not cause issues when parsing --- pkg/openvpn/parser_test.go | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/pkg/openvpn/parser_test.go b/pkg/openvpn/parser_test.go index 427d9ff..d5711fd 100644 --- a/pkg/openvpn/parser_test.go +++ b/pkg/openvpn/parser_test.go @@ -91,3 +91,29 @@ func TestConnectedClientsParsedCorrectly(t *testing.T) { t.Errorf("Clients are not parsed correctly") } } + +const badFields = `OpenVPN CLIENT LIST +Updated,test +Common Name,Real Address,Bytes Received,Bytes Sent,Connected Since +user1,1.2.3.4,foo,foo,test +ROUTING TABLE +Virtual Address,Common Name,Real Address,Last Ref +10.240.1.222,user4,1.2.3.7:fooo,test +GLOBAL STATS +Max bcast/mcast queue length,foo +END +` + +func TestParsingWrongValuesIsNotAnIssue(t *testing.T) { + status, e := parse(bufio.NewReader(strings.NewReader(badFields))) + if e != nil { + t.Errorf("should have worked") + } + if status.GlobalStats.MaxBcastMcastQueueLen != 0 { + t.Errorf("Parsing wrong MaxBcastMcastQueueLen value lead to unexpected result") + } + expectedTime := time.Time{} + if !expectedTime.Equal(status.UpdatedAt) { + t.Errorf("parsing incorrect time value should have yieleded a default time object") + } +} From 5a1ac966ca4760ce34d497f6c7631ac5acce0114 Mon Sep 17 00:00:00 2001 From: Patrick Jahns Date: Sun, 26 Apr 2020 22:44:35 +0200 Subject: [PATCH 2/2] feat: ability to disable per client metrics As the common_name can have a high cardinality on high traffic openvpn servers with a lot of clients, this allows to drop that metric and only expose metrics with a lower cardinality --- pkg/collector/openvpn.go | 56 ++++++++++++++++++++++------------------ pkg/command/command.go | 10 +++++-- pkg/config/config.go | 12 ++++++--- 3 files changed, 48 insertions(+), 30 deletions(-) diff --git a/pkg/collector/openvpn.go b/pkg/collector/openvpn.go index 227a1cf..30413f8 100644 --- a/pkg/collector/openvpn.go +++ b/pkg/collector/openvpn.go @@ -12,6 +12,7 @@ type OpenVPNCollector struct { logger log.Logger name string statusFile string + collectClientMetrics bool LastUpdated *prometheus.Desc ConnectedClients *prometheus.Desc BytesReceived *prometheus.Desc @@ -21,11 +22,12 @@ type OpenVPNCollector struct { } // NewOpenVPNCollector returns a new OpenVPNCollector -func NewOpenVPNCollector(logger log.Logger, name string, statusFile string) *OpenVPNCollector { +func NewOpenVPNCollector(logger log.Logger, name string, statusFile string, collectClientMetrics bool) *OpenVPNCollector { return &OpenVPNCollector{ - logger: logger, - statusFile: statusFile, - name: name, + logger: logger, + statusFile: statusFile, + name: name, + collectClientMetrics: collectClientMetrics, LastUpdated: prometheus.NewDesc( prometheus.BuildFQName(namespace, "", "last_updated"), @@ -70,10 +72,12 @@ func NewOpenVPNCollector(logger log.Logger, name string, statusFile string) *Ope func (c *OpenVPNCollector) Describe(ch chan<- *prometheus.Desc) { ch <- c.LastUpdated ch <- c.ConnectedClients - ch <- c.BytesSent - ch <- c.BytesReceived - ch <- c.ConnectedSince ch <- c.MaxBcastMcastQueueLen + if c.collectClientMetrics { + ch <- c.BytesSent + ch <- c.BytesReceived + ch <- c.ConnectedSince + } } // Collect is called by the Prometheus registry when collecting metrics. @@ -100,24 +104,26 @@ func (c *OpenVPNCollector) Collect(ch chan<- prometheus.Metric) { "bytesReceived", client.BytesReceived, "bytesSent", client.BytesSent, ) - ch <- prometheus.MustNewConstMetric( - c.BytesReceived, - prometheus.GaugeValue, - client.BytesReceived, - c.name, client.CommonName, - ) - ch <- prometheus.MustNewConstMetric( - c.BytesSent, - prometheus.GaugeValue, - client.BytesSent, - c.name, client.CommonName, - ) - ch <- prometheus.MustNewConstMetric( - c.ConnectedSince, - prometheus.GaugeValue, - float64(client.ConnectedSince.Unix()), - c.name, client.CommonName, - ) + if c.collectClientMetrics { + ch <- prometheus.MustNewConstMetric( + c.BytesReceived, + prometheus.GaugeValue, + client.BytesReceived, + c.name, client.CommonName, + ) + ch <- prometheus.MustNewConstMetric( + c.BytesSent, + prometheus.GaugeValue, + client.BytesSent, + c.name, client.CommonName, + ) + ch <- prometheus.MustNewConstMetric( + c.ConnectedSince, + prometheus.GaugeValue, + float64(client.ConnectedSince.Unix()), + c.name, client.CommonName, + ) + } } level.Debug(c.logger).Log( "updatedAt", status.UpdatedAt, diff --git a/pkg/command/command.go b/pkg/command/command.go index 7d47eb1..7bda351 100644 --- a/pkg/command/command.go +++ b/pkg/command/command.go @@ -68,10 +68,15 @@ func Run() error { Usage: "The OpenVPN status file(s) to export (example test:./example/version1.status )", Required: true, }, + &cli.BoolFlag{ + Name: "disable-client-metrics", + Usage: "Disables per client (bytes_received, bytes_sent, connected_since) metrics", + }, } app.Before = func(c *cli.Context) error { - cfg.StatusFile = c.StringSlice("status-file") + cfg.StatusCollector.StatusFile = c.StringSlice("status-file") + cfg.StatusCollector.ExportClientMetrics = !c.Bool("disable-client-metrics") return nil } @@ -105,7 +110,7 @@ func run(c *cli.Context, cfg *config.Config) error { version.GoVersion, version.Started, )) - for _, statusFile := range cfg.StatusFile { + for _, statusFile := range cfg.StatusCollector.StatusFile { serverName, statusFile := parseStatusFileSlice(statusFile) level.Info(logger).Log( @@ -117,6 +122,7 @@ func run(c *cli.Context, cfg *config.Config) error { logger, serverName, statusFile, + cfg.StatusCollector.ExportClientMetrics, )) } diff --git a/pkg/config/config.go b/pkg/config/config.go index ac1bfab..860bef9 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -14,9 +14,15 @@ type Logs struct { // Config defines the general configuration object type Config struct { - Server Server - Logs Logs - StatusFile []string + Server Server + Logs Logs + StatusCollector StatusCollector +} + +// StatusCollector contains configuration for the OpenVPN status collector +type StatusCollector struct { + ExportClientMetrics bool + StatusFile []string } // Load initializes a default configuration struct.