mirror of
https://github.com/simonw/datasette.git
synced 2026-09-27 12:24:07 +02:00
Drop the db.execute(table=) telemetry label
Simon's review: `table=` was a new parameter on the public `execute()` signature that had no effect on execution - it existed only to set the `db.collection.name` span attribute. Removing it takes the whole attribute out of phase 1. `db.query` spans keep `db.namespace`, `db.query.text`, `db.operation.name` and the rest; `db.collection.name` returns in a later PR once there is a mechanism worth committing to in the public API. Side effect worth having: this PR no longer touches `views/table.py` or `views/row.py` at all - both files are now byte-identical to main - so it is purely the database layer it claims to be. The registry conformance tests already enforce the rest: an attribute left in `telemetry_registry` but never emitted fails `test_every_registered_attribute_is_emitted`, so the registry entry, the generated docs block and the workload that reached it all come out together. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
bb70f3fa1a
commit
fd6bf7c4b1
8 changed files with 8 additions and 101 deletions
|
|
@ -853,7 +853,7 @@ async def test_invoke_startup_produces_one_trace_not_dozens_of_orphans(otel_span
|
|||
)
|
||||
|
||||
|
||||
# --- Semantic conventions: span kind, scope, db.operation/collection -------
|
||||
# --- Semantic conventions: span kind, scope, db.operation.name -------------
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
|
@ -999,58 +999,6 @@ async def test_execute_write_script_has_no_operation_name(otel_spans):
|
|||
assert "db.operation.name" not in script_spans[0].attributes
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_db_collection_name_set_from_table_argument(ds_client, otel_spans):
|
||||
db = ds_client.ds.get_database("fixtures")
|
||||
await db.execute("select pk from facetable limit 1", table="facetable")
|
||||
|
||||
spans = _spans_for_namespace(otel_spans, "fixtures")
|
||||
assert spans
|
||||
assert spans[-1].attributes["db.collection.name"] == "facetable"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_db_collection_name_absent_without_table_argument(ds_client, otel_spans):
|
||||
"""
|
||||
db.collection.name comes only from an explicit table= argument and is
|
||||
never derived from the SQL.
|
||||
|
||||
Deriving it would be a parse, and on an instance where anybody can create
|
||||
a table the value set has no ceiling. Without this test the one above
|
||||
would still pass if the table name were being read out of the query text.
|
||||
"""
|
||||
db = ds_client.ds.get_database("fixtures")
|
||||
await db.execute("select pk from facetable limit 1")
|
||||
|
||||
spans = _spans_for_namespace(otel_spans, "fixtures")
|
||||
assert spans
|
||||
span = spans[-1]
|
||||
assert span.attributes["db.query.text"] == "select pk from facetable limit 1"
|
||||
assert "db.collection.name" not in span.attributes
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"path,table",
|
||||
(
|
||||
("/fixtures/facetable.json", "facetable"),
|
||||
("/fixtures/simple_primary_key/1.json", "simple_primary_key"),
|
||||
),
|
||||
)
|
||||
@pytest.mark.asyncio
|
||||
async def test_table_and_row_pages_set_db_collection_name(
|
||||
ds_client, otel_spans, path, table
|
||||
):
|
||||
"The table and row views know their table, so their queries carry it."
|
||||
response = await ds_client.get(path)
|
||||
assert response.status_code == 200
|
||||
|
||||
spans = _spans_for_namespace(otel_spans, "fixtures")
|
||||
assert spans
|
||||
assert any(
|
||||
span.attributes.get("db.collection.name") == table for span in spans
|
||||
), f"expected a db.query span from {path} carrying db.collection.name"
|
||||
|
||||
|
||||
# --- Callback-style calls: execute_fn / execute_write_fn / execute_isolated_fn
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -46,7 +46,6 @@ EXPECTED_ATTRIBUTES = {
|
|||
"db.query.text",
|
||||
"datasette.callback",
|
||||
"db.operation.name",
|
||||
"db.collection.name",
|
||||
"datasette.param_count",
|
||||
"datasette.param_sets",
|
||||
"datasette.time_limit_ms",
|
||||
|
|
@ -140,9 +139,6 @@ async def exercise():
|
|||
custom_time_limit=1,
|
||||
)
|
||||
|
||||
# db.collection.name - set only by views that already know their table
|
||||
assert (await ds.client.get(f"/{name}/t?_facet=v")).status_code == 200
|
||||
assert (await ds.client.get(f"/{name}/t/1.json")).status_code == 200
|
||||
return ds
|
||||
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue