3.0.5 release

CHANGELOG.md

@@ -5,6 +5,17 @@ Notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [3.0.5] - 2023-12-30
+
+### Changed
+
+- Fix incorrect function decorator preventing startup in telegraf mode.
+- Ensure the ntpmon user is in the chrony or ntp system groups if they are
+  present.  This fixes the inability to read chrony logs by default on Debian
+  and Ubuntu.
+- Add test suite for line_protocol and fix some resultant bugs.
+- Reduce polling frequency on peer stats log to once every 3 seconds.
+
 ## [3.0.4] - 2023-12-29
 
 ### Changed

Makefile

@@ -10,14 +10,16 @@ PREFIX=/usr/local
 SHAREDIR=share/$(NAME)
 SYSTEMD_SERVICE_DIR=/lib/systemd/system
 USER=$(NAME)
-VERSION=3.0.4
+VERSION=3.0.5
 RELEASE=1
 
 TESTS=\
+  unit_tests/test_classifier.py \
+  unit_tests/test_line_protocol.py \
   unit_tests/test_peer_stats.py \
+  unit_tests/test_peers.py \
   unit_tests/test_tailer.py \
-  unit_tests/test_classifier.py \
-  unit_tests/test_peers.py
+
 
 test: pytest datatest
 

SECURITY.md

@@ -0,0 +1,18 @@
+# Security Policy
+
+## Supported Versions
+
+Use this section to tell people about which versions of your project are
+currently being supported with security updates.
+
+| Version | Supported          | Remark                                |
+| ------- | ------------------ | ------                                |
+| 3.0.x   | :white_check_mark: | Current development release           |
+| 2.1.x   | :white_check_mark: | Stable release - security fixes only  |
+| 2.0.x   | :x:                | Previous stable release - unsupported |
+| 1.x     | :x:                | Unsupported                           |
+
+## Reporting a Vulnerability
+
+Please [open an issue](https://github.com/paulgear/ntpmon/issues/new).  At the
+moment I can see no need to create private issues for security flaws.

debian/changelog

@@ -1,3 +1,9 @@
+ntpmon (3.0.5-1) focal; urgency=medium
+
+  * New upstream release.
+
+ -- Paul Gear <github@libertysys.com.au>  Sat, 30 Dec 2023 14:03:10 +1000
+
 ntpmon (3.0.4-1) focal; urgency=medium
 
   * New upstream release to fix test suite.

debian/postinst

@@ -33,6 +33,16 @@ case "$1" in
                 --system \
                 "$NAME"
         fi
+        TMP=$(mktemp)
+        for GROUP in _chrony ntp; do
+            if getent group "$GROUP" >$TMP; then
+                if grep -qw "$NAME" $TMP; then
+                    : $NAME already in $GROUP
+                else
+                    adduser "$NAME" "$GROUP"
+                fi
+            fi
+        done
         ;;
 
     abort-upgrade|abort-remove|abort-deconfigure)

src/line_protocol.py

@@ -9,22 +9,28 @@
 # With telegraf we can only use timestamps in nanosecond format
 
 
+import re
+
+
 exclude_fields = []
 
 exclude_tags = []
 
 
+# https://docs.influxdata.com/influxdb/v2/reference/syntax/line-protocol/#special-characters
+def escape_tag_value(s: str) -> str:
+    return s.replace(" ", "\\ ").replace(",", "\\,").replace("=", "\\=")
+
+
 def format_tags(metrics: dict, additional_tags: dict) -> str:
+    all_metrics = {}
+    all_metrics.update(additional_tags)
+    all_metrics.update(metrics)
     return ",".join(
         [
-            f"{transform_identifier(tag)}={metrics[tag]}"
-            for tag in sorted(metrics.keys())
-            if tag not in exclude_tags and type(metrics[tag]) == str
-        ]
-        + [
-            f"{transform_identifier(tag)}={additional_tags[tag]}"
-            for tag in sorted(additional_tags.keys())
-            if tag not in exclude_tags
+            f"{transform_identifier(tag)}={escape_tag_value(all_metrics[tag])}"
+            for tag in sorted(all_metrics.keys())
+            if tag not in exclude_tags and type(all_metrics[tag]) == str
         ]
     )
 
@@ -50,8 +56,10 @@ def format_fields(metrics: dict) -> str:
 
 
 def timestamp_to_line_protocol(timestamp: float) -> (int, int):
+    if timestamp < 0:
+        raise ValueError("timestamps cannot be negative")
     seconds = int(timestamp)
-    nanoseconds = int((timestamp - seconds) * 1_000_000_000)
+    nanoseconds = round((timestamp - seconds) * 1_000_000_000)
     return (seconds, nanoseconds)
 
 
@@ -67,5 +75,8 @@ def to_line_protocol(metrics: dict, which: str, additional_tags: dict = {}) -> s
     return f"{which}{tags} {format_fields(metrics)}{timestamp}"
 
 
+punctuation = re.compile(r'[-!@#$%^&()<>,./\?+=:;"\'\[\]\{\}\*\s]+')
+
+
 def transform_identifier(id: str) -> str:
-    return id.replace("-", "_").strip("_")
+    return punctuation.sub("_", id).strip("_")

