Skip to content

Instrument the OpenAI client, and stop shipping raw prompts in span events - #86

Merged
brentrager merged 1 commit into
mainfrom
ts-gen-ai-parity
Aug 15, 2026
Merged

Instrument the OpenAI client, and stop shipping raw prompts in span events#86
brentrager merged 1 commit into
mainfrom
ts-gen-ai-parity

Conversation

@brentrager

Copy link
Copy Markdown
Contributor

The gap, corrected

The brief was "TypeScript has no GenAI support." That premise is wrong, and worth stating plainly: packages/core/src/gen-ai-attributes.ts has shipped since SMOODEV-1155, exports setGenAIAttributes / recordGenAIMessage from the package index, and is the file the other four SDKs were ported from (Rust, Go and Python all say so in their headers).

What TypeScript actually lacked:

before after
gen_ai.* attribute helper ✅ already shipped unchanged
tests none — zero 34
framework integration none wrapOpenAI
PII scrub on recorded content none routed through scrubString

What this adds

wrapOpenAI(client, options) — proxies chat.completions.create and emits a GenAI-semconv span (chat {model}, SpanKind.CLIENT). Request model / temperature / top_p / max_tokens / seed / tool names go out with the call; response model, id, finish reason, truncation and token usage come back off the response. Errors mark the span ERROR, record the exception, and rethrow — request attributes still land, so a failed call stays attributable.

Streaming is handled, not skipped. A stream: true call hands span ownership to the iterator: the span stays open until the stream drains, ends when a consumer breaks out early, and collects the usage chunk that only appears under stream_options.include_usage. The Stream object is proxied rather than replaced, so .controller / .tee() keep working.

