feat(agent-server): sanitized product-analytics telemetry with split consent policy by VascoSch92 · Pull Request #4172 · OpenHands/software-agent-sdk

VascoSch92 · 2026-07-21T09:26:53Z

HUMAN:

Add telemetry in agent-server.

AGENT:

Why

We need server-side visibility into local Agent Canvas execution and errors. The repo has no PostHog client, dependency, or event emitter today, and the two existing signals are unsuitable for product analytics: LLM.log_completions writes full prompts, responses and tracebacks to disk, and Laminar/OpenTelemetry produces distributed traces. Neither may be forwarded wholesale.

This adds a versioned, allowlisted diagnostic-event contract owned by openhands-agent-server, behind an injectable sink that defaults to a no-op, with the PostHog exporter enabled only at the composition boundary.

Design note: the original issue framed consent plumbing as a design blocker, and this PR first resolved it with a typed settings field plus a deployment "mode". The OSS-5715 follow-up thread overruled that. Consent now lives in misc_settings.telemetry.consent, there is no cloud-mode concept, and writes go through the existing /api/settings endpoint. The event schema, sanitizer, sink and subscriber layers were compatible with either design and are unchanged.

Summary

Consent lives in misc_settings.telemetry.consent, written through the existing PATCH /api/settings. No settings schema change, no dedicated endpoint, and it persists across restarts for free. misc_settings.telemetry is documented as the single sanctioned exception to the opaque-container rule. Precedence: DO_NOT_TRACK → OH_TELEMETRY_CONSENT in override mode → persisted value → legacy Canvas key (read-only fallback) → OH_TELEMETRY_CONSENT as a seed → unset.
No cloud mode. Nothing in agent-server special-cases a hosted deployment; enforcement is seeding the sandbox's misc settings at runtime init, which lives outside this repo.
Exporters are selectable — OH_TELEMETRY_EXPORTER = none (default) | posthog | http. The new generic HTTP exporter POSTs sanitized batches to a configured endpoint, so no vendor credentials need to live in the sandbox.
Privacy enforced by the type system. Every property is a constrained scalar — never a bare str — so a prompt or traceback is a construction-time ValidationError. The sanitizer never reads str(exc) nor calls into traceback. Ids are keyed digests, magnitudes bucketed, and the exporter only calls capture() so it cannot mint or merge an identity.
Delivery cannot block or fail a conversation. emit() is synchronous, because PubSub.__call__ awaits its subscribers. Events go onto a deque(maxlen=…) drained by one retained task; failed batches are dropped, not re-queued; shutdown is hard-capped.

Issue Number

No issue number was supplied with this work item. Happy to link one.

Acceptance criteria

Every item from the issue is addressed:

Requirement	Where
Keep `openhands-sdk` vendor-neutral	No PostHog dep or vendor API; the only SDK change is a docstring on `user_id`
Sanitized diagnostic-event contract in agent-server	`telemetry/models.py`
Injectable no-op sink + optional PostHog exporter at the composition boundary	`telemetry/sink.py`, `telemetry/posthog_exporter.py`, wired in `api.py` / `init_router.py`
Sanitize at event construction, before any exporter	Pydantic validation on construction; sinks accept only `DiagnosticEvent`, never a dict
Consent explicitly resolved, including revocation	`telemetry/policy.py`, full truth table tested
Versioned schema over minimal lifecycle + failure events	`TELEMETRY_SCHEMA_VERSION`; 7 events
Properties allowlisted; no prompts, messages, source, secrets, full paths, bodies, tracebacks	`EXPECTED_PROPERTY_NAMES` + deny test; leak audit below
Core SDK has no PostHog dependency or vendor API	Asserted in a subprocess test
Exporter optional, disabled by default for library/headless	`[project.optional-dependencies] posthog`; default exporter `none`
Consent states explicitly represented and tested	`test_telemetry_policy.py`, `test_telemetry_settings_path.py`
No telemetry before consent; revocation stops delivery and discards queue	Nothing is buffered while disabled; the settings path re-resolves and clears the queue before returning
Delivery async, bounded, cannot block or fail a conversation	Sync `emit()` + bounded deque + retained drain task
Release/version, coarse runtime props, normalized error class/fingerprint, privacy-safe correlation ids	`RuntimeProperties`, `normalize_exception`, keyed `conversation_ref`
Canvas ↔ agent-server correlation without duplicate identities	`distinct_id` reuses host `user_id` verbatim; `capture()` only
Tests for sanitization, consent transitions, queue limits, exporter failure, disabled behavior	234 tests in `tests/agent_server/telemetry/`
Docs distinguish analytics from completion logging and Laminar/OTel	Comparison table in the agent-server README

How to Test

