From 97e7e43cbd075e90a8ad0cf042f7fe5e7051db93 Mon Sep 17 00:00:00 2001 From: Nytelife26 Date: Mon, 24 Jun 2024 16:21:54 +0100 Subject: [PATCH 1/7] refactor: fully type SuiteResults This also fixes a bug where --json would omit some entries. --- cfspeedtest/__main__.py | 6 +- cfspeedtest/cloudflare.py | 135 +++++++++++++++++++++----------------- 2 files changed, 76 insertions(+), 65 deletions(-) diff --git a/cfspeedtest/__main__.py b/cfspeedtest/__main__.py index 6de3f87..34e6fdc 100644 --- a/cfspeedtest/__main__.py +++ b/cfspeedtest/__main__.py @@ -28,7 +28,7 @@ def cfspeedtest() -> None: ) args = parser.parse_args() - setup_log(silent=args.json) + setup_log(silent=args.json and not args.version) set_verbosity(debug=args.debug) if args.version: @@ -36,11 +36,11 @@ def cfspeedtest() -> None: log.debug("Python %s", sys.version) sys.exit(0) - results = CloudflareSpeedtest().run_all(megabits=not args.bps) + results = CloudflareSpeedtest().run_all() if args.json: setup_log() - log.info(json.dumps(CloudflareSpeedtest.results_to_dict(results))) + log.info(json.dumps(results.to_full_dict())) if __name__ == "__main__": diff --git a/cfspeedtest/cloudflare.py b/cfspeedtest/cloudflare.py index cc88211..7eb4bf2 100644 --- a/cfspeedtest/cloudflare.py +++ b/cfspeedtest/cloudflare.py @@ -9,6 +9,7 @@ import logging import statistics import time +from collections import UserDict from enum import Enum from typing import Any, NamedTuple @@ -128,7 +129,52 @@ def _calculate_percentile(data: list[float], percentile: float) -> float: return edges[0] + (edges[1] - edges[0]) * rem -SuiteResults = dict[str, dict[str, TestResult]] +def bits_to_megabits(bits: int) -> float: + """Convert bits to megabits, rounded to 2 decimal places.""" + return round(bits / 1e6, 2) + + +class SuiteResults(UserDict): + """The results of a test suite.""" + + def __init__(self, *, megabits: bool = False): + super().__init__() + self.setdefault("tests", {}) + self._megabits = megabits + + @property + def meta(self) -> TestMetadata: + return self["meta"] + + @meta.setter + def meta(self, value: TestMetadata) -> None: + self["meta"] = value + for meta_field, meta_value in value._asdict().items(): + log.info("%s: %s", meta_field, meta_value) + + @property + def tests(self) -> dict[str, TestResult]: + return self["tests"] + + def add_test(self, label: str, result: TestResult): + self.tests[label] = result + log.info("%s: %s", label, result.value) + + @property + def percentile_90th_down_bps(self) -> TestResult: + return self["90th_percentile_down_bps"] + + @property + def percentile_90th_up_bps(self) -> TestResult: + return self["90th_percentile_up_bps"] + + def to_full_dict(self) -> dict: + return { + "meta": self.meta._asdict(), + "tests": {k: v._asdict() for k, v in self.tests.items()}, + "90th_percentile_down_bps": self.percentile_90th_down_bps, + "90th_percentile_up_bps": self.percentile_90th_up_bps, + } class CloudflareSpeedtest: @@ -155,10 +201,7 @@ def __init__( # noqa: D417 no logging will occur. """ - self.results = results or {} - self.results.setdefault("tests", {}) - self.results.setdefault("meta", {}) - + self.results = results or SuiteResults() self.tests = tests self.request_sess = requests.Session() self.timeout = timeout @@ -199,74 +242,42 @@ def run_test(self, test: TestSpec) -> TestTimers: ) return coll - def _sprint( - self, label: str, result: TestResult, *, meta: bool = False - ) -> None: - """Add an entry to the suite results and log it.""" - log.info("%s: %s", label, result.value) - save_to = self.results["meta"] if meta else self.results["tests"] - save_to[label] = result + def run_test_latency(self, test: TestSpec) -> None: + """Run a test specification and collect latency results.""" + timers = self.run_test(test) + latencies = timers.to_latencies() + jitter = timers.jitter_from(latencies) + if jitter: + jitter = round(jitter, 2) + self.results.add_test( + "latency", TestResult(round(statistics.mean(latencies), 2)) + ) + self.results.add_test("jitter", TestResult(jitter)) + + def run_test_speed(self, test: TestSpec) -> list[int]: + """Run a test specification and collect speed results.""" + speeds = self.run_test(test).to_speeds(test) + self.results.add_test( + f"{test.name}_{test.type.name.lower()}_bps", + TestResult(int(statistics.mean(speeds))), + ) + return speeds - def run_all(self, *, megabits: bool = False) -> SuiteResults: + def run_all(self) -> SuiteResults: """Run the full test suite.""" - meta = self.metadata() - self._sprint("ip", TestResult(meta.ip), meta=True) - self._sprint("isp", TestResult(meta.isp)) - self._sprint("location_code", TestResult(meta.location_code), meta=True) - self._sprint("location_city", TestResult(meta.city), meta=True) - self._sprint("location_region", TestResult(meta.region), meta=True) + self.results.meta = self.metadata() data = {"down": [], "up": []} for test in self.tests: - timers = self.run_test(test) - if test.name == "latency": - latencies = timers.to_latencies() - jitter = timers.jitter_from(latencies) - if jitter: - jitter = round(jitter, 2) - self._sprint( - "latency", - TestResult(round(statistics.mean(latencies), 2)), - ) - self._sprint("jitter", TestResult(jitter)) + self.run_test_latency(test) continue - - speeds = timers.to_speeds(test) - data[test.type.name.lower()].extend(speeds) - # TODO: reduce code duplication of megabits reporting - mean_speed = int(statistics.mean(speeds)) - label_suffix = "bps" - if megabits: - mean_speed = round(mean_speed / 1e6, 2) - label_suffix = "mbps" - self._sprint( - f"{test.name}_{test.type.name.lower()}_{label_suffix}", - TestResult(mean_speed), - ) + data[test.type.name.lower()].extend(self.run_test_speed(test)) for k, v in data.items(): result = None if len(v) > 0: result = int(_calculate_percentile(v, 0.9)) - label_suffix = "bps" - if megabits: - result = round(result / 1e6, 2) if result else result - label_suffix = "mbps" - self._sprint( - f"90th_percentile_{k}_{label_suffix}", - TestResult(result), - ) + self.results[f"90th_percentile_{k}_bps"] = TestResult(result) return self.results - - @staticmethod - def results_to_dict( - results: SuiteResults, - ) -> dict[str, dict[str, dict[str, float]]]: - """Convert the test results to a full dictionary.""" - return { - k: {sk: sv._asdict()} - for k, v in results.items() - for sk, sv in v.items() - } From 64736ebec49d98fe7a8edcf7fd39e5787567450a Mon Sep 17 00:00:00 2001 From: Nytelife26 Date: Mon, 24 Jun 2024 23:27:28 +0100 Subject: [PATCH 2/7] refactor: use pairwise for jitter --- cfspeedtest/cloudflare.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/cfspeedtest/cloudflare.py b/cfspeedtest/cloudflare.py index 7eb4bf2..41dee73 100644 --- a/cfspeedtest/cloudflare.py +++ b/cfspeedtest/cloudflare.py @@ -11,6 +11,7 @@ import time from collections import UserDict from enum import Enum +from itertools import pairwise from typing import Any, NamedTuple import requests @@ -98,12 +99,7 @@ def jitter_from(latencies: list[float]) -> float | None: """Compute jitter as average deviation between consecutive latencies.""" if len(latencies) < 2: return None - return statistics.mean( - [ - abs(latencies[i] - latencies[i - 1]) - for i in range(1, len(latencies)) - ] - ) + return statistics.mean([abs(b - a) for a, b in pairwise(latencies)]) class TestMetadata(NamedTuple): From 67a3a471578ccc99aa1838e52270e4f4c6db5510 Mon Sep 17 00:00:00 2001 From: Nytelife26 Date: Tue, 25 Jun 2024 00:16:25 +0100 Subject: [PATCH 3/7] refactor: return values from run_test_latency --- cfspeedtest/cloudflare.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cfspeedtest/cloudflare.py b/cfspeedtest/cloudflare.py index 41dee73..5b59760 100644 --- a/cfspeedtest/cloudflare.py +++ b/cfspeedtest/cloudflare.py @@ -238,17 +238,17 @@ def run_test(self, test: TestSpec) -> TestTimers: ) return coll - def run_test_latency(self, test: TestSpec) -> None: + def run_test_latency(self, test: TestSpec) -> tuple[float, float | None]: """Run a test specification and collect latency results.""" timers = self.run_test(test) latencies = timers.to_latencies() jitter = timers.jitter_from(latencies) if jitter: jitter = round(jitter, 2) - self.results.add_test( - "latency", TestResult(round(statistics.mean(latencies), 2)) - ) + latency = round(statistics.mean(latencies), 2) + self.results.add_test("latency", TestResult(latency)) self.results.add_test("jitter", TestResult(jitter)) + return (latency, jitter) def run_test_speed(self, test: TestSpec) -> list[int]: """Run a test specification and collect speed results.""" From 5e83d66fd2be6b047e9ddf90b5f2528e68e0adfb Mon Sep 17 00:00:00 2001 From: Nytelife26 Date: Fri, 28 Jun 2024 00:04:53 +0100 Subject: [PATCH 4/7] refactor: reimplement megabit output --- cfspeedtest/__main__.py | 4 ++-- cfspeedtest/cloudflare.py | 42 +++++++++++++++++++++++++++------------ 2 files changed, 31 insertions(+), 15 deletions(-) diff --git a/cfspeedtest/__main__.py b/cfspeedtest/__main__.py index 34e6fdc..8985c13 100644 --- a/cfspeedtest/__main__.py +++ b/cfspeedtest/__main__.py @@ -4,7 +4,7 @@ import sys from argparse import ArgumentParser -from cfspeedtest.cloudflare import CloudflareSpeedtest +from cfspeedtest.cloudflare import CloudflareSpeedtest, SuiteResults from cfspeedtest.logger import log, set_verbosity, setup_log from cfspeedtest.version import __version__ @@ -36,7 +36,7 @@ def cfspeedtest() -> None: log.debug("Python %s", sys.version) sys.exit(0) - results = CloudflareSpeedtest().run_all() + results = CloudflareSpeedtest(SuiteResults(megabits=not args.bps)).run_all() if args.json: setup_log() diff --git a/cfspeedtest/cloudflare.py b/cfspeedtest/cloudflare.py index 5b59760..56e9d82 100644 --- a/cfspeedtest/cloudflare.py +++ b/cfspeedtest/cloudflare.py @@ -137,6 +137,7 @@ def __init__(self, *, megabits: bool = False): super().__init__() self.setdefault("tests", {}) self._megabits = megabits + log.info("megabits: %s", self._megabits) @property def meta(self) -> TestMetadata: @@ -146,30 +147,44 @@ def meta(self) -> TestMetadata: def meta(self, value: TestMetadata) -> None: self["meta"] = value for meta_field, meta_value in value._asdict().items(): - log.info("%s: %s", meta_field, meta_value) + log.info("meta.%s: %s", meta_field, meta_value) @property def tests(self) -> dict[str, TestResult]: return self["tests"] - def add_test(self, label: str, result: TestResult): + def add_test(self, label: str, result: TestResult) -> None: + if ( + self._megabits + and result.value is not None + and label not in {"latency", "jitter"} + ): + result = TestResult(bits_to_megabits(result.value), result.time) self.tests[label] = result + log.info("tests.%s: %s", label, result.value) + + def add_90th(self, test_type: TestType, result: TestResult) -> None: + if self._megabits and result.value is not None: + result = TestResult(bits_to_megabits(result.value), result.time) + label = f"90th_percentile_{test_type.name.lower()}" + self[label] = result log.info("%s: %s", label, result.value) @property - def percentile_90th_down_bps(self) -> TestResult: - return self["90th_percentile_down_bps"] + def percentile_90th_down(self) -> TestResult: + return self["90th_percentile_down"] @property - def percentile_90th_up_bps(self) -> TestResult: - return self["90th_percentile_up_bps"] + def percentile_90th_up(self) -> TestResult: + return self["90th_percentile_up"] def to_full_dict(self) -> dict: return { + "megabits": self._megabits, "meta": self.meta._asdict(), "tests": {k: v._asdict() for k, v in self.tests.items()}, - "90th_percentile_down_bps": self.percentile_90th_down_bps, - "90th_percentile_up_bps": self.percentile_90th_up_bps, + "90th_percentile_down": self.percentile_90th_down, + "90th_percentile_up": self.percentile_90th_up, } @@ -231,7 +246,8 @@ def run_test(self, test: TestSpec) -> TestTimers: ) coll.full.append(time.time() - start) coll.server.append( - float(r.headers["Server-Timing"].split("=")[1].split(",")[0]) / 1e3 + float(r.headers["Server-Timing"].split("=")[1].split(",")[0]) + / 1e3 ) coll.request.append( r.elapsed.seconds + r.elapsed.microseconds / 1e6 @@ -254,7 +270,7 @@ def run_test_speed(self, test: TestSpec) -> list[int]: """Run a test specification and collect speed results.""" speeds = self.run_test(test).to_speeds(test) self.results.add_test( - f"{test.name}_{test.type.name.lower()}_bps", + f"{test.name}_{test.type.name.lower()}", TestResult(int(statistics.mean(speeds))), ) return speeds @@ -263,17 +279,17 @@ def run_all(self) -> SuiteResults: """Run the full test suite.""" self.results.meta = self.metadata() - data = {"down": [], "up": []} + data = {TestType.Down: [], TestType.Up: []} for test in self.tests: if test.name == "latency": self.run_test_latency(test) continue - data[test.type.name.lower()].extend(self.run_test_speed(test)) + data[test.type].extend(self.run_test_speed(test)) for k, v in data.items(): result = None if len(v) > 0: result = int(_calculate_percentile(v, 0.9)) - self.results[f"90th_percentile_{k}_bps"] = TestResult(result) + self.results.add_90th(k, TestResult(result)) return self.results From 671f8870365fc7e58ea1b5de05188e8af7de38c8 Mon Sep 17 00:00:00 2001 From: Nytelife26 Date: Fri, 28 Jun 2024 00:10:30 +0100 Subject: [PATCH 5/7] feat: add label property to TestSpec --- cfspeedtest/cloudflare.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/cfspeedtest/cloudflare.py b/cfspeedtest/cloudflare.py index 56e9d82..4d13218 100644 --- a/cfspeedtest/cloudflare.py +++ b/cfspeedtest/cloudflare.py @@ -40,6 +40,11 @@ def bits(self) -> int: """The size of the test in bits.""" return self.size * 8 + @property + def label(self) -> str: + """The output label for the test.""" + return f"{self.name}_{self.type.name.lower()}" + TestSpecs = tuple[TestSpec, ...] @@ -270,7 +275,7 @@ def run_test_speed(self, test: TestSpec) -> list[int]: """Run a test specification and collect speed results.""" speeds = self.run_test(test).to_speeds(test) self.results.add_test( - f"{test.name}_{test.type.name.lower()}", + test.label, TestResult(int(statistics.mean(speeds))), ) return speeds From 2d8685a3d230a1133e24e0919e1ca80641ed2c3f Mon Sep 17 00:00:00 2001 From: Nytelife26 Date: Fri, 28 Jun 2024 00:45:06 +0100 Subject: [PATCH 6/7] fix: use consistent format for 90th_percentile dict keys --- cfspeedtest/cloudflare.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cfspeedtest/cloudflare.py b/cfspeedtest/cloudflare.py index 4d13218..d243882 100644 --- a/cfspeedtest/cloudflare.py +++ b/cfspeedtest/cloudflare.py @@ -188,8 +188,8 @@ def to_full_dict(self) -> dict: "megabits": self._megabits, "meta": self.meta._asdict(), "tests": {k: v._asdict() for k, v in self.tests.items()}, - "90th_percentile_down": self.percentile_90th_down, - "90th_percentile_up": self.percentile_90th_up, + "90th_percentile_down": self.percentile_90th_down._asdict(), + "90th_percentile_up": self.percentile_90th_up._asdict(), } From 441dcada956b6f54e8900e8ad5c40462a370aac4 Mon Sep 17 00:00:00 2001 From: Nytelife26 Date: Fri, 28 Jun 2024 01:10:39 +0100 Subject: [PATCH 7/7] chore: export SuiteResults --- cfspeedtest/__init__.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/cfspeedtest/__init__.py b/cfspeedtest/__init__.py index 9b1e5db..10d2873 100644 --- a/cfspeedtest/__init__.py +++ b/cfspeedtest/__init__.py @@ -1,5 +1,10 @@ """cfspeedtest, a Cloudflare speedtest suite in Python.""" -from cfspeedtest.cloudflare import CloudflareSpeedtest, TestSpec, TestType +from cfspeedtest.cloudflare import ( + CloudflareSpeedtest, + SuiteResults, + TestSpec, + TestType, +) -__all__ = ("CloudflareSpeedtest", "TestSpec", "TestType") +__all__ = ("CloudflareSpeedtest", "SuiteResults", "TestSpec", "TestType")