From d791deb195df1bf43bd076bd51d6b229cddf6ad6 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone <exarkun@twistedmatrix.com> Date: Mon, 6 Dec 2021 10:11:15 -0500 Subject: [PATCH] Add helpers for computing and observing spending metrics --- src/_zkapauthorizer/_storage_server.py | 51 ++++++++++++++ .../tests/test_storage_server.py | 67 ++++++++++++++++++- 2 files changed, 116 insertions(+), 2 deletions(-) diff --git a/src/_zkapauthorizer/_storage_server.py b/src/_zkapauthorizer/_storage_server.py index 160f3d6..a0de104 100644 --- a/src/_zkapauthorizer/_storage_server.py +++ b/src/_zkapauthorizer/_storage_server.py @@ -150,6 +150,57 @@ class LeaseRenewalRequired(Exception): """ +def observe_spending_successes(metric, observations): + # type: (Histogram, Iterable[Tuple[int, int]]) -> None + """ + Put some spending observations into a Histogram. + """ + for (size, count) in observations: + for _ in range(count): + metric.observe(size) + + +def compute_spending_metrics(bytes_per_pass, sizes): + # type: (int, List[int]) -> Generator[Tuple[int, int]] + """ + Attribute portions of a payment for one or more shares to the individual + shares. This supports maintaining a histogram of spending where + information is placed in buckets by the size of the data is relates to. + + This is somewhat less straightforward than one might hope because payment + for more than one share combines all of the share sizes for the purposes + of pricing. We have to reverse engineer that combination to attribute + portions of the spending to each share. We do this by noting that price + is proportional to size and by allowing for some imprecision when a share + size does not fall exactly on a multiple of pass value. + + :param bytes_per_pass: The number of bytes one pass pays for for one + storage period. + + :param sizes: The sizes of the shares that were paid for. + + :return: A generator of tuples that describe a share size in bytes and a + number of passes spent to store the share of that size. Each element + from ``sizes`` will be represented in this result. + """ + if len(sizes) == 0: + return + overrun = 0 + # Make a copy so we can pop one off. Also sort the sizes so we have a + # consistent result for a given collection of sizes independent of the + # order they're considered. + values = sorted(list(sizes)) + last_allocated_size = values.pop() + for allocated_size in values: + share_result, overrun = divmod(allocated_size + overrun, bytes_per_pass) + yield (allocated_size, share_result) + + share_result, overrun = divmod(last_allocated_size + overrun, bytes_per_pass) + if overrun > 0: + share_result += 1 + yield (last_allocated_size, share_result) + + @implementer(RIPrivacyPassAuthorizedStorageServer) # It would be great to use `frozen=True` (value-based hashing) instead of # `cmp=False` (identity based hashing) but Referenceable wants to set some diff --git a/src/_zkapauthorizer/tests/test_storage_server.py b/src/_zkapauthorizer/tests/test_storage_server.py index 76c241a..cfb17e9 100644 --- a/src/_zkapauthorizer/tests/test_storage_server.py +++ b/src/_zkapauthorizer/tests/test_storage_server.py @@ -21,6 +21,7 @@ from __future__ import absolute_import, division from random import shuffle from time import time +from prometheus_client import CollectorRegistry, Histogram from challenge_bypass_ristretto import RandomToken, random_signing_key from foolscap.referenceable import LocalReferenceable from hypothesis import given, note @@ -30,7 +31,7 @@ from testtools.matchers import AfterPreprocessing, Equals, MatchesAll from twisted.internet.task import Clock from twisted.python.runtime import platform -from .._storage_server import _ValidationResult +from .._storage_server import _ValidationResult, compute_spending_metrics, observe_spending_successes from ..api import MorePassesRequired, ZKAPAuthorizerStorageServer from ..storage_common import ( add_lease_message, @@ -144,7 +145,7 @@ def read_count(storage_server): return sum(b.get() for b in buckets) def read_bucket(storage_server, size): - bounds = self.storage_server._get_buckets() + bounds = storage_server._get_buckets() for bucket_number, upper_bound in enumerate(bounds): if size <= upper_bound: break @@ -688,4 +689,66 @@ class PassValidationTests(TestCase): "Unexpected histogram bucket value", ) + + +class SpendingMetricTests(TestCase): + @given( + integers(min_value=1), + lists(sizes()), + ) + def test_total_passes_observed(self, bytes_per_pass, sizes): + """ + The total number of spent passes reported by ``compute_spending_metrics`` + equals the total number of passes it is called with. + """ + num_passes = required_passes(bytes_per_pass, sizes) + observations = compute_spending_metrics(bytes_per_pass, sizes) + total_recorded = sum(count for (size, count) in observations) + self.assertThat( + total_recorded, + Equals(num_passes), + "expected {} passes but metrics only accounted for {}".format( + num_passes, + total_recorded, + ), + ) + + @given( + integers(min_value=1), + lists(sizes()), + ) + def test_share_sizes_observed(self, bytes_per_pass, sizes): + """ + Every size passed to ``compute_spending_metrics`` has a corresponding + element in the result. + """ + observations = compute_spending_metrics(bytes_per_pass, sizes) + sizes_seen = list(size for (size, count) in observations) + self.assertThat( + sorted(sizes), + Equals(sorted(sizes_seen)), + ) + + def test_observe_spending(self): + """ + ``observe_spending_successes`` adds observations to a Prometheus histogram + metric. + """ + registry = CollectorRegistry() + metric = Histogram( + "zkapauthorizer_tests", + "", + registry=registry, + buckets=[1, 2, 3, float("inf")], + ) + observe_spending_successes( + metric, + iter([(1, 3), (2, 5), (3, 7), (4, 11), (3, 13), (5, 17)]), + ) + self.assertThat( + list(b.get() for b in metric._buckets), + Equals([3, 5, 20, 28]), + ) + + # Counter of invalid ZKAP spend attempts -- GitLab