uv run pytest tests/agent_server/telemetry/ -v     # 234 passed
uv run pytest tests/agent_server/ -q               # 1714 passed
uv run pytest tests/cross/ -q                      # 360 passed, 1 pre-existing failure
uv run pre-commit run --all-files                  # all hooks pass

Two pre-existing failures you may hit locally, both of which reproduce unchanged on a clean main worktree:

tests/cross/test_remote_conversation_live_server.py::test_openai_chat_completions_gateway_over_real_server needs a real LLM provider.
Running tests/agent_server/ and tests/cross/ in one process yields 25 failures from cross-suite state pollution (test_conversation_restore_behavior, test_agent_loading, test_agent_secrets_integration). Verified identical on clean main. CI runs them as separate jobs, which is why it is green — run them separately locally too.

End-to-end against a real server

Unit tests alone are not sufficient, so the three required scenarios were driven against a real uvicorn process with a capturing stand-in for the exporter, via curl. Re-run against the new consent path after the follow-up rework.

1. Opted-in produces sanitized events. Exact captured payload:

{
  "event": "agent_server.request_failed",
  "distinct_id": "anon:855e4331473d401587ab806f547d892f",
  "properties": {
    "schema_version": 1, "server_version": "1.36.1", "python_version": "3.13",
    "platform": "darwin", "deferred_init": false,
    "source": "openhands-agent-server",
    "route_template": "/api/_boom", "method": "GET", "status_code": 500,
    "error_class": "ValueError", "error_category": "config",
    "error_fingerprint": "445aa04176d34c95", "error_id": "7b474744..."
  }
}

Leak audit. The route raises ValueError(f"secret={CANARY} path=/Users/alice/x.py"); a parameterized route was hit at /api/_boom/CONV-SENSITIVE-999/detail/TOK-SECRET:

absent   sk-ant-CANARY-DO-NOT-LEAK       absent   CONV-SENSITIVE-999
absent   'secret=' (message fragment)    absent   TOK-SECRET
absent   /Users/alice                    absent   Traceback
route_template: '/api/_boom/{conversation_id}/detail/{secret_token}'

2. Opted-out produces nothing.

Driven entirely through PATCH /api/settings — there is no telemetry endpoint:

OPTED OUT : 500 raised                            -> captured events: 0
GRANT     : misc_settings.telemetry.consent=granted
            500 raised x2                         -> captured events: 2
REVOKE    : misc_settings.telemetry.consent=denied
            500 raised x3                         -> before 2, after 2  (discarded)

Consent survives a process restart, since misc_settings is already persisted.

3. Exporter failure leaves execution unaffected. With ConnectionError on every send:

server still serving      -> HTTP 200
'export failing' warnings -> 1     (per streak, not per failure)
shutdown took 0.5s                 (bounded)

4. Disabled by default. create_app(Config()) yields NoOpTelemetrySink and never imports the exporter module (subprocess-asserted). The test asserts our module stays unimported rather than "posthog" not in sys.modules, because the pre-existing browser-use dependency imports posthog transitively — unrelated to this change.

Video/Screenshots

Not applicable — no GUI surface. The transcripts above are the equivalent evidence.

Type

Notes

Follow-up alignment (latest commit). Reverted from this PR's original design: the typed PersistedSettings.telemetry_consent field, the v2→v3 schema bump and its baseline fixture, the cloud_locked/local_opt_in/disabled mode concept, OH_TELEMETRY_MODE, the deployment_mode event property, and the dedicated consent router.

⚠️ Blocking follow-up outside this repo

Cloud gets no telemetry until OpenHands/runtime-api seeds the sandbox agent-server with misc_settings.telemetry.consent = granted and managed = true at runtime init, before any conversation starts. That is the direct consequence of removing the cloud-mode concept: nothing in this repo special-cases Cloud any more, so enforcement has to happen at seeding time.

Similarly out of scope: the backend endpoint the http exporter targets — the one that revalidates runtime auth and TOS/consent before forwarding to PostHog — lives in the OpenHands main app. Its payload contract is documented in the agent-server README so it can be built against independently.

Confirmed during review: OH_TELEMETRY_CONSENT_MODE semantics (seed default, override opt-in); keeping DO_NOT_TRACK and dropping the redundant OH_TELEMETRY_DISABLED alias; dual-read of the legacy Canvas consent key without dual-write.

Reviewer attention: telemetry/service.py must call get_settings_store(config), never the no-arg form — that singleton's directory and cipher are fixed by the first call, and telemetry initialises before ConversationService primes it. An earlier revision left the process with cipher=None. Regression-tested.

Not covered: no test drives a full LLM-backed conversation; lifecycle is verified against the real event object and a real StoredConversation, not a live model run. request_failed / server_* always use anon: since no user context is available at those points.

