From b89e84d4d12014ddbeaf8d3406e18177a4696e88 Mon Sep 17 00:00:00 2001
From: Jean-Paul Calderone <exarkun@twistedmatrix.com>
Date: Wed, 1 Dec 2021 13:47:31 -0500
Subject: [PATCH] Replace the ZKAP spending counter with a histogram
---
src/_zkapauthorizer/_storage_server.py | 42 +++++++++++++++++--
.../tests/test_storage_server.py | 33 +++++++++++++--
2 files changed, 68 insertions(+), 7 deletions(-)
diff --git a/src/_zkapauthorizer/_storage_server.py b/src/_zkapauthorizer/_storage_server.py
index 1c1488b..80d7e2d 100644
--- a/src/_zkapauthorizer/_storage_server.py
+++ b/src/_zkapauthorizer/_storage_server.py
@@ -41,7 +41,7 @@ from attr.validators import instance_of, provides
from challenge_bypass_ristretto import SigningKey, TokenPreimage, VerificationSignature
from eliot import start_action
from foolscap.api import Referenceable
-from prometheus_client import CollectorRegistry, Counter
+from prometheus_client import CollectorRegistry, Histogram
from twisted.internet.defer import Deferred
from twisted.internet.interfaces import IReactorTime
from twisted.python.reflect import namedAny
@@ -182,11 +182,44 @@ class ZKAPAuthorizerStorageServer(Referenceable):
)
_metric_spending_successes = attr.ib(init=False)
+ def _get_buckets(self):
+ """
+ Create the upper bounds for the ZKAP spending histogram. The bounds are
+ set as a function of the pass value.
+
+ For example, if the value of a pass is 1 MB then the upper bounds for
+ the buckets will be:
+
+ 32 KB
+ 64 KB
+ 128 KB
+ 256 KB
+ 512 KB
+ 1 MB
+ 10 MB
+ 100 MB
+ 1 GB
+ INF
+ """
+ return (
+ self._pass_value / 32,
+ self._pass_value / 16,
+ self._pass_value / 8,
+ self._pass_value / 4,
+ self._pass_value / 2,
+ self._pass_value,
+ self._pass_value * 10,
+ self._pass_value * 100,
+ self._pass_value * 1000,
+ float("inf")
+ )
+
def __attrs_post_init__(self):
- self._metric_spending_successes = Counter(
+ self._metric_spending_successes = Histogram(
"zkapauthorizer_server_spending_successes",
- "ZKAP Spending Successes Counter",
+ "ZKAP Spending Successes histogram",
registry=self._registry,
+ buckets=self._get_buckets(),
)
def remote_get_version(self):
@@ -215,7 +248,8 @@ class ZKAPAuthorizerStorageServer(Referenceable):
passes,
self._signing_key,
)
- self._metric_spending_successes.inc(len(validation.valid))
+ for _ in range(len(validation.valid)):
+ self._metric_spending_successes.observe(allocated_size)
# Note: The *allocate_buckets* protocol allows for some shares to
# already exist on the server. When this is the case, the cost of the
diff --git a/src/_zkapauthorizer/tests/test_storage_server.py b/src/_zkapauthorizer/tests/test_storage_server.py
index 3e7ce1b..b696e22 100644
--- a/src/_zkapauthorizer/tests/test_storage_server.py
+++ b/src/_zkapauthorizer/tests/test_storage_server.py
@@ -582,7 +582,23 @@ class PassValidationTests(TestCase):
list(RandomToken.create() for i in range(expected)),
)
- before_count = self.storage_server._metric_spending_successes._value.get()
+ buckets = self.storage_server._get_buckets()
+ for bucket_number, upper_bound in enumerate(buckets):
+ if size <= upper_bound:
+ break
+
+ def read_count():
+ buckets = self.storage_server._metric_spending_successes._buckets
+ return sum(b.get() for b in buckets)
+
+ def read_bucket():
+ buckets = self.storage_server._metric_spending_successes._buckets
+ note(list((n, b.get()) for n, b in enumerate(buckets)))
+ return buckets[bucket_number].get()
+
+ before_count = read_count()
+ before_bucket = read_bucket()
+
alreadygot, allocated = self.storage_server.doRemoteCall(
"allocate_buckets",
(),
@@ -597,9 +613,20 @@ class PassValidationTests(TestCase):
),
)
- after_count = self.storage_server._metric_spending_successes._value.get()
+ after_count = read_count()
+ after_bucket = read_bucket()
- self.assertThat(
+ note("bucket_number {}".format(bucket_number))
+
+ self.expectThat(
after_count - before_count,
Equals(expected),
+ "Unexpected histogram sum value",
)
+ self.assertThat(
+ after_bucket - before_bucket,
+ Equals(expected),
+ "Unexpected histogram bucket value",
+ )
+
+# Counter of invalid ZKAP spend attempts
--
GitLab