From 1e4da5dac66210a85cf0e3bee739a6839edeeb46 Mon Sep 17 00:00:00 2001
From: Jean Paul Galea <jeanpaul@yubico.com>
Date: Tue, 8 Sep 2015 16:41:15 +0200
Subject: [PATCH] Rewrite ykval-munin-vallatency plugin.

- avoid having to use the same internal and label name,
	as it's problematic.

- internal name has a lot of restrictions:
	s/^[^A-Za-z_]/_/
	s/[^A-Za-z0-9_]/_/g

- which doesn't allow us to show proper label names,
	the names that users will see.

- label displays :80 or :443 depending on scheme.

- avoid ugliness with shortname() and instead use endpoints().
---
 ykval-common.php           | 59 ++++++++++++++++++++++++++++++++++++--
 ykval-munin-vallatency.php | 40 ++++++++++++++------------
 2 files changed, 77 insertions(+), 22 deletions(-)

diff --git a/ykval-common.php b/ykval-common.php
index 241f656..f1c2b2c 100644
--- a/ykval-common.php
+++ b/ykval-common.php
@@ -364,12 +364,57 @@ function total_time ($url)
 }
 
 /**
- * Return the hostname, and if defined the port, for a given URL.
+ * Given a list of urls, create internal and label names for munin.
+ *
+ * @argument $urls
+ * @return array|bool array or false on failure.
+ */
+function endpoints ($urls)
+{
+	$endpoints = array();
+
+	foreach ($urls as $url)
+	{
+		// internal munin name must be a-zA-Z0-9_,
+		//	so sha1 hex should be fine.
+		//
+		// munin also truncates at some length,
+		//	so we just take the first few characters of the hashsum.
+		$internal = substr(sha1($url), 0, 20);
+
+		// actual label name shown for graph values
+		if (($label = hostport($url)) === FALSE)
+		{
+			return false;
+		}
+
+		$endpoints[] = array($internal, $label, $url);
+	}
+
+	// check for truncated sha1 collisions (or actual duplicate URLs!)
+	$internal = array();
+
+	foreach($endpoints as $endpoint)
+	{
+		$internal[] = $endpoint[0];
+	}
+
+	if (count(array_unique($internal)) !== count($endpoints))
+		return false;
+
+	return $endpoints;
+}
+
+/**
+ * Given a URL, return the hostname and port,
+ *	if the port is defined or can be determined from the scheme.
+ *
+ * Otherwise just return the hostname.
  *
  * @argument $url string
  * @return string|bool short name or false on failure
  */
-function shortname ($url)
+function hostport ($url)
 {
 	if (($url = parse_url($url)) === FALSE)
 		return false;
@@ -378,7 +423,15 @@ function shortname ($url)
 		return false;
 
 	if (array_key_exists('port', $url) === TRUE && $url['port'] !== NULL)
-		return $url['host'] . '_' . $url['port'];
+		return $url['host'].':'.$url['port'];
+
+	if (array_key_exists('scheme', $url) === TRUE
+			&& strtolower($url['scheme']) === 'http')
+		return $url['host'].':80';
+
+	if (array_key_exists('scheme', $url) === TRUE
+			&& strtolower($url['scheme']) === 'https')
+		return $url['host'].':443';
 
 	return $url['host'];
 }
diff --git a/ykval-munin-vallatency.php b/ykval-munin-vallatency.php
index 3d5b225..8273bc3 100755
--- a/ykval-munin-vallatency.php
+++ b/ykval-munin-vallatency.php
@@ -37,17 +37,8 @@ set_include_path(implode(PATH_SEPARATOR, array(
 require_once 'ykval-config.php';
 require_once 'ykval-common.php';
 
-$urls = $baseParams['__YKVAL_SYNC_POOL__'];
 
-$shortnames = array_map('shortname', $urls);
-foreach($shortnames as $shortname)
-{
-	if ($shortname === FALSE)
-	{
-		echo "Cannot parse URL from sync pool list\n";
-		exit(1);
-	}
-}
+$urls = $baseParams['__YKVAL_SYNC_POOL__'];
 
 if ($argc == 2 && strcmp($argv[1], 'autoconf') == 0)
 {
@@ -61,6 +52,12 @@ if ($argc == 2 && strcmp($argv[1], 'autoconf') == 0)
 	exit(0);
 }
 
+if (($endpoints = endpoints($urls)) === FALSE)
+{
+	echo "Cannot parse URLs from sync pool list";
+	exit(1);
+}
+
 if ($argc == 2 && strcmp($argv[1], 'config') == 0)
 {
 	echo "multigraph ykval_vallatency\n";
@@ -69,25 +66,30 @@ if ($argc == 2 && strcmp($argv[1], 'config') == 0)
 	echo "graph_category ykval\n";
 	echo "graph_width 400\n";
 
-	foreach ($shortnames as $shortname)
+	foreach ($endpoints as $endpoint)
 	{
-		echo "${shortname}_avgwait.label ${shortname}\n";
-		echo "${shortname}_avgwait.type GAUGE\n";
-		echo "${shortname}_avgwait.info Average VAL round-trip latency\n";
-		echo "${shortname}_avgwait.min 0\n";
-		echo "${shortname}_avgwait.draw LINE1\n";
+		list($internal, $label, $url) = $endpoint;
+
+		echo "${internal}_avgwait.label ${label}\n";
+		echo "${internal}_avgwait.type GAUGE\n";
+		echo "${internal}_avgwait.info Average VAL round-trip latency\n";
+		echo "${internal}_avgwait.min 0\n";
+		echo "${internal}_avgwait.draw LINE1\n";
 	}
 
 	exit(0);
 }
 
 echo "multigraph ykval_vallatency\n";
-foreach ($urls as $url)
+
+foreach ($endpoints as $endpoint)
 {
-	$shortname = shortname($url);
+	list ($internal, $label, $url) = $endpoint;
 
 	if (($total_time = total_time($url)) === FALSE)
 		$total_time = 'error';
 
-	echo "${shortname}_avgwait.value ${total_time}\n";
+	echo "${internal}_avgwait.value ${total_time}\n";
 }
+
+exit(0);