datamodel: stabilize monitoring schema

diff --git a/NEWS b/NEWS
index e8bd0cf8c72f506bea97b4086b13faa84aeac861..93b4b73c2077b12a188c2af9e54ee5be8c034247 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -26,6 +26,8 @@ Incompatible changes
   - /dnssec/trust-anchor-sentinel -> /dnssec/sentinel
   - /dnssec/trust-anchor-signal-query -> /dnssec/signal-query
   - /logging/dnssec-bogus -> /dnssec/log-bogus
+  - /monitoring/enabled -> /monitoring/metrics
+  - /monitoring/graphite -> /monitoring/graphite/enabled
   - /network/tls/files-watchdog -> /network/tls/watchdog
   - /rate-limiting -> /rate-limiting/enabled
 

diff --git a/doc/_static/config.schema.json b/doc/_static/config.schema.json
index 534eceee063f24a3fad748aa503815378c71558a..9d354a05a47404b86fc9b23197025736f1fc8826 100644 (file)
--- a/doc/_static/config.schema.json
+++ b/doc/_static/config.schema.json
@@ -1514,70 +1514,82 @@
             "description": "Metrics exposisition configuration (Prometheus, Graphite)",
             "type": "object",
             "properties": {
-                "enabled": {
+                "metrics": {
                     "type": "string",
                     "enum": [
                         "manager-only",
                         "lazy",
                         "always"
                     ],
-                    "description": "configures, whether statistics module will be loaded into resolver",
+                    "description": "configures, whether metrics/statistics will be collected by the resolver",
                     "default": "lazy"
                 },
                 "graphite": {
-                    "anyOf": [
-                        {
-                            "type": "string",
-                            "enum": [
-                                false
-                            ]
+                    "type": "object",
+                    "properties": {
+                        "enabled": {
+                            "type": "boolean",
+                            "default": false
                         },
-                        {
-                            "type": "object",
-                            "properties": {
-                                "host": {
-                                    "anyOf": [
-                                        {
-                                            "type": "string"
-                                        },
-                                        {
-                                            "type": "string"
-                                        },
-                                        {
-                                            "type": "string",
-                                            "pattern": "(?=^.{,253}\\.?$)(^(?!-)[^.]{,62}[^.-](\\.(?!-)[^.]{,62}[^.-])*\\.?$)|^\\.$"
-                                        }
-                                    ]
+                        "host": {
+                            "anyOf": [
+                                {
+                                    "type": "null"
                                 },
-                                "port": {
-                                    "type": "integer",
-                                    "minimum": 1,
-                                    "maximum": 65535,
-                                    "default": 2003
+                                {
+                                    "type": "string"
                                 },
-                                "prefix": {
-                                    "type": "string",
-                                    "default": ""
+                                {
+                                    "type": "string"
                                 },
-                                "interval": {
+                                {
                                     "type": "string",
-                                    "pattern": "^(\\d+)(us|ms|s|m|h|d)$",
-                                    "default": "5s"
-                                },
-                                "tcp": {
-                                    "type": "boolean",
-                                    "default": false
+                                    "pattern": "(?=^.{,253}\\.?$)(^(?!-)[^.]{,62}[^.-](\\.(?!-)[^.]{,62}[^.-])*\\.?$)|^\\.$"
                                 }
-                            }
+                            ],
+                            "default": null
+                        },
+                        "port": {
+                            "type": "integer",
+                            "minimum": 1,
+                            "maximum": 65535,
+                            "default": 2003
+                        },
+                        "prefix": {
+                            "type": "string",
+                            "default": ""
+                        },
+                        "interval": {
+                            "type": "string",
+                            "pattern": "^(\\d+)(us|ms|s|m|h|d)$",
+                            "default": "5s"
+                        },
+                        "tcp": {
+                            "type": "boolean",
+                            "default": false
                         }
-                    ],
+                    },
                     "description": "optionally configures where should graphite metrics be sent to",
-                    "default": false
+                    "default": {
+                        "enabled": false,
+                        "host": null,
+                        "port": 2003,
+                        "prefix": "",
+                        "interval": "5s",
+                        "tcp": false
+                    }
                 }
             },
             "default": {
-                "enabled": "lazy",
-                "graphite": false
+                "metrics": "lazy",
+                "graphite": {
+                    "enabled": false,
+                    "host": null,
+                    "port": 2003,
+                    "prefix": "",
+                    "interval": "5s",
+                    "tcp": false
+                }
             }
         },
         "rate-limiting": {

diff --git a/doc/user/config-monitoring-stats.rst b/doc/user/config-monitoring-stats.rst
index 43c998700ebc9a7fb569b561362667967c08dd7e..e5c41f443b8944cda70e86bbc7e6bce32759825d 100644 (file)
--- a/doc/user/config-monitoring-stats.rst
+++ b/doc/user/config-monitoring-stats.rst
@@ -18,15 +18,15 @@ exposed as :ref:`config-monitoring-prometheus`.
 
 .. option:: monitoring:
 
-   .. option:: enabled: manager-only|lazy|always
+   .. option:: metrics: manager-only|lazy|always
 
       :default: lazy
 
       Configures, whether statistics module will be loaded into resolver.
 
-      * ``manager-only`` - Disables statistics collection in all `kresd` workers.
-      * ``lazy`` - Statistics collection is enabled at the time of request.
-      * ``always`` - Statistics collection is always on.
+      * ``manager-only`` - Disables metrics/statistics collection in all `kresd` workers.
+      * ``lazy`` - Metrics/statistics collection is enabled at the time of request.
+      * ``always`` - Metrics/statistics collection is always on.
 
 You can see all built-in statistics in `built-in statistics <./dev/modules-stats.html#mod-stats-list>`_ section.
 
@@ -62,16 +62,19 @@ Example configuration:
 
    monitoring:
      graphite:
+       enabled: true
        host: 127.0.0.1 # graphite server address
        port: 200       # optional graphite server port (2003 is default)
        interval: 5s    # optional publish interval (5s is default)
 
-.. option:: monitoring/graphite: <graphite-config>|false
+.. option:: monitoring/graphite:
 
-   :default: false
+   .. option:: enabled: true|false
 
-   Graphite module is disabled by default.
-   It is automatically enabled when configured.
+      :default: false
+
+      Enabled Graphite bridge module. It is disabled by default.
+      Configured :option:`host <host: <address or hostname>>` is also required to enable Graphite bridge.
 
    .. option:: host: <address or hostname>
 

diff --git a/python/knot_resolver/datamodel/monitoring_schema.py b/python/knot_resolver/datamodel/monitoring_schema.py
index f7a49f2273f88b8631253b89f6dd5536ca6fa75a..9308e333840e661f6390a7f7332cb1e289c0bdc7 100644 (file)
--- a/python/knot_resolver/datamodel/monitoring_schema.py
+++ b/python/knot_resolver/datamodel/monitoring_schema.py
@@ -5,19 +5,24 @@ from knot_resolver.utils.modeling import ConfigSchema
 
 
 class GraphiteSchema(ConfigSchema):
-    host: Union[IPAddress, DomainName]
+    enabled: bool = False
+    host: Union[None, IPAddress, DomainName] = None
     port: PortNumber = PortNumber(2003)
     prefix: EscapedStr = EscapedStr("")
     interval: TimeUnit = TimeUnit("5s")
     tcp: bool = False
 
+    def _validate(self) -> None:
+        if self.enabled and not self.host:
+            raise ValueError("'host' option must be configured to enable graphite bridge")
+
 
 class MonitoringSchema(ConfigSchema):
     """
     ---
-    enabled: configures, whether statistics module will be loaded into resolver
+    metrics: configures, whether metrics/statistics will be collected by the resolver
     graphite: optionally configures where should graphite metrics be sent to
     """
 
-    enabled: Literal["manager-only", "lazy", "always"] = "lazy"
-    graphite: Union[Literal[False], GraphiteSchema] = False
+    metrics: Literal["manager-only", "lazy", "always"] = "lazy"
+    graphite: GraphiteSchema = GraphiteSchema()

diff --git a/python/knot_resolver/datamodel/templates/monitoring.lua.j2 b/python/knot_resolver/datamodel/templates/monitoring.lua.j2
index 624b59ab721ce61e87d55fe3a80a2e125cd1cad6..665682d9d4563c5be739a0dc799b3ffd6ac1701d 100644 (file)
--- a/python/knot_resolver/datamodel/templates/monitoring.lua.j2
+++ b/python/knot_resolver/datamodel/templates/monitoring.lua.j2
@@ -14,7 +14,7 @@ else
        end
 end
 
-{% if cfg.monitoring.enabled == "always" %}
+{% if cfg.monitoring.metrics == "always" %}
 modules.load('stats')
 {% endif %}
 

diff --git a/python/knot_resolver/manager/metrics/collect.py b/python/knot_resolver/manager/metrics/collect.py
index 733f40cc0c5d7dc8af239ef0e8cccf67c790df5e..d596d6445bdbb3f3d8223cc757b38f759a0adf45 100644 (file)
--- a/python/knot_resolver/manager/metrics/collect.py
+++ b/python/knot_resolver/manager/metrics/collect.py
@@ -10,12 +10,12 @@ logger = logging.getLogger(__name__)
 
 
 async def collect_kresd_workers_metrics(config: KresConfig) -> Optional[Dict[KresID, object]]:
-    if config.monitoring.enabled == "manager-only":
+    if config.monitoring.metrics == "manager-only":
         logger.debug("Skipping kresd stat collection due to configuration")
         return None
 
     cmd = "collect_statistics()"
-    if config.monitoring.enabled == "lazy":
+    if config.monitoring.metrics == "lazy":
         cmd = "collect_lazy_statistics()"
     logger.debug(f"Collecting stats from all kresd workers using method '{cmd}'")
 

diff --git a/python/knot_resolver/manager/metrics/prometheus.py b/python/knot_resolver/manager/metrics/prometheus.py
index 5e8a130dced9a0cf2b619fe98f5d0c4ce5b463e6..659e110cd933ca4d2d8f44024db6c240b9ef387b 100644 (file)
--- a/python/knot_resolver/manager/metrics/prometheus.py
+++ b/python/knot_resolver/manager/metrics/prometheus.py
@@ -400,7 +400,7 @@ if PROMETHEUS_LIB:
         Starts graphite bridge if required
         """
         global _graphite_bridge
-        if config.monitoring.graphite is not False and _graphite_bridge is None:
+        if config.monitoring.graphite.enabled and _graphite_bridge is None:
             logger.info(
                 "Starting Graphite metrics exporter for [%s]:%d",
                 str(config.monitoring.graphite.host),
@@ -414,14 +414,14 @@ if PROMETHEUS_LIB:
             )
 
     async def _deny_turning_off_graphite_bridge(old_config: KresConfig, new_config: KresConfig) -> Result[None, str]:
-        if old_config.monitoring.graphite and not new_config.monitoring.graphite:
+        if old_config.monitoring.graphite.enabled and not new_config.monitoring.graphite.enabled:
             return Result.err(
                 "You can't turn off graphite monitoring dynamically. If you really want this feature, please let the developers know."
             )
 
         if (
-            old_config.monitoring.graphite is not None
-            and new_config.monitoring.graphite is not None
+            old_config.monitoring.graphite.enabled
+            and new_config.monitoring.graphite.enabled
             and old_config.monitoring.graphite != new_config.monitoring.graphite
         ):
             return Result.err("Changing graphite exporter configuration in runtime is not allowed.")