Why the OpenAI client over the Vercel AI SDK. The OpenAI wire shape is the lingua franca: Groq, Together, Fireworks, DeepSeek, Azure OpenAI and our own LiteLLM gateway at llm.smoo.ai are all reachable through this same client, so one wrapper instruments all of them ({ system: 'groq' } attributes the span to the real provider). It also needs no dependency — the client is duck-typed structurally, so @smooai/observability never imports openai. The Vercel AI SDK would have meant a peer dep on a fast-moving package and its own LanguageModelV* middleware types for strictly less coverage. grep across the smooai monorepo agrees: openai is a dependency of three packages, ai / @ai-sdk/* of none.

A cost seam. Nothing anywhere emits gen_ai.usage.cost_usd — which is exactly why the product's cost column is empty. Providers don't return a price, so a caller has to supply one:

wrapOpenAI(new OpenAI(), {
    costUsd: ({ inputTokens = 0, outputTokens = 0 }) => inputTokens * 2.5e-6 + outputTokens * 1e-5,
});

PII discipline. recordGenAIMessage now scrubs content before it leaves the process, and wrapOpenAI records no prompt or completion content unless you pass { recordContent: true }. Prompts and tool arguments are the most PII-dense payload this SDK can touch.

Dependency note: the TS scrubString is credentials-only today (Bearer tokens, api keys, password=). Keyed per-org hashing of names / emails / phones exists in Rust (rust/observability/src/pii.rs) and is being ported to TS in a parallel PR. This wires through whatever pii exposes rather than duplicating it, so that PR is inherited here for free with no change to this file.

Verification

Attribute keys are asserted literally, not derived from the source — a typo'd key silently produces an unroutable span. Two tests pin the contract with rust/api-prime/src/handlers/observability/ingest_traces.rs: gen_ai.system is the sole routing trigger into gen_ai_events, and gen_ai.operation.name is a straight passthrough with no fallback, so the wrapper always sets it (unset ⇒ NULL column).

Every key was mutation-checked — broken one at a time, the specific failing test recorded, then restored and verified byte-identical by sha256:

32 mutations applied, 32 killed, 0 survivors. Every one of the 19 attribute keys, all 4 message-event keys, the gen_ai.{role}.message event name, the PII scrub itself, and 8 wrapper-level behaviours (operation.name always set, default system, each usage-field mapping, the cost seam, streaming span lifetime, truncation).

Judged by exit code, not by reported text:

gen_ai tests   EXIT=0   (34 passed)
pnpm typecheck EXIT=0
pnpm lint      EXIT=0
pnpm test      EXIT=0   (295 passed, 16 files)
pnpm build     EXIT=0
pnpm format:check EXIT=0   (oxfmt, not prettier)

Docs

README gains a GenAI parity table across all five SDKs, honest about the two divergences it surfaced — Rust emits gen_ai.tool.names as a comma-joined string where the other four emit a string array, and TypeScript is the only SDK that scrubs recorded message content.

🤖 Generated with Claude Code

…vents

The TS SDK already had the gen_ai semconv vocabulary (Rust, Go, Python and
.NET were all ported FROM it) — what it did not have was a single test, a
framework integration, or any PII discipline on the content it records.

- `wrapOpenAI(client, opts)` proxies `chat.completions.create` and emits the
  GenAI semconv span. Streaming included: the span outlives the response and
  closes when the stream drains or the consumer breaks out early, picking up
  the `include_usage` chunk. The client is duck-typed, so this adds no
  dependency on `openai` and covers Groq / Together / DeepSeek / Azure and
  our own LiteLLM gateway — all of which speak the OpenAI wire shape.
- A cost seam. Nothing anywhere emits `gen_ai.usage.cost_usd`, which is why
  the dashboard's cost column is empty; providers do not return a price, so
  a caller has to supply one. `costUsd({...})` is that seam.
- `recordGenAIMessage` now routes content through the SDK's PII scrub, and
  `wrapOpenAI` keeps content recording off by default. Prompts and tool
  arguments are the most PII-dense payload this SDK can touch. The TS scrub
  is credentials-only today; when keyed per-org hashing lands it is inherited
  here for free, because this call site routes through the one entry point.
- 34 tests asserting every attribute key literally, plus the routing contract
  against ingest_traces.rs — `gen_ai.system` is the trigger, and
  `gen_ai.operation.name` is a passthrough with no fallback, so a caller who
  leaves it unset writes a NULL column. Every key was mutation-checked: 32
  mutations, 32 killed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@changeset-bot

changeset-bot Bot commented Aug 15, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 42ef343

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@smooai/observability Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@brentrager
brentrager merged commit 1104624 into main Aug 15, 2026
6 checks passed
brentrager added a commit that referenced this pull request Aug 15, 2026
#86 routed prompt content through `scrubString` and noted the TS scrub was
credentials-only "until keyed per-org hashing lands". It has landed in this
PR, and that call site inherited it exactly as predicted — no second redactor.
The comment now describes what the code does instead of what it will do.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
brentrager added a commit that referenced this pull request Aug 15, 2026
* Hash PII in the TS, Go, Python and .NET SDKs, not just Rust

All four scrubbed credentials only — `Bearer`, `password=`,
`token`/`api_key`/`secret=`, `sk-…` — while their module docs claimed "PII
scrubbing". Emails, phone numbers and street addresses went to the backend
untouched. Rust fixed this in #82; this brings the other four to parity with
byte-identical output, which matters now that telemetry is actually being
exported (chat-ws was just wired up).

Personal identifiers are hashed, not dropped: `a@b.com` -> `[email:9f2a41c8]`.
`[redacted]` destroys the ability to ask "is this the same user as that other
trace?"; a keyed hash keeps correlation while storing nothing reversible, and
the type prefix stays visible so you can still see what kind of value was there.

- HMAC-SHA256, keyed — a bare digest of an email is rainbow-tabled in seconds.
- Org id inside the HMAC message, so identical PII hashes differently per org.
  The kind is in there too, NUL-separated, so values that normalize alike can't
  collide.
- Fail closed: no key => `[email:redacted]`, never plaintext and never a hash
  under a guessable key.
- Credentials matched FIRST and dropped entirely, never hashed — a hash of a
  live token is still a token oracle, and PII inside a secret (`token=a@b.com`)
  goes with the secret.
- Normalized by kind (phone -> digits, email -> lowercase) so `(415) 555-0142`
  and `415-555-0142` correlate.

Key comes from `SMOOAI_OBSERVABILITY_PII_HASH_KEY` at bootstrap, or the
per-SDK setter; set-once, and the setters refuse a second key — rotating it
silently forks every correlation already stored. The browser TS bundle has no
env, so it calls `setPiiHashKey` explicitly (now exported from the entry).

`piiToken(kind, raw, orgId)` is the search seam: hash a typed query term the
same way and match the stored token.

Existing org-less signatures keep working (they hash under the empty salt);
`*ForOrg` variants are additive.

The TS SDK ships a small sync SHA-256/HMAC rather than a dependency:
`scrubString` is sync and runs in the browser bundle, where `node:crypto` is
unavailable and WebCrypto is async-only. Pinned by the RFC 4231 / FIPS 180-4
vectors.

All five SDKs assert the same `cross_sdk_parity_vectors` — computed
independently — so any drift in message framing, normalization or truncation
breaks exactly one SDK's suite.

Two unrelated pre-existing format failures are fixed here because this PR
touches those lanes and would otherwise be red: `CrashChild.cs` whitespace
(`dotnet format`) and a stray blank line in `bootstrap.rs` (`cargo fmt`).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Correct the gen_ai scrub comment that PR #86 left pointing at a future

#86 routed prompt content through `scrubString` and noted the TS scrub was
credentials-only "until keyed per-org hashing lands". It has landed in this
PR, and that call site inherited it exactly as predicted — no second redactor.
The comment now describes what the code does instead of what it will do.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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