src/ntpmon.py

@@ -122,7 +122,7 @@ async def peer_stats_task(args: argparse.Namespace, output: outputs.Output) -> N
     tailer = None
 
     while True:
-        await asyncio.sleep(0.5)
+        await asyncio.sleep(3)
 
         if implementation is None:
             implementation = process.get_implementation()

src/outputs.py

@@ -250,7 +250,7 @@ def __init__(self, args: argparse.Namespace) -> None:
         super().__init__()
         self.file = sys.stdout if args.debug else self.get_telegraf_file(args.connect)
 
-    @classmethod
+    @staticmethod
     def get_telegraf_file(connect: str) -> TextIOWrapper:
         """Return a TextIOWrapper for writing data to telegraf"""
         (host, port) = connect.split(":")

testdata/OK/05aae847-27b5-41a9-9f09-820f2732c498

@@ -0,0 +1,8 @@
+^,*,130.95.128.58,3,10,377,167,0.000076249,0.000156259,0.011293960
+=,-,130.95.13.18,4,10,377,904,0.000112006,0.000183676,0.055506174
+^,-,203.0.178.191,2,10,377,29,0.002167636,0.002167636,0.132863492
+^,-,110.141.196.84,1,10,377,343,0.004494720,0.004572738,0.027861051
+^,-,203.114.73.24,2,10,177,365,0.002114314,0.002192088,0.097485669
+^,-,120.146.26.214,2,10,377,1313,0.000196896,0.000535064,0.068387702
+^,-,128.199.123.83,2,10,377,371,-0.021903355,-0.021825654,0.323896408
+^,-,139.99.107.37,2,10,337,154,-0.020766487,-0.020766487,0.103270806

unit_tests/test_line_protocol.py

@@ -0,0 +1,62 @@
+#
+# Copyright:    (c) 2023 Paul D. Gear
+# License:      AGPLv3 <http://www.gnu.org/licenses/agpl.html>
+
+import pytest
+
+import line_protocol
+
+
+def test_escape_tag_value() -> None:
+    assert line_protocol.escape_tag_value("hello=world") == "hello\\=world"
+    assert line_protocol.escape_tag_value("hello world") == "hello\\ world"
+    assert line_protocol.escape_tag_value("hello, world") == "hello\\,\\ world"
+
+
+def test_timestamp_to_line_protocol() -> None:
+    assert line_protocol.timestamp_to_line_protocol(1) == (1, 0)
+    assert line_protocol.timestamp_to_line_protocol(1.123_456_789) == (1, 123_456_789)
+    assert line_protocol.timestamp_to_line_protocol(1.9) == (1, 900_000_000)
+    assert line_protocol.timestamp_to_line_protocol(1.999) == (1, 999_000_000)
+    assert line_protocol.timestamp_to_line_protocol(1.999_999) == (1, 999_999_000)
+    assert line_protocol.timestamp_to_line_protocol(1.999_999_999) == (1, 999_999_999)
+    with pytest.raises(ValueError):
+        # We should never get to either of these assert statements - the
+        # ValueError should be raised first.
+        assert line_protocol.timestamp_to_line_protocol(-1) == (-1, 0)
+        assert False
+
+
+def test_to_line_protocol() -> None:
+    metrics = {
+        "associd": 0,
+        "frequency": -11.673,
+        "leap": False,
+        "offset": +0.0000145826,
+        "precision": -23,
+        "processor": "x86_64",
+        "refid": "100.66.246.50",
+        "reftime": "e93a0505.8336edfd",
+        "rootdelay": 1.026,
+        "rootdisp": 8.218,
+        "stratum": 2,
+        "sys jitter": 0.082849,
+        "system": "Linux/5.10.0-26-amd64",
+        "test": True,
+        "version": "ntpd 4.2.8p15@1.3728-o Wed Sep 23 11:46:38 UTC 2020 (1)",
+    }
+    assert (
+        line_protocol.to_line_protocol(metrics, "ntpmon", additional_tags={"hostname": "ntp1", "processor": "amd64"})
+        == "ntpmon,hostname=ntp1,processor=x86_64,refid=100.66.246.50,reftime=e93a0505.8336edfd,system=Linux/5.10.0-26-amd64,"
+        "version=ntpd\\ 4.2.8p15@1.3728-o\\ Wed\\ Sep\\ 23\\ 11:46:38\\ UTC\\ 2020\\ (1) "
+        "frequency=-11.673,offset=1.45826e-05,rootdelay=1.026,rootdisp=8.218,"
+        "sys_jitter=0.082849,associd=0i,precision=-23i,stratum=2i,leap=0i,test=1i"
+    )
+
+
+def test_transform_identifier() -> None:
+    assert line_protocol.transform_identifier("_chrony") == "chrony"
+    assert line_protocol.transform_identifier("-chrony-was-here-") == "chrony_was_here"
+    assert line_protocol.transform_identifier("hello, world") == "hello_world"
+    assert line_protocol.transform_identifier("a = hello(world)") == "a_hello_world"
+    assert line_protocol.transform_identifier("def hello(world) -> str:") == "def_hello_world_str"