datamodel: stabilize monitoring schema

diff --git a/NEWS b/NEWS
index 75bac01bf0c7b8cae5eee13c5ecfa74d982ddc2a..1961fa446bd8cbd5e7a74ddbc8e698a9b6dde6f5 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -25,6 +25,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 4353e16876f307a134564f66f666ee01799db4c0..ed39ff714834ab9258b189abee67c34b9bad205a 100644 (file)
--- a/doc/_static/config.schema.json
+++ b/doc/_static/config.schema.json
@@ -1619,70 +1619,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 b10f12c26a28119a903916513d6db3dc8576b3e5..86b6a259b7f825ef4b40e88a5e05f0cc6afd6cc7 100644 (file)
--- a/python/knot_resolver/manager/metrics/prometheus.py
+++ b/python/knot_resolver/manager/metrics/prometheus.py
@@ -406,7 +406,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),
@@ -422,14 +422,14 @@ if PROMETHEUS_LIB:
     async def _deny_turning_off_graphite_bridge(
         old_config: KresConfig, new_config: KresConfig, force: bool = False
     ) -> 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.")