Skip to content

Commit eaf9d2f

Browse files
alexkuzmikclaude
andauthored
[OPIK-8196] [SDK] BI events improvements (#8118)
* [OPIK-8196] [SDK] feat: report the account behind a configure run in its BI events `opik configure` and `opik mcp configure` are where MCP adoption starts, and their events carried nothing the MCP funnels key on, so a configure run could not be tied to the person who went on to use the MCP server - or to drop it. One runner in twenty was attributable, by the accident of a workspace named after its owner. Three keys now go with those four events, because no single one covers the population. Measured against the local stdio funnel's own grain, 761 installs over 30 days: - `api_key_sha256`, 628 installs (83%). The MCP server reports the digest of the same key this command configures, so two runs of one credential meet even when neither side resolved a name. The widest bridge, and the only one present on the first run of a new install. - `user_id`, the Comet login, 72 installs (9%) and a strict subset of the above. Thin alone, reported anyway: it is the warehouse's own user key and the only key that names a person rather than a credential. - `install_id`, read from `~/.opik-mcp/install-id` when the server has already written one. Exact, and the only bridge for the 133 installs (17%) with no credential at all. Read, never written - the server reports `install_id_freshly_generated` on whichever run creates that file, so writing it here would file every onboarding as a returning install. `workspace`, `workspace_kind` and `identity_lookup` go with them, reusing the MCP server's own names and values so one query reads both products. `no_credential` is the single value it has no analogue for: a run that has not reached its credential yet, rather than one that never will. The login is a plaintext personal identifier, so the privacy contract in `opik.analytics` is amended explicitly - hashing it would make it unjoinable, which is the only reason to report it - and the public docs now disclose the key digest rather than promising nothing about the key at all. These two commands are the only place any of it is reported. None of it is something a user feels: nothing is looked up when nothing would be reported (a new `analytics.reporting_allowed`, so `OPIK_ANALYTICS_ENABLE` governs the lookup too), the lookup is cloud-only and bounded at three seconds, cached so a command pays for it at most once, and every failure is silent and countable rather than an unexplained absence. Verified against the live pipeline rather than assumed: the id and digest this computes match real MCP events at every funnel stage, and a new property on a configure event was seen arriving in the warehouse. Implements OPIK-8196: [SDK] Add user information to `opik configure` and `opik mcp configure` BI events. Identity conventions follow OPIK_7907, OPIK_8042 and OPIK_8044; the funnels being joined are those of OPIK_7613. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(analytics): arrange the reporting-allowed yes explicitly `reporting_allowed` reads the environment the suite itself runs in, and CI runs with `OPIK_ANALYTICS_ENABLE=false` - so the one case asserting a yes was asserting that the environment happened to permit reporting, which locally it did and in CI it does not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(cli): record why the key digest is a fast hash CodeQL reads a SHA-256 of something named like a credential as password storage. The distinction that makes it correct here - a label nothing verifies against, and a value that must match byte for byte what the MCP server emits, which a salted KDF could never do - was only in the ticket, not in the code a reviewer reads. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(cli): address review comments on the configuration identity - Bound the resolved-account cache and key it by the key's digest instead of the key itself, so a process that configures several accounts no longer holds credentials in an unbounded cache to recognise one it has seen. - Refuse the lookup when no analytics URL is configured. `_start_worker` already declines to report without a destination, so the enrichment was paying for a round trip whose event is then dropped. - Stop claiming the raw key never leaves the process. It does - the configure flow authenticates with it - and the claim that matters is narrower: only the digest is ever reported. - Say four things rather than three in the privacy disclosure, which listed four. - Exercise `event_properties()` itself, gate and configuration loading included, rather than only `_properties` with a hand-built config; and assert `identity_lookup` beside the login on the result event, which otherwise passes when that metadata disappears. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * revert(docs): drop the analytics disclosure from this PR Kept out at review request; the SDK change stands on its own here. The user-facing disclosure of what the two configure commands report still needs to ship - it is not in this PR, so the published docs currently describe attribution by workspace name only. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent ebc39d3 commit eaf9d2f

9 files changed

Lines changed: 827 additions & 1 deletion

File tree

sdks/python/src/opik/analytics/__init__.py

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,27 @@
77
name, the same identifier used for error reports. Trace, span, prompt and dataset
88
contents are never sent, nor is the API key.
99
10+
One exception, and it is deliberate: the two `configuration` events - `opik configure`
11+
and `opik mcp configure` - also report the Comet login of the account being
12+
configured, when there is one to resolve. It is the value the MCP server reports as
13+
its own `user_id`, and the warehouse's own user key, so it is what ties a
14+
configuration run to what that person went on to do; hashing it would make it
15+
unjoinable, which is the whole point of reporting it. See
16+
`opik.cli.account_identity`. No other event carries it, and no other personal data
17+
goes with it.
18+
1019
Reporting happens on a background thread, is switched off by
1120
`OPIK_ANALYTICS_ENABLE=false`, and never raises into calling code.
1221
"""
1322

14-
from .api import Component, flush, internal, shutdown, track_event
23+
from .api import (
24+
Component,
25+
flush,
26+
internal,
27+
reporting_allowed,
28+
shutdown,
29+
track_event,
30+
)
1531
from .rules import register_rule
1632
from .worker import PropertyValue
1733

@@ -21,6 +37,7 @@
2137
"flush",
2238
"internal",
2339
"register_rule",
40+
"reporting_allowed",
2441
"shutdown",
2542
"track_event",
2643
]

sdks/python/src/opik/analytics/api.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,30 @@ def _reset_after_fork() -> None:
200200
os.register_at_fork(after_in_child=_reset_after_fork)
201201

202202

203+
def reporting_allowed() -> bool:
204+
"""
205+
Whether an event reported from this process would be sent anywhere.
206+
207+
For a call site that has to do work to enrich an event - a lookup, a round-trip
208+
- and must not do that work when nothing will be reported. Asking here keeps
209+
`OPIK_ANALYTICS_ENABLE` the single switch that governs analytics, the work done
210+
to produce it included.
211+
212+
Every reason `_start_worker` refuses to report has to be a reason here too, or
213+
an enrichment pays for an event that is then dropped - which is why a missing
214+
destination counts, not just the opt-out.
215+
"""
216+
if _DISABLED:
217+
return False
218+
219+
try:
220+
config_ = config.OpikConfig()
221+
return rules.reporting_allowed(config_) and bool(config_.analytics_url)
222+
except Exception:
223+
LOGGER.debug("Failed to decide whether analytics may report", exc_info=True)
224+
return False
225+
226+
203227
def _disable_after_rejection() -> None:
204228
"""
205229
Called from the worker thread when the destination has told us to stop. Turning
Lines changed: 302 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,302 @@
1+
"""Who ran a configuration command, for the `configuration` analytics events.
2+
3+
`opik configure` and `opik mcp configure` are where MCP adoption starts, and their
4+
events carried nothing the MCP funnels key on - so a configure run could not be tied
5+
to the person who went on to use the MCP server, or to drop it. One runner in twenty
6+
was attributable.
7+
8+
Three keys are reported, because no single one covers the population. Measured over
9+
30 days against the local stdio funnel's own grain, 761 installs:
10+
11+
- `api_key_sha256` - **628 installs (83%)**. The MCP server reports the digest of the
12+
same key this command configures, so two runs of the same credential meet here even
13+
when nobody's name could be resolved on either side. The widest bridge by far, and
14+
the only one available on the first run of a brand-new install.
15+
- `user_id` - the Comet login, **72 installs (9%)**, a strict subset of the above. Thin
16+
on its own, and worth reporting anyway: it is the warehouse's own user key and the
17+
only key that names a person rather than a credential.
18+
- `install_id` - the MCP server's own per-machine id, read from `~/.opik-mcp/install-id`
19+
when it is already there. Exact, and the only bridge for the 133 installs (17%) with
20+
no credential at all - a local or open source Opik, which has no accounts. Read,
21+
never written: the server reports `install_id_freshly_generated` on the run that
22+
creates that file, and creating it here would report every onboarding as a returning
23+
install.
24+
25+
The login is a plaintext personal identifier, which is a narrow, deliberate amendment
26+
to the analytics privacy contract in `opik.analytics`: hashing it would make it
27+
unjoinable, which is the only reason to report it. The key is reported only as a
28+
one-way digest; the key itself is never sent anywhere but the Opik deployment it
29+
authenticates against. These two commands are the only place either is reported, and
30+
nothing else personal goes with them.
31+
32+
Three rules, so that none of this is ever something a user feels:
33+
34+
- **Nothing is looked up when nothing would be reported.** This is work done for
35+
analytics, so `OPIK_ANALYTICS_ENABLE=false` switches it off too.
36+
- **Cloud only, and bounded.** `account-details` does not exist on a self-hosted or
37+
local Opik, and no configure run should spend a timeout on an answer that cannot
38+
exist there.
39+
- **Failure is silent, and countable.** Every path reports `identity_lookup`, so an
40+
unattributed run says which reason it was rather than being an absence someone has
41+
to guess at.
42+
43+
`identity_lookup`, `workspace` and `workspace_kind` deliberately reuse the MCP
44+
server's own property names and values - `resolved` / `miss` / `none_expected`, and
45+
`configured` / `resolved` / `placeholder` / `unknown` - so one BI query reads both
46+
products. `no_credential` is the one value the MCP has no analogue for: it is a run
47+
that has not reached its credential yet, rather than one that will never have it.
48+
49+
Note what this does NOT change: the identity an event is *attributed* to. Every SDK
50+
event, these included, is still keyed on the workspace-derived `anonymous_id`, while
51+
the MCP server keys its events on the login. So the login reported here joins the two
52+
by value - which is what the warehouse joins on - and not as one PostHog person.
53+
"""
54+
55+
import dataclasses
56+
import hashlib
57+
import logging
58+
import pathlib
59+
import uuid
60+
from typing import Any, Dict, Optional, Tuple
61+
62+
import httpx
63+
64+
import opik.config as opik_config
65+
import opik.url_helpers as url_helpers
66+
from opik import analytics
67+
68+
LOGGER = logging.getLogger(__name__)
69+
70+
# Where the MCP server keeps its per-machine id. Matches
71+
# `opik_mcp.analytics.identity._install_id_path`; a different path here would report
72+
# an id nothing else has ever seen.
73+
_MCP_INSTALL_ID_PATH = pathlib.Path.home() / ".opik-mcp" / "install-id"
74+
75+
# Tight on purpose: this runs inside a command someone is waiting on. The answer is
76+
# worth a moment and never a hang, and it is cached below, so a command pays for it
77+
# at most once however many events it reports.
78+
_TIMEOUT_SECONDS = 3.0
79+
80+
# A CLI run configures one account, so this exists to serve the second event of a
81+
# pair rather than to hold a directory. Bounded so nothing accumulates in a process
82+
# that calls `opik.configure()` in a loop.
83+
_MAX_CACHED_ACCOUNTS = 4
84+
85+
86+
@dataclasses.dataclass(frozen=True)
87+
class _Account:
88+
"""What `account-details` says about the holder of an API key."""
89+
90+
user_name: Optional[str]
91+
default_workspace_name: Optional[str]
92+
93+
94+
# Keyed by `(key digest, base url)`: recognising a key we already resolved needs no
95+
# more than its digest, so the process never holds a credential to do it.
96+
_RESOLVED: Dict[Tuple[str, str], Optional[_Account]] = {}
97+
98+
99+
def event_properties() -> Dict[str, analytics.PropertyValue]:
100+
"""The identity properties to report with a `configuration` event.
101+
102+
`identity_lookup` and `workspace_kind` are always present: they say where the
103+
other two values came from, which is what makes a missing one countable instead
104+
of indistinguishable from a version that never reported it.
105+
106+
Never raises, and returns nothing at all when analytics is switched off.
107+
"""
108+
try:
109+
if not analytics.reporting_allowed():
110+
return {}
111+
112+
return _properties(opik_config.OpikConfig())
113+
except Exception:
114+
LOGGER.debug("Failed to resolve the account to report", exc_info=True)
115+
return {"identity_lookup": "miss", "workspace_kind": "unknown"}
116+
117+
118+
def _properties(
119+
config_: opik_config.OpikConfig,
120+
) -> Dict[str, analytics.PropertyValue]:
121+
account, lookup = _resolve(config_)
122+
workspace, workspace_kind = _workspace(config_, account)
123+
124+
properties: Dict[str, analytics.PropertyValue] = {
125+
"identity_lookup": lookup,
126+
"workspace_kind": workspace_kind,
127+
}
128+
if account is not None and account.user_name:
129+
properties["user_id"] = account.user_name
130+
if workspace is not None:
131+
properties["workspace"] = workspace
132+
if config_.api_key:
133+
# Reported whatever the deployment: a self-hosted install resolves no login,
134+
# but its key still meets the MCP server's digest of the same key.
135+
properties["api_key_sha256"] = _digest(config_.api_key)
136+
137+
install_id = _mcp_install_id()
138+
if install_id is not None:
139+
properties["install_id"] = install_id
140+
141+
return properties
142+
143+
144+
def _resolve(config_: opik_config.OpikConfig) -> Tuple[Optional[_Account], str]:
145+
"""The account behind this run, and why it came out that way."""
146+
# Asked before the credential, because the two answers mean opposite things: a
147+
# missing key is a run that has not got there yet, while a deployment with no
148+
# accounts is one that can never be attributed at all.
149+
if not config_.is_cloud_installation:
150+
# No account-details endpoint on a self-hosted Opik, and no accounts at all
151+
# on the open source one. Unattributable by construction, not a gap to close
152+
# - which is what the MCP server means by this value too.
153+
return None, "none_expected"
154+
155+
if not config_.api_key:
156+
# `opik configure` reports its first event before it has asked for a key, so
157+
# this is the ordinary state of a first-ever run rather than a failure.
158+
return None, "no_credential"
159+
160+
account = _fetch(config_.api_key, url_helpers.get_base_url(config_.url_override))
161+
if account is None or not account.user_name:
162+
return account, "miss"
163+
164+
return account, "resolved"
165+
166+
167+
def _workspace(
168+
config_: opik_config.OpikConfig, account: Optional[_Account]
169+
) -> Tuple[Optional[str], str]:
170+
"""The workspace to report, and where its name came from.
171+
172+
Reported explicitly even though every event is already attributed to a
173+
workspace: that attribution is read once per process, and `opik configure`
174+
changes the workspace while it runs - so on the run that matters most it names
175+
the workspace the user had before this command, or the shared `default`
176+
sentinel. This is the one the command actually configured.
177+
"""
178+
configured = (config_.workspace or "").strip()
179+
180+
if configured and configured != opik_config.OPIK_WORKSPACE_DEFAULT_NAME:
181+
# Someone named this workspace deliberately, and they may be working outside
182+
# their account default - so it outranks the resolved name.
183+
return configured, "configured"
184+
185+
resolved = account.default_workspace_name if account is not None else None
186+
if resolved:
187+
return resolved, "resolved"
188+
189+
if configured:
190+
# The literal `default`: one name shared by every install that never set
191+
# one, so it is reported but must never be joined on as a workspace.
192+
return configured, "placeholder"
193+
194+
return None, "unknown"
195+
196+
197+
def _fetch(api_key: str, base_url: str) -> Optional[_Account]:
198+
"""Ask Comet who holds this key. `None` on any failure whatsoever.
199+
200+
Cached for the process: a command reports an entry event and a result event, and
201+
the second must not cost a second round-trip. The cache is keyed by a digest of
202+
the key rather than the key itself, and bounded, so a long-lived process that
203+
configures several accounts never accumulates credentials in memory - it holds
204+
only what it needs to recognise a key it has already resolved.
205+
206+
TLS is always verified, matching the analytics sender: `check_tls_certificate`
207+
exists for a self-hosted deployment's own certificate, and this only ever calls
208+
Comet's cloud host.
209+
"""
210+
cache_key = (_digest(api_key), base_url)
211+
if cache_key in _RESOLVED:
212+
return _RESOLVED[cache_key]
213+
214+
account = _request(api_key, base_url)
215+
216+
# A plain dict with a bound rather than an LRU: one CLI run resolves one
217+
# account, so eviction order cannot matter, and dropping the oldest keeps the
218+
# common case (one entry) allocation-free.
219+
if len(_RESOLVED) >= _MAX_CACHED_ACCOUNTS:
220+
_RESOLVED.pop(next(iter(_RESOLVED)))
221+
_RESOLVED[cache_key] = account
222+
223+
return account
224+
225+
226+
def _request(api_key: str, base_url: str) -> Optional[_Account]:
227+
try:
228+
with httpx.Client(timeout=_TIMEOUT_SECONDS, verify=True) as client:
229+
response = client.get(
230+
url_helpers.get_account_details_url(base_url),
231+
headers={"Authorization": api_key},
232+
)
233+
234+
if response.status_code != 200:
235+
LOGGER.debug("account-details returned %s", response.status_code)
236+
return None
237+
238+
body = response.json()
239+
except Exception:
240+
LOGGER.debug("account-details lookup failed", exc_info=True)
241+
return None
242+
243+
if not isinstance(body, dict):
244+
return None
245+
246+
return _Account(
247+
user_name=_text(body.get("userName")),
248+
default_workspace_name=_text(body.get("defaultWorkspaceName")),
249+
)
250+
251+
252+
def _digest(api_key: str) -> str:
253+
"""SHA-256 hex of the key, the transform `opik_mcp.credential_identity` uses.
254+
255+
Lowercase hex of the UTF-8 bytes, because BI joins on the exact string: a
256+
different encoding or casing here would produce a digest that matches nothing.
257+
258+
A plain fast hash is the right primitive, and CodeQL's
259+
`py/weak-sensitive-data-hashing` will disagree because the input is named like
260+
a credential. That rule is about *storing a password for verification*, where
261+
a fast hash lets whoever steals the store brute-force human-chosen secrets.
262+
Neither half holds here:
263+
264+
- This is a label, not a verifier. Nothing compares it to anything to grant
265+
access; it exists so two reports of the same credential can be recognised as
266+
one setup. Only the digest is ever reported - the key itself goes nowhere
267+
except the Opik deployment it authenticates against, which is where the
268+
configure flow already sends it.
269+
- A password KDF cannot do the job. bcrypt / scrypt / PBKDF2 are salted, so the
270+
same key would digest differently on every machine and join to nothing - and
271+
an unsalted slow hash would still have to match what the MCP server emits,
272+
which is this.
273+
274+
An Opik API key is also a high-entropy generated value rather than something a
275+
person chose, so the guessing attack the rule guards against does not apply.
276+
"""
277+
return hashlib.sha256(api_key.encode("utf-8")).hexdigest()
278+
279+
280+
def _mcp_install_id() -> Optional[str]:
281+
"""The MCP server's per-machine id if it has already written one.
282+
283+
Deliberately read-only, and deliberately not created. The server reports
284+
`install_id_freshly_generated` on whichever run writes that file, and the install
285+
funnel counts new installs with it - so writing it from here would file every
286+
person who onboards through this command as a returning install instead.
287+
288+
Normalised through `UUID` exactly as the server does, so a hand-edited or
289+
truncated file reports nothing rather than an id that joins to nothing.
290+
"""
291+
try:
292+
return str(uuid.UUID(_MCP_INSTALL_ID_PATH.read_text().strip()))
293+
except Exception:
294+
LOGGER.debug("No MCP install id to report", exc_info=True)
295+
return None
296+
297+
298+
def _text(value: Any) -> Optional[str]:
299+
if not isinstance(value, str) or not value.strip():
300+
return None
301+
302+
return value.strip()

sdks/python/src/opik/cli/configure.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
import opik.config as opik_config
1010
from opik import analytics
11+
from opik.cli import account_identity
1112
from opik.cli import assistants
1213
from opik.cli import install_view
1314
from opik.cli import status_view
@@ -333,6 +334,9 @@ def configure(
333334
# "never asked" — the flag is also how an agent drives this.
334335
install_mcp=str(install_mcp),
335336
install_skills=str(install_skills),
337+
# Whoever is already configured, if anyone: a first-ever run has no
338+
# credential yet at this point, and says so.
339+
**account_identity.event_properties(),
336340
)
337341

338342
# With no terminal there is nobody to ask, and every question here has a sane
@@ -356,6 +360,10 @@ def configure(
356360
"result",
357361
clients_written=outcome.clients,
358362
skills_installed=outcome.skills,
363+
# Resolved again rather than reused from the entry event: this is the run
364+
# that just wrote ~/.opik.config, so it is the first point at which a
365+
# first-ever configure has an account to name at all.
366+
**account_identity.event_properties(),
359367
)
360368

361369

0 commit comments

Comments
 (0)