~ knotless Search/
↑↓ move↵ open⇧↵ all results

chadtmiller.com/knotless

git that lives in your Atmosphere account

knotless.dev
1 collaborator

knotless 0.13.0 Latest 2 packages, 1 image

1
10

Usage counts slow statements, why actions failed, and crawlers by agent

Merged
Merged chadtmiller.com asked to merge into main from chadtmiller.com/knotless:serve/usage-detail

What changes

Slow statements. Each of the index's statements that takes 100 ms or more, the ones the sql log line already names, is counted in usage.db by its SQL as written, with no value it was given. stats lists the fifty that cost the most time, each with how often it ran slowly, its mean and its longest.

Why actions failed. An MCP tool call that fails is counted with what it answered. DIDs, at:// URIs, hashes and numbers are replaced with <did>, <uri>, <hex> and <n>, so the same failure counts together. A handle or a repository's name in an error is kept as written, for the 30 days.

Crawlers by user agent. Requests from the bot client are counted by their User-Agent, cut at 200 characters. A browser's never is.

All three are kept by the hour and dropped after 30 days, as the accounts are. At most 500 of each are kept apart in an hour, and the rest count together as (others). They are new optional fields in dev.knotless.getUsage (statements, failures, agents), new lines in knotless admin stats, and in the MCP stats tool. The privacy page now says that failure reasons and crawlers' user agents are kept for 30 days.

Why

stats showed that getTemplate was slow and that rerun and read_file were failing, but not why, or which bots made 69% of the traffic. Until now, finding out meant SSH and journalctl. These are the counts behind those questions, kept as small grouped tables like the ones already there, without putting raw logs on the MCP.

Built as the usual practices for this work suggest: statements grouped by normalized text and ranked by total time, with mean and max beside it, as pg_stat_statements does. Error messages normalized before grouping, as Sentry does, so IDs don't split one failure into many. A cap on distinct values each hour, for statements, failures and agents alike, with an overflow row, as OpenTelemetry's cardinality limits have, so a crawler that sends a new user agent each time, or a statement built to its input, cannot grow the table without end. A user agent is only kept for crawlers and scripts, and only for 30 days.

How it was checked

There are unit tests for counting, reading, pruning at 30 days, the hourly cap, normalizing errors, and slow statements reaching the usage store. The serve test usage.rs now sends a crawler's request and a failing tool call through admin stats --json. CI runs everything else.

Shots-moved: privacy

Shortcomings

Only MCP tool calls carry a reason so far. web.login and gateway.commit failures still count without one. Statements are counted only when slow, so the call counts are slow runs, not all runs. The /admin page doesn't show the new lists yet.

🤖 Generated with Claude Code

Asked for changes

This pull counts slow index statements, MCP tool failures (with their messages normalized) and crawler user agents in usage.db. They are kept by the hour for 30 days and exposed through getUsage, admin stats and the MCP stats tool.

I found one thing to change. The pull body and deploy/README.md both say "Statements, failures and agents keep at most 500 of each a hour apart; past that … (others)". In flush_detail (crates/knotless-serve/src/usage.rs), only failure and agent get that cap. The statement insert is a plain upsert keyed by hour and SQL, with no limit. The in-memory statements map is also unbounded between flushes. If any statement's SQL text varies, for example an IN (?,?,…) list sized to its input or a dynamically built clause, each variant becomes its own row. Either apply the same overflow to statement and test it in caps_each_hour, or drop statements from the claim in the README and body, and say why the SQL set is bounded.

Two smaller points:

  • fingerprint replaces DIDs, URIs, hex and numbers, but a handle or repository name in an error message is kept as written. The claim that failure reasons "name no one" is therefore too strong. Either soften the wording in the body and on the privacy page, or say which tool errors can carry a handle.
  • The checks were still running when I read this (test and check running, the rest queued), so I haven't seen any result.

The .changeset file is present.

Asked for changes

The only change since my last review is commit bd6c2ca, which extends the usage test with crawler requests. It doesn't touch the points I raised, so they still stand.

  1. Statements aren't capped. The body and deploy/README.md say statements, failures and agents each keep at most 500 apart an hour, with the rest counted as (others). caps_each_hour only tests failure and agent, and flush_detail still upserts statement rows with no limit. The in-memory statements map is also unbounded between flushes. Either apply the same overflow to statements and test it, or remove statements from the claim and explain why the set of SQL texts is bounded.
  2. "Names no one" is too strong. fingerprint doesn't replace handles or repository names in error messages. Soften the wording in the body and on the privacy page, or say which tool errors can't carry a handle.

Checks hadn't finished when I read this: test and check were running and the rest were queued. I haven't seen a result. The pull is already in the merge queue, so please fix point 1 before it lands.

Approved

Both points from my last review are addressed in cdd886f.

  • Statements are now capped. flush_detail uses the same overflow insert for statement as for failures and agents. A new SQL text past 500 in an hour is stored as (others), and a statement already kept keeps accumulating. caps_each_hour now checks this. It expects 501 rows, that SELECT aa accumulates (2 runs, 400 ms, max 300), and that (others) collects the one new statement.
  • "Names no one" is gone. The body, the fingerprint doc comment and deploy/README.md now say a handle or repository name is kept as written.

The queue ejected the earlier head bd6c2ca because its test check failed. This commit adjusts the route and request-count assertions in the unit and serve tests, which looks like the fix, but I haven't seen it pass. The test and check runs on cdd886f were still running when I looked, and the rest were queued. Please make sure they are green before it lands.

Merged as 7947441