mixin: remove enableActiveDirectory and enableTime

collectors.ad and collectors.time are the only switches for the Active
Directory and clock alerts, like every other opt-in collector. Setting
the removed keys fails the build with a message naming the replacement.

Signed-off-by: Jan-Otto Kröpke <mail@jkroepke.de>

contrib/mixin/README.md

@@ -62,9 +62,7 @@ modifying its source. For example, create `custom.libsonnet` in this directory:
     nicSelector: 'nic!="Loopback"',
     rateInterval: '5m',
     cpuHighUsageThreshold: 95,
-    enableActiveDirectory: true,
-    enableTime: true,
-    collectors+: { process: true },
+    collectors+: { ad: true, time: true, process: true },
     alerts+: { WindowsHostRebooted: false },
     keepFiringFor: '10m',
     runbookURLPattern: 'https://runbooks.example.com/windows/%s/',
@@ -223,15 +221,17 @@ older servers.
 The network error and discard thresholds are percentages of all packets on the
 interface. Earlier versions of the mixin had `networkErrorRateThreshold` and
 `networkDiscardRateThreshold` in packets per second; setting them now fails
-the build with a message that names the replacements.
+the build with a message that names the replacements. The same goes for
+`enableActiveDirectory` and `enableTime`, replaced by `collectors+: { ad: true }`
+and `collectors+: { time: true }`.
 
 `criticalSeverity` defaults to `critical` and `infoSeverity` to `info`; override
 them to match your routing labels. Below the critical disk threshold, both disk
 severity levels fire; configure Alertmanager inhibition if only the critical
 notification should be sent.
 
-Active Directory alerts need `collectors.ad`, which `enableActiveDirectory`
-sets. Clock alerts need `collectors.time`, which `enableTime` sets. A zero NTP
+Active Directory alerts need `collectors.ad`, clock alerts `collectors.time`;
+both are off by default. A zero NTP
 source count may be expected on hosts using another time provider;
 `WindowsClockSourceLocal` covers domain members, whose source is `NT5DS`, and
 fires only for the local clock or no synchronization. The offset compares the
@@ -424,7 +424,7 @@ tab to the full dashboard:
 names, such as `cpu`, `physical_disk`, `diskdrive`, `smb` or `update`. The
 dashboard renders only the tabs, rows, panels and variables of enabled
 collectors, so a tab for a collector you do not run does not appear. The
-defaults match the exporter's default collectors; `time` follows `enableTime`.
+defaults match the exporter's default collectors.
 The `os` collector is required, because the variables select hosts through
 `windows_os_hostname`. An unknown collector name fails the build. Enable
 optional collectors on top of the defaults with `collectors+:`; replacing the

contrib/mixin/alerts/alerts.libsonnet

@@ -6,6 +6,8 @@
   assert std.length(unknown) == 0 : 'Unknown alerts in _config.alerts: ' + std.join(', ', unknown),
   assert !std.objectHas(c, 'networkErrorRateThreshold') && !std.objectHas(c, 'networkDiscardRateThreshold') :
          'networkErrorRateThreshold and networkDiscardRateThreshold (packets per second) were replaced by networkErrorRatioThreshold and networkDiscardRatioThreshold (percent of all packets)',
+  assert !std.objectHas(c, 'enableActiveDirectory') && !std.objectHas(c, 'enableTime') :
+         'enableActiveDirectory and enableTime were replaced by collectors+: { ad: true } and collectors+: { time: true }',
   // A collector missing from a replaced collectors object counts as disabled;
   // an alert missing from a replaced alerts object counts as enabled.
   local collectorEnabled(collector) = std.get(c.collectors, collector, false),

contrib/mixin/config.libsonnet

@@ -10,11 +10,6 @@
     sharedVolumeSelector: 'shared_volume!="ClusterPerformanceHistory"',
     rateInterval: '5m',
 
-    // Opt in only for targets with the ad collector enabled. Sets collectors.ad.
-    enableActiveDirectory: false,
-    // Requires the time collector with its ntp subcollector enabled. Sets collectors.time.
-    enableTime: false,
-
     // Grafana dashboard metadata. A UID is the v2 metadata.name; change the
     // UIDs to import the dashboards next to the sample ones.
     // Every tab also renders as a dashboard of its own with the UID
