diff --git a/datasette/app.py b/datasette/app.py index 37c4e882..6a06e091 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -51,6 +51,7 @@ from .renderer import json_renderer from .resources import DatabaseResource, TableResource from .telemetry import ( TelemetryMiddleware, + _in_datasette_client, clamp_http_method, request_span, tracer, @@ -171,8 +172,9 @@ app_root = Path(__file__).parent.parent logger = logging.getLogger(__name__) -# Context variable to track when code is executing within a datasette.client request -_in_datasette_client = contextvars.ContextVar("in_datasette_client", default=False) +# _in_datasette_client itself lives in telemetry.py so the request span +# middleware can read it without a circular import; its writers +# (_DatasetteClientContext) and reader (in_client()) both live here. class _DatasetteClientContext: diff --git a/datasette/telemetry.py b/datasette/telemetry.py index 1f74a3fb..19462585 100644 --- a/datasette/telemetry.py +++ b/datasette/telemetry.py @@ -14,6 +14,7 @@ run-to-run variation. Installing an SDK provider is what costs something measurable. """ +import contextvars import re from opentelemetry import trace as otel_trace @@ -25,6 +26,7 @@ from .telemetry_registry import ( ERROR_TYPE, HTTP_REQUEST_METHOD, HTTP_RESPONSE_STATUS_CODE, + INTERNAL_CLIENT, SERVER_ADDRESS, URL_PATH, URL_SCHEME, @@ -32,6 +34,14 @@ from .telemetry_registry import ( ) from .version import __version__ +# True while code is executing within a datasette.client request. Defined +# here rather than in app.py (which owns its writers and the in_client() +# accessor) so TelemetryMiddleware can read it without a circular import: +# an in-process sub-request runs the full ASGI stack, so it emits a second, +# nested SERVER span - datasette.internal_client marks those so kind-based +# dashboards can filter the double-count out. +_in_datasette_client = contextvars.ContextVar("in_datasette_client", default=False) + # The semantic-convention version whose spellings this instrumentation # actually emits. Deliberately NOT the latest release. # @@ -301,6 +311,8 @@ class TelemetryMiddleware: user_agent = _first_header(headers, b"user-agent") if user_agent: span.set_attribute(USER_AGENT_ORIGINAL, user_agent) + if _in_datasette_client.get(): + span.set_attribute(INTERNAL_CLIENT, True) # A copy, not a mutation: the scope belongs to the server, and # every other layer in Datasette extends it the same way. diff --git a/datasette/telemetry_registry.py b/datasette/telemetry_registry.py index 0850a4ff..283b8886 100644 --- a/datasette/telemetry_registry.py +++ b/datasette/telemetry_registry.py @@ -136,6 +136,16 @@ USER_AGENT_ORIGINAL = Attribute( "The ``User-Agent`` header, verbatim. Omitted if the client sent none.", optional=True, ) +INTERNAL_CLIENT = Attribute( + "datasette.internal_client", + "``True`` when the request was made in-process through " + "``datasette.client`` rather than arriving over the network. Such a " + "sub-request runs the full ASGI stack, so it emits its own nested " + "``SERVER`` span inside the outer request's - filter on this attribute " + "to keep kind-based dashboards from double-counting requests. Omitted " + "for real inbound requests.", + optional=True, +) ERROR_TYPE = Attribute( "error.type", "Set when the request failed: the exception class name if one escaped the " @@ -283,6 +293,7 @@ HTTP_REQUEST = SpanName( USER_AGENT_ORIGINAL, HTTP_RESPONSE_STATUS_CODE, ERROR_TYPE, + INTERNAL_CLIENT, ), dynamic=True, kind=SpanKind.SERVER, diff --git a/docs/internals.rst b/docs/internals.rst index c780a34e..6465ea49 100644 --- a/docs/internals.rst +++ b/docs/internals.rst @@ -2397,6 +2397,7 @@ That is the route's compiled regular expression, not a prettified ``/{database}/ - ``user_agent.original`` *(optional)* - The ``User-Agent`` header, verbatim. Omitted if the client sent none. - ``http.response.status_code`` *(optional)* - The status of the response, read from the ASGI ``http.response.start`` message rather than from a :ref:`internals_response` object - several views, including static files, file downloads and streaming CSV, send that message themselves and never build one. Omitted if the connection closed before anything was sent. - ``error.type`` *(optional)* - Set when the request failed: the exception class name if one escaped the application, otherwise the status code as a string for a 5xx response. A 4xx does **not** set this and does not set an error status - per semantic conventions a client error is not a server span's failure. + - ``datasette.internal_client`` *(optional)* - ``True`` when the request was made in-process through ``datasette.client`` rather than arriving over the network. Such a sub-request runs the full ASGI stack, so it emits its own nested ``SERVER`` span inside the outer request's - filter on this attribute to keep kind-based dashboards from double-counting requests. Omitted for real inbound requests. ``db.query`` A SQL operation issued by Datasette, covering the full round trip including any time spent queued for a thread. Callback-style calls - ``execute_fn()``, ``execute_write_fn()`` and ``execute_isolated_fn()`` - appear here too, distinguished by ``datasette.callback`` in place of ``db.query.text``. diff --git a/tests/test_http_span.py b/tests/test_http_span.py index a69713cb..ca2239d5 100644 --- a/tests/test_http_span.py +++ b/tests/test_http_span.py @@ -856,3 +856,34 @@ def test_no_provider_takes_the_fast_path(): ) # Same fast path, other observable: nothing is stashed in the scope either. assert report["scope_keys"] == [False, False] + + +@pytest.mark.asyncio +async def test_internal_client_requests_are_marked(ds, otel_spans): + """ + An in-process `datasette.client` request runs the full ASGI stack, so it + emits its own SERVER span - `datasette.internal_client` marks those so + kind-based dashboards can filter the double-count out. A request that + arrives through the raw ASGI app (the shape of a real inbound request, + without the DatasetteClient wrapper setting the ContextVar) must not + carry the attribute. + """ + otel_spans.clear() + assert (await ds.client.get("/")).status_code == 200 + server = _server_spans(otel_spans) + assert server + assert all( + span.attributes.get("datasette.internal_client") is True for span in server + ) + + import httpx + + transport = httpx.ASGITransport(app=ds.app()) + async with httpx.AsyncClient( + transport=transport, base_url="http://localhost" + ) as client: + otel_spans.clear() + assert (await client.get("/")).status_code == 200 + server = _server_spans(otel_spans) + assert server + assert all("datasette.internal_client" not in span.attributes for span in server) diff --git a/tests/test_telemetry_registry.py b/tests/test_telemetry_registry.py index f37dc8f0..242d64d3 100644 --- a/tests/test_telemetry_registry.py +++ b/tests/test_telemetry_registry.py @@ -93,6 +93,7 @@ EXPECTED_HTTP_ATTRIBUTES = { "user_agent.original", "http.response.status_code", "error.type", + "datasette.internal_client", } # The registry's own name for the request span is that template, not anything