Skip to content

feat(analytics): report to collect.mindshub.ai and send an identifying agent (ENG-1355) - #333

Open
lucas-koontz wants to merge 1 commit into
stagingfrom
feat/eng-1355-repoint-analytics-collector
Open

feat(analytics): report to collect.mindshub.ai and send an identifying agent (ENG-1355)#333
lucas-koontz wants to merge 1 commit into
stagingfrom
feat/eng-1355-repoint-analytics-collector

Conversation

@lucas-koontz

Copy link
Copy Markdown
Contributor

User story

As a harness engineer who shipped per-turn cost accounting last week
I want anton's turn_completed event to reach a collector that keeps its properties
So that ENG-1286's spend ceiling can be set from real turn costs instead of guessed at

Why this matters

turn_completed has never arrived in PostHog. The collector it was pointed at matched a lowercase anton_ / ds_connect_ prefix on the action name and copied five known property names, discarding everything else and answering HTTP 200 either way. send_event is fire-and-forget behind a bare except, so nothing reported it. The turn_cost log line worked the whole time, which is what made it look healthy.

This is the client half of the fix. The collector half is mindsdb/mindshub_services#186.

What happens today

sequenceDiagram
    participant S as anton session
    participant A as anton/analytics.py
    participant L as zoomInfoCollector
    participant P as PostHog 355390

    S->>A: send_event("turn_completed", 28 props)
    A->>L: GET ?action=turn_completed&...
    L--xP: action fails the prefix filter, nothing relayed
    L-->>A: HTTP 200
    A->>A: response discarded, no error raised
    Note over S,P: The log line is complete.<br/>The sink is empty. Nothing says so.
Loading

What should happen

sequenceDiagram
    participant S as anton session
    participant A as anton/analytics.py
    participant L as collect.mindshub.ai
    participant P as PostHog 355390

    S->>A: send_event("turn_completed", 28 props)
    A->>L: GET ?action=turn_completed&... (User-Agent set)
    L->>P: capture, all properties minus a denylist
    L-->>A: HTTP 200, relayed true
    A->>A: response discarded, as before
    Note over L,P: A drop or a relay failure now<br/>increments a counter and alarms.
Loading

What changed

File Change
anton/config/settings.py analytics_url default is now https://collect.mindshub.ai/collect. Still overridable with ANTON_ANALYTICS_URL.
anton/analytics.py _fire sends User-Agent: anton-analytics/1.0 instead of urllib's default.
anton/analytics.py Module docstring corrected. It claimed every extra kwarg became a queryable property and that this was not an allowlist. Both halves were false.
tests/test_analytics.py Two tests: the user agent is sent and is not urllib's default; the baked default is not an execute-api id.

On the docstring. It is worth reading the diff on it rather than skimming. The old text told any author that no collector work was needed to add a property, which is a plausible reason this defect reached Passed QA. The new text says what the collector actually does, notes that it was different before 2026-08-11, and tells a reader to check the collector before assuming a missing property is the caller's fault.

On the user agent. Cloudflare's bot protection answers Python-urllib/3.x with 403 on the mindshub.ai zone. The collector host is deliberately not proxied, so this is a second line rather than the only one, but _fire discards its response either way and a blocked event would disappear with nothing reporting it.

Acceptance criteria

  • A real turn produces exactly one turn_completed row in project 355390 whose properties match that turn's turn_cost log line, with the per-role token sums reconciling to tokens_total and unknown_tokens at 0.
  • ANTON_ANALYTICS_URL still overrides the default.
  • ANTON_ANALYTICS_ENABLED=false still suppresses the event entirely.
  • CI traffic is still dropped rather than sent.
  • Still true afterwards: send_event raises nothing when the endpoint is unreachable, refuses the request, or returns a non-200.

How to test

  1. Merge order matters. mindsdb/mindshub_services#186 and mindsdb/terraform#162 must be deployed first, so collect.mindshub.ai answers. Confirm with curl -s "https://collect.mindshub.ai/collect?action=anton_probe&aid=qaprobe1355" returning relayed: true.
  2. Install this branch and run one turn with analytics on, no ANTON_ANALYTICS_ENABLED=false.
  3. Read the turn_cost line for that turn from ~/Library/Logs/anton/cowork-server.log.
  4. Wait 60 seconds, then in project 355390: select * from events where event = 'turn_completed' and properties.conversation_id = '<the session id from step 3>'. Expect exactly one row.
  5. Diff its properties against the log line. planning_tokens + coding_tokens + router_tokens + unknown_tokens must equal tokens_total.
  6. Confirm the override: ANTON_ANALYTICS_URL=https://example.test/collect and check the request goes there instead.
  7. Confirm the opt-out: ANTON_ANALYTICS_ENABLED=false and check nothing is sent.
  8. Exclude probe rows from any validation query: where properties.aid not like '%probe1288%' and properties.aid not like '%probe1355%'.

