Commit graph

3 commits

Author SHA1 Message Date
Alex Garcia
b20ee1de21 Trace callback-style calls: execute_fn, execute_write_fn, execute_isolated_fn
The database instrumentation covered the four SQL-string entry points but
not the callback entry points, which are the documented way for plugins to
run arbitrary SQL - so the JSON write API's inserts and deletes, the
startup catalog scan, and every plugin built on execute_fn/execute_write_fn
were invisible to a trace, or worse, showed orphan-looking db.write.* spans
with no db.query above them.

Each callback method now opens the same db.query CLIENT span as its
SQL-string sibling, carrying a new optional datasette.callback attribute
(the callable's qualified name, captured before _wrap_fn_with_hooks() can
rename it) in place of db.query.text, which is now marked optional. A bare
execute_fn() also wraps the callback in a db.query.execute child, so the
"gap between the spans is thread-wait" story holds for plugin callbacks
too. No db.operation.name: there is no statement to take a keyword from,
and the registry says that attribute is omitted rather than guessed.

The previous bodies move to private _execute_fn()/_execute_write_fn() and
the SQL-string methods call those, so an execute() emits exactly the spans
it did before - pinned by test_execute_does_not_double_wrap. Database's own
introspection helpers stay on the public method deliberately: they are real
SQLite round trips, which lifts a table page from ~58 to ~100 (no-op) spans.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012U7coQfVu8nK2R4q2mCULA
2026-09-17 09:23:13 -07:00
Alex Garcia
27041f519d Review polish: drop unused prefix machinery, tighten comments and docs
- Remove the registry's unused prefix=True slot, its span_for() branch,
  its doc-rendering case and its test - nothing in the stack sets it.
- Stop promising a "later phase" query-duration metric dimension in the
  db.operation.name description; the cardinality rationale stands alone.
- Replace baked-in benchmark numbers in the telemetry module docstring
  with the docs' own phrasing (below run-to-run variation).
- Compact the duplicated copy_context() and enqueue-site comments in
  database.py to pointers at their canonical tellings.
- Make the "catch people out" gotchas skimmable as a bullet list and
  give the changelog's "nothing is removed" line a clear antecedent.
- Add a test that a result cut short by max_returned_rows records
  datasette.truncated=True - previously only ever asserted False.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012U7coQfVu8nK2R4q2mCULA
2026-09-17 09:23:02 -07:00
Alex Garcia
c90586db06 Name every span and attribute once, in a registry the docs are built from
The span and attribute names were string literals spread across four call
sites in database.py and one in app.py, with a hand-written reference page
that would have been true only on the day it was written. That drift is not
hypothetical: an earlier iteration of this work carried a README asserting
parameter values were never recorded for two branches after that had stopped
being true.

datasette/telemetry_registry.py now holds each name once, with its
documentation. Attribute and SpanName subclass str, so a registry entry *is*
the string OpenTelemetry wants - no wrapper API over the OTel calls, no
parallel structure to keep in step, and a typo becomes an ImportError rather
than a silently misnamed attribute. docs/internals.rst renders the span
reference from it via cog, and `cog --check docs/*.rst` already runs in CI,
so the reference cannot drift from the definitions.

Nothing changes on the wire: the emitted span names and attribute keys are
byte-identical before and after, verified by diffing a dump of both.

tests/test_telemetry_registry.py exercises a real workload and compares it
against the registry in both directions - emitted-but-unregistered catches
instrumentation added without documentation, registered-but-never-emitted
catches documentation that has outlived its code. Because the call sites now
take their names from the registry, neither direction can catch a rename:
move DB_NAMESPACE to "db.namespace2" and code and registry still agree while
every dashboard breaks. So the literal names are also written out in the test
and asserted against the registry and against the wire separately. That pair
is the only comparison in the file not derived from the registry itself.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-09-17 09:22:45 -07:00