Repository navigation
Proposal: outcome and duration on product analytics events (turns, tools, approvals, connectors, schedules) #960
frannouter
started this conversation in
Ideas
Replies: 1 comment
|
approval_resolved and tool_called don't share a key, so once an approved action runs and fails, or succeeds and turns out wrong, the stream can't say whether a person or a rule let it through. I'd put request_id and by on the tool_called that executes the approved action, which makes failure rate per approval path one join instead of a trace search. Does a turn resumed after an approval know the request_id it resumes from, or would that have to come from the span? |
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
Summary
PRODUCT_ANALYTICSalready gives a deployment a stream of product events (gadget_created,connection_created,workpiece_created, ...). We run a deployment of Cloudflare OS and have been extending that stream for a while. Thisis not a PR: the contributing guide asks for a discussion first, and the change is bigger than a dozen lines in total.
We would like to know whether the maintainers want any of it, and if so, in which shape. If the answer is no, that is
fine too.
The gap
As far as we can tell from
packages/workshop-backend/src/analytics.ts, the events that exist today have twolimitations:
gadget_created,workpiece_created,connection_createdand the rest areemitted after the happy path. A deployment can count what people did, but not what they tried and could not do, so
it cannot tell "nobody uses scheduled tasks" from "everybody who tries them hits an error".
waited, or how long connecting a connector took. These are the numbers an operator needs to answer "is the agent
getting slower?", "which tool fails most?" or "where do people abandon the connect flow?".
In
PRODUCT_ANALYTICS, nothing covers the agent turn, the tools it calls, approvals, or the connect flow, which iswhere most of the questions come from once real people use a deployment.
Why not the existing agent spans?
We know
agent-tracing.tsalready emitsinvoke_agent,chat,execute_toolandtool_approvalspans withoutcomes and timings, and they answer "what happened in this turn?" well. We would still like the same facts in the
stream, for three reasons:
connector" is one query, not a trace search.
questions need.
workpiece_createdor
gadget_openedfor the same workspace, to ask whether people who hit an error come back.There is also a limit that
workshop-backend/AGENTS.mddocuments: a span's trace ends at the next event the DurableObject receives once the invocation that opened it has returned, so a turn keeps all its spans only while the request
that started it stays open. An event written to the stream does not depend on that.
What we propose
New event names in the existing style (
object_verb, snake_case, English). Properties are of the same kind the currentevents use (ids, counts, booleans, closed lists). The exact names are open (the existing events say
gatekeeper_idandvendor_id, we wroteconnection_idandvendor; see the questions below). Every event is emitted throughrecordAnalytics(or a batch helper for the tool calls, below), so it also gets itsactivity/v1metric, except where noted.agent_turn_finishedchat_id,model_id,duration_ms,tool_calls(count),outcome(ok,error,cancelled,limit,no_model),error_code(only on error)error_shownchat_id,code,retryable(boolean)tool_calledchat_id,tool_name,connector_vendor(for a connector's tool),duration_ms,outcome(ok,error,cancelled),error_classapproval_requestedconnection_id,vendor,action_kind,chat_id,request_id,auto(a rule will approve it)approval_resolvedconnection_id,vendor,action_kind,request_id,outcome(approved,rejected,automatic),by(person,rule),wait_msconnector_connect_started/connector_connected/connector_connect_failed/connector_disconnectedvendor,reconnect(boolean),duration_ms(onconnected),stageanderror_class(onfailed;stageis a closed list:start,handoff,save,reconnect,abandoned)schedule_created/_enabled/_disabled/_deletedhook_id,by(agent,gadget,person) on_createdschedule_runhook_id,outcome(ok,error,rejected),duration_ms,delay_ms(how late it started)screen_viewed/ui_actionroute(a route template, never a URL),previous_route,action(closed list)Notes on the ones that are not obvious:
tool_calledis the expensive one. A long turn makes dozens of calls, and one send per call would hold the turnup. We collect the calls of a turn and send them in one
PRODUCT_ANALYTICS.send([...])when the turn ends, eachstamped with the moment its call finished. We also give it no
activity/v1metric: a turn with many calls would writea lot of Analytics Engine points per invocation; we expected a busy turn to approach the per-invocation write cap, so we did not
measure it; please check the current limit in Cloudflare's Analytics Engine documentation.
schedule_runand the UI events go to the stream only, for the same reason (a task that runs every minute, or apage view, would flood Analytics Engine).
approval_requestedandapproval_resolvedsharerequest_id. The id is only unique inside its workspace, sothe join is
gadget_id+request_id.agent_turn_finishedafter an approval: a turn resumed after an approval is a new turn, so it is another event.screen_viewedandui_actionare the largest piece and the least necessary. The client batches them and asingle backend RPC records them, against a closed list of routes and actions defined in
workshop-shared. We woulddrop this part first if the maintainers want a smaller scope.
We would also keep the envelope out of the proposal. Our deployment adds a tenant, a hashed person and a version to
each record; that is specific to how we host many tenants and has no place upstream.
The rule: never content
The stream is for behavior, not for what people write. We hold ourselves to this and would propose the same for any of
the events above:
a free-text field.
match a known list or a strict pattern; otherwise the event goes without the name. A tool the model made up is
recorded with no
tool_name.ProductAnalyticsInput, so a value outside its list does not compile. A few are typedstringand kept closed wherethey are emitted (a setting's value, the family of a failure site).
@is enforced by a test: after every test in the analytics suites, a check reads all the records that were sentand fails if any of them contains an
@(an email, or something a person typed). The check does not know eachproperty's list or pattern, so a new free-text property without an
@would pass it: that one is for review to catch.If useful, we can contribute that check as its own PR before any new event, so the rule is in place first.
Analytics must never break the product
All of the emissions go through
recordAnalytics, which already wraps the send in atry/catchand sends inctx.waitUntil. The call sites that run in the path of the action itself (deleting a workspace or a gadget and itsconnection, saving an agent step with its request cards, retrying a blueprint publish) have a test that makes
PRODUCT_ANALYTICS.sendthrow, or the whole analytics call, and checks that the action still completes. The rest relyon that
try/catch.How it could be split into small PRs
The guide asks for PRs of about a dozen lines. Each event needs a member in
ProductAnalyticsInput, a case intoActivityMetric(the switch is exhaustive), and one emission, so a simple event is roughly that size. Some will bea bit over,
agent_turn_finishedfirst (it has the most properties and needs the duration and tool count tracked inthe turn), and we would ask for guidance on those rather than squeeze them. We would send them
one at a time, each with its test, in this order, and stop wherever the maintainers say stop:
agent_turn_finished(likely somewhat over a dozen lines): the type member, the metric case, and one call in thefinallyof the agent turn. Smallestand the most useful on its own.
error_shown: one call where the error message is posted to the chat.approval_requestedandapproval_resolved: two members, two cases, two calls. They go together because therequest id only means something with both.
connector_connect_started,connector_connected,connector_connect_failed: one PR for the connect flow and onefor
connector_disconnected. The failed event is the one with a subtlety (one failure per attempt, and an expiry thatmust not double-count), so it gets its own PR and its own description.
schedule_created,schedule_enabled,schedule_disabled,schedule_deleted: one PR per event or one for allfour, whichever the maintainers prefer, since they are four lines each.
schedule_run: one call where the hook firing callback finishes.tool_calledlast, since it is the only one that needs a helper: a small batch function so a turn sends once. Weexpect that to be over a dozen lines and would ask for guidance before writing it.
screen_viewedandui_actiononly if there is interest. This one touches the frontend,workshop-sharedand anRPC, so it is a different conversation.
Each PR would stay out of
workshop-backendbeyond the emission itself, and would not rename or change any existingevent.
Questions for the maintainers
outcomeandduration_msadded to the existing success events as well? We left them aloneto keep the diffs small, but we are happy to try.
outcome,duration_ms,byandwait_ms, in English like the rest of the upstream, and for theconnector
connection_id/vendorwhere the existing events saygatekeeper_id/vendor_id. Tell us if youprefer the existing names or others.
Thanks for reading.
All reactions