@@ -44,7 +39,8 @@
     // collectors, and the alerts only the rules whose collectors are enabled.
     // The defaults match the exporter's default collectors.
     collectors: {
-      ad: $._config.enableActiveDirectory,
+      // Only for targets with the ad collector enabled, such as domain controllers.
+      ad: false,
       cpu: true,
       dhcp: false,
       diskdrive: false,
@@ -68,7 +64,8 @@
       storage_spaces: false,
       system: true,
       tcp: false,
-      time: $._config.enableTime,
+      // Needs the time collector with its ntp and clock_source subcollectors.
+      time: false,
       udp: false,
       update: false,
     },

contrib/mixin/tests/alerts.test.jsonnet

@@ -2,7 +2,7 @@
 // keep_firing_for and the group layout.
 local mixin = import '../mixin.libsonnet';
 local config = (import '../config.libsonnet')._config;
-local everything = mixin { _config+:: { enableActiveDirectory: true, enableTime: true, collectors: { [name]: true for name in std.objectFields(config.collectors) } } };
+local everything = mixin { _config+:: { collectors: { [name]: true for name in std.objectFields(config.collectors) } } };
 local optionalGroups = ['windows-active-directory', 'windows-time', 'windows-scheduled-task', 'windows-update', 'windows-hyperv', 'windows-cluster', 'windows-storage-spaces', 'windows-dhcp', 'windows-dns'];
 local names(alerts) = std.sort([rule.alert for group in alerts.groups for rule in group.rules]);
 local groupNames(alerts) = [group.name for group in alerts.groups];
@@ -15,7 +15,7 @@ assert all == std.set(std.objectFields(config.alerts)) : 'alerts map differs fro
 assert groupNames(mixin.prometheusAlerts) == ['windows-exporter', 'windows-cpu', 'windows-memory', 'windows-disk', 'windows-network', 'windows-service', 'windows-system'] : std.manifestJson(groupNames(mixin.prometheusAlerts));
 assert groupNames(everything.prometheusAlerts) == groupNames(mixin.prometheusAlerts) + optionalGroups : std.manifestJson(groupNames(everything.prometheusAlerts));
 // Opt-in collectors add their alerts to the default groups of their area too.
-local adOnly = mixin { _config+:: { enableActiveDirectory: true } };
+local adOnly = mixin { _config+:: { collectors+: { ad: true } } };
 assert groupNames(adOnly.prometheusAlerts) == groupNames(mixin.prometheusAlerts) + ['windows-active-directory'];
 local pagefile = mixin { _config+:: { collectors+: { pagefile: true, diskdrive: true, license: true } } };
 assert groupNames(pagefile.prometheusAlerts) == groupNames(mixin.prometheusAlerts);

contrib/mixin/tests/config.libsonnet

@@ -9,8 +9,6 @@
     diskFreeWarningThreshold: 20,
     diskFreeCriticalThreshold: 10,
     criticalSeverity: 'page',
-    enableActiveDirectory: true,
-    enableTime: true,
     runbookURLPattern: 'https://runbooks.example.com/windows/%s/',
     memoryCommitHighUsageThreshold: 80,
     diskLatencyThresholdSeconds: 0.02,
@@ -20,6 +18,7 @@
     adPendingReplicationThreshold: 10,
     adReplicationFailureRateThreshold: 0.5,
     collectors+: {
+      ad: true,
       dhcp: true,
       diskdrive: true,
       dns: true,
@@ -29,6 +28,7 @@
       pagefile: true,
       scheduled_task: true,
       storage_spaces: true,
+      time: true,
       update: true,
     },
     pagefileUsageThreshold: 75,

docs/runbooks/windowsadpendingreplication.md

@@ -6,7 +6,7 @@ title: WindowsADPendingReplication
 
 Pending Active Directory replication operations exceed the configured threshold
 for 15 minutes. The default is 50 operations. This alert is generated only with
-`enableActiveDirectory: true` and requires the `ad` collector on domain controllers.
+`collectors+: { ad: true }` and requires the `ad` collector on domain controllers.
 
 ## Impact
 

docs/runbooks/windowsclocknotsynchronising.md

@@ -5,7 +5,7 @@ title: WindowsClockNotSynchronising
 ## Meaning
 
 Windows Time reports no active NTP time source for ten minutes. This alert is
-generated only with `enableTime: true` and requires the `time` collector and its
+generated only with `collectors+: { time: true }` and requires the `time` collector and its
 `ntp` subcollector. Enable it on hosts expected to synchronize through NTP;
 hosts intentionally using another clock source may legitimately report zero.