Verified locally

Check Result
pytest tests/ (full suite) 1680 passed, 28 skipped
pytest tests/test_analytics.py tests/test_settings.py after rebase onto current staging 34 passed

Note on rollout

The endpoint reaches the fleet through a normal release plus the sidecar auto-updater, so no flag day. Installs older than that release keep reporting to the previous collector, which stays running and relays into the same PostHog project, so there is no gap in the event stream while the tail drains. Rows are told apart by their source property.

Ships with

  • mindsdb/mindshub_services#186, the collector.
  • mindsdb/terraform#162, the host and the alarms.
  • mindsdb/cowork, the same repoint for the desktop app's four install events.

Refs ENG-1355

…g agent

The baked default was a raw execute-api id belonging to an AWS account nobody can
deploy to, so the endpoint could not be changed without shipping a release to
every install. Points it at a hostname we own instead, where the collector is in
a repo and behind CI.

Also corrects this module's own docstring, which stated that every extra kwarg
became a queryable PostHog property and that this was not an allowlist. That was
false for as long as it had been written: the collector copied five property
names and dropped everything else, so turn_completed reached PostHog carrying one
of its 28 properties, and only when its action name happened to match a lowercase
prefix. The claim is true now, minus a denylist, and the docstring says which.

The request carries an explicit user agent because urllib's default
Python-urllib/3.x is answered with 403 by the bot protection on the mindshub.ai
zone, and _fire discards its response, so an event blocked for looking like a
script would vanish with no trace.

Lucas Koontz
ENG-1355 - Analytics collector lambda silently drops turn_completed and its properties

Refs: ENG-1355
@entelligence-ai-pr-reviews

Copy link
Copy Markdown

EntelligenceAI PR Summary

Analytics now sends events to the hostname-controlled https://collect.mindshub.ai/collect endpoint, allowing the collector to preserve turn_completed’s full cost and token properties for downstream spend-ceiling calculations. Requests use the identifying anton-analytics/1.0 User-Agent to avoid Cloudflare blocking, while existing opt-out and asynchronous fire-and-forget behavior remain unchanged. Tests cover the custom User-Agent and protect the new default collector URL from regressing to an unmanaged API Gateway endpoint.

flowchart TD
    classDef newBehavior fill:#dcfce7,stroke:#16a34a,color:#14532d;
    Config["AntonSettings analytics_url"] --> SendEvent["send_event builds event URL"]
    SendEvent --> Fire["_fire sends asynchronous HTTP request"]
    Fire --> Collector["collect.mindshub.ai collector"]
    Collector --> PostHog["PostHog preserves event properties"]
    class Config,SendEvent,Fire,Collector newBehavior;
Loading

🟢 Green = new or changed in this PR


Review Scorecard

Dimension Rating Basis
Code Quality ●●●●○ 4/5 — Good no critical/significant findings — rated Good
Blast Radius Medium touches infra / deploy / config, 2 file(s) / ~48 line(s) changed (size only — not a blast signal)
Merge Confidence ●●●●○ 4/5 — Mostly Safe code quality 4/5 × Medium blast radius

Safe to merge with a minor test follow-up — the Medium blast radius is appropriately contained, and the analytics change cleanly preserves opt-out and asynchronous fire-and-forget behavior while adding the controlled collector endpoint and identifying User-Agent. The only issue is that the URL regression test in tests/test_analytics.py is weaker than intended and does not fully verify the exact default collector URL.

Key Findings:

  • tests/test_analytics.py asserts only an https://collect.* pattern, so a misconfigured default such as https://collect.attacker.example/... could pass; assert the complete https://collect.mindshub.ai/collect URL.
Evaluated against
  • 3/3 changed files reviewed
  • criteria: correctness, security & access control, robustness & error handling, concurrency & data integrity, repo conventions / steering docs
  • steering docs: none found in repo
Files requiring special attention
  • tests/test_analytics.py

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant