Register metrics and give histograms bucket boundaries suited to seconds

Both histograms declared unit="s" but inherited OpenTelemetry's default
boundaries, which are tuned for milliseconds - so every SQLite query
landed in the single (0, 5] second bucket and every quantile query
returned noise.

The boundaries are the semantic conventions' recommended set for
db.client.operation.duration plus 0.0001 and 0.0005 at the bottom, since
SQLite is in-process and many real queries take tens of microseconds.

(Adapted from 024f2029: that commit assumed the metrics were already in
telemetry_registry.py, which on this lineage held spans only - so this
commit also brings the MetricName registry machinery, the registry
entries for all eight phase-3 metrics, the cog-generated Metric
reference in internals.rst, and the datasette.operation attribute. The
template and facet histograms it also touched belong to phase 5 and are
not included.)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F2h9ANGZ7paWSpqs5DUAcG
This commit is contained in:
Alex Garcia 2026-09-01 12:24:01 -07:00
commit fa04620156
6 changed files with 352 additions and 18 deletions

View file

@ -321,3 +321,67 @@ def test_registry_holds_instances_weakly():
gc.collect()
assert ref() is None
assert not any(isinstance(ds, FakeDatasette) for ds in telemetry._live_instances())
HISTOGRAM_PROBES = [
# (instrument attribute on telemetry, metric name, isolating attributes)
(
"sql_operation_duration",
"db.client.operation.duration",
{"db.namespace": "bucket_probe_operation"},
),
(
"write_queue_wait",
"datasette.write.queue_wait",
{"db.namespace": "bucket_probe_queue_wait"},
),
]
# One value inside each of six distinct registry buckets. Under OpenTelemetry's
# default boundaries - [0, 5, 10, 25, ...], meant for milliseconds - the first
# five of these all land in (0, 5] and only 7.0 lands elsewhere, so the
# "occupies six buckets" assertion below fails if the advisory is ever dropped.
SPREAD = [0.00005, 0.0003, 0.002, 0.03, 0.8, 7.0]
@pytest.mark.parametrize(
"instrument_name,metric_name,attributes",
HISTOGRAM_PROBES,
ids=[metric for _, metric, _ in HISTOGRAM_PROBES],
)
def test_histograms_spread_values_across_buckets(
otel_metrics, instrument_name, metric_name, attributes
):
"""
The registry's boundaries reach the SDK, and a realistic spread of
seconds-scale durations occupies more than one bucket.
Recording onto the instrument directly rather than driving a workload is
deliberate: real durations here are all tens of microseconds and would
share a bucket no matter what the boundaries were, which is exactly the
situation this test exists to detect.
`explicit_bounds` is compared against the registry rather than against the
instrument's own configuration - the instrument is built *from* the
registry, so that comparison would be a value against itself. What is
checked here is that the advisory survived the trip through the SDK.
"""
from datasette.telemetry_registry import METRICS
metric = next(m for m in METRICS if m == metric_name)
instrument = getattr(telemetry, instrument_name)
for value in SPREAD:
instrument.record(value, attributes)
otel_metrics.collect()
point = otel_metrics.point(metric_name, attributes)
assert (
tuple(point.explicit_bounds) == metric.buckets
), "the registry's boundaries did not reach the SDK"
assert point.count == len(SPREAD)
occupied = [count for count in point.bucket_counts if count]
assert len(occupied) == len(SPREAD), (
f"expected each of {SPREAD} in its own bucket, got bucket counts "
f"{list(point.bucket_counts)} for bounds {list(point.explicit_bounds)}"
)

View file

@ -382,6 +382,29 @@ def test_registry_entries_are_usable_as_plain_strings():
assert f"{reg.DB_QUERY}.execute" == "db.query.execute"
def test_every_histogram_declares_bucket_boundaries():
"""
Every histogram must carry explicit boundaries, and only histograms may.
OpenTelemetry's default boundaries start at 5 and are meant for
milliseconds, so a seconds-valued histogram that inherits them records
everything into one bucket. This is a registry self-consistency check, not
a check that the boundaries reached the SDK - for that see
`test_histograms_spread_values_across_buckets` in test_telemetry_metrics.py.
"""
for metric in reg.METRICS:
if metric.kind == reg.HISTOGRAM:
assert metric.buckets, f"{metric} is a histogram with no boundaries"
assert list(metric.buckets) == sorted(
set(metric.buckets)
), f"{metric} boundaries must be ascending and unique"
assert metric.buckets[0] > 0, f"{metric} has a non-positive boundary"
else:
assert (
metric.buckets is None
), f"{metric} is a {metric.kind} and cannot have bucket boundaries"
def test_dynamic_span_lookup():
"""
`dynamic=True` matching, which is how the request span resolves.