From 33aeb1cb5c4e796e06cfd3b39db280a7cf162738 Mon Sep 17 00:00:00 2001
From: Matthew Holt <mholt@users.noreply.github.com>
Date: Fri, 23 Mar 2018 23:44:16 -0600
Subject: [PATCH] telemetry: Add CLI option to selectively disable some metrics

Also fix a couple metrics that were named wrong or reported in excess.
---
 caddy.go                |  2 +-
 caddy/caddymain/run.go  | 22 +++++++++++++---------
 caddytls/setup.go       |  2 --
 telemetry/collection.go | 11 ++++++++++-
 telemetry/telemetry.go  | 20 +++++++++++++++-----
 5 files changed, 39 insertions(+), 18 deletions(-)

diff --git a/caddy.go b/caddy.go
index 851e5315f..cc9dfee45 100644
--- a/caddy.go
+++ b/caddy.go
@@ -617,7 +617,7 @@ func ValidateAndExecuteDirectives(cdyfile Input, inst *Instance, justValidate bo
 		return fmt.Errorf("error inspecting server blocks: %v", err)
 	}
 
-	telemetry.Set("http_num_server_blocks", len(sblocks))
+	telemetry.Set("num_server_blocks", len(sblocks))
 
 	return executeDirectives(inst, cdyfile.Path(), stype.Directives(), sblocks, justValidate)
 }
diff --git a/caddy/caddymain/run.go b/caddy/caddymain/run.go
index cd1b47409..1d9f29573 100644
--- a/caddy/caddymain/run.go
+++ b/caddy/caddymain/run.go
@@ -46,6 +46,7 @@ func init() {
 	flag.StringVar(&caddytls.DefaultCAUrl, "ca", "https://acme-v01.api.letsencrypt.org/directory", "URL to certificate authority's ACME server directory")
 	flag.BoolVar(&caddytls.DisableHTTPChallenge, "disable-http-challenge", caddytls.DisableHTTPChallenge, "Disable the ACME HTTP challenge")
 	flag.BoolVar(&caddytls.DisableTLSSNIChallenge, "disable-tls-sni-challenge", caddytls.DisableTLSSNIChallenge, "Disable the ACME TLS-SNI challenge")
+	flag.StringVar(&disabledMetrics, "disabled-metrics", "", "Comma-separated list of telemetry metrics to disable")
 	flag.StringVar(&conf, "conf", "", "Caddyfile to load (default \""+caddy.DefaultConfigFile+"\")")
 	flag.StringVar(&cpu, "cpu", "100%", "CPU cap")
 	flag.BoolVar(&plugins, "plugins", false, "List installed plugins")
@@ -91,6 +92,8 @@ func Run() {
 	// initialize telemetry client
 	if enableTelemetry {
 		initTelemetry()
+	} else if disabledMetrics != "" {
+		mustLogFatalf("[ERROR] Cannot disable specific metrics because telemetry is disabled")
 	}
 
 	// Check for one-time actions
@@ -326,21 +329,22 @@ func initTelemetry() {
 		}
 	}
 
-	telemetry.Init(id)
+	telemetry.Init(id, strings.Split(disabledMetrics, ","))
 }
 
 const appName = "Caddy"
 
 // Flags that control program flow or startup
 var (
-	serverType string
-	conf       string
-	cpu        string
-	logfile    string
-	revoke     string
-	version    bool
-	plugins    bool
-	validate   bool
+	serverType      string
+	conf            string
+	cpu             string
+	logfile         string
+	revoke          string
+	version         bool
+	plugins         bool
+	validate        bool
+	disabledMetrics string
 )
 
 // Build information obtained with the help of -ldflags
diff --git a/caddytls/setup.go b/caddytls/setup.go
index 8100088b3..ef29ed2e0 100644
--- a/caddytls/setup.go
+++ b/caddytls/setup.go
@@ -254,7 +254,6 @@ func setupTLS(c *caddy.Controller) error {
 				return c.Errf("Unable to load certificate and key files for '%s': %v", c.Key, err)
 			}
 			log.Printf("[INFO] Successfully loaded TLS assets from %s and %s", certificateFile, keyFile)
-			telemetry.Increment("tls_manual_cert_count")
 		}
 
 		// load a directory of certificates, if specified
@@ -355,7 +354,6 @@ func loadCertsInDir(cfg *Config, c *caddy.Controller, dir string) error {
 				return c.Errf("%s: failed to load cert and key for '%s': %v", path, c.Key, err)
 			}
 			log.Printf("[INFO] Successfully loaded TLS assets from %s", path)
-			telemetry.Increment("tls_manual_cert_count")
 		}
 		return nil
 	})
diff --git a/telemetry/collection.go b/telemetry/collection.go
index 3e7e2fc5b..91183f83b 100644
--- a/telemetry/collection.go
+++ b/telemetry/collection.go
@@ -28,7 +28,11 @@ import (
 // may safely be used. If this function is not
 // called, the collector functions may still be
 // invoked, but they will be no-ops.
-func Init(instanceID uuid.UUID) {
+//
+// Any metrics keys that are passed in the second
+// argument will be permanently disabled for the
+// lifetime of the process.
+func Init(instanceID uuid.UUID, disabledMetricsKeys []string) {
 	if enabled {
 		panic("already initialized")
 	}
@@ -37,6 +41,11 @@ func Init(instanceID uuid.UUID) {
 		panic("empty UUID")
 	}
 	instanceUUID = instanceID
+	disabledMetricsMu.Lock()
+	for _, key := range disabledMetricsKeys {
+		disabledMetrics[key] = false
+	}
+	disabledMetricsMu.Unlock()
 	enabled = true
 }
 
diff --git a/telemetry/telemetry.go b/telemetry/telemetry.go
index bc193e316..6a83b2fa2 100644
--- a/telemetry/telemetry.go
+++ b/telemetry/telemetry.go
@@ -158,12 +158,15 @@ func emit(final bool) error {
 		// update the list of enabled/disabled keys, if any
 		for _, key := range reply.EnableKeys {
 			disabledMetricsMu.Lock()
-			delete(disabledMetrics, key)
+			// only re-enable this metric if it is temporarily disabled
+			if temp, ok := disabledMetrics[key]; ok && temp {
+				delete(disabledMetrics, key)
+			}
 			disabledMetricsMu.Unlock()
 		}
 		for _, key := range reply.DisableKeys {
 			disabledMetricsMu.Lock()
-			disabledMetrics[key] = struct{}{}
+			disabledMetrics[key] = true // all remotely-disabled keys are "temporarily" disabled
 			disabledMetricsMu.Unlock()
 		}
 
@@ -359,10 +362,17 @@ var (
 	updateTimer   *time.Timer
 	updateTimerMu sync.Mutex
 
-	// disabledMetrics is a list of metric keys
+	// disabledMetrics is a set of metric keys
 	// that should NOT be saved to the buffer
-	// or sent to the telemetry server.
-	disabledMetrics   = make(map[string]struct{})
+	// or sent to the telemetry server. The value
+	// indicates whether the entry is temporary.
+	// If the value is true, it may be removed if
+	// the metric is re-enabled remotely later. If
+	// the value is false, it is permanent
+	// (presumably becaues the user explicitly
+	// disabled it) and can only be re-enabled
+	// with user consent.
+	disabledMetrics   = make(map[string]bool)
 	disabledMetricsMu sync.RWMutex
 
 	// instanceUUID is the ID of the current instance.