…consent policy Adds an opt-in, allowlisted diagnostic-event pipeline to openhands-agent-server. The core SDK stays vendor-neutral: no PostHog dependency, no vendor API, and the only SDK change is a docstring clarifying that user_id also feeds analytics. Policy is split across its two natural owners. The deployment supplies the mode (Config.telemetry.mode: cloud_locked | local_opt_in | disabled, default disabled); the user supplies consent (PersistedSettings.telemetry_consent, schema v2->v3, additive). Consent is deliberately a typed field rather than a key inside misc_settings, which the agent-server documents as never interpreting. DO_NOT_TRACK / OH_TELEMETRY_DISABLED override every mode. Privacy is enforced structurally rather than by review. Every event property is a constrained scalar, never a bare str, so a prompt or traceback becomes a construction-time ValidationError. The sanitizer never reads str(exc) and never calls into traceback; failures are grouped by a fingerprint built from the exception type and first-party module/line pairs. Conversation ids are keyed digests, magnitudes are bucketed, and the exporter only ever calls capture() so it cannot create or merge a PostHog identity. Delivery is a synchronous emit() onto a maxlen deque drained by a single retained task, because PubSub awaits its subscribers and inline I/O would stall event fan-out for a whole conversation.

… events Three correctness gaps found while auditing the implementation against the production event path rather than against the tests. confirmation_mode does not exist on StoredConversation -- the field is confirmation_policy. The getattr default silently reported False for every conversation. Now reports the policy kind, which also preserves ConfirmRisky that a bool would have collapsed into NeverConfirm. total_tokens_bucket and cost_bucket were hardcoded to "unknown" and never populated. They are now derived from stats.usage_to_metrics in the terminal state snapshot and bucketed. iteration_count_bucket is removed: it is not obtainable from the snapshot, so it could only ever have been "unknown". Most subscriber tests drove a synthetic ConversationStateUpdateEvent with key="execution_status". EventService always publishes via from_conversation_state(), which emits key="full_state", so a bug in that branch could not have been caught. Adds tests that use the real production constructor, and syncs the README property list with the schema.

Removes restatement and over-long explanation, keeping only comments that carry non-obvious "why" a reader cannot get from the code: the settings-store singleton constraint, the initial-state-push baseline, the backlog re-arm, and the bounded backoff. Structural section dividers and the grouping labels inside the error-category table are kept -- both match existing repo style and aid navigation of long literals. Net -46 comment lines across the telemetry package and touched modules.

Drops section dividers, grouping labels and any comment restating the code. What remains explains a constraint the code cannot show on its own: the settings-store singleton priming, __name__ vs __qualname__, the attach-time state push being a baseline, the backlog re-arm, the lazy vendor imports, and the bounded backoff. Production comment lines 138 -> 33.

- Drop `from __future__ import annotations` everywhere; the package targets 3.12+ and had no forward references needing it. - Annotate every module constant with `typing.Final`. - Event names become a `StrEnum` (`EventName`) instead of seven string constants plus a parallel `Literal` alias that could drift. - `resolve()` uses match statements. - Drop the four `*_bucket` wrappers; callers pass the bounds to `bucket()` directly, and the bounds are now public. - Source provider names from `litellm.provider_list` instead of a hand-maintained set. This also improves accuracy: `litellm_proxy/<deployment>` now reports `litellm_proxy` rather than `other`, still without the deployment name. - `_walk_frames` is a generator; `_categorize` consumes an iterator. - `slots=True` on ErrorFingerprint, TelemetryDecision, ConversationTelemetryContext and TelemetrySubscriber. - Explicit `return None` in `_capture_usage`. Tests updated for the two behavioural consequences: `slots` removes `__dict__` (leak audits now use `dataclasses.asdict`), and `litellm_proxy` is a recognised provider.

…ents - Every module constant now carries an explicit type: Final[str], Final[int], Final[float], Final[frozenset[str]], Final[re.Pattern[str]]. - TelemetryConsent and ConsentMode become StrEnum, matching EventName; the resolution path compares identity instead of strings. - __all__ moves to the top of policy.py and subscriber.py. - Bucket edges are wrapped in a Bounds NamedTuple that owns the lookup, so a value cannot be paired with the wrong scale, and the lookup is a bisect rather than a linear scan. - _handle dispatches with a match statement; the state-update branch moves to its own method rather than nesting three levels deep. _MODEL_FAMILY_HINTS stays hardcoded, with the reason recorded inline: litellm models providers, not model families, and the same family is served by many of them (claude-* appears under anthropic, bedrock, azure_ai and deepinfra), so deriving it from models_by_provider yields 4+ candidates per family. The provider-prefixed path already uses litellm's real provider list, and a test asserts every hint target is one of them.

Shutdown could hang for minutes. aclose() capped the final flush but then awaited exporter.aclose() with no timeout. For PostHog that is where the real network I/O happens: capture() only enqueues, so aclose() -> client.shutdown() is flush() + join(), and posthog 6.7.7's consumer defaults to retries=10 with timeout=15 per batch. An unreachable endpoint blocked the uvicorn lifespan there. Both legs are now bounded by shutdown_flush_timeout. The regression test fails with TimeoutError against the previous code. View-only rehydrations emitted spurious conversation_finished events. The subscriber attaches on every _start_event_service path, including the lazy attach when a user merely opens an existing conversation. close() then emitted a terminal event for a session in which nothing ran -- carrying a non-terminal value such as "paused" in a field named terminal_status, with no matching conversation_started, and again on every view-then-restart cycle for the same conversation_ref. Inflated finished counts relative to started. close() now emits only when a run was actually observed (status reached running past the attach-time baseline). The crash-mid-run safety net is kept: such a run reports conversation_failed with terminal_status="interrupted" rather than echoing a raw non-terminal status. test_close_emits_a_terminal_event_when_none_was_observed encoded the old behaviour as correct and is replaced by tests asserting silence for view-only rehydration at idle/paused/finished/error, and that an interrupted run is still reported.

Converting TelemetryConsent from a Literal to a StrEnum means pyright no longer accepts a plain "granted"/"denied" where the enum is expected. The earlier pre-commit run only covered changed files and so missed these; `pre-commit run --all-files` is now clean.

malhotra5 · 2026-07-21T17:34:09Z

NIT: for the telemetry service would be nice to automatically parse a dedicated header like X-OpenHands-Telemetry-Distinct-Id, this way we can attribute other endpoint activities like llm profile creation

The FE would be responsible for attaching the header properly so that the BE can appropriately attribute

headers["X-OpenHands-Telemetry-Distinct-Id"] = posthog.get_distinct_id()

malhotra5

LGTM

Couple of nits, and would be great to test this in canvas

Addresses a review nit: request-scoped activity (a failed request today, and future events such as LLM-profile creation) has no conversation user_id, so it fell back to the anonymous id. The frontend can now send its PostHog identity in X-OpenHands-Telemetry-Distinct-Id, and request_failed attributes to that person instead. The value is trusted the same way as user_id, and sanitized the same way: it is stripped of control characters, bounded to 256 chars, and falls back to the anonymous id when empty. Conversation-scoped events are unaffected; they continue to use the user_id from the stored conversation.

VascoSch92 · 2026-07-22T08:37:38Z

Good call — done in dc11aafef.

The agent-server now reads X-OpenHands-Telemetry-Distinct-Id and uses it as the PostHog distinct_id for request-scoped events. request_failed is wired up as the first consumer (it previously always fell back to anon: since there is no conversation user_id at that point), so the pattern is in place for LLM-profile creation and other endpoint events as they start emitting.

# frontend
headers["X-OpenHands-Telemetry-Distinct-Id"] = posthog.get_distinct_id()

It is trusted the same way as user_id — the frontend owns setting it — and sanitized the same way: control characters stripped, bounded to 256 chars, empty falls back to the anonymous id. Conversation-scoped events are unchanged; they keep using the user_id from the stored conversation. Documented in the agent-server README under Telemetry → Identity.

Two review findings. emit_server_started() was called before deferred init reached ready. A failure in a later step (get_instance, bash/service enter, root-path update) rolls the handler back to dormant without emitting server_stopped or resetting the lifecycle flag, so a retry emitted a second unpaired server_started. Moved the emit to after the ready transition, so a failed init emits no start and a retry emits exactly one. emit_server_started() is now also idempotent as defense in depth: a call while a start is already outstanding is a no-op. request_failed was not emitted for a BaseExceptionGroup that contains no HTTPException — that branch returned a genuine 500 but skipped telemetry. It now emits request_failed like the plain-unhandled path. Both regression tests fail against the previous code (verified by reverting each fix in isolation).

# Conflicts: # uv.lock

…ts emit ConversationService is instantiated at import time (sockets.py module scope), before the lifespan builds the telemetry sink, so it captured get_telemetry_sink()'s pre-init NoOpTelemetrySink. _maybe_subscribe_telemetry gated on that frozen NoOp, so conversation_started/finished/failed/error never emitted regardless of consent; server-lifecycle and request_failed events were unaffected because they resolve the sink fresh at emit time. Resolve get_telemetry_sink() at conversation start instead, matching those paths. Drop the now-unused telemetry_sink field and its get_instance() capture, and add a regression test that builds the service while the sink is a NoOp, enables telemetry, and asserts a new conversation still emits conversation_started.

VascoSch92 added 4 commits July 21, 